fix(services): reject/ignore explicit null in partial updates for NOT NULL fields - #14040
Draft
stilla[bot] wants to merge 1 commit into
Draft
fix(services): reject/ignore explicit null in partial updates for NOT NULL fields#14040stilla[bot] wants to merge 1 commit into
stilla[bot] wants to merge 1 commit into
Conversation
…domains Batch fix for the "explicit null bypasses exclude_unset partial update" bug class: an update schema types a field Optional so it can be omitted, but the field maps to a NOT NULL column. Pydantic v2 marks an explicitly-sent JSON `null` as set, so `model_dump(exclude_unset=True)` keeps it and the None reaches the ORM object, causing an IntegrityError (sometimes surfacing later via an unrelated autoflush). Mirrors the fix pattern established for `discount.duration` in PR #10491 — checking `model_fields_set` / guarding the raw dict application — and follows each file's existing sibling-field behavior (reject vs. silently ignore): - discount: reject explicit null for `type` (via the existing "cannot be changed" path) and `name`. - benefit: reject explicit null for `description` (the Sentry incident field). - product: exclude `name`/`visibility`/`is_archived` from the trailing raw-dict loop so it can't overwrite the earlier `is not None` handling; add the missing `visibility` guard (silently ignore null). - organization: drop explicit-null `name`, `socials`, `embed_hosts`, `default_presentment_currency`, `default_tax_behavior`, `sso_enforced`, `customer_email_settings`, `customer_portal_settings` from update_dict, mirroring feature_settings/subscription_settings/dispute_settings (ignore). - checkout: skip explicit-null `is_business_customer`, `allow_discount_codes`, `require_billing_address`, `customer_metadata` in the raw-dict loop (ignore). - meter: exclude+conditionally re-add `name`/`unit`, mirroring filter/aggregation (ignore). - custom_field: skip explicit null in the raw setattr loop (ignore). - license_key: skip explicit-null `status` in the raw setattr loop (ignore). Adds service tests mirroring the discount.duration null-rejection coverage. Co-Authored-By: Stilla <stilla@stilla.ai>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
OpenAPI ChangesNo changes detected in the OpenAPI schema. |
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.
Context
Triggered by production Sentry issue SERVER-4ZZ: an
IntegrityError(NotNullViolationErroronbenefits.description) surfaced deep inside an unrelatedSELECT, because SQLAlchemy's autoflush tried to persist a pendingUPDATE benefits SET description=NULLfrom a priorPATCH /v1/benefits/{id}call.Root cause:
BenefitUpdateBase.descriptionis typedstr | None(so it can be omitted from a partial update), butbenefits.descriptionisNOT NULL. Pydantic v2 marks an explicitly-sent JSONnullas "set" inmodel_fields_set, so it survivesexclude_unset=Truefiltering and getssetattr'd straight onto the ORM object with no guard.This exact bug class was already fixed once for
discount.durationin #10491 (guard changed fromis not Noneto"duration" in model_fields_set). This PR applies the same defensive pattern everywhere else it was found unfixed across the codebase.Fields fixed
Reject explicit null (raises
PolarRequestValidationError, matching each file's existing idiom for invalid input):discount.type(reused the existing type-mismatch validation path)discount.namebenefit.description(the original incident field)Silently ignore explicit null (preserves the existing value, matching sibling fields in the same method that already do this):
product.name,product.visibility,product.is_archivedorganization.name,socials,embed_hosts,default_presentment_currency,default_tax_behavior,sso_enforced,customer_email_settings,customer_portal_settingscheckout.is_business_customer,allow_discount_codes,require_billing_address,customer_metadatameter.name,meter.unitcustom_field.name,slug,propertieslicense_key.statusTesting
ruff checkandmypypass on all changed files.No migrations or schemas were changed — service-layer guards only.
Sent by @pieterbeulque from Stilla investigation.