feat: adapt UserDatasets to datasets read-all scope

This is a simplified approach, without requiring an extra OAuth request per each dataset
CartoDB · Dec 10, 2020 · 3f24210 · 3f24210 · vercel · Dec 10, 2020
1 parent a573413
commit 3f24210
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## Not released
 - Refactor to use local UserMenuLogin for managing Login button [#144](https://github.com/CartoDB/carto-react-template/pull/144)
+- Add a new forceOAuthLogin option to appSlice, so a fullscreen Login protects the whole app [#146](https://github.com/CartoDB/carto-react-template/pull/146)
+- Refactor to adapt UserDatasets to datasets read-all scope [#147](https://github.com/CartoDB/carto-react-template/pull/147)
+- Fix issue with UserDatasets removal [#147](https://github.com/CartoDB/carto-react-template/pull/147)
 
 ## 1.0.0-beta6 (2020-12-04)
 - Improve layout for mobile (specially for iOS) [#141](https://github.com/CartoDB/carto-react-template/pull/141)

diff --git a/template-sample-app/template/src/components/layers/OAuthLayer.js b/template-sample-app/template/src/components/layers/OAuthLayer.js
@@ -17,10 +17,10 @@ export default function OAuthLayer() {
       autohighlight: true,
       stroked: true,
       filled: true,
-      lineWidthMinPixels: 2,
+      lineWidthMinPixels: 1,
       getFillColor: [238, 77, 90],
-      pointRadiusMinPixels: 2.5,
-      getLineColor: [255, 77, 90],
+      pointRadiusMinPixels: 5,
+      getLineColor: [238, 238, 238],
       getRadius: 30,
       getLineWidth: 1,
       onHover: (info) => {

diff --git a/template-sample-app/template/src/components/views/datasets/UserDatasets.js b/template-sample-app/template/src/components/views/datasets/UserDatasets.js
@@ -1,16 +1,13 @@
-import React, { useState, useEffect, useCallback } from 'react';
+import React, { useEffect, useCallback } from 'react';
 import { useSelector, useDispatch } from 'react-redux';
 
 import {
-  setOAuthApp,
-  setTokenAndUserInfoAsync,
   selectOAuthCredentials,
   addLayer,
   addSource,
   removeLayer,
   removeSource,
 } from '@carto/react/redux';
-import { useOAuthLogin } from '@carto/react/oauth';
 
 import { makeStyles } from '@material-ui/core/styles';
 import {
@@ -22,7 +19,8 @@ import {
   Typography,
 } from '@material-ui/core';
 import { ChevronRight, HighlightOff } from '@material-ui/icons';
-import { setBottomSheetOpen, setError } from 'config/appSlice';
+
+import { setBottomSheetOpen } from 'config/appSlice';
 
 const useStyles = makeStyles((theme) => ({
   loadingSpinner: {
@@ -34,111 +32,46 @@ const useStyles = makeStyles((theme) => ({
   },
 }));
 
-const scopeForDataset = (dataset) => {
-  return `datasets:r:${dataset.table_schema}.${dataset.name}`;
-};
-
 const toTitleCase = (str) => str.charAt(0).toUpperCase() + str.slice(1).toLowerCase();
 
+const OAUTH_LAYER = 'oauthLayer';
+const OAUTH_SOURCE = 'oauthSource';
+
 export default function UserDatasets(props) {
   const dispatch = useDispatch();
   const classes = useStyles();
 
-  const { oauthLayer } = useSelector((state) => state.carto.layers);
   const credentials = useSelector(selectOAuthCredentials);
-  const oauthApp = useSelector((state) => state.oauth.oauthApp);
-  const token = useSelector((state) => state.oauth.token);
-
-  const [dataset, setDataset] = useState(null);
-  const [newTokenRequest, setNewTokenRequest] = useState(false);
-  const [initialToken, setInitialToken] = useState(null);
+  const { oauthLayer } = useSelector((state) => state.carto.layers);
 
-  // Load dataset & layer to store (so to Map)
   const loadDataset = useCallback(
     (selectedDataset) => {
       const { name: datasetName, table_schema: schema } = selectedDataset;
       const dataSourceCredentials = { ...credentials };
 
       dispatch(
         addSource({
-          id: 'oauthSource',
+          id: OAUTH_SOURCE,
           data: `SELECT * FROM "${schema}".${datasetName}`,
           credentials: dataSourceCredentials,
         })
       );
 
-      dispatch(addLayer({ id: 'oauthLayer', source: 'oauthSource', name: datasetName }));
+      dispatch(addLayer({ id: OAUTH_LAYER, source: OAUTH_SOURCE, layerAttributes: { name: datasetName } }));
+
+      dispatch(setBottomSheetOpen(false));
     },
     [credentials, dispatch]
   );
 
-  // Remove dataset & layer from store (so from Map)
   const removeDataset = useCallback(() => {
-    dispatch(removeLayer('oauthLayer'));
-    dispatch(removeSource('oauthSource'));
-  }, [dispatch]);
-
-  const oauthUpdatedFor = useCallback(
-    (dataset) => {
-      return oauthApp.scopes.includes(scopeForDataset(dataset));
-    },
-    [oauthApp]
-  );
-
-  const authorizeAndLoadDataset = (selectedDataset) => {
-    if (oauthUpdatedFor(selectedDataset)) {
-      loadDataset(selectedDataset);
-    } else {
-      setDataset(selectedDataset); // start the process..., monitored during useEffects
-      setNewTokenRequest(true);
-      setInitialToken(token);
-    }
-    dispatch(setBottomSheetOpen(false));
-  };
-
-  const onParamsRefreshed = (oauthParams) => {
-    if (oauthParams.error) {
-      dispatch(setError(oauthParams.error));
-    } else {
-      dispatch(setTokenAndUserInfoAsync(oauthParams));
-    }
-  };
-
-  const [handleLogin] = useOAuthLogin(oauthApp, onParamsRefreshed);
+    dispatch(removeLayer(OAUTH_LAYER));
+    dispatch(removeSource(OAUTH_SOURCE));
+  }, [dispatch]);  
 
   // cleanup when leaving
   useEffect(() => removeDataset, [removeDataset]);
 
-  useEffect(() => {
-    if (dataset && newTokenRequest && !oauthUpdatedFor(dataset)) {
-      // step 1: require a new OAuth process, including the scope for the dataset
-      const newScopes = new Set(
-        (oauthApp.scopes ? [...oauthApp.scopes] : []).concat(scopeForDataset(dataset))
-      );
-
-      const newOAuth = { ...oauthApp, scopes: [...newScopes] };
-      dispatch(setOAuthApp(newOAuth));
-    }
-  });
-
-  useEffect(() => {
-    if (dataset && newTokenRequest && oauthUpdatedFor(dataset)) {
-      // step 2: login again, once that the new scopes are ready (including the desired datasets)
-      handleLogin();
-      setNewTokenRequest(false);
-    }
-  }, [dataset, newTokenRequest, oauthUpdatedFor, handleLogin]);
-
-  useEffect(() => {
-    const tokenHasBeenRefreshed = token !== initialToken;
-    if (dataset && oauthUpdatedFor(dataset) && tokenHasBeenRefreshed) {
-      // step 3: load dataset, once there is a new token that includes its access
-      loadDataset(dataset);
-      setDataset(null); // ...and finish the process for this dataset
-      setInitialToken(null);
-    }
-  }, [dataset, oauthUpdatedFor, token, initialToken, loadDataset]);
-
   // Loading...
   if (props.loading) {
     return (
@@ -159,7 +92,7 @@ export default function UserDatasets(props) {
     <List component='nav' disablePadding={true}>
       {props.datasets.map((dataset) => {
         const labelId = `checkbox-list-label-${dataset.name}`;
-        const datasetLoaded = oauthLayer && oauthLayer.name === dataset.name;
+        const datasetLoaded = oauthLayer && oauthLayer.layerAttributes.name === dataset.name;
         const secondary = toTitleCase(`${dataset.privacy}`);
 
         return (
@@ -170,7 +103,7 @@ export default function UserDatasets(props) {
             button
             role={undefined}
             onClick={() =>
-              datasetLoaded ? removeDataset() : authorizeAndLoadDataset(dataset)
+              datasetLoaded ? removeDataset() : loadDataset(dataset)
             }
           >
             <ListItemText id={labelId} primary={dataset.name} secondary={secondary} />

diff --git a/template-sample-app/template/src/components/views/stores/StoresDetail.js b/template-sample-app/template/src/components/views/stores/StoresDetail.js
@@ -72,12 +72,22 @@ export default function StoresDetail() {
       });
 
     // Set selected store on the layer
-    dispatch(updateLayer(LAYER_ID, { selectedStore: id }));
-
+    dispatch(updateLayer(
+      {
+        id: LAYER_ID,
+        layerAttributes: { selectedStore: id }
+      }
+    ));
+
     dispatch(setBottomSheetOpen(true));
 
     return () => {
-      dispatch(updateLayer(LAYER_ID, { selectedStore: null }));
+      dispatch(updateLayer(
+        {
+          id: LAYER_ID,
+          layerAttributes: { selectedStore: null }
+        }
+      ));
       abortController.abort();
     };
   }, [dispatch, source, id, location.state]);

diff --git a/template-sample-app/template/src/config/initialStateSlice.js b/template-sample-app/template/src/config/initialStateSlice.js
@@ -24,6 +24,7 @@ export const oauthInitialState = {
     scopes: [
       'user:profile', // to load avatar photo
       'datasets:metadata', // to list all your datasets,
+      'datasets:r:*', // to read any of your datasets
       'dataservices:geocoding', // to use geocoding through Data Services API
       'dataservices:isolines', // to launch isochrones or isodistances through Data Services API
     ],