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

Fixing SearchParameter validation #6616

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

TipzCM
Copy link
Collaborator

@TipzCM TipzCM commented Jan 13, 2025

closes #6615

Copy link

github-actions bot commented Jan 13, 2025

This Pull Request has failed the formatting check

Please run mvn spotless:apply or mvn clean install -DskipTests to fix the formatting issues.

You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling pre-commit install --hook-type pre-push. This will cause formatting to run automatically whenever you push.

… of LogbackTestExtension to validate that it really skipped the validation logic.
Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.50%. Comparing base (406db33) to head (59e4250).
Report is 181 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6616      +/-   ##
============================================
- Coverage     83.54%   83.50%   -0.04%     
- Complexity    27432    28568    +1136     
============================================
  Files          1707     1797      +90     
  Lines        106185   111173    +4988     
  Branches      13397    13968     +571     
============================================
+ Hits          88710    92838    +4128     
- Misses        11750    12348     +598     
- Partials       5725     5987     +262     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -142,6 +146,16 @@ public void validateSearchParamOnUpdate(IBaseResource theResource, RequestDetail
if (isNotSearchParameterResource(theResource)) {
return;
}

// avoid a loop when loading our hard-coded core FhirContext SearchParameters
//to address https://gitlab.com/simpatico.ai/cdr/-/issues/6986 so that we can skip Search Param validation if been set in the request
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i appreciate what you're doing here, but since gitlab is proprietary software, and this is opensource stuff, directing users to an issue they might not be able to even see isn't going to be as helpful as we think

---
type: fix
issue: 6615
title: "added login to skip the SearchParam Validation during update and uses of LogbackTestExtension to validate that it really skipped the validation logic."
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Capitalize "a" to start (these get published)

And i think you mean "added logging"

don't need to give details on the test; just the fix

Copy link
Collaborator

@JasonRoberts-smile JasonRoberts-smile left a comment

Choose a reason for hiding this comment

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

looks good to me

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.

Updating search parameters with 'skip_validation' does not skip validation
3 participants