Repository navigation
OpenConceptLab/ocl_online#451 | FHIR $validate-code and $translate need a code and its system, and read only that code - #947
Conversation
…ed a code and its system, and read only that code - A POSTed Parameters resource is read with the value types R4 gives each input, and a code or URI sent as valueString is read too. When both are sent, the declared type wins. Before, a code sent as valueString was dropped, and the request went on as if it had no code. - CodeSystem $validate-code without a code, or with a code but no url, is a 400 OperationOutcome naming the missing parameter. Before, it validated against every concept on the server, and a display check read all of them. A url that doesn't resolve, or that the requester can't view, still answers result=false, with or without a code (OpenConceptLab/ocl_online#224). - ValueSet $validate-code without a code, or with a code but no system, is a 400. Before, it answered for the first concept in the value set, or on the server. Without url, the global route still checks the code against the code system alone. - ConceptMap $translate without a code, or with a code but no system, is a 400. It no longer tests whether the ConceptMap, the code or the system match anything before narrowing further: each test read every matching mapping. - The 400 is a DRF ValidationError carrying an OperationOutcome (issue code invalid, expression naming the parameter), shaped like $lookup's not-found. - core/code_systems/tests_operation_bounds.py checks every concept and mapping read these operations make, for GET and POST, with and without code, url, system and display: each must name its rows by code or id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wdpwbn9tU8NmVL8oW2A75d
…its own checks, never the mapping list's search, facets or CSV export - $translate inherited the mapping list view's shortcuts. A q, sort or faceted filter parameter sent it to Elasticsearch, facetsOnly and searchStatsOnly answered with facets or search stats, the IncludeFacets header wrapped the answer in facets, and csv=true answered with a CSV export of the matching mappings instead of the translation. The search ones skipped apply_filters, so the request needed no code or system, and the translation's url, code and system filters weren't applied. - Each shortcut is off for $translate. Their parameters are ignored, like any other parameter the operation doesn't define. - The two $validate-code views and $lookup read through get_queryset directly, so they never took these shortcuts. tests_operation_bounds.py now checks all four operations with each shortcut's parameter or header: the answer is the same as without it, a request without a code is still a 400, and no search, facet, search-stats or export call is made. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wdpwbn9tU8NmVL8oW2A75d
…ters resource - POST $translate failed with a TypeError before reading its inputs: it passed instance twice through the view's get_serializer, which renders the answer. The Parameters resource is now read by the serializer class itself, as a GET's query is. - tests_operation_bounds.py checks $translate by POST, with declared value types and with valueString, as it checks GET: a missing code or system is a 400, and every mapping read names its rows by code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMvtGDtJmnhfUDvHQAw4HF
paynejd
left a comment
There was a problem hiding this comment.
Codex review, pass 1 (codex-cli 0.163.0, commit ade3d91): clean, no new findings.
One finding from a review of this code before the rebase is now fixed in d01a0aa. POST $translate failed with a TypeError before reading its inputs, which was already the case on master. The full review is on the tracking ticket.
paynejd
left a comment
There was a problem hiding this comment.
Codex review, pass 2 (codex-cli 0.163.0, commit d01a0aa): clean. Both earlier findings are resolved, and there are no new ones.
Simplification review: 3 test cuts were proposed and all 3 declined. Each test pins behaviour this PR documents.
The review loop is done. The full passes are on the tracking ticket.
|
@snyaggarwal thanks for merging. The master build for this is run https://github.com/OpenConceptLab/oclapi2/actions/runs/39531004834, in tests now. Jon asked that you take it through the staging and production approvals. I'll confirm the new behaviour on production once it's out. |
|
@paynejd this is deployed |
Tracking: OpenConceptLab/ocl_online#451
FHIR
$validate-codeand$translatenow require the inputs that name what they check, and read only the concept or mapping those inputs name.$translatealso stops answering through the mapping list's search, facet and export shortcuts. The same change is on the #451 FHIR integration branch. This PR brings it to master ahead of that branch.Review and merge: @snyaggarwal reviews, and merges once the review is done (Jon's request).
Behaviour changes (for the release notes)
$validate-codewith nocodeis a 400 OperationOutcome (issue codeinvalid,expression: ["code"]).$validate-codewith acodebut nourlis a 400 namingurl. It used to answerresult=false.urlthat doesn't resolve, or that the requester can't view, still answersresult=false, with or without a code (unchanged from oclapi2#943).$validate-codeneedscodeandsystem, and either missing is a 400. A value set that doesn't resolve still answers false.$translateneedscodeandsystem, and either missing is a 400. It narrows byurl,codeandsystembefore reading mappings, instead of testing after each filter whether anything matched.$translateignores the mapping list's own parameters and headers (q,sortAsc/sortDesc, faceted andextras.*filters,facetsOnly,searchStatsOnly,csv,IncludeFacets), as it ignores any parameter the operation doesn't define. It always answers with the translation.valueStringis read too, and the declared type wins when both are sent. This also applies to POST$expand'surl.$translatereads its Parameters resource. It used to fail with a server error before reading its inputs.Tests
core/code_systems/tests_operation_bounds.pycovers CodeSystem and ValueSet$validate-code,$translateand$lookup, by GET and POST (with declared value types and withvalueString), with and withoutcode,url,systemanddisplay. It checks:The tests are in
core/code_systems/becausecore/fhir/andcore/parameters/have no__init__.pyon master, so tests there aren't collected. The #451 integration branch adds them.How to verify
python manage.py test core.code_systems.tests_operation_bounds --keepdbGET /fhir/CodeSystem/$validate-code?url=<a code system's canonical>withoutcodeis a 400 OperationOutcome namingcode. Adding&code=<a code in it>answersresult=true.GET /fhir/ConceptMap/$translate?system=<canonical>&code=<code>&csv=trueanswers with the translation Parameters, not a CSV.🤖 Generated with Claude Code
https://claude.ai/code/session_01RMvtGDtJmnhfUDvHQAw4HF