Skip to content

fix(jsonapi): default Resource relationships to serde_json::Value#86

Merged
shadowhand merged 1 commit intorefactor-extract-domain-modulefrom
fix-jsonapi-relationships-default
Apr 28, 2026
Merged

fix(jsonapi): default Resource relationships to serde_json::Value#86
shadowhand merged 1 commit intorefactor-extract-domain-modulefrom
fix-jsonapi-relationships-default

Conversation

@shadowhand
Copy link
Copy Markdown
Contributor

Summary

Fixes a JSON:API deserialization regression introduced in #77 and surfaced in code review on #78.

The bug

Resource<A, R = ()> was added in #77 as a generic envelope. The previous hand-rolled *Resource types (TeamResource, ClinicianResource, etc.) had no relationships field at all, so serde silently skipped the key. The new generic declares the field explicitly with R defaulting to (), which only deserializes from null or an absent key.

Real JSON:API responses for clinicians, teams, etc. carry populated relationships objects (e.g. {"team": {"data": {...}}}). Against the live API those calls would have failed at runtime with invalid type: map, expected unit. Every test mock omitted relationships, so the regression slipped through CI.

Reproduced locally:

relationships field R = () (before) R = serde_json::Value (after)
absent OK OK
null OK OK
{} FAIL OK
populated object FAIL OK

The fix

Switch the default in src/jsonapi.rs from R = () to R = serde_json::Value. Value accepts any JSON shape and Default::default() returns Value::Null. Callers that don't read .relationships are unaffected; callers that do (the typed-relationships path in test_typed_relationships) are unaffected since they pass an explicit R.

Test additions

Three regression tests covering null, empty-object, and populated-object cases. The third would have caught this in #77 had it been there. Test count: 291 → 294 (276 lib + 7 bin + 11 integration).

Spotted by

Devin AI's review on #78.

Test plan

  • cargo fmt --check clean
  • cargo clippy --all-targets --all-features -- -D warnings clean
  • cargo test — 294 pass

https://claude.ai/code/session_012fVDyKBysF1ERtvVuEAw2Y


Generated by Claude Code

The generic `Resource<A, R = ()>` introduced in #77 silently regressed
deserialization: the previous hand-rolled `*Resource` types had no
`relationships` field at all, so serde skipped it; the new generic
declares it explicitly as `R` and defaults `R` to `()`, which only
deserializes from null or absent.

Real JSON:API responses for clinicians, teams, and others carry
populated relationships objects (e.g. `{"team": {"data": {...}}}`).
Against the live API those calls would have failed at runtime with
`invalid type: map, expected unit`. Test mocks all omitted the
relationships key, so the regression slipped through CI.

Switching the default to `serde_json::Value` accepts any JSON shape
on the field — null, object, array, primitive, or absent — and
callers that don't read it can ignore it. Callers that need typed
relationships still pass an explicit `R` (see test_typed_relationships
and the existing call sites for `prepare`'s WorkspaceAttrs etc.,
which use named structs and are unaffected).

Adds three regression tests covering null, empty object, and populated
object cases — the third would have caught this had it been there.

Spotted by Devin AI in review of #78.
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@shadowhand shadowhand merged commit b5df7db into refactor-extract-domain-module Apr 28, 2026
4 checks passed
@shadowhand shadowhand deleted the fix-jsonapi-relationships-default branch April 28, 2026 02:35
shadowhand added a commit that referenced this pull request Apr 28, 2026
…fault

fix(jsonapi): default Resource relationships to serde_json::Value
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.

2 participants