Skip to content

test(rest): pin the issue codes on #478's two search-entry failure paths - #518

Open
aacruzgon wants to merge 1 commit into
fix/504-batch-entry-issue-codesfrom
fix/478-search-entry-issue-codes
Open

test(rest): pin the issue codes on #478's two search-entry failure paths#518
aacruzgon wants to merge 1 commit into
fix/504-batch-entry-issue-codesfrom
fix/478-search-entry-issue-codes

Conversation

@aacruzgon

@aacruzgon aacruzgon commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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:

Err(e) => {
    let (status, _, details) = e.client_response();
    create_error_result(status.as_u16(), &details)
}

That is the same code-discard #504 removed from every other call site in the file: client_response
computes the correct FHIR issue code, the _ throws it away, and the wrapper stamps processing over
the result. Left alone, a search entry would have been the one path in the handler still answering
processing after #504.

The rebase below this commit migrated both to entry_failure. This PR pins that, so they cannot
drift back.

Changes

Three tests in batch_entry_issue_codes.rs:

Test Drives Asserts
a_failed_search_entry_reports_its_issue_code batch: Patient?_query=byName, Patient?code:not-in=… 400 invalid, 501 not-supported
a_failed_transaction_search_entry_reports_its_issue_code transaction: Patient?_query=byName 400 invalid, per-entry, bundle still 200
a_failed_search_is_described_identically_by_both_surfaces both surfaces, same bad search severity, code and details.text all agree, plus a literal pin

The parity test's literal pin is load-bearing: processing == processing satisfies the three
equalities 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 >= 400 guard and return TransactionError::BundleError, which carries
neither 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:

Bare type-level GETGET Patient in a batch entry is read as an instance read with an empty
id and answers 404 not-found with the message Resource Patient/ not found. Executing it as a
search is #478.

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

cargo test -p helios-rest   →  1117 passed, 0 failed
cargo fmt --all --check     →  clean

Verified non-vacuous. With both sites reverted to their original code-discard, all three fail:

left: String("processing")   right: "invalid"
left: String("processing")   right: String("invalid")
left: String("processing")   right: "invalid"

Clippy: CI's invocation (ci.yml:497, which carries -A collapsible_if and 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 bare cargo clippy --all-targets --all-features -- -D warnings, but not of what CI runs. All 42 crates/rest warnings 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/rest lint set is 42 → 42, zero introduced, zero removed against the base with forced recompilation.

Notes

Refs #478, #504

@claude

claude Bot commented Aug 6, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@aacruzgon
aacruzgon force-pushed the fix/478-search-entry-issue-codes branch from 6abd69d to 0a1dcea Compare August 6, 2026 17:25
Base automatically changed from fix/478-bundle-get-search to fix/504-batch-entry-issue-codes August 7, 2026 14:48
@aacruzgon
aacruzgon force-pushed the fix/478-search-entry-issue-codes branch from 0a1dcea to ff60e9a Compare August 7, 2026 14:52
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
aacruzgon force-pushed the fix/478-search-entry-issue-codes branch from ff60e9a to 61af249 Compare August 9, 2026 20:32
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

1 participant