refactor: implement PR feedback until March 16, 2022

dhis2 · Apr 4, 2022 · aac9c87 · aac9c87
1 parent 98de59f
commit aac9c87
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 52 deletions.
diff --git a/src/context-selection/org-unit-selector-bar-item/get-offline-levels-to-load.js b/src/context-selection/org-unit-selector-bar-item/get-offline-levels-to-load.js
@@ -13,18 +13,18 @@ export default function getOfflineLevelsToLoad({
     organisationUnitLevels,
     userOrganisationUnits,
 }) {
-    return userOrganisationUnits.reduce((acc, userOrgUnit) => {
-        const foundLevel = organisationUnitLevels.find(
-            (orgUnitLevel) => orgUnitLevel.level === userOrgUnit.level
-        )
+    return userOrganisationUnits
+        .map((userOrgUnit) => {
+            const foundLevel = organisationUnitLevels.find(
+                (orgUnitLevel) => orgUnitLevel.level === userOrgUnit.level
+            )
 
-        if (!foundLevel) {
-            return acc
-        }
-
-        return [
-            ...acc,
-            { id: userOrgUnit.id, offlineLevels: foundLevel.offlineLevels },
-        ]
-    }, [])
+            if (foundLevel) {
+                return {
+                    id: userOrgUnit.id,
+                    offlineLevels: foundLevel.offlineLevels,
+                }
+            }
+        })
+        .filter((value) => !!value)
 }
diff --git a/src/context-selection/org-unit-selector-bar-item/load-offline-level.js b/src/context-selection/org-unit-selector-bar-item/load-offline-level.js
@@ -32,14 +32,16 @@ export default async function loadOfflineLevel({
     const { children: childrenSize } = orgData.orgUnit
     const nextOfflineLevels = offlineLevels - 1
 
-    let children
-    if (childrenSize) {
-        const orgChildren = await dataEngine.query(QUERY_ORG_CHILDREN_FROM_UI, {
-            variables,
-        })
-        children = orgChildren.orgUnit.children
+    if (!childrenSize) {
+        return
     }
 
+    const orgChildren = await dataEngine.query(QUERY_ORG_CHILDREN_FROM_UI, {
+        variables,
+    })
+
+    const children = orgChildren.orgUnit.children
+
     if (
         children &&
         // nextOfflineLevels = 0 -> Load the org unit but not its children

diff --git a/src/context-selection/org-unit-selector-bar-item/use-load-offline-levels.js b/src/context-selection/org-unit-selector-bar-item/use-load-offline-levels.js
@@ -4,40 +4,50 @@ import loadOfflineLevel from './load-offline-level.js'
 import useOfflineLevelsToLoad from './use-offline-levels-to-load.js'
 import useOrganisationUnitLevels from './use-organisation-unit-levels.js'
 
+const loadAllOfflineLevels = ({
+    dataEngine,
+    offlineLevelsToLoadData,
+    onDone,
+}) => {
+    const allRequests = offlineLevelsToLoadData.map((offlineLevelToLoad) =>
+        loadOfflineLevel({
+            dataEngine,
+            ...offlineLevelToLoad,
+        })
+    )
+
+    Promise.all(allRequests).finally(() => {
+        // @TODO: Think about what should happen here
+        // Do we want to notify the user about success?
+        // What do we want to display when an error occurs?
+        onDone()
+    })
+}
+
 /**
  * As the service worker caches request responses,
  * it should suffice to simply perform all request the org unit tree would
  * perform as well in advance
  */
 export default function useLoadOfflineLevels() {
-    const [loading, setLoading] = useState(true)
-    const [error, setError] = useState(undefined)
-    const [data, setData] = useState(false)
-
+    const [done, setDone] = useState(false)
     const dataEngine = useDataEngine()
     const organisationUnitLevels = useOrganisationUnitLevels()
-    const offlineLevelsToLoad = useOfflineLevelsToLoad(organisationUnitLevels)
+    const offlineLevelsToLoad = useOfflineLevelsToLoad(
+        organisationUnitLevels.data
+    )
     const { data: offlineLevelsToLoadData } = offlineLevelsToLoad
 
     useEffect(() => {
         // Can't pass async function to useEffect
-        offlineLevelsToLoadData &&
-            (async () => {
-                await Promise.all(
-                    offlineLevelsToLoadData.map((offlineLevelToLoad) => {
-                        const payload = {
-                            dataEngine,
-                            ...offlineLevelToLoad,
-                        }
-
-                        return loadOfflineLevel(payload)
-                    })
-                )
-                    .then(() => setData(true))
-                    .catch(setError)
-                    .finally(() => setLoading(false))
-            })()
-    }, [dataEngine, offlineLevelsToLoadData, setLoading, setData])
+        if (offlineLevelsToLoadData) {
+            loadAllOfflineLevels({
+                dataEngine,
+                offlineLevelsToLoadData,
+                onDone: () => setDone(true),
+            })
+        }
+    }, [dataEngine, offlineLevelsToLoadData, setDone])
 
-    return { loading, error, data }
+    return done
 }
diff --git a/src/context-selection/org-unit-selector-bar-item/use-load-offline-levels.test.js b/src/context-selection/org-unit-selector-bar-item/use-load-offline-levels.test.js
@@ -34,6 +34,8 @@ describe('useLoadOfflineLevels', () => {
             { offlineLevels: 1, id: 'bar' },
         ],
     }
+
+    useOrganisationUnitLevels.mockImplementation(() => ({}))
     useOfflineLevelsToLoad.mockImplementation(() => offlineLevelToLoad)
 
     afterEach(() => {
@@ -45,6 +47,7 @@ describe('useLoadOfflineLevels', () => {
     it('should call "loadOfflineLevel" for each offline level to load', async () => {
         const { result, waitForNextUpdate } = renderHook(useLoadOfflineLevels)
 
+        expect(result.current).toBe(false)
         await waitForNextUpdate()
 
         expect(loadOfflineLevel).toHaveBeenCalledTimes(2)
@@ -59,10 +62,6 @@ describe('useLoadOfflineLevels', () => {
             id: 'bar',
         })
 
-        expect(result.current).toEqual({
-            loading: false,
-            error: undefined,
-            data: true,
-        })
+        expect(result.current).toBe(true)
     })
 })
diff --git a/src/context-selection/org-unit-selector-bar-item/use-offline-levels-to-load.js b/src/context-selection/org-unit-selector-bar-item/use-offline-levels-to-load.js
@@ -12,14 +12,12 @@ export default function useOfflineLevelsToLoad(organisationUnitLevels) {
     ]
 
     return useQuery(queryKey, {
-        enable:
-            organisationUnitLevels.data &&
-            // Only fetch the user's org units when there are any offline levels
-            organisationUnitLevels.data.length,
+        // Only fetch the user's org units when there are any offline levels
+        enable: organisationUnitLevels?.length,
         select: ({ organisationUnits: userOrganisationUnits }) => {
             return getOfflineLevelsToLoad({
                 userOrganisationUnits,
-                organisationUnitLevels: organisationUnitLevels.data,
+                organisationUnitLevels,
             })
         },
     })