Skip to content

feat: add cg snomed mapping to config search#1390

Merged
fzhao99 merged 36 commits into
mainfrom
bob/840-cg-snomed-mapping
Jun 22, 2026
Merged

feat: add cg snomed mapping to config search#1390
fzhao99 merged 36 commits into
mainfrom
bob/840-cg-snomed-mapping

Conversation

@fzhao99

@fzhao99 fzhao99 commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

🔀 PULL REQUEST

💡 Summary

Adds SNOMED CG code / display name to the creation modal search / confirmation flow

Screen.Recording.2026-06-17.at.12.19.54.PM.mov

🔗 Related Issue

Fixes #840

✅ Acceptance Criteria

🧪 How to test

  • Select a new config from the modal. Notice the new table that displays child RSG codes / displays. Note down one of the codes
  • Clear the selection and search by the display name / code. Notice a search match renders for it now.
  • Create the config and notice the creation works as intended

ℹ️ Additional Information

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🔒 Security Scan Results

⚠️ Found 15 vulnerabilities

Severity Total
🟠 High 12
🟡 Medium 3

📦 refiner-app

No vulnerabilities found

📦 refiner-lambda

Severity Count
🟠 High 4

📦 refiner-ops

Severity Count
🟠 High 8
🟡 Medium 3

View detailed results: Security tab
Last updated: 2026-06-22 16:10:45 UTC

Comment thread client/src/components/Combobox/index.tsx
Comment thread client/src/components/Modal/index.tsx
Comment thread client/src/hooks/useSearch.ts Outdated
Comment thread refiner/app/db/codes/model.py
Comment thread client/src/components/Modal/index.tsx
Comment thread client/src/pages/Configurations/index.tsx Outdated
@fzhao99 fzhao99 marked this pull request as ready for review June 17, 2026 16:55
@jakewheeler

Copy link
Copy Markdown
Collaborator

When searching for influenza I am unable to see the entire block of content within the element. I'm thinking we should probably allow the box to be as large as it needs to in order to fit the content?
image

Comment thread client/src/pages/Configurations/index.tsx Outdated
Comment thread client/src/pages/Configurations/index.tsx Outdated
Comment thread refiner/app/api/v1/conditions.py Outdated
Comment thread client/src/pages/Configurations/index.tsx Outdated
threshold: 0.25,
distance: 500, // Broaden the distance so Fuse doesn't penalize long strings
includeMatches: true,
findAllMatches: true,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We might be able to avoid a lot of the string length related logic by setting minMatchCharLength to 3.

Let me know if I'm missing anything but this seems to be working for me.

@fzhao99 fzhao99 Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Doing it this way seems to make Fuse break when there are strings with spaces in them (note the resetting of the match results after spaces). I toyed around with a few settings to try to get it to work but couldn't seem to figure out how.

Screen.Recording.2026-06-22.at.10.17.45.AM.mov

Lmk if you can repro. It seems to work fine if we do the checking in React, so I might just leave it like that unless there's a setting you can find that makes it behave like we expect

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm seeing the same thing. I think we'd need to tweak the hook to account for a minimum string length which we don't need to do here, so let's leave it as you have it.

@fzhao99

fzhao99 commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

When searching for influenza I am unable to see the entire block of content within the element. I'm thinking we should probably allow the box to be as large as it needs to in order to fit the content?

@jakewheeler setting the height here to max-h-fit seems to overload the browser during the search experience. I've bumped up the max height a little bit to display more of the search options at once, but if you can repro the issue with fit, think we should leave a set max height to make the rendering less jumpy.

Screen.Recording.2026-06-22.at.9.48.50.AM.mov

Lmk if I misunderstood / you have another thought on how to set the max height.

@fzhao99 fzhao99 requested a review from jakewheeler June 22, 2026 14:46
Comment on lines -52 to -59
conditions = await get_latest_conditions_db(db=db)
return [
GetConditionsResponse(
id=condition.id,
display_name=condition.display_name,
)
for condition in conditions
]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Apologies if my wording about the response model was confusing. I meant to suggest that we actually go back to this model where we explicitly map what we get from the DB into a response object that only has the properties we intend to provide to the client.

The current change doesn't really buy us anything since we are directly passing what we receive from the DB directly to the client still. We just have one more wrapper involved now.

})
).toBeVisible();
await expect(page.getByRole('searchbox')).not.toBeVisible();
const setupConfigurationButton = page.getByRole('button', {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice tests 👍

@jakewheeler jakewheeler left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! 👏

@fzhao99 fzhao99 added this pull request to the merge queue Jun 22, 2026
Merged via the queue into main with commit 6d6066d Jun 22, 2026
21 checks passed
@fzhao99 fzhao99 deleted the bob/840-cg-snomed-mapping branch June 22, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[USER STORY] CG to SNOMED Mapping - Set Up Configuration

2 participants