Delete mutation/tumor/treatment as last step to avoid stale index #444

calvinlu3 · 2024-09-20T20:44:21Z

fixes https://github.com/oncokb/oncokb-pipeline/issues/551

zhx828 · 2024-09-21T01:12:47Z

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_PATH_TYPE {


Name is too generic. Maybe

Suggested change

export enum FIREBASE_PATH_TYPE {

export enum FIREBASE_LIST_PATH_TYPE {

zhx828 · 2024-09-21T01:15:10Z

src/main/webapp/app/service/firebase/firebase-gene-review-service.ts

@@ -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];


Is innerMap used?

I use it on the next line, so I don't have to do itemsToDelete[firebasePathType][firebasepath]

zhx828 · 2024-09-21T01:16:30Z

src/main/webapp/app/service/firebase/firebase-gene-review-service.ts

+      for (const pathType of [FIREBASE_PATH_TYPE.TREATMENT_LIST, FIREBASE_PATH_TYPE.TUMOR_LIST, FIREBASE_PATH_TYPE.MUTATION_LIST]) {
+        for (const [firebasePath, deleteIndices] of Object.entries(itemsToDelete[pathType])) {
+          await this.firebaseRepository.deleteFromArray(firebasePath, deleteIndices);
+        }
+      }


I see here we are trying to delete deeper list first to avoid mismatch. But how are you addressing deleting/updating the next index?

Good question.
firebaseRepository.deleteFromArray(firebasePath, deleteIndices) will perform the deletion of the array of index in a transaction. The index will never be stale because we loop through the original list to construct a new firebase list.

Check out the code: https://github.com/oncokb/oncokb-transcript/blob/rc/src/main/webapp/app/stores/firebase/firebase-repository.ts#L83-L92

zhx828 · 2024-09-21T01:18:04Z

We need some comments about these logics. We ned test to make sure after approving deletion will not affect the update on other indices.

bprize15

One minor comment, otherwise LGTM

bprize15 · 2024-09-23T13:22:42Z

src/main/webapp/app/service/firebase/firebase-gene-review-service.ts

+      // Todo: We should use multi-location updates for deletions once all our arrays use firebase auto-generated keys
+      // instead of using sequential number indices.
+      /* eslint-disable no-console */
+      console.log(itemsToDelete);


bprize15 · 2024-09-23T15:57:31Z

src/main/webapp/app/service/firebase/firebase-gene-review-service.ts

          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) {


Why do we only refresh if the length is 1? If a review level corresponds to some change that needs to be reviewed, wouldn't we want to refresh if length > 0?

The only time reviewLevels length is > 1 is when we use the accept all feature. In that case a refresh is not needed because everything should be accepted all together.

zhx828

Once Ben's comment is addressed, it's good to go.

bprize15

LGTM

* Handle when Instant json element is a json object The completedDate in the DB could be ISO 8601 string, or json object. I think this has to do with the recent spring boot upgrade, but I can't find the specific version of the change. I will handle both scenarios for now * Textarea in review mode should fit text content (#427) * Update pom version * Fix reject cancer type name review (#428) * Add reference to oncokb sop alteration nomenclature in add mutation modal helper (#432) * Update pom version * Avoid fetching management info repeatedly * Do not rerender the side bar when loading session The getSession is an async method which updates the loading status. * Fix drug code not selectable (#435) * Bump actions to latest version (#438) * resize text area when input changes (#440) * add sorting by firebase index (#443) * Delete mutation/tumor/treatment as last step to avoid stale index (#444) * Review no longer removes data for core submission * Added comments to useLastReviewOnly * Add children review paths * Fixed approve all * Fixed UI tests * Fixed UI tests * break in middle of word to fix collapsible title overlfow (#442) * Change searchEntities to readHandler instead of updateHandler (#445) * Fixed submit all * Fixed submission bug * Fixed data validation tool (#447) * Allow curating mutation summary (#433) * Fixed submit all * Fixed tests * Fixed gene type submissions * Removed last review check inside getevidences and gene type * Added mutation summary --------- Co-authored-by: Hongxin <[email protected]> Co-authored-by: Hongxin <[email protected]> Co-authored-by: Calvin Lu <[email protected]> Co-authored-by: oncokb-bot <[email protected]> Co-authored-by: bprize15 <[email protected]>

* Handle when Instant json element is a json object The completedDate in the DB could be ISO 8601 string, or json object. I think this has to do with the recent spring boot upgrade, but I can't find the specific version of the change. I will handle both scenarios for now * Textarea in review mode should fit text content (#427) * Update pom version * Fix reject cancer type name review (#428) * Add reference to oncokb sop alteration nomenclature in add mutation modal helper (#432) * Update pom version * Avoid fetching management info repeatedly * Do not rerender the side bar when loading session The getSession is an async method which updates the loading status. * Fix drug code not selectable (#435) * Bump actions to latest version (#438) * resize text area when input changes (#440) * add sorting by firebase index (#443) * Delete mutation/tumor/treatment as last step to avoid stale index (#444) * break in middle of word to fix collapsible title overlfow (#442) * Change searchEntities to readHandler instead of updateHandler (#445) * Fixed data validation tool (#447) * Allow curating mutation summary (#433) * Parse protein change case insensitive (#441) * Fix adding CDx biomarker association (#450) * use firebase properties directly in backend (#451) * fix references tab (#452) * Added Heap (#448) * Added Heap * Updated CSP * Reduce width so sidebar does not go offscren (#446) * allow comma in mutation name when transcripts present (#453) * allow comma in mutation name when transcripts present * add missing colon * rename transcripts to reference genomes * Make core API call non-blocking * Add tests for stale indices check (#455) * Added loading icon when review is accepted * Update setup-java action * Update pom version * Put feature flag in app config * Now showing loading icon on accept all --------- Co-authored-by: Hongxin <[email protected]> Co-authored-by: Hongxin <[email protected]> Co-authored-by: Calvin Lu <[email protected]> Co-authored-by: oncokb-bot <[email protected]> Co-authored-by: bprize15 <[email protected]>

* Added core submission logic * Added Save Gene Logic (#385) * Added name changes to review logic (#393) * Added name change to core submission logic * Added missing unit tests * Addessed PR comments * Removed unneeded uuid * added comments to collectUUIDs function * Added gene type submission (#396) * Can now delete evidences (#398) * fixed logging in * fixed build errors * updated drugs mapping * updated drugs mapping * Add releaseGene param when saving gene data to core * Update to allow save all genes * Fix excludedRCTs * Add method to undo gene release * Use reviewed content only (#430) * Use reviewed content only * Ignore if in review added state * Save Genes now checks if added * Delete parent node if added field is true * Fixed build * don't save undefined * Addressed PR comments * Addressed PR comments * Feature/fix submit to core (#439) * Handle when Instant json element is a json object The completedDate in the DB could be ISO 8601 string, or json object. I think this has to do with the recent spring boot upgrade, but I can't find the specific version of the change. I will handle both scenarios for now * Textarea in review mode should fit text content (#427) * Update pom version * Fix reject cancer type name review (#428) * Add reference to oncokb sop alteration nomenclature in add mutation modal helper (#432) * Update pom version * Avoid fetching management info repeatedly * Do not rerender the side bar when loading session The getSession is an async method which updates the loading status. * Fix drug code not selectable (#435) * Bump actions to latest version (#438) * resize text area when input changes (#440) * add sorting by firebase index (#443) * Delete mutation/tumor/treatment as last step to avoid stale index (#444) * Review no longer removes data for core submission * Added comments to useLastReviewOnly * Add children review paths * Fixed approve all * Fixed UI tests * Fixed UI tests * break in middle of word to fix collapsible title overlfow (#442) * Change searchEntities to readHandler instead of updateHandler (#445) * Fixed submit all * Fixed submission bug * Fixed data validation tool (#447) * Allow curating mutation summary (#433) * Fixed submit all * Fixed tests * Fixed gene type submissions * Removed last review check inside getevidences and gene type * Added mutation summary --------- Co-authored-by: Hongxin <[email protected]> Co-authored-by: Hongxin <[email protected]> Co-authored-by: Calvin Lu <[email protected]> Co-authored-by: oncokb-bot <[email protected]> Co-authored-by: bprize15 <[email protected]> * Added Vus submissions to core (#456) * Make core API call non-blocking (#454) * Handle when Instant json element is a json object The completedDate in the DB could be ISO 8601 string, or json object. I think this has to do with the recent spring boot upgrade, but I can't find the specific version of the change. I will handle both scenarios for now * Textarea in review mode should fit text content (#427) * Update pom version * Fix reject cancer type name review (#428) * Add reference to oncokb sop alteration nomenclature in add mutation modal helper (#432) * Update pom version * Avoid fetching management info repeatedly * Do not rerender the side bar when loading session The getSession is an async method which updates the loading status. * Fix drug code not selectable (#435) * Bump actions to latest version (#438) * resize text area when input changes (#440) * add sorting by firebase index (#443) * Delete mutation/tumor/treatment as last step to avoid stale index (#444) * break in middle of word to fix collapsible title overlfow (#442) * Change searchEntities to readHandler instead of updateHandler (#445) * Fixed data validation tool (#447) * Allow curating mutation summary (#433) * Parse protein change case insensitive (#441) * Fix adding CDx biomarker association (#450) * use firebase properties directly in backend (#451) * fix references tab (#452) * Added Heap (#448) * Added Heap * Updated CSP * Reduce width so sidebar does not go offscren (#446) * allow comma in mutation name when transcripts present (#453) * allow comma in mutation name when transcripts present * add missing colon * rename transcripts to reference genomes * Make core API call non-blocking * Add tests for stale indices check (#455) * Added loading icon when review is accepted * Update setup-java action * Update pom version * Put feature flag in app config * Now showing loading icon on accept all --------- Co-authored-by: Hongxin <[email protected]> Co-authored-by: Hongxin <[email protected]> Co-authored-by: Calvin Lu <[email protected]> Co-authored-by: oncokb-bot <[email protected]> Co-authored-by: bprize15 <[email protected]> --------- Co-authored-by: Hongxin <[email protected]> Co-authored-by: Hongxin <[email protected]> Co-authored-by: Calvin Lu <[email protected]> Co-authored-by: oncokb-bot <[email protected]> Co-authored-by: bprize15 <[email protected]>

Delete objects as the last step

8d62d20

calvinlu3 added the fix Fix tag for release label Sep 20, 2024

calvinlu3 requested review from bprize15 and zhx828 September 20, 2024 20:44

zhx828 requested changes Sep 21, 2024

View reviewed changes

bprize15 approved these changes Sep 23, 2024

View reviewed changes

zhx828 self-requested a review September 23, 2024 14:54

zhx828 approved these changes Sep 23, 2024

View reviewed changes

calvinlu3 added 2 commits September 23, 2024 10:58

Update based on comments

4d4e181

Handle individual accepts by refreshing page

bb4ae6e

bprize15 reviewed Sep 23, 2024

View reviewed changes

zhx828 approved these changes Sep 23, 2024

View reviewed changes

bprize15 approved these changes Sep 23, 2024

View reviewed changes

calvinlu3 merged commit f269cf8 into oncokb:rc Sep 23, 2024
2 checks passed

calvinlu3 deleted the fix/deleted-mutation branch September 23, 2024 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete mutation/tumor/treatment as last step to avoid stale index #444

Delete mutation/tumor/treatment as last step to avoid stale index #444

calvinlu3 commented Sep 20, 2024

zhx828 Sep 21, 2024

zhx828 Sep 21, 2024

calvinlu3 Sep 21, 2024

zhx828 Sep 21, 2024

calvinlu3 Sep 21, 2024

zhx828 commented Sep 21, 2024

bprize15 left a comment

bprize15 Sep 23, 2024

bprize15 Sep 23, 2024

calvinlu3 Sep 23, 2024

zhx828 left a comment

bprize15 left a comment

	export enum FIREBASE_PATH_TYPE {
	export enum FIREBASE_LIST_PATH_TYPE {

Delete mutation/tumor/treatment as last step to avoid stale index #444

Delete mutation/tumor/treatment as last step to avoid stale index #444

Conversation

calvinlu3 commented Sep 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhx828 commented Sep 21, 2024

bprize15 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhx828 left a comment

Choose a reason for hiding this comment

bprize15 left a comment

Choose a reason for hiding this comment