fix: Cast paginated list items to their element type - #113
Merged
Conversation
Fixes #112. Every list endpoint with a named `*List` envelope in the spec cast each item with the envelope's `from_map/1` rather than the element's, so 35 of 40 paginated endpoints returned items with every field dropped and `membership.organization_id` raised `KeyError`. 3.0.0 shipped this with a green suite because the generated fixtures and assertions were wrong in the same direction: the fixture nested an envelope inside `data` — a shape the API never returns — the test asserted the envelope struct, and the `@spec` claimed `Page.t(FooList.t())`, so dialyzer saw a consistent story too. Regenerated with the emitter fix in workos/oagen-emitters#210. The `*List` envelope structs go away with it: `Page` is what these functions have always returned, and the envelope structs only ever surfaced as the empty husks this bug produced. The two envelopes that non-paginated endpoints return by name (`VersionListResponse`, `GroupRoleAssignmentList`) are unaffected.
Spec drift the regeneration for #112 picked up, unrelated to that fix. Kept separate so the pagination fix can be reviewed, released, or reverted on its own.
|
Too many files changed for review (131 files, 100 file limit). Bypass the limit by tagging |
Contributor
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.
Fixes #112. Requires workos/oagen-emitters#210 — this branch is the regenerated output of that emitter fix.
Summary
Page.from_map/4casts each element ofdata, but the generated code passed the*Listenvelope'sfrom_map/1instead of the element's. Every item came back as an empty%FooList{object: "list", data: nil, list_metadata: nil}, and reading a real field raisedKeyError. The reporter hit it onlist_organization_memberships; it also affectedlist_users,list_invitations,list_user_api_keys, all ofWorkOS.Authorization, and 28 more.AuditLogAction,AuditLogSchema,RedirectUri,CORSOriginResponse,UserSessionsListItem), confirming the reporter's codegen hypothesis.data— a shape the API never returns — the test asserted%WorkOS.UserList{object: "list"}, and the@specclaimedPage.t(UserList.t()), so dialyzer agreed as well. The bug was self-consistent end to end.*Listenvelope structs are removed. Once the caster was fixed, 24 of 27 were referenced by nothing.Pageis what these functions have always returned, so the envelopes only ever surfaced as the empty husks this bug produced.VersionListResponseandGroupRoleAssignmentListremain — non-paginated endpoints return them by name.agent.registration.refreshed) that the regeneration picked up. Split out so this fix can be reviewed and released on its own.Compatibility
This drops 47 modules from the public surface, so code that named a
*Liststruct in a@specor pattern match will no longer compile. In practice that code could only ever have been working around this bug — the structs were never populated with anything.