From 72992c41f19b123132230f2b6f48e77d7b9ff323 Mon Sep 17 00:00:00 2001 From: evans-g-crsj Date: Thu, 1 Jul 2021 10:59:58 -0400 Subject: [PATCH] #3352 incorporate comments from code review --- src/store/SavedQueriesTypes.ts | 13 ++++-- src/store/actionCreators/SavedQueries.ts | 24 +++++------ src/store/reducers/SavedQueries.ts | 51 ++++++++++++++---------- 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/src/store/SavedQueriesTypes.ts b/src/store/SavedQueriesTypes.ts index 285d8e248..e63352a93 100644 --- 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; diff --git a/src/store/actionCreators/SavedQueries.ts b/src/store/actionCreators/SavedQueries.ts index e3a277962..71074ab15 100644 --- 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,10 +62,10 @@ 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)); } }; @@ -73,7 +73,7 @@ export const fetchSavedQueries = ( api: Api, loggedInUser: LoggedInUser, ): ThunkAction => 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 index 7c20c86d7..ba0587107 100644 --- 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,11 +16,23 @@ 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 }; } @@ -25,34 +40,30 @@ export default (state = initialState, action: SavedQueriesActionTypes): SavedQue 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;