kids-first#3352 incorporate comments from code review

evans-g-crsj · Jul 1, 2021 · 72992c4 · 72992c4
1 parent 61e84a7
commit 72992c4
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 36 deletions.
diff --git a/src/store/SavedQueriesTypes.ts b/src/store/SavedQueriesTypes.ts
@@ -48,6 +48,7 @@ export enum SavedQueriesActions {
   failureFetchingAllSavedQueries = 'failureFetchingAllSavedQueries',
   failureDeletingSavedQuery = 'failureDeletingSavedQuery',
   addAllSavedQueries = 'addAllSavedQueries',
+  requestFetchAllSavedQueries = 'requestFetchAllSavedQueries',
 }
 
 type AddSavedQueriesAction = {
@@ -57,12 +58,12 @@ type AddSavedQueriesAction = {
 
 type FailureFetchingAllSavedQueriesAction = {
   type: SavedQueriesActions.failureFetchingAllSavedQueries;
-  error: Error | null;
+  error: Error;
 };
 
 type FailureDeletingSavedQueryAction = {
   type: SavedQueriesActions.failureDeletingSavedQuery;
-  error: Error | null;
+  error: Error;
   queryId: string;
 };
 
@@ -71,6 +72,10 @@ type ToggleLoadingAllSavedQueriesAction = {
   isLoadingAllQueries: boolean;
 };
 
+type RequestFetchAllSavedQueriesAction = {
+  type: SavedQueriesActions.requestFetchAllSavedQueries;
+};
+
 type FetchAllSavedQueriesAction = {
   type: SavedQueriesActions.fetchAllSavedQueries;
 };
@@ -83,7 +88,6 @@ type RequestDeleteSavedQueryAction = {
 type SuccessDeletingSavedQueryAction = {
   type: SavedQueriesActions.successDeletingSavedQuery;
   queryId: string;
-  queryType: QueryType;
 };
 
 export enum SavedQueriesStatuses {
@@ -105,6 +109,7 @@ export type SavedQueriesActionTypes =
   | FailureFetchingAllSavedQueriesAction
   | FailureDeletingSavedQueryAction
   | AddSavedQueriesAction
-  | SuccessDeletingSavedQueryAction;
+  | SuccessDeletingSavedQueryAction
+  | RequestFetchAllSavedQueriesAction;
 
 export type DispatchSavedQueries = ThunkDispatch<RootState, null, SavedQueriesActionTypes>;
diff --git a/src/store/actionCreators/SavedQueries.ts b/src/store/actionCreators/SavedQueries.ts
@@ -20,12 +20,12 @@ const toggleFetchAllQueriesLoading = (isLoadingAllQueries: boolean): SavedQuerie
   isLoadingAllQueries,
 });
 
-const addQueries = (queries: SplitSavedQueries): SavedQueriesActionTypes => ({
+const addSavedQueries = (queries: SplitSavedQueries): SavedQueriesActionTypes => ({
   type: SavedQueriesActions.addAllSavedQueries,
   queries,
 });
 
-const failureFetchingAllSavedQueries = (error: Error | null): SavedQueriesActionTypes => ({
+const failureFetchingAllSavedQueries = (error: Error): SavedQueriesActionTypes => ({
   type: SavedQueriesActions.failureFetchingAllSavedQueries,
   error,
 });
@@ -35,21 +35,21 @@ const requestDeleteSavedQueryAction = (queryId: string): SavedQueriesActionTypes
   queryId,
 });
 
-const successDeletingSavedQueryAction = (
-  queryId: string,
-  queryType: QueryType,
-): SavedQueriesActionTypes => ({
+const successDeletingSavedQueryAction = (queryId: string): SavedQueriesActionTypes => ({
   type: SavedQueriesActions.successDeletingSavedQuery,
   queryId,
-  queryType,
 });
 
-const failureDeletingQuery = (queryId: string, error: Error | null): SavedQueriesActionTypes => ({
+const failureDeletingSavedQuery = (queryId: string, error: Error): SavedQueriesActionTypes => ({
   type: SavedQueriesActions.failureDeletingSavedQuery,
   queryId,
   error,
 });
 
+const requestFetchAllSavedQueries = (): SavedQueriesActionTypes => ({
+  type: SavedQueriesActions.requestFetchAllSavedQueries,
+});
+
 export const deleteParticularSavedQuery = (
   api: Api,
   queryId: string,
@@ -62,18 +62,18 @@ export const deleteParticularSavedQuery = (
       dispatch(deleteVirtualStudy({ virtualStudyId: queryId, loggedInUser }));
     }
     await deleteSavedQuery(api, queryId);
-    dispatch(successDeletingSavedQueryAction(queryId, queryType));
+    dispatch(successDeletingSavedQueryAction(queryId));
   } catch (error) {
     console.error(error);
-    dispatch(failureDeletingQuery(queryId, error));
+    dispatch(failureDeletingSavedQuery(queryId, error));
   }
 };
 
 export const fetchSavedQueries = (
   api: Api,
   loggedInUser: LoggedInUser,
 ): ThunkAction<void, RootState, null, SavedQueriesActionTypes> => async (dispatch) => {
-  dispatch(toggleFetchAllQueriesLoading(true));
+  dispatch(requestFetchAllSavedQueries());
   try {
     const queriesFromRiff = await fetchAllSavedQueries(api, loggedInUser);
     const queriesVirtualStudies = await getVirtualStudies(api, loggedInUser.egoId);
@@ -90,7 +90,7 @@ export const fetchSavedQueries = (
     const cohortQueries = Array.isArray(queriesVirtualStudies)
       ? queriesVirtualStudies.map((vs) => ({ ...vs, id: vs.virtualStudyId }))
       : [];
-    dispatch(addQueries({ [QueryType.cohort]: cohortQueries, [QueryType.file]: fileQueries }));
+    dispatch(addSavedQueries({ [QueryType.cohort]: cohortQueries, [QueryType.file]: fileQueries }));
   } catch (e) {
     console.error(e);
     dispatch(failureFetchingAllSavedQueries(e));

diff --git a/src/store/reducers/SavedQueries.ts b/src/store/reducers/SavedQueries.ts
@@ -1,9 +1,12 @@
+import omit from 'lodash/omit';
+
 import {
   QueryType,
   SavedQueriesActions,
   SavedQueriesActionTypes,
   SavedQueriesState,
   SavedQueriesStatuses,
+  SplitSavedQueries,
 } from '../SavedQueriesTypes';
 
 const initialState: SavedQueriesState = {
@@ -13,46 +16,54 @@ const initialState: SavedQueriesState = {
   errorFetchAllQueries: null,
 };
 
+const removeGivenQueryFromQueries = (
+  queryIdToRemove: string,
+  queries: SplitSavedQueries,
+): SplitSavedQueries => {
+  const fileQueries = queries[QueryType.file].filter((q) => q.id !== queryIdToRemove);
+  const cohortQueries = queries[QueryType.cohort].filter((q) => q.id !== queryIdToRemove);
+  return { [QueryType.file]: fileQueries, [QueryType.cohort]: cohortQueries };
+};
+
 export default (state = initialState, action: SavedQueriesActionTypes): SavedQueriesState => {
   switch (action.type) {
     case SavedQueriesActions.toggleFetchAllSavedQueries: {
       return { ...state, isLoadingAllQueries: action.isLoadingAllQueries };
     }
+    case SavedQueriesActions.requestFetchAllSavedQueries: {
+      return { ...initialState, isLoadingAllQueries: true };
+    }
     case SavedQueriesActions.addAllSavedQueries: {
       return { ...state, queries: action.queries };
     }
     case SavedQueriesActions.failureFetchingAllSavedQueries: {
       return { ...state, errorFetchAllQueries: action.error };
     }
     case SavedQueriesActions.requestDeleteSavedQuery: {
-      const queryId = action.queryId;
-
-      const queryIdToStatusShallowCopy = { ...state.queryIdToStatus };
-      queryIdToStatusShallowCopy[queryId] = SavedQueriesStatuses.deleting;
-
-      return { ...state, queryIdToStatus: queryIdToStatusShallowCopy };
+      return {
+        ...state,
+        queryIdToStatus: {
+          ...state.queryIdToStatus,
+          [action.queryId]: SavedQueriesStatuses.deleting,
+        },
+      };
     }
     case SavedQueriesActions.successDeletingSavedQuery: {
       const queryId = action.queryId;
-      const queryType = action.queryType;
-
-      const queryIdToStatusShallowCopy = { ...state.queryIdToStatus };
-      delete queryIdToStatusShallowCopy[queryId];
-
-      const queriesShallowCopy = { ...state.queries };
-      const queriesForTypeShallowCopy = [...queriesShallowCopy[queryType]];
-      const filteredQueriesForType = queriesForTypeShallowCopy.filter((q) => q.id !== queryId);
       return {
         ...state,
-        queries: { ...queriesShallowCopy, [queryType]: filteredQueriesForType },
-        queryIdToStatus: queryIdToStatusShallowCopy,
+        queries: removeGivenQueryFromQueries(queryId, state.queries),
+        queryIdToStatus: omit(state.queryIdToStatus, [queryId]),
       };
     }
     case SavedQueriesActions.failureDeletingSavedQuery: {
-      const queryId = action.queryId;
-      const queryIdToStatusShallowCopy = { ...state.queryIdToStatus };
-      queryIdToStatusShallowCopy[queryId] = SavedQueriesStatuses.error;
-      return { ...state, queryIdToStatus: queryIdToStatusShallowCopy };
+      return {
+        ...state,
+        queryIdToStatus: {
+          ...state.queryIdToStatus,
+          [action.queryId]: SavedQueriesStatuses.error,
+        },
+      };
     }
     default:
       return state;