test(rest): pin the issue codes on #478's two search-entry failure paths - #518
Open
aacruzgon wants to merge 1 commit into
Open
test(rest): pin the issue codes on #478's two search-entry failure paths#518aacruzgon wants to merge 1 commit into
aacruzgon wants to merge 1 commit into
Conversation
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
aacruzgon
force-pushed
the
fix/478-search-entry-issue-codes
branch
from
August 6, 2026 17:25
6abd69d to
0a1dcea
Compare
Base automatically changed from
fix/478-bundle-get-search
to
fix/504-batch-entry-issue-codes
August 7, 2026 14:48
aacruzgon
force-pushed
the
fix/478-search-entry-issue-codes
branch
from
August 7, 2026 14:52
0a1dcea to
ff60e9a
Compare
The search-entry dispatch added two failure sites, one per bundle arm, and covered neither: all five of its tests assert `200 OK`. Both sites were written as `let (status, _, details) = e.client_response(); create_error_result( status.as_u16(), &details)` — the same code-discard #504 removed from every other call site. The rebase migrated them to `entry_failure`; this pins that so they cannot drift back. Three tests. A batch search entry failing on `_query` (400 `invalid`) and on `:not-in` (501 `not-supported`); the same failure inside a `transaction`; and parity with `GET [base]/Patient?_query=…`, asserting severity, code and `details.text` all agree plus a literal pin, since `processing == processing` would satisfy the equalities on its own. The transaction case is worth naming. #504 stated that no per-entry outcome is reachable on the transaction arm, because the backends discard an entry result at their `status >= 400` guard and return `TransactionError::BundleError`. That was true of the tree #504 landed on and is false here: this search loop bypasses the backend executor and surfaces the failure as that entry's own outcome — deliberately, since a search failure cannot roll back writes that already committed. So this is the first reachable per-entry outcome on that arm, and the first place its issue code is asserted. Verified non-vacuous: with both sites reverted to their original code-discard, all three fail — `left: String("processing"), right: "invalid"`. Also corrects a README bullet #504 added. It documented `GET Patient` in a batch entry as an instance read answering `404 not-found` with the message `Resource Patient/ not found`, and named executing it as a search as future work. That work is the commit below this one, so the bullet is now false and is removed rather than left to mislead. The per-entry code table gains a row for search-entry failures. Tests: 1117 pass. Refs #478, #504
aacruzgon
force-pushed
the
fix/478-search-entry-issue-codes
branch
from
August 9, 2026 20:32
ff60e9a to
61af249
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#481 added search-entry dispatch to both bundle arms, and with it two new failure sites — one per
arm — with no coverage on either. All five of its tests assert
200 OK.Both sites were written as:
That is the same code-discard #504 removed from every other call site in the file:
client_responsecomputes the correct FHIR issue code, the
_throws it away, and the wrapper stampsprocessingoverthe result. Left alone, a search entry would have been the one path in the handler still answering
processingafter #504.The rebase below this commit migrated both to
entry_failure. This PR pins that, so they cannotdrift back.
Changes
Three tests in
batch_entry_issue_codes.rs:a_failed_search_entry_reports_its_issue_codePatient?_query=byName,Patient?code:not-in=…invalid, 501not-supporteda_failed_transaction_search_entry_reports_its_issue_codePatient?_query=byNameinvalid, per-entry, bundle still 200a_failed_search_is_described_identically_by_both_surfacesdetails.textall agree, plus a literal pinThe parity test's literal pin is load-bearing:
processing == processingsatisfies the threeequalities on its own, so without it a regression that flattened both surfaces would pass.
The transaction case corrects a claim #504 made
#504 stated that no per-entry outcome is reachable on the transaction arm — the backends discard an
entry result at their
status >= 400guard and returnTransactionError::BundleError, which carriesneither a status nor a code. That was true of the tree #504 landed on. It is false here. #481's
search loop bypasses the backend executor entirely and surfaces a search failure as that entry's own
outcome, deliberately — a failed search cannot roll back writes that already committed.
So this is the first reachable per-entry outcome on the transaction arm, and the first place its issue
code is asserted. #504's PR body and README say otherwise; the README bullet is corrected below and
the claim is narrowed to the tree it was true of.
Documentation
Removes a Current Limitations bullet #504 added:
That work is the commit directly below this one, so the bullet is now false. Removed rather than left
to mislead. The per-entry code table gains a row for search-entry failures, and the parity sentence
gains the search surface.
Testing
Verified non-vacuous. With both sites reverted to their original code-discard, all three fail:
Clippy: CI's invocation (
ci.yml:497, which carries-A collapsible_ifand seven sibling allow-flags) passes clean — exit 0, zero errors. Worth correcting explicitly, because this PR previously repeated the parent PRs' claim that the gate "still exits 101 on pre-existing lints tracked in #513": that is true of the barecargo clippy --all-targets --all-features -- -D warnings, but not of what CI runs. All 42crates/restwarnings and all six files that fail the bare form sit inside the allowed lint families, so the Linting job was never going to fail. Separately measured:crates/restlint set is 42 → 42, zero introduced, zero removed against the base with forced recompilation.Notes
mainonto fix(rest): render batch entry failures through the single-resource error mapping #516 to make this possible — thereconciliation needs
entry_failure, which only exists after batch: every per-entry error carries OperationOutcome codeprocessing— a 403, 404 and 405 are indistinguishable without parsing English #504. angela's original branch ispreserved at
backup/481-original-pre-restack; the rebase and its five-hunk resolution aredescribed in a comment on #481.
execute_search_bundlerejects_queryas a known-but-unimplemented control parameter (
InvalidParameter→ 400invalid) and:not-inas a modifier nobackend implements (
NotImplemented→ 501not-supported). Two different codes through one path,which is what makes the test meaningful rather than a single-value check.
The behaviour is deliberate and documented in the code, but asserting it needs a bundle whose write
entries commit and whose search then fails, and the ordering that makes it deterministic is
method_processing_order's, not something a test should depend on. Left as-is.Refs #478, #504