diff --git a/src/main/webapp/app/pages/curation/review/ReviewPage.tsx b/src/main/webapp/app/pages/curation/review/ReviewPage.tsx index 6907cc72f..ec611078b 100644 --- a/src/main/webapp/app/pages/curation/review/ReviewPage.tsx +++ b/src/main/webapp/app/pages/curation/review/ReviewPage.tsx @@ -47,14 +47,18 @@ const ReviewPage: React.FunctionComponent = (props: IReviewPag const [editorsToAcceptChangesFrom, setEditorsToAcceptChangesFrom] = useState([]); const [isAcceptingAll, setIsAcceptingAll] = useState(false); - const fetchFirebaseData = () => { + const [isAccepting, setIsAccepting] = useState(false); + + const fetchFirebaseData = async () => { if (!props.firebaseDb) { return; } // Fetch the data when the user enters review mode. We don't use a listener // because there shouldn't be another user editing the gene when it is being reviewed. - get(ref(props.firebaseDb, firebaseGenePath)).then(snapshot => setGeneData(snapshot.val())); - get(ref(props.firebaseDb, firebaseMetaReviewPath)).then(snapshot => setMetaReview(snapshot.val())); + const geneDataSnapshot = await get(ref(props.firebaseDb, firebaseGenePath)); + setGeneData(geneDataSnapshot.val()); + const metaReviewSnapshot = await get(ref(props.firebaseDb, firebaseMetaReviewPath)); + setMetaReview(metaReviewSnapshot.val()); }; useEffect(() => { @@ -105,7 +109,7 @@ const ReviewPage: React.FunctionComponent = (props: IReviewPag try { setIsAcceptingAll(true); await props.acceptReviewChangeHandler?.(hugoSymbol ?? '', reviewLevels, isGermline, true); - fetchFirebaseData(); + await fetchFirebaseData(); } catch (error) { notifyError(error); } finally { @@ -190,10 +194,20 @@ const ReviewPage: React.FunctionComponent = (props: IReviewPag hugoSymbol={hugoSymbol ?? ''} isGermline={isGermline} baseReviewLevel={rootReview} - handleAccept={props.acceptReviewChangeHandler} + handleAccept={async (hugoArg, reviewLevelsArg, isGermlineArg, isAcceptAllArg) => { + setIsAccepting(true); + try { + const returnVal = await props.acceptReviewChangeHandler?.(hugoArg, reviewLevelsArg, isGermlineArg, isAcceptAllArg); + if (returnVal?.shouldRefresh) { + await fetchFirebaseData(); + } + } finally { + setIsAccepting(false); + } + }} handleReject={props.rejectReviewChangeHandler} handleCreateAction={props.createActionHandler} - disableActions={isAcceptingAll} + disableActions={isAcceptingAll || isAccepting} isRoot={true} firebase={{ path: getGenePathFromValuePath(hugoSymbol ?? '', rootReview.valuePath, isGermline), diff --git a/src/main/webapp/app/service/firebase/firebase-gene-review-service.ts b/src/main/webapp/app/service/firebase/firebase-gene-review-service.ts index 198ebd9e7..878f1e6ff 100644 --- a/src/main/webapp/app/service/firebase/firebase-gene-review-service.ts +++ b/src/main/webapp/app/service/firebase/firebase-gene-review-service.ts @@ -6,7 +6,12 @@ import { FirebaseRepository } from 'app/stores/firebase/firebase-repository'; import _ from 'lodash'; import { Review } from '../../shared/model/firebase/firebase.model'; import { buildHistoryFromReviews } from '../../shared/util/firebase/firebase-history-utils'; -import { extractArrayPath, parseFirebaseGenePath } from '../../shared/util/firebase/firebase-path-utils'; +import { + extractArrayPath, + FIREBASE_LIST_PATH_TYPE, + getFirebasePathType, + parseFirebaseGenePath, +} from '../../shared/util/firebase/firebase-path-utils'; import { ReviewLevel, TumorReviewLevel, @@ -95,6 +100,12 @@ export class FirebaseGeneReviewService { const geneFirebasePath = getFirebaseGenePath(isGermline, hugoSymbol); const vusFirebasePath = getFirebaseVusPath(isGermline, hugoSymbol); + const itemsToDelete: { [key in FIREBASE_LIST_PATH_TYPE]: { [path in string]: number[] } } = { + [FIREBASE_LIST_PATH_TYPE.MUTATION_LIST]: {}, + [FIREBASE_LIST_PATH_TYPE.TUMOR_LIST]: {}, + [FIREBASE_LIST_PATH_TYPE.TREATMENT_LIST]: {}, + }; + let updateObject = {}; const reviewHistory = buildHistoryFromReviews(this.authStore.fullName, reviewLevels); @@ -115,17 +126,15 @@ export class FirebaseGeneReviewService { } else if (isDeleteReview(reviewLevel)) { const { firebaseArrayPath, deleteIndex } = extractArrayPath(reviewLevel.valuePath); const firebasePath = geneFirebasePath + '/' + firebaseArrayPath; + const firebasePathType = getFirebasePathType(firebaseArrayPath + '/' + deleteIndex); + if (firebasePathType !== undefined) { + const innerMap = itemsToDelete[firebasePathType]; + innerMap[firebasePath] ? innerMap[firebasePath].push(deleteIndex) : (innerMap[firebasePath] = [deleteIndex]); + } if (review.demotedToVus) { const variants = parseAlterationName(reviewLevel.currentVal)[0].alteration.split(', '); updateObject = { ...updateObject, ...this.firebaseVusService.getVusUpdateObject(vusFirebasePath, variants) }; } - try { - // Todo: We should use multi-location updates for deletions once all our arrays use firebase auto-generated keys - // instead of using sequential number indices. - await this.firebaseRepository.deleteFromArray(firebasePath, [deleteIndex]); - } catch (error) { - throw new SentryError('Failed to accept deletion in review mode', { hugoSymbol, reviewLevel, isGermline }); - } } else if (isCreateReview(reviewLevel) && isAcceptAll) { const createUpdateObject = await this.getCreateUpdateObject(hugoSymbol, reviewLevel, isGermline, ActionType.ACCEPT); updateObject = { ...updateObject, ...createUpdateObject }; @@ -140,7 +149,36 @@ export class FirebaseGeneReviewService { try { await this.firebaseRepository.update('/', updateObject); } catch (error) { - throw new SentryError('Failed to accept changes in review mode', { hugoSymbol, reviewLevels, isGermline, isAcceptAll, updateObject }); + throw new SentryError('Failed to accept changes in review mode', { + hugoSymbol, + reviewLevels, + isGermline, + isAcceptAll, + updateObject, + }); + } + + // We are deleting last because the indices will change after deleting from array. + let hasDeletion = false; + try { + // Todo: We should use multi-location updates for deletions once all our arrays use firebase auto-generated keys + // instead of using sequential number indices. + for (const pathType of [ + FIREBASE_LIST_PATH_TYPE.TREATMENT_LIST, + FIREBASE_LIST_PATH_TYPE.TUMOR_LIST, + FIREBASE_LIST_PATH_TYPE.MUTATION_LIST, + ]) { + for (const [firebasePath, deleteIndices] of Object.entries(itemsToDelete[pathType])) { + hasDeletion = true; + await this.firebaseRepository.deleteFromArray(firebasePath, deleteIndices); + } + } + // If user accepts a deletion individually, we need to refresh the ReviewPage with the latest data to make sure the indices are up to date. + if (reviewLevels.length === 1 && hasDeletion) { + return { shouldRefresh: true }; + } + } catch (error) { + throw new SentryError('Failed to accept deletions in review mode', { hugoSymbol, reviewLevels, isGermline, itemsToDelete }); } }; diff --git a/src/main/webapp/app/shared/util/firebase/firebase-path-utils.spec.ts b/src/main/webapp/app/shared/util/firebase/firebase-path-utils.spec.ts index b0c9611c9..b8446e71c 100644 --- a/src/main/webapp/app/shared/util/firebase/firebase-path-utils.spec.ts +++ b/src/main/webapp/app/shared/util/firebase/firebase-path-utils.spec.ts @@ -1,5 +1,11 @@ import { FB_COLLECTION } from 'app/config/constants/firebase'; -import { buildFirebaseGenePath, extractArrayPath, parseFirebaseGenePath } from './firebase-path-utils'; +import { + buildFirebaseGenePath, + extractArrayPath, + FIREBASE_LIST_PATH_TYPE, + getFirebasePathType, + parseFirebaseGenePath, +} from './firebase-path-utils'; describe('FirebasePathUtils', () => { describe('parseFirebaseGenePath', () => { @@ -41,4 +47,15 @@ describe('FirebasePathUtils', () => { expect(extractArrayPath(path)).toEqual({ firebaseArrayPath, deleteIndex }); }); }); + + describe('getFirebasePathType', () => { + test.each([ + { path: 'mutations/0', pathType: FIREBASE_LIST_PATH_TYPE.MUTATION_LIST }, + { path: 'mutations/0/tumors/0', pathType: FIREBASE_LIST_PATH_TYPE.TUMOR_LIST }, + { path: 'mutations/0/tumors/23/TIs/0/treatments/4', pathType: FIREBASE_LIST_PATH_TYPE.TREATMENT_LIST }, + { path: 'mutations/0/mutation_effect', pathType: undefined }, + ])('should return correct path type', ({ path, pathType }) => { + expect(getFirebasePathType(path)).toEqual(pathType); + }); + }); }); diff --git a/src/main/webapp/app/shared/util/firebase/firebase-path-utils.ts b/src/main/webapp/app/shared/util/firebase/firebase-path-utils.ts index a9352248e..7ef3efc00 100644 --- a/src/main/webapp/app/shared/util/firebase/firebase-path-utils.ts +++ b/src/main/webapp/app/shared/util/firebase/firebase-path-utils.ts @@ -42,3 +42,20 @@ export const extractArrayPath = (valuePath: string) => { const firebaseArrayPath = pathParts.join('/'); return { firebaseArrayPath, deleteIndex }; }; + +export enum FIREBASE_LIST_PATH_TYPE { + MUTATION_LIST, + TUMOR_LIST, + TREATMENT_LIST, +} +export const getFirebasePathType = (path: string) => { + if (/mutations\/\d+$/i.test(path)) { + return FIREBASE_LIST_PATH_TYPE.MUTATION_LIST; + } + if (/tumors\/\d+$/i.test(path)) { + return FIREBASE_LIST_PATH_TYPE.TUMOR_LIST; + } + if (/TIs\/\d+\/treatments\/\d+$/i.test(path)) { + return FIREBASE_LIST_PATH_TYPE.TREATMENT_LIST; + } +};