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

Add validation of parameters for POST requests #552

Closed
rob-reynolds opened this issue Jul 13, 2022 · 4 comments · Fixed by #628
Closed

Add validation of parameters for POST requests #552

rob-reynolds opened this issue Jul 13, 2022 · 4 comments · Fixed by #628
Assignees
Labels
enhancement New feature or request

Comments

@rob-reynolds
Copy link

rob-reynolds commented Jul 13, 2022

Validation of parameters for GET requests is currently supported (i.e., query string parameters). Extend with validation of parameters for other verbs (i.e. body parameters).

Also, update existing usages of the query string parameter validation to use the new body parameters functionality.

@JPercival JPercival added the enhancement New feature or request label Aug 9, 2022
@JPercival
Copy link
Contributor

JPercival commented Aug 9, 2022

Seems unusual that the validation would not work with POST requests. They are parsed from the body of the request prior to the operation implementation being invoked. Can we validate that it doesn't work for POST?

@rob-reynolds
Copy link
Author

The issue was identified, reported, and addressed by @c-schuler here: https://github.com/DBCG/cqf-ruler/blob/c97f0cfa06f0cff2082f805947dae4fcaf2ddeae/plugin/ra/src/main/java/org/opencds/cqf/ruler/ra/r4/RiskAdjustmentProvider.java#L58.

I don't know what our policy is on validating issues. I would think the expectation is that the person who identified the issue also validated it, but we know that doesn't always happen. So when we have issues we suspect aren't valid do we kick them back to the person who identified them or do we have the person who's working on the issue start with validating it's an actual issue?

Maybe the simplest approach is we require a repro.

And in this case we could ask @c-schuler to provide a repro?

@rob-reynolds
Copy link
Author

@rob-reynolds
Copy link
Author

And make repro a (nearly) required field in bug issues.

c-schuler added a commit that referenced this issue Sep 29, 2022
…dated report and care-gaps tests and operations to use updated logic
This was unlinked from pull requests Sep 30, 2022
@JPercival JPercival linked a pull request Sep 30, 2022 that will close this issue
@JPercival JPercival self-assigned this Oct 3, 2022
JPercival added a commit that referenced this issue Oct 14, 2022
* #552: updated Operations utility for POST parameter validation ... updated report and care-gaps tests and operations to use updated logic

* Updated care-gaps operation and tests

* Updating ra resolve and evaluate-measure operations and tests

Co-authored-by: Jonathan Percival <jonathan.i.percival@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants