Improve loading states and fix authentication data fetching (#48)

* feat: replace redux-localstorage with redux-persist

- Update store configuration to use redux-persist instead of redux-localstorage
- Add PersistGate wrapper component for state rehydration
- Configure whitelist-based persistence for better control over persisted state

* feat: add loading state management for data fetching

- Add loading state tracking to Redux store with initial states for unit/service
- Implement GET_RESOURCE_START action to set loading states to true
- Update dataReducer to handle loading state transitions and normalize state structure
- Modify Dashboard component to use loading state instead of checking for undefined data
- Add setResourceFetchStart action dispatch in Main component before fetch operations

* fix: reset geolocation stage after login to prevent stuck state

- Add reset parameter to getInitialLocation() to reset currentStage
- Fix bug where geolocation wouldn't work after login due to persistent currentStage
- Ensure getNearestUnits() is called properly on subsequent app sessions
- Pass reset=true explicitly when calling getInitialLocation in Main component

* fix: ensure user authentication before data fetching in AppComponent

* fix: remove
unnecessary import

Author: jorilindell

package-lock.json

@@ -42,7 +42,7 @@
     "react-router-dom": "^6.23.1",
     "redux": "^5.0.1",
     "redux-actions": "^3.0.0",
-    "redux-localstorage": "^0.4.1",
+    "redux-persist": "^6.0.0",
     "redux-promise": "^0.6.0",
     "sass": "^1.77.4",
     "sass-loader": "^14.2.1",

package.json

@@ -2,6 +2,12 @@ import _ from 'lodash';
 import { createAction } from 'redux-actions';
 import * as ApiClient from '../lib/municipal-services-client.js';
 
+export const setResourceFetchStart = createAction(
+  'GET_RESOURCE_START',
+  (resourceType) => resourceType,
+  (resourceType) => ({ resourceType })
+);
+
 export const fetchUnitsWithServices = createAction(
   'GET_RESOURCE',
   ApiClient.fetchUnitsWithServices,

src/actions/index.js

@@ -8,7 +8,7 @@ import { UnitListElement } from './UnitList';
 class DashBoard extends React.Component {
 
   render() {
-    if (this.props.nearest === undefined || this.props.frequent === undefined) {
+    if (this.props.loading) {
       return <div>Ladataan...</div>;
     }
     let nearest;
@@ -48,14 +48,18 @@ function topUnits(n, units, unitsByUpdateCount) {
 function mapStateToProps(state) {
   if (_.keys(state.data.unit).length > 0) {
     return {
+      loading: state.data.loading.unit,
       nearest: _.map(state.data.unitsByDistance, (u) => {
         return state.data.unit[u.id];
       }),
       frequent: topUnits(20, state.data.unit, state.unitsByUpdateCount),
       userLocation: state.userLocation
     };
   }
-  return {};
+  return {
+    loading: state.data.loading.unit,
+    userLocation: state.userLocation
+  };
 }
 
 function mapDispatchToProps() {

src/components/DashBoard.js

@@ -6,7 +6,7 @@ import { connect } from 'react-redux';
 import { Link, Outlet } from 'react-router-dom';
 import _ from 'lodash';
 
-import { fetchUnitsWithServices, fetchResource, setUserLocation, getNearestUnits } from '../actions/index';
+import { fetchUnitsWithServices, fetchResource, setUserLocation, getNearestUnits, setResourceFetchStart } from '../actions/index';
 import { getCurrentSeason } from './utils';
 import { getInitialLocation } from '../lib/geolocation';
 import { withRouter } from '../hooks/index';
@@ -28,12 +28,20 @@ function requireAuth(navigate, auth) {
 class AppComponent extends React.Component {
   componentDidMount() {
     let { navigate, auth } = this.props;
-    requireAuth(navigate, auth);
+    
+    // Only proceed with data fetching if user is authenticated
+    if (!hasAuth(auth)) {
+      requireAuth(navigate, auth);
+      return;
+    }
+    
     const services = constants.SERVICE_GROUPS[this.props.serviceGroup].services;
+    this.props.setResourceFetchStart('service');
     this.props.fetchResource(
       'service', { id: services.join(',') },
       ['id', 'name'], ['observable_properties']
     );
+    this.props.setResourceFetchStart('unit');
     this.props.fetchUnitsWithServices(
       services, this.props.maintenanceOrganization, {
         selected: ['id', 'name', 'services', 'location', 'extensions'],
@@ -43,7 +51,7 @@ class AppComponent extends React.Component {
       if (services) {
         this.props.getNearestUnits(position, services, this.props.maintenanceOrganization);
       }
-    });
+    }, true);
   }
 
   render() {
@@ -113,6 +121,9 @@ const mapDispatchToProps = (dispatch) => {
     },
     getNearestUnits: (...args) => {
       dispatch(getNearestUnits(...args));
+    },
+    setResourceFetchStart: (resourceType) => {
+      dispatch(setResourceFetchStart(resourceType));
     }
   };
 };

src/components/Main.js

@@ -6,7 +6,9 @@ import { createMemoryHistory } from 'history';
 import { compose, applyMiddleware, legacy_createStore as createStore } from 'redux';
 import { Provider } from 'react-redux';
 import promiseMiddleware from 'redux-promise';
-import persistState from 'redux-localstorage';
+import { persistStore, persistReducer } from 'redux-persist';
+import storage from 'redux-persist/lib/storage'; // localStorage
+import { PersistGate } from 'redux-persist/integration/react';
 import 'bootstrap-sass';
 
 import queueHandler from './actions/queueHandler';
@@ -36,15 +38,38 @@ import isomorphicFetch from 'isomorphic-fetch';
 isomorphicFetch;
 moment.locale('fi');
 
-const stateToPersist = ['data', 'auth', 'updateQueue', 'unitsByUpdateTime', 'unitsByUpdateCount', 'serviceGroup'];
+const persistConfig = {
+  key: 'root',
+  storage,
+  whitelist: ['auth', 'updateQueue', 'unitsByUpdateTime', 'unitsByUpdateCount', 'serviceGroup'], // Only persist these
+  transforms: [
+    // Transform to exclude loading from data if we include data
+    {
+      in: (state, key) => {
+        if (key === 'data' && state) {
+          // eslint-disable-next-line no-unused-vars
+          const { loading, ...stateWithoutLoading } = state;
+          return stateWithoutLoading;
+        }
+        return state;
+      },
+      // eslint-disable-next-line no-unused-vars
+      out: (state, key) => state
+    }
+  ]
+};
 
-const finalCreateStore = compose(
-  applyMiddleware(promiseMiddleware),
-  persistState(stateToPersist))(createStore);
+const persistedReducer = persistReducer(persistConfig, rootReducer);
 
-const enableReduxDevTools = window.__REDUX_DEVTOOLS_EXTENSION__ && window.__REDUX_DEVTOOLS_EXTENSION__();
+const store = createStore(
+  persistedReducer,
+  compose(
+    applyMiddleware(promiseMiddleware),
+    window.__REDUX_DEVTOOLS_EXTENSION__ && window.__REDUX_DEVTOOLS_EXTENSION__()
+  )
+);
 
-const store = finalCreateStore(rootReducer, enableReduxDevTools);
+const persistor = persistStore(store);
 window.store = store;
 
 const rootElement = document.getElementById('app');
@@ -53,24 +78,26 @@ const root = createRoot(rootElement);
 // Render the main component into the dom
 root.render(
   <Provider store={store}>
-    <BrowserRouter history={createMemoryHistory()}>
-      <Routes>
-        <Route path="/login" element={<LoginScreen />} />
-        <Route path="/" element={<App />}>
-            <Route exact path="/" element={<DashBoard />} />
-            <Route path="/group" element={<GroupList />} />
-            <Route path="/group/:groupId" element={<UnitList />} />
-            <Route path="/group/:groupId/mass-edit" element={<UnitMassEditPropertySelect />} />
-            <Route path="/group/:groupId/mass-edit/:propertyId" element={<UnitMassEdit />} />
-            <Route path="/unit/:unitId" element={<UnitDetails />} />
-            <Route path="/unit/:unitId/history" element={<UnitHistory />} />
-            <Route path="/unit/:unitId/update/:propertyId/:valueId" element={<UpdateConfirmation />} />
-            <Route path="/unit/:unitId/delete/:propertyId" element={<DeleteConfirmation />} />
-            <Route path="/queue" element={<UpdateQueue />} />
-            <Route path="*" element={<NotFound />} />
-        </Route>
-      </Routes>
-    </BrowserRouter>
+    <PersistGate loading={<div>Ladataan...</div>} persistor={persistor}>
+      <BrowserRouter history={createMemoryHistory()}>
+        <Routes>
+          <Route path="/login" element={<LoginScreen />} />
+          <Route path="/" element={<App />}>
+              <Route exact path="/" element={<DashBoard />} />
+              <Route path="/group" element={<GroupList />} />
+              <Route path="/group/:groupId" element={<UnitList />} />
+              <Route path="/group/:groupId/mass-edit" element={<UnitMassEditPropertySelect />} />
+              <Route path="/group/:groupId/mass-edit/:propertyId" element={<UnitMassEdit />} />
+              <Route path="/unit/:unitId" element={<UnitDetails />} />
+              <Route path="/unit/:unitId/history" element={<UnitHistory />} />
+              <Route path="/unit/:unitId/update/:propertyId/:valueId" element={<UpdateConfirmation />} />
+              <Route path="/unit/:unitId/delete/:propertyId" element={<DeleteConfirmation />} />
+              <Route path="/queue" element={<UpdateQueue />} />
+              <Route path="*" element={<NotFound />} />
+          </Route>
+        </Routes>
+      </BrowserRouter>
+    </PersistGate>
   </Provider>
 );
 

src/index.js

@@ -37,7 +37,7 @@ function createSuccessCallback(callback) {
   return (position) => {
     currentStage++;
     if (currentStage < _.size(POSITION_OPTIONS) - 1) {
-      getInitialLocation(callback);
+      getInitialLocation(callback, false); // Don't reset on subsequent calls
     }
     callback(position);
   };
@@ -46,10 +46,15 @@ function createSuccessCallback(callback) {
 function error() {
 }
 
-export function getInitialLocation(callback) {
+export function getInitialLocation(callback, reset = true) {
   if (DISABLED) {
     return;
   }
+  // Reset stage for new geolocation session
+  if (reset) {
+    currentStage = 0;
+  }
+  
   const options = POSITION_OPTIONS[STAGES[currentStage]];
   if (options) {
     navigator.geolocation.getCurrentPosition(createSuccessCallback(callback), error, options);

src/lib/geolocation.js

@@ -8,7 +8,8 @@ const initialDataState = {
   unitsByDistance: [],
   observable_property: {},
   observation: {},
-  service: {}
+  service: {},
+  loading: {}
 };
 
 const initialAuthState = {
@@ -27,21 +28,37 @@ const serviceGroup = 'skiing';
 
 function dataReducer(state = initialDataState, action) {
   switch (action.type) {
+    case 'GET_RESOURCE_START':
+      const startResourceType = action.meta.resourceType;
+      return {
+        ...state,
+          loading: {
+            ...state.loading,
+            [startResourceType]: true
+          }
+      };
     case 'GET_RESOURCE':
       const resourceType = action.meta.resourceType;
       let existingResources = state[resourceType];
       if (action.meta.replaceAll === true) {
         existingResources = {};
       }
-      return Object.assign(
-        {}, state,
-        {[resourceType]: Object.assign(
-          existingResources,
-          action.payload[resourceType])});
+      return {
+        ...state,
+        [resourceType]: {
+          ...existingResources,
+          ...action.payload[resourceType]
+        },
+        loading: {
+          ...state.loading,
+          [resourceType]: false
+        }
+      };
     case 'GET_NEAREST_UNITS':
-      return Object.assign(
-        {}, state,
-        {unitsByDistance: action.payload});
+      return {
+        ...state,
+        unitsByDistance: action.payload
+      };
   }
   return state;
 }

src/reducers/index.js

@@ -6300,10 +6300,10 @@ redux-actions@^3.0.0:
     just-curry-it "5.3.0"
     reduce-reducers "1.0.4"
 
-redux-localstorage@^0.4.1:
-  version "0.4.1"
-  resolved "https://registry.yarnpkg.com/redux-localstorage/-/redux-localstorage-0.4.1.tgz#faf6d719c581397294d811473ffcedee065c933c"
-  integrity sha512-dUha0YoH+BSZ2q15pakB+JWeqiuXUf3Ir4rObOpNrZ96HEdciGAjkL10k3KGdLI7qvQw/c096asw/SQ6TPjU/A==
+redux-persist@^6.0.0:
+  version "6.0.0"
+  resolved "https://registry.yarnpkg.com/redux-persist/-/redux-persist-6.0.0.tgz#b4d2972f9859597c130d40d4b146fecdab51b3a8"
+  integrity sha512-71LLMbUq2r02ng2We9S215LtPu3fY0KgaGE0k8WRgl6RkqxtGfl7HUozz1Dftwsb0D/5mZ8dwAaPbtnzfvbEwQ==
 
 redux-promise@^0.6.0:
   version "0.6.0"