fix(fixed/databases): add missing 'subscription' field to AccountFixedSubscriptionDatabases#81
Merged
Conversation
…dSubscriptionDatabases
The Essentials list endpoint (`GET /fixed/subscriptions/{id}/databases`)
returns a wrapper whose `subscription` field carries the actual databases.
The OpenAPI schema for `AccountFixedSubscriptionDatabases` lists only
`accountId` and `links`, but the spec's own `example` for the same schema
clearly shows a single `subscription` object containing `subscriptionId`,
`numberOfDatabases`, `databases[]`, and `links`.
The Rust struct mirrored only the broken schema. The result: every call to
`FixedDatabaseHandler::list()` returned an `account_id`+`links` wrapper
with the `databases` array silently dropped. The existing test passed
because it only asserted on the two fields that did deserialize.
This commit:
- Adds the missing `subscription: Option<FixedSubscriptionDatabasesInfo>`
field on `AccountFixedSubscriptionDatabases`.
- Introduces `FixedSubscriptionDatabasesInfo` mirroring the Pro side's
`SubscriptionDatabasesInfo`. Note: Essentials wraps it as a single
object (not an array — that's an architectural difference from Pro
documented in the doc comments).
- Replaces the previous vacuous test with one that exercises the full
shape including a populated `databases` array, asserting database id
and name come through.
Harmonizing `list()` to return `Vec<FixedDatabase>` directly (as the
Pro side already does) is out of scope here and tracked under #65.
Closes #73
Refs #40
fc91b86 to
c57d32e
Compare
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
subscriptionfield toAccountFixedSubscriptionDatabasessoFixedDatabaseHandler::list()actually surfaces the databases array.FixedSubscriptionDatabasesInfomirroring the Pro side'sSubscriptionDatabasesInfo, but as a single object (not an array — Essentials returns one, not many).test_get_fixed_subscription_databaseswith one that asserts a populateddatabases[]round-trips: database id, name, and thesubscription_id/number_of_databasessummary.Why
The OpenAPI schema for
AccountFixedSubscriptionDatabasesdeclares onlyaccountIdandlinksas properties — but the spec's ownexamplefor the same schema showssubscriptionas an object containingdatabases[]. The Rust struct mirrored the schema, not the example. Result: every list call returned an empty wrapper with no way to access the databases. The existing test passed only because it asserted on the two fields that did deserialize.Scope
Tightly scoped to fixing the response shape. Harmonizing
list()to returnVec<FixedDatabase>directly (as the Pro side already does via the harmonization work) is out of scope here and remains tracked under #65.Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— all tests pass (288 total)cargo test --test fixed_databases_tests— 11 passCloses #73
Refs #40 (spec deltas tracking)