feat(organization): support partial updates of settings groups - #13815
feat(organization): support partial updates of settings groups#13815pieterbeulque wants to merge 2 commits into
Conversation
`customer_email_settings`, `subscription_settings`, and
`customer_portal_settings` on `PATCH /v1/organizations/{id}` were typed
as their full TypedDicts and applied by wholesale replacement, so a
caller had to send the complete object — omitting any nested field
returned a 422, and there was no way to toggle a single flag.
Give them the same partial-merge treatment `feature_settings` and
`dispute_settings` already have: dedicated `*Update` schemas with every
field optional, merged into the current settings in the service.
`customer_portal_settings` is deep-merged so a nested group can be
changed without resending its siblings. Full-object updates keep
working unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jfigrw4Jbz1MHSEMxgLBVY
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
OpenAPI ChangesSchemas
|
![cubic-dev-ai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Preview Environment |
When a legacy organization lacked an explicit `payment_method_expiration_reminder`, its fallback is derived from `subscription_cycled`. The partial-update merge resolved defaults against the *old* stored settings before applying the update, so changing `subscription_cycled` in the same PATCH left the persisted reminder following the old cycle. Merge the partial update first, then resolve defaults, so the reminder fallback follows the `subscription_cycled` value set in the same request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jfigrw4Jbz1MHSEMxgLBVY
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Adds optional-field update schemas and merge logic so PATCH can partially update organization settings groups, with new endpoint tests pinning the merge behavior and backwards compatibility. Diff is scoped and self-contained, no security, data, or operational tradeoffs introduced.
Re-trigger cubic
|
|
||
|
|
||
| class OrganizationCustomerEmailSettingsUpdate(Schema): | ||
| """Partial update for customer email settings. |
There was a problem hiding this comment.
Those doctoring will make their way to the OpenAPI, SDK and docs, so they should be a bit less technical or dropped entirely.
| } | ||
| # Resolve defaults after merging so a legacy organization that lacks | ||
| # `payment_method_expiration_reminder` derives its fallback from the | ||
| # `subscription_cycled` value set in this same request, not the old one. |
There was a problem hiding this comment.
That's why when we add a new setting, we shouldn't be lazy and make a proper migration. Otherwise, we need this kind of workarounds.
@cubic-dev Please remember that for the next time you see a new or removed organization setting.
There was a problem hiding this comment.
@frankie567 Saved as a learning. You can edit it anytime in Settings → AI review → Learnings.
There was a problem hiding this comment.
Yeah, maybe I'll clean up the data first so that we don't need to do this...
3 participants
Summary
Related Issue: N/A
PATCH /v1/organizations/{id}couldn't partially update thecustomer_email_settings,subscription_settings, orcustomer_portal_settingsgroups: callers had to send each group in full. This makes them behave likefeature_settingsanddispute_settings, which already support partial merges.What
OrganizationCustomerEmailSettingsUpdate,OrganizationSubscriptionSettingsUpdate, andOrganizationCustomerPortalSettingsUpdate(with nestedCustomerPortalUsageSettingsUpdate/CustomerPortalSubscriptionSettingsUpdate/CustomerPortalCustomerSettingsUpdate) — where every field is optional.OrganizationUpdatenow references these*Updatevariants instead of the full model TypedDicts.customer_portal_settingsis deep-merged so a nested group (e.g.subscription) can be changed without resending its siblings.clients/packages/client/src/v1.ts).Why
Previously these three groups were typed as their full TypedDicts and written by wholesale assignment. Because those TypedDicts require every key, omitting any nested field returned a
422, and there was no way to toggle a single flag (e.g. turning off onecustomer_emailnotification) without re-sending the whole object.feature_settingsanddispute_settingsalready merge partial input; this brings the remaining settings groups in line.How
*Updateschema mirrors its settings model but with all fields| None = None, so a subset validates.subscription_settings.proration_behaviorkeeps its public JSON-schema restriction (invoice/prorate/next_period), so the generated client surface is unchanged.OrganizationService.update, each group is merged withmodel_dump(mode="json", exclude_unset=True, exclude_none=True)on top of the stored value, and the two newly-handled groups are excluded from the genericupdate_dictso they aren't re-applied.customer_email_settingsis merged ontoresolve_default_customer_email_settings(...)so the stored value stays complete.Testing
customer_email_settings,subscription_settings, and deep-mergedcustomer_portal_settingsupdates, plus a full-objectcustomer_email_settingsupdate.TestUpdateOrganization(20 passed) andtests/organization/test_service.py+test_schemas.py(350 passed) against a local Postgres/Redis.ruff check,ruff format --check, andmypypass on the changed files.Checklist
uv run task lint && uv run task lint_types)🤖 Generated with Claude Code
https://claude.ai/code/session_01Jfigrw4Jbz1MHSEMxgLBVY
Generated by Claude Code