fix(dashboard): ensure fresh store state when merging data

We were merging API responses to a stale version of the store object. That got merged to the global store state, causing some changes to be lost or overwritten.
garden-io · Sep 24, 2019 · bf5b5d0 · bf5b5d0
1 parent ea25f30
commit bf5b5d0
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 100 deletions.
diff --git a/dashboard/src/containers/entity-result.tsx b/dashboard/src/containers/entity-result.tsx
@@ -47,15 +47,15 @@ interface Props {
  */
 export default ({ name, moduleName, type, onClose }: Props) => {
   const {
-    actions: { loadTestResult, loadTaskResult },
+    actions,
     store: { entities: { tasks, tests }, requestStates: { fetchTestResult, fetchTaskResult } },
   } = useApi()
 
   const loadResults = () => {
     if (type === "test") {
-      loadTestResult({ name, moduleName, force: true })
+      actions.loadTestResult({ name, moduleName, force: true })
     } else if (type === "run" || type === "task") {
-      loadTaskResult({ name, force: true })
+      actions.loadTaskResult({ name, force: true })
     }
   }
 

diff --git a/dashboard/src/containers/graph.tsx b/dashboard/src/containers/graph.tsx
@@ -36,34 +36,34 @@ export interface GraphOutputWithNodeStatus extends GraphOutput {
 
 export default () => {
   const {
-    actions: { loadGraph, loadConfig },
+    actions,
     store: { entities: { modules, services, tests, tasks, graph }, requestStates: { fetchGraph, fetchTaskStates } },
   } = useApi()
 
+  const {
+    actions: { selectGraphNode, stackGraphToggleItemsView, clearGraphNodeSelection },
+    state: { selectedGraphNode, isSidebarOpen, stackGraph: { filters } },
+  } = useUiState()
+
   useEffect(() => {
     async function fetchData() {
-      return await loadConfig()
+      return await actions.loadConfig()
     }
     fetchData()
   }, [])
 
   useEffect(() => {
     async function fetchData() {
-      return await loadGraph()
+      return await actions.loadGraph()
     }
     fetchData()
   }, [])
 
-  const {
-    actions: { selectGraphNode, stackGraphToggleItemsView, clearGraphNodeSelection },
-    state: { selectedGraphNode, isSidebarOpen, stackGraph: { filters } },
-  } = useUiState()
-
   if (fetchGraph.error) {
     return <PageError error={fetchGraph.error} />
   }
 
-  if (fetchGraph.loading) {
+  if (!fetchGraph.initLoadComplete) {
     return <Spinner />
   }
 

diff --git a/dashboard/src/containers/logs.tsx b/dashboard/src/containers/logs.tsx
@@ -15,30 +15,30 @@ import Spinner from "../components/spinner"
 
 export default () => {
   const {
-    actions: { loadConfig, loadLogs },
+    actions,
     store: { entities: { logs, services }, requestStates: { fetchLogs, fetchConfig } },
   } = useApi()
 
   const serviceNames: string[] = Object.keys(services)
 
   useEffect(() => {
     async function fetchData() {
-      return await loadConfig()
+      return await actions.loadConfig()
     }
     fetchData()
   }, [])
 
   useEffect(() => {
     async function fetchData() {
-      return await loadLogs({ serviceNames })
+      return await actions.loadLogs({ serviceNames })
     }
 
     if (serviceNames.length) {
       fetchData()
     }
-  }, [fetchConfig.didFetch]) // run again only after config had been fetched
+  }, [fetchConfig.initLoadComplete]) // run again only after config had been fetched
 
-  if (fetchConfig.loading || fetchLogs.loading) {
+  if (!(fetchConfig.initLoadComplete && fetchLogs.initLoadComplete)) {
     return <Spinner />
   }
 
@@ -49,6 +49,6 @@ export default () => {
   }
 
   return (
-    <Logs onRefresh={loadLogs} logs={logs} />
+    <Logs onRefresh={actions.loadLogs} logs={logs} />
   )
 }
diff --git a/dashboard/src/containers/overview.tsx b/dashboard/src/containers/overview.tsx
@@ -77,12 +77,12 @@ const mapTasks = (taskEntities: Task[], moduleName: string): ModuleProps["taskCa
 
 export default () => {
   const {
+    actions,
     store: {
       projectRoot,
       entities: { modules, services, tests, tasks },
       requestStates: { fetchConfig, fetchStatus },
     },
-    actions: { loadConfig, loadStatus },
   } = useApi()
 
   const {
@@ -94,18 +94,9 @@ export default () => {
     },
   } = useUiState()
 
-  // TODO use useAsyncEffect?
-  // https://dev.to/n1ru4l/homebrew-react-hooks-useasynceffect-or-how-to-handle-async-operations-with-useeffect-1fa8
   useEffect(() => {
     async function fetchData() {
-      return await loadConfig()
-    }
-    fetchData()
-  }, [])
-
-  useEffect(() => {
-    async function fetchData() {
-      return await loadStatus()
+      return await actions.loadConfig()
     }
     fetchData()
   }, [])
@@ -118,7 +109,10 @@ export default () => {
     return <PageError error={fetchConfig.error || fetchStatus.error} />
   }
 
-  if (fetchConfig.loading || fetchStatus.loading) {
+  // Note that we don't call the loadStatus function here since the Sidebar ensures that the status is always loaded.
+  // FIXME: We should be able to call loadStatus safely and have the handler check if the status
+  // has already been fetched or is pending.
+  if (!(fetchConfig.initLoadComplete && fetchStatus.initLoadComplete)) {
     return <Spinner />
   }
 

diff --git a/dashboard/src/containers/sidebar.tsx b/dashboard/src/containers/sidebar.tsx
@@ -43,13 +43,13 @@ const builtinPages: Page[] = [
 
 const SidebarContainer = () => {
   const {
-    actions: { loadStatus },
+    actions,
     store: { entities: { providers } },
   } = useApi()
 
   useEffect(() => {
     async function fetchData() {
-      return await loadStatus()
+      return await actions.loadStatus()
     }
     fetchData()
   }, [])

diff --git a/dashboard/src/contexts/api-handlers.ts b/dashboard/src/contexts/api-handlers.ts
@@ -53,7 +53,7 @@ interface LoadHandlerParams {
 export async function loadConfigHandler({ store, dispatch, force = false }: LoadHandlerParams) {
   const requestKey = "fetchConfig"
 
-  if (!force && store.requestStates[requestKey].didFetch) {
+  if (!force && store.requestStates[requestKey].initLoadComplete) {
     return
   }
 
@@ -66,8 +66,9 @@ export async function loadConfigHandler({ store, dispatch, force = false }: Load
     return
   }
 
-  const processedStore = processConfig(store, res)
-  dispatch({ store: processedStore, type: "fetchSuccess", requestKey })
+  const produceNextStore = (currentStore: Store) => processConfig(currentStore, res)
+
+  dispatch({ type: "fetchSuccess", requestKey, produceNextStore })
 }
 
 function processConfig(store: Store, config: ConfigDump) {
@@ -111,23 +112,21 @@ function processConfig(store: Store, config: ConfigDump) {
     }
   }
 
-  const processedStore = produce(store, storeDraft => {
+  return produce(store, storeDraft => {
     storeDraft.entities.modules = modules
     storeDraft.entities.services = services
     storeDraft.entities.tests = tests
     storeDraft.entities.tasks = tasks
     storeDraft.projectRoot = config.projectRoot
   })
-
-  return processedStore
 }
 
 interface LoadLogsHandlerParams extends LoadHandlerParams, FetchLogsParams { }
 
 export async function loadLogsHandler({ serviceNames, store, dispatch, force = false }: LoadLogsHandlerParams) {
   const requestKey = "fetchLogs"
 
-  if ((!force && store.requestStates[requestKey].didFetch) || !serviceNames.length) {
+  if ((!force && store.requestStates[requestKey].initLoadComplete) || !serviceNames.length) {
     return
   }
   dispatch({ requestKey, type: "fetchStart" })
@@ -140,7 +139,9 @@ export async function loadLogsHandler({ serviceNames, store, dispatch, force = f
     return
   }
 
-  dispatch({ store: processLogs(store, res), type: "fetchSuccess", requestKey })
+  const produceNextStore = (currentStore: Store) => processLogs(currentStore, res)
+
+  dispatch({ type: "fetchSuccess", requestKey, produceNextStore })
 }
 
 function processLogs(store: Store, logs: ServiceLogEntry[]) {
@@ -152,7 +153,7 @@ function processLogs(store: Store, logs: ServiceLogEntry[]) {
 export async function loadStatusHandler({ store, dispatch, force = false }: LoadHandlerParams) {
   const requestKey = "fetchStatus"
 
-  if (!force && store.requestStates[requestKey].didFetch) {
+  if (!force && store.requestStates[requestKey].initLoadComplete) {
     return
   }
 
@@ -166,11 +167,13 @@ export async function loadStatusHandler({ store, dispatch, force = false }: Load
     return
   }
 
-  dispatch({ store: processStatus(store, res), type: "fetchSuccess", requestKey })
+  const produceNextStore = (currentStore: Store) => processStatus(currentStore, res)
+
+  dispatch({ type: "fetchSuccess", requestKey, produceNextStore })
 }
 
 function processStatus(store: Store, status: StatusCommandResult) {
-  const processedStore = produce(store, storeDraft => {
+  return produce(store, storeDraft => {
     for (const serviceName of Object.keys(status.services)) {
       storeDraft.entities.services[serviceName] = {
         ...storeDraft.entities.services[serviceName],
@@ -191,8 +194,6 @@ function processStatus(store: Store, status: StatusCommandResult) {
     }
     storeDraft.entities.providers = status.providers
   })
-
-  return processedStore
 }
 
 interface LoadTaskResultHandlerParams extends LoadHandlerParams, FetchTaskResultParams { }
@@ -202,7 +203,7 @@ export async function loadTaskResultHandler(
 ) {
   const requestKey = "fetchTaskResult"
 
-  if (!force && store.requestStates[requestKey].didFetch) {
+  if (!force && store.requestStates[requestKey].initLoadComplete) {
     return
   }
 
@@ -216,7 +217,9 @@ export async function loadTaskResultHandler(
     return
   }
 
-  dispatch({ store: processTaskResult(store, res), type: "fetchSuccess", requestKey })
+  const produceNextStore = (currentStore: Store) => processTaskResult(currentStore, res)
+
+  dispatch({ type: "fetchSuccess", requestKey, produceNextStore })
 }
 
 function processTaskResult(store: Store, result: TaskResultOutput) {
@@ -232,7 +235,7 @@ interface LoadTestResultParams extends LoadHandlerParams, FetchTestResultParams
 export async function loadTestResultHandler({ store, dispatch, force = false, ...fetchParams }: LoadTestResultParams) {
   const requestKey = "fetchTestResult"
 
-  if (!force && store.requestStates[requestKey].didFetch) {
+  if (!force && store.requestStates[requestKey].initLoadComplete) {
     return
   }
 
@@ -246,7 +249,9 @@ export async function loadTestResultHandler({ store, dispatch, force = false, ..
     return
   }
 
-  dispatch({ store: processTestResult(store, res), type: "fetchSuccess", requestKey })
+  const produceNextStore = (currentStore: Store) => processTestResult(currentStore, res)
+
+  dispatch({ type: "fetchSuccess", requestKey, produceNextStore })
 }
 
 function processTestResult(store: Store, result: TestResultOutput) {
@@ -260,7 +265,7 @@ function processTestResult(store: Store, result: TestResultOutput) {
 export async function loadGraphHandler({ store, dispatch, force = false }: LoadHandlerParams) {
   const requestKey = "fetchGraph"
 
-  if (!force && store.requestStates[requestKey].didFetch) {
+  if (!force && store.requestStates[requestKey].initLoadComplete) {
     return
   }
 
@@ -274,7 +279,9 @@ export async function loadGraphHandler({ store, dispatch, force = false }: LoadH
     return
   }
 
-  dispatch({ store: processGraph(store, res), type: "fetchSuccess", requestKey })
+  const produceNextStore = (currentStore: Store) => processGraph(currentStore, res)
+
+  dispatch({ type: "fetchSuccess", requestKey, produceNextStore })
 }
 
 function processGraph(store: Store, graph: GraphOutput) {