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

Implementing chain search for the MockClient #3714

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jjdonov
Copy link

@jjdonov jjdonov commented Jan 13, 2024

⚠️ this is a draft PR for feedback after speaking w @rahul1

Hello, I was interested in getting chained searches working w/ the mock client, and was hoping to get some feedback on the approach taken so far, and in particular to check on the usage of SearchParamDetails and the columnName attribute.

Approach Overview:

  • The existing chain parsing logic was in server. So that it can be used both by server and fhir-router its moved to core
  • match.ts only handles matching w/ a single resource. Matching for a chain requires being able to resolve the linked resources. e.g. To cope with that, the matchesChain is implemented in MemoryRepository instead of as a part of match

Open Questions

  • The reverse chaining implementation makes use of SearchParamDetails.columnName to make sure the linked resource has a reference to the parent. columnName suggests that it might be a server-centric concept, but was already in core. Is this safe to use?

Feedback already provided by @rahul1

  • ensure fwd chains can be "chained"
  • implement circuit breaker on chain depth (believe this is 5 already server side)
  • test cases to cover mixing chain and reverse chains

Copy link

vercel bot commented Jan 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
medplum-app ⬜️ Ignored (Inspect) Visit Preview Jan 22, 2024 7:24pm
medplum-storybook ⬜️ Ignored (Inspect) Visit Preview Jan 22, 2024 7:24pm
medplum-www ⬜️ Ignored (Inspect) Visit Preview Jan 22, 2024 7:24pm

Copy link

vercel bot commented Jan 13, 2024

@jjdonov is attempting to deploy a commit to the Medplum Team on Vercel.

A member of the Team first needs to authorize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

1 participant