Skip to content

Support DX Decimal fields via a field manager - #114

Open
ramonski wants to merge 1 commit into
2.xfrom
fix/decimal-field-manager
Open

ramonski wants to merge 1 commit into
2.xfrom
fix/decimal-field-manager

Conversation

@ramonski

@ramonski ramonski commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

A zope.schema.Decimal field could not be written through the API at all. JSON has no type for a decimal.Decimal, and a string, a float and an integer all fail the field's own validation.

Description of the issue/feature this PR addresses

analysis_profile_price and analysis_profile_vat on AnalysisProfile are the only Decimal fields in the stack today. Setting either of them returned {"analysis_profile_price": "wrong type"}, so a profile's package price could only be entered through the browser form.

Current behavior before PR

Two things went wrong, and the second one hid behind the first.

The generic ZopeSchemaFieldManager validates the raw JSON value, which a Decimal field refuses. But the value never got that far: DexterityDataManager.set prefers the content type's own set<Name> mutator over the field manager, and setAnalysisProfilePrice passes the value straight to its mutator:

def setAnalysisProfilePrice(self, value):
    mutator = self.mutator("analysis_profile_price")
    mutator(self, value)

The raw string therefore landed in the field, and the object failed api.validate afterwards with wrong type. Registering an adapter alone does not fix this, which is why NORMALIZING_FIELD_MANAGERS exists.

Desired behavior after PR is merged

DecimalFieldManager coerces a string, an integer or a float to Decimal on set, and serializes back to a string, the only JSON form that keeps the exact value:

@staticmethod
def to_decimal(value):
    if value is None or isinstance(value, Decimal):
        return value
    if isinstance(value, bool):
        return value
    if isinstance(value, (six.string_types, int, float)):
        try:
            return Decimal(str(value).strip())
        except (InvalidOperation, ValueError):
            return value
    return value

A value that is not a number is handed on untouched, so the field reports it rather than this adapter raising an error of its own.

The manager also joins NORMALIZING_FIELD_MANAGERS, alongside UID references and durations, so the data manager goes through it instead of the setter. Neither Decimal setter does anything beyond calling its mutator, so nothing is lost by that route.

This is the same shape as #102 for Duration and #100 for UID references.

Verification

  • bin/test-senaite -s senaite.jsonapi: 38 tests, 0 failures, 0 errors.
  • The create doctest gained the Decimal case: a price as a string and a VAT as a plain integer both arrive as Decimal, and the field manager reads the price back as '24.00'.
  • The failure this fixes was found on a running instance, composing an AnalysisProfile over the API: before the change the request came back with {"analysis_profile_price": "wrong type"}. The end-to-end re-check on that instance is still outstanding, because the data manager change needs a restart to take effect.

I confirm I have tested the PR thoroughly and coded it according to PEP8 standards.

@ramonski
ramonski requested a review from xispa October 6, 2026 06:28
@ramonski ramonski added the Enhancement ✨ Improvement to existing functionality label Oct 6, 2026
@ramonski
ramonski force-pushed the fix/decimal-field-manager branch 3 times, most recently from 783cece to 3173cf6 Compare October 7, 2026 05:44
A `zope.schema.Decimal` only accepts a `decimal.Decimal`, and JSON has
no type for one. A price arrived as a string, a float or an integer and
every one of them failed the field's own validation, so a Decimal field
could not be written through the API at all.

The new DecimalFieldManager coerces those three on set and serializes
back to a string, which is the only JSON form that keeps the exact
value. A value that is not a number is handed on untouched, so the
field reports it rather than this adapter raising an error of its own.

The manager also joins NORMALIZING_FIELD_MANAGERS. Without that the
data manager prefers the content type's own set<Name> mutator, which
stores the value exactly as it arrives: `setAnalysisProfilePrice` calls
its mutator with the raw string, and the object then fails validation
with "wrong type". This is the same reason UID references and durations
are in that list.

`analysis_profile_price` and `analysis_profile_vat` on AnalysisProfile
are the only Decimal fields in the stack today, and neither setter does
anything beyond calling its mutator, so nothing is lost by going
through the field manager instead.
@ramonski
ramonski force-pushed the fix/decimal-field-manager branch from 3173cf6 to c7503b7 Compare October 7, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement ✨ Improvement to existing functionality

Development

Successfully merging this pull request may close these issues.

1 participant