From 522a92717e1036aa1af71d5857222286503782a6 Mon Sep 17 00:00:00 2001 From: John Konecny <24961694+jfkonecn@users.noreply.github.com> Date: Mon, 23 Sep 2024 13:56:54 -0400 Subject: [PATCH] Fixed approve all --- .../firebase/firebase-gene-review-service.ts | 12 +- .../core-submission-utils.spec.ts | 169 ++++++++++++++---- .../core-submission-utils.ts | 77 +++----- 3 files changed, 171 insertions(+), 87 deletions(-) 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 c90878637..3985f13d0 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 @@ -31,6 +31,7 @@ import { import { EvidenceApi } from 'app/shared/api/manual/evidence-api'; import { createGeneTypePayload, isGeneTypeChange } from 'app/shared/util/core-gene-type-submission/core-gene-type-submission'; import { GeneTypeApi } from 'app/shared/api/manual/gene-type-api'; +import { flattenReviewPaths, useLastReviewedOnly } from 'app/shared/util/core-submission-shared/core-submission-utils'; export class FirebaseGeneReviewService { firebaseRepository: FirebaseRepository; @@ -130,8 +131,13 @@ export class FirebaseGeneReviewService { let geneTypePayload: ReturnType | undefined = undefined; let hasEvidences = false; try { - for (const reviewLevel of reviewLevels) { - if (!(isCreateReview(reviewLevel) && isAcceptAll)) { + const flattenedReviewLevels = reviewLevels.flatMap(flattenReviewPaths); + // Generate a new version of the gene object (`approvedGene`) for the getEvidence payload. + // This ensures that if multiple valuePaths modify the same part of the payload, + // the changes are applied consistently, preventing any section from being overwritten unintentionally. + const approvedGene = useLastReviewedOnly(gene, ...flattenedReviewLevels.map(x => x.valuePath)) as Gene; + for (const reviewLevel of flattenedReviewLevels) { + if (!isCreateReview(reviewLevel)) { if (reviewLevel.reviewInfo.review.removed) { const deleteEvidencesPayload = pathToDeleteEvidenceArgs({ valuePath: reviewLevel.valuePath, gene }); if (deleteEvidencesPayload !== undefined) { @@ -141,7 +147,7 @@ export class FirebaseGeneReviewService { geneTypePayload = createGeneTypePayload(gene, reviewLevel.valuePath); } else { const args = pathToGetEvidenceArgs({ - gene, + gene: approvedGene, valuePath: reviewLevel.valuePath, updateTime: new Date().getTime(), drugListRef, diff --git a/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.spec.ts b/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.spec.ts index 4425632c6..b48b6ff4c 100644 --- a/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.spec.ts +++ b/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.spec.ts @@ -1,8 +1,9 @@ import 'jest-expect-message'; -import { findAllChildReviewPaths, useLastReviewedOnly } from './core-submission-utils'; +import { flattenReviewPaths, useLastReviewedOnly } from './core-submission-utils'; import { Gene } from 'app/shared/model/firebase/firebase.model'; import { GENE_TYPE } from 'app/config/constants/firebase'; import _ from 'lodash'; +import { BaseReviewLevel, ReviewLevel } from '../firebase/firebase-review-utils'; type RecursivePartial = { [P in keyof T]?: RecursivePartial; @@ -206,47 +207,143 @@ describe('useLastReviewedOnly', () => { }); describe('findAllChildReviewPaths', () => { - const tests: [RecursivePartial, string, string[]][] = [ + const tests: [ + RecursivePartial[], + RecursivePartial[], + ][] = [ [ - { - summary: 'XXXXXXXXXXX', - summary_review: { - lastReviewed: 'YYYYYYYYYYY', - added: true, - updateTime: 0, - updatedBy: 'Test User', - }, - }, - 'summary', - ['summary'], - ], - [ - { - summary: 'XXXXXXXXXXX', - summary_review: { - lastReviewed: 'YYYYYYYYYYY', - updateTime: 0, - updatedBy: 'Test User', + [ + { + valuePath: 'mutations/12/name', + children: [ + { + valuePath: 'mutations/12/name/mutation_effect', + children: [ + { + valuePath: 'mutations/12/mutation_effect/description', + children: [], + reviewInfo: { + reviewPath: 'mutations/12/mutation_effect/description_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/12/mutation_effect/effect', + children: [], + reviewInfo: { + reviewPath: 'mutations/12/mutation_effect/effect_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/12/mutation_effect/oncogenic', + children: [], + reviewInfo: { + reviewPath: 'mutations/12/mutation_effect/oncogenic_review', + reviewAction: 2, + }, + }, + ], + }, + ], + reviewInfo: { + reviewPath: 'mutations/12/name_review', + reviewAction: 0, + }, }, - mutations: [ - { - name: '', - name_review: { - lastReviewed: 'YYYYYYYYYYY', - updateTime: 0, - updatedBy: 'Test User', + { + valuePath: 'mutations/13/name', + children: [ + { + children: [ + { + valuePath: 'mutations/13/mutation_effect/description', + children: [], + reviewInfo: { + reviewPath: 'mutations/13/mutation_effect/description_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/13/mutation_effect/effect', + children: [], + reviewInfo: { + reviewPath: 'mutations/13/mutation_effect/effect_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/13/mutation_effect/oncogenic', + children: [], + reviewInfo: { + reviewPath: 'mutations/13/mutation_effect/oncogenic_review', + reviewAction: 2, + }, + }, + ], }, + ], + reviewInfo: { + reviewPath: 'mutations/13/name_review', + reviewAction: 0, }, - ], - }, - 'mutations/0', - ['mutations/0/name'], + }, + ], + [ + { + valuePath: 'mutations/12/mutation_effect/description', + children: [], + reviewInfo: { + reviewPath: 'mutations/12/mutation_effect/description_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/12/mutation_effect/effect', + children: [], + reviewInfo: { + reviewPath: 'mutations/12/mutation_effect/effect_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/12/mutation_effect/oncogenic', + children: [], + reviewInfo: { + reviewPath: 'mutations/12/mutation_effect/oncogenic_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/13/mutation_effect/description', + children: [], + reviewInfo: { + reviewPath: 'mutations/13/mutation_effect/description_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/13/mutation_effect/effect', + children: [], + reviewInfo: { + reviewPath: 'mutations/13/mutation_effect/effect_review', + reviewAction: 2, + }, + }, + { + valuePath: 'mutations/13/mutation_effect/oncogenic', + children: [], + reviewInfo: { + reviewPath: 'mutations/13/mutation_effect/oncogenic_review', + reviewAction: 2, + }, + }, + ], ], ]; - test.each(tests)('Should find all child review paths in %j for path "%s"', (gene, path, expected) => { - const geneCopy = _.cloneDeep(gene); - expect(findAllChildReviewPaths(gene as Gene, path)).toEqual(expected); - expect(geneCopy, 'The passed gene should be untouched').toEqual(gene); + test.each(tests)('Should find all child review paths in %j for path "%s"', (reviews, expected) => { + const actual = reviews.flatMap(x => flattenReviewPaths(x as BaseReviewLevel)); + expect(actual).toEqual(expected); }); }); diff --git a/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.ts b/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.ts index 15d381697..9e6bbc75f 100644 --- a/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.ts +++ b/src/main/webapp/app/shared/util/core-submission-shared/core-submission-utils.ts @@ -1,4 +1,5 @@ -import { Gene, Review } from 'app/shared/model/firebase/firebase.model'; +import { Review } from 'app/shared/model/firebase/firebase.model'; +import { BaseReviewLevel, ReviewLevel } from '../firebase/firebase-review-utils'; /** * Determines whether the `path` should be protected based on the review path. @@ -6,12 +7,18 @@ import { Gene, Review } from 'app/shared/model/firebase/firebase.model'; * Protection is based on whether the current path or parent path aligns with the `reviewPath`, * meaning that the data was just accepted by a user. * - * @param reviewPath - The reference path for review, indicating an accepted field. Undefined when nothing was accepted. + * @param [reviewPaths] - The reference path for review, indicating an accepted field. Undefined when nothing was accepted. * @param path - The path of the current element being evaluated. * @returns - Returns true if the `path` should be protected, false otherwise. */ -function isPathProtected(reviewPath: string | undefined, path: string): boolean { - return reviewPath !== undefined && path.length > 0 && (path.includes(reviewPath) || reviewPath.includes(path)); +function isPathProtected(reviewPaths: string[], path: string): boolean { + for (const reviewPath of reviewPaths) { + const shouldProtect = reviewPath !== undefined && path.length > 0 && (path.includes(reviewPath) || reviewPath.includes(path)); + if (shouldProtect) { + return true; + } + } + return false; } /** @@ -23,17 +30,17 @@ function isPathProtected(reviewPath: string | undefined, path: string): boolean * only use their`lastReviewed` value. * * @param obj - The object to be filtered for last reviewed elements. - * @param [reviewPath] - The path used to determine if an element should be protected. + * @param [reviewPaths] - The paths used to determine if an element should be protected. * @returns - The filtered object or undefined if no elements are accepted. */ -export function useLastReviewedOnly(obj: T, reviewPath?: string): T | undefined { +export function useLastReviewedOnly(obj: T, ...reviewPaths: string[]): T | undefined { function processObjectRecursively(currentObj: T, parentPath: string): T | undefined { const resultObj: T = {} as T; for (const [key, value] of Object.entries(currentObj as Record)) { const currentPath = `${parentPath}${parentPath.length > 0 ? '/' : ''}${key}`; - const isCurrentPathProtected = isPathProtected(reviewPath, currentPath); - const isParentPathProtected = isPathProtected(reviewPath, parentPath); + const isCurrentPathProtected = isPathProtected(reviewPaths, currentPath); + const isParentPathProtected = isPathProtected(reviewPaths, parentPath); // If the key ends with "_review", copy it to the new object if (key.endsWith('_review')) { @@ -84,48 +91,22 @@ export function useLastReviewedOnly(obj: T, reviewPath?: string): T | undefin return processObjectRecursively(obj, ''); } -export function findAllChildReviewPaths(gene: Gene, reviewPath: string): string[] { - const objectKeys = reviewPath.split('/'); - let pathObject: unknown = gene; +export function flattenReviewPaths(reviewLevel: BaseReviewLevel): ReviewLevel[] { + const arr: ReviewLevel[] = []; - { - let currentPath = ''; - for (const objectKey of objectKeys) { - currentPath += `/${objectKey}`; - if (Array.isArray(pathObject) || (typeof pathObject === 'object' && pathObject !== null)) { - pathObject = pathObject[objectKey]; - } else { - throw new Error(`${currentPath} is not an object or array in the path "${reviewPath}" "${JSON.stringify(pathObject)}"`); - } + if (reviewLevel.children !== undefined && reviewLevel.children.length > 0) { + for (const childLevel of reviewLevel.children) { + arr.push(...flattenReviewPaths(childLevel)); } + } else if ( + 'reviewInfo' in reviewLevel && + typeof reviewLevel.reviewInfo === 'object' && + reviewLevel.reviewInfo !== null && + 'reviewAction' in reviewLevel.reviewInfo && + reviewLevel.reviewInfo.reviewAction !== undefined + ) { + arr.push(reviewLevel as ReviewLevel); } - if (typeof pathObject !== 'object' || pathObject === null) { - return [reviewPath]; - } - - function findAllChildReviewPathsRecursively(obj: unknown, currentPath: string): string[] { - if (currentPath.endsWith('_review')) { - return [currentPath.replace('_review', '')]; - } else if (Array.isArray(obj)) { - const arr: string[] = []; - let i = 0; - for (const element of obj) { - const temp = findAllChildReviewPathsRecursively(element, `${currentPath}/${i}`); - arr.push(...temp); - i++; - } - return arr; - } else if (typeof obj === 'object' && obj !== null) { - const arr: string[] = []; - for (const [key, value] of Object.entries(obj)) { - const temp = findAllChildReviewPathsRecursively(value, `${currentPath}/${key}`); - arr.push(...temp); - } - return arr; - } else { - return []; - } - } - return findAllChildReviewPathsRecursively(pathObject, reviewPath); + return arr; }