Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix/DEVSU-2445 rapid report kb matches bug fix #572

Conversation

bnguyen-bcgsc
Copy link
Contributor

@bnguyen-bcgsc bnguyen-bcgsc commented Nov 6, 2024

Extension of [https://github.com//pull/571]

  • DEVSU 2445 & 2467 & 2505
  • Update kb matches and evidence level splitting logic in rapid report with new kb matched statements response data structure
  • Update valueGetter logic for observed variants column in kb matches tables
  • Switch positions of observed variants and known variants columns back to original
  • Rework kbMatches statement categories handling in smallMutations, copyNumber, structuralVariants, and expressionOutliers to work with new kbmatches data structure
  • Fix ExpOutliersType missing in GeneType

- Update kb matches and evidence level splitting logic in rapid report with new kb matched statements response data structure
- Update valueGetter logic for observed variants column in kb matches tables
- Switch positions of observed variants and known variants columns back to original
@bnguyen-bcgsc bnguyen-bcgsc added the bug Something isn't working label Nov 6, 2024
@bnguyen-bcgsc bnguyen-bcgsc self-assigned this Nov 6, 2024
…ta structure (evidenceLevel and category being nested in kbmatchedstatements array since each variant can now have multiple statements)

- Rework kbMatches statement categories handling in smallMutations, copyNumber, structuralVariants, and expressionOutliers to work with new kbmatches data structure
- Fix ExpOutliersType missing in GeneType
…ches data structure

- Update mockData for rapid report unit tests
Copy link

github-actions bot commented Nov 13, 2024

Test Results

    1 files  ±0    28 suites  ±0   4m 55s ⏱️ +2s
145 tests ±0  145 ✔️ ±0  0 💤 ±0  0 ±0 
142 runs  ±0  142 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit d228772. ± Comparison against base commit 4c0e053.

♻️ This comment has been updated with latest results.

elewis2
elewis2 previously approved these changes Nov 13, 2024
valueGetter: (params) => {
const { data: { kbMatches } } = params;
if (kbMatches) {
const kbVariants = kbMatches?.map((match) => match.kbVariant).filter((kbVariant) => kbVariant !== undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably just use .filter no?

kbMatches?.filter((match) => match?.kbVariant);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yea you're right let me simplify it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kttkjl actually I tried it and it didn't work, we still have to extract kbVariant from each match with .map() right?

Copy link
Member

@kttkjl kttkjl Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, match.kbVariant && match.kbVariant !== undefined, maybe the shorthand is not enough, oh yeah, we do have to map out the values too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in which case we'll use reduce

kbMatches?.reduce((acc, match) => {
  if (match.kbVariant !== undefined) {
    acc.push(match.kbVariant);
  }
  return acc;
}, []);

kttkjl
kttkjl previously approved these changes Nov 14, 2024
Copy link
Member

@kttkjl kttkjl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Programmatically makes sense, just reduce vs map + filter

@bnguyen-bcgsc bnguyen-bcgsc dismissed stale reviews from kttkjl and elewis2 via d228772 November 14, 2024 18:52
@bnguyen-bcgsc bnguyen-bcgsc merged commit 274096a into develop Nov 14, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants