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

Split custom Medplum search parameter definitions into separate file #2178

Merged
merged 7 commits into from
Jun 27, 2023

Conversation

mattwiller
Copy link
Member

No description provided.

@mattwiller mattwiller added the fhir-datastore Related to the FHIR datastore, includes API and FHIR operations label Jun 6, 2023
@mattwiller mattwiller self-assigned this Jun 6, 2023
@mattwiller mattwiller requested a review from a team as a code owner June 6, 2023 23:14
@vercel
Copy link

vercel bot commented Jun 6, 2023

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

Name Status Preview Comments Updated (UTC)
medplum-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 10:42pm
medplum-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 10:42pm

Copy link
Member

@codyebberson codyebberson left a comment

Choose a reason for hiding this comment

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

Nice, this is a good change.

If you grep for readJson('fhir/r4/search-parameters.json'), there are quite a few places in the code base where we would need to add search-parameters-medplum.json too. Probably not all of the tests, but we should add them to the packages/generator/src/ files (docs.ts, migrate.ts, searchparams.ts, storybook.ts).

@@ -250,11 +250,7 @@ export function indexSearchParameterBundle(bundle: Bundle<SearchParameter>): voi
* @see {@link IndexedStructureDefinition} for more details on indexed StructureDefinitions.
*/
export function indexSearchParameter(searchParam: SearchParameter): void {
if (!searchParam.base) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this change? If base is undefined, then the function is a no-op, yeah?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept it as a no-op, just streamlined the undefined check so it would instead iterate over an empty array

for (const entry of searchParams.entry as BundleEntry<SearchParameter>[]) {
indexSearchParameter(entry.resource as SearchParameter);
}
indexSearchParameterBundle(readJson('fhir/r4/search-parameters.json') as Bundle<SearchParameter>);
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

@@ -20,11 +17,6 @@ export async function main(configName: string): Promise<void> {

const config = await loadConfig(configName);

// Preload the schema
Copy link
Member

Choose a reason for hiding this comment

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

Oy, we were double loading the schema? 🤦‍♂️ Nice catch.

IIRC, we did intentionally load the schema before initializing express, to the case where we lazy load the schema inside an HTTP request. Maybe we were doing that anyway (sigh 🤦‍♂️), but I think the theory still holds? So might be worth adding getStructureDefinitions() here?

@reshmakh reshmakh added this to the June 15, 2023 milestone Jun 7, 2023
@reshmakh reshmakh modified the milestones: June 15, 2023, June 30, 2023 Jun 17, 2023
@mattwiller mattwiller marked this pull request as draft June 21, 2023 19:02
@coveralls
Copy link

Coverage Status

coverage: 94.191% (+0.02%) from 94.174% when pulling 0fb872e on split_custom_search_params into 9b7bf74 on main.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@codyebberson codyebberson merged commit cacf2fd into main Jun 27, 2023
@codyebberson codyebberson deleted the split_custom_search_params branch June 27, 2023 16:42
codyebberson added a commit that referenced this pull request Jun 30, 2023
Update client-cloudwatch-logs manual mock to use aws-sdk-client-mock (#2342)
Log resource for validator latency warning (#2338)
Minor tweaks to aws init tool and docs (#2340)
Allow contentReference elements to override cardinality rules (#2339)
Fixes #2315 - allow bot success without return value (#2336)
Fix app env vars in publish job (#2335)
Fixes #2310 - CLI rest with --fhir-url-path (#2313)
Unify all of the sleep helper functions (#2331)
Increase Fargate health check grace period (#2330)
Remove PWA and service worker (#2329)
Updating subscription documentation to reflect behavior when resources are deleted (#2328)
Add app to publish.sh (#2326)
Fixes #2306 - remove console.log from export tests due to race conditions (#2322)
Fixed landing page for storybook (#2323)
Bring back sourcemaps (#2309)
Split custom Medplum search parameter definitions into separate file (#2178)
Poll status follow Location header (#2307)
Copy .env.defaults to .env if not exists (#2308)
Allow extra URL path content with Bot execute (#2305)
Fixes #2182- All Patient Export (#2293)
Strict validation of property cardinality (#2300)
Request from ACN (#2299)
Fixed CLI shebang (#2301)
ESBuild (#2298)
Add logs and parity tests for new validator (#2291)
Fixes #2277 - Updated access policies docs (#2292)
Add non-null requirements to values in GraphQL array types (#2290)
Validating Fixed and Pattern Values (#2288)
Fixes #2195 - updated cdk init docs (#2289)
codyebberson added a commit that referenced this pull request Jul 1, 2023
Update client-cloudwatch-logs manual mock to use aws-sdk-client-mock (#2342)
Log resource for validator latency warning (#2338)
Minor tweaks to aws init tool and docs (#2340)
Allow contentReference elements to override cardinality rules (#2339)
Fixes #2315 - allow bot success without return value (#2336)
Fix app env vars in publish job (#2335)
Fixes #2310 - CLI rest with --fhir-url-path (#2313)
Unify all of the sleep helper functions (#2331)
Increase Fargate health check grace period (#2330)
Remove PWA and service worker (#2329)
Updating subscription documentation to reflect behavior when resources are deleted (#2328)
Add app to publish.sh (#2326)
Fixes #2306 - remove console.log from export tests due to race conditions (#2322)
Fixed landing page for storybook (#2323)
Bring back sourcemaps (#2309)
Split custom Medplum search parameter definitions into separate file (#2178)
Poll status follow Location header (#2307)
Copy .env.defaults to .env if not exists (#2308)
Allow extra URL path content with Bot execute (#2305)
Fixes #2182- All Patient Export (#2293)
Strict validation of property cardinality (#2300)
Request from ACN (#2299)
Fixed CLI shebang (#2301)
ESBuild (#2298)
Add logs and parity tests for new validator (#2291)
Fixes #2277 - Updated access policies docs (#2292)
Add non-null requirements to values in GraphQL array types (#2290)
Validating Fixed and Pattern Values (#2288)
Fixes #2195 - updated cdk init docs (#2289)
@reshmakh
Copy link
Member

reshmakh commented Aug 1, 2023

Related:#2006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fhir-datastore Related to the FHIR datastore, includes API and FHIR operations
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants