Repository navigation
Conversation
github-actions
Bot
added
the
Scope: Backend
joshuarli
force-pushed
the
josh/typing-contracts/c1-empty-number-fields
branch
from
November 3, 2026 16:50
144ae53 to
70a4a2d
Compare
EmptyDecimalField and EmptyIntegerField both convert a blank string to None in run_validation, which DRF calls before to_internal_value. The blank check in their to_internal_value overrides could not be reached through DRF, but #129614 typed it by casting None to Decimal and int, so both overrides claimed a non-null result while holding a None branch. Dropping the overrides leaves run_validation (typed `Decimal | None` / `int | None`) as the only blank path, and to_internal_value inherits DRF's honest `Decimal` / `int` contract. MissedMarginField now uses that contract instead of `Any -> Any`.
joshuarli
force-pushed
the
josh/typing-contracts/c1-empty-number-fields
branch
from
November 3, 2026 17:03
70a4a2d to
f292a21
Compare
joshuarli
marked this pull request as ready for review
November 3, 2026 17:06
Member
|
Nice, that was my conclusion as well (that in the end None cast was not dangerous at all, despite looking very off). Good to see there's redundant code you can just remove instead of struggle to type correctly. |
vjeranc
approved these changes
Nov 9, 2026
This branch has not been deployed
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.
EmptyDecimalFieldandEmptyIntegerFieldno longer castNonetoDecimal/int. Since #129614, theirto_internal_valueoverrides claimed a non-null result while returningNonefor blank input.That branch was unreachable through DRF. Both fields already map
""toNoneinrun_validation, which DRF calls beforeto_internal_value, so the overrides are removed rather than re-typed. Blank input still validates toNonethroughrun_validation(typedDecimal | None/int | None), andto_internal_valuenow inherits DRF's accurateDecimal/intcontract.MissedMarginFielduses that contract instead ofAny -> Any.Behavior change: none through serializers. Calling
field.to_internal_value("")directly now raisesValidationErrorlike the base DRF field instead of returningNone. Nothing in sentry or getsentry calls it directly. Blank input through serializers stays covered bytest_resolve_ageintests/sentry/core/endpoints/test_project_details.py.