Problem/Motivation

It would be convenient if the fraction field item value could be set from a single integer/decimal/string value.

From the Drupal field API this would effectively be a shortcut in place of using Fraction::createFromDecimal() to explicitly set the numerator and denominator properties of the fraction field.

More importantly, this would allow fraction field items to be set via JSONAPI by only providing a single decimal value instead of separate numerator and denominator values. This means API consumers do not need to implement logic to convert a decimal value to a fraction numerator + denominator.

Currently if the fraction field item is provided as a single value in a JSONAPI request a 500 response is returned with the following watchdog message:

	Error: Call to a member function getClass() on null in Drupal\jsonapi\Normalizer\FieldItemNormalizer->denormalize() (line 125 of /opt/drupal/web/core/modules/jsonapi/src/Normalizer/FieldItemNormalizer.php)
#0 /opt/drupal/web/modules/jsonapi_extras/src/Normalizer/JsonApiNormalizerDecoratorBase.php(45): Drupal\jsonapi\Normalizer\FieldItemNormalizer->denormalize(200.57, 'Drupal\\fraction...', 'api_json', Array)
#1 /opt/drupal/vendor/symfony/serializer/Serializer.php(196): Drupal\jsonapi_extras\Normalizer\JsonApiNormalizerDecoratorBase->denormalize(200.57, 'Drupal\\fraction...', 'api_json', Array)
#2 /opt/drupal/web/core/modules/jsonapi/src/Serializer/Serializer.php(75): Symfony\Component\Serializer\Serializer->denormalize(200.57, 'Drupal\\fraction...', 'api_json', Array)
#3 /opt/drupal/web/core/modules/jsonapi/src/Normalizer/FieldNormalizer.php(65): Drupal\jsonapi\Serializer\Serializer->denormalize(200.57, 'Drupal\\fraction...', 'api_json', Array)
#4 /opt/drupal/vendor/symfony/serializer/Serializer.php(196): Drupal\jsonapi\Normalizer\FieldNormalizer->denormalize(200.57, '\\Drupal\\Core\\Fi...', 'api_json', Array)
#5 /opt/drupal/web/core/modules/jsonapi/src/Serializer/Serializer.php(75): Symfony\Component\Serializer\Serializer->denormalize(200.57, '\\Drupal\\Core\\Fi...', 'api_json', Array)
#6 /opt/drupal/web/core/modules/jsonapi/src/Normalizer/ContentEntityDenormalizer.php(85): Drupal\jsonapi\Serializer\Serializer->denormalize(200.57, '\\Drupal\\Core\\Fi...', 'api_json', Array)
#7 /opt/drupal/web/core/modules/jsonapi/src/Normalizer/EntityDenormalizerBase.php(99): Drupal\jsonapi\Normalizer\ContentEntityDenormalizer->prepareInput(Array, Object(Drupal\jsonapi_extras\ResourceType\ConfigurableResourceType), 'api_json', Array)
#8 /opt/drupal/web/modules/jsonapi_extras/src/Normalizer/JsonApiNormalizerDecoratorBase.php(45): Drupal\jsonapi\Normalizer\EntityDenormalizerBase->denormalize(Array, 'Drupal\\quantity...', 'api_json', Array)
#9 /opt/drupal/web/modules/jsonapi_extras/src/Normalizer/ContentEntityDenormalizer.php(16): Drupal\jsonapi_extras\Normalizer\JsonApiNormalizerDecoratorBase->denormalize(Array, 'Drupal\\quantity...', 'api_json', Array)

This error stems from the fact that the fraction field item's "main property name" is NULL, but I think that is OK, NULL is an allowed value. This change was made in #3217975: Invalid mainPropertyName value.

We might actually be hitting a bit of an edge case in the JSONAPI FieldItemNormalizer logic: https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/modules/json...

This seems related to #3048348: Denormalizing NULL for an optional @FieldType=address or @FieldType=geolocation field fails due to either no main property name or computed read-only main property but isn't quite the same scenario. We want to provide a single value to a field type that has *no* main property name. It's kind of like a reverse computed field.

This behavior is similar to how the drupal/geofield Geofield field type works - you provide a single raw geometry value which then populates many other properties of the field (like a geometry bounding box). The difference is that none of those field properties are computed, they are all saved in the database. And notable that field type does have a main property name.

Steps to reproduce

Submit a single value (integer, decimal or string) for a fraction field (rather than separate numerator and denominator values). Observe a 500 error.

Proposed resolution

Implement the FractionItem::setValue() method to allow the fraction field to be populated from a single value.
Use Fraction::createFromDecimal to populate the field's numerator and denominator properties.

Remaining tasks

Test and implement.

User interface changes

None.

API changes

Fraction field values can be provided as a single value.

Data model changes

None.

Issue fork fraction-3243488

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

paul121 created an issue. See original summary.

paul121 opened merge request !8

paul121’s picture

paul121 commented

I've implemented a setValue function in my MR. We should probably add tests for this, but I think it would be best to do so in a new Kernel test class dedicated to the FractionItem field type. So this needs works, but setting to "Needs review" to determine if we should take this further!

Overall this change works fine with the Drupal field API, but is still not enough to make this work for JSONAPI requests (the same reason as described above). For this reason it seems like this might be a bug in JSONAPI, but it is quite the edge case. Still, a 500 error is not great.

What this does make possible for JSONAPI, however, is specifying a single value *in an array*.

Where the fraction field is named "value", sending the single decimal value in an array works:

    "attributes": {
        "value": {
            "value": 12.3
        }
    }

But sending the decimal as a single value does not:

    "attributes": {
        "value": 12.3,
    },

An example of this usage in Drupal field API, tested with drush php:

Specifying a single decimal value:

>>> $q = Drupal\quantity\Entity\Quantity::create(['type' => 'standard']);
>>> $q->set('value', 12.3);
>>> $q->save()
=> 1
>>> $q->get('value')->numerator
=> "123"
>>> $q->get('value')->denominator
=> "10"
>>> $q->get('value')->value
=> "12.300000000"

Or specifying the value in an array:

>>> $q = Drupal\quantity\Entity\Quantity::create(['type' => 'standard']);
>>> $q->set('value', ['value' => 12.3]);
>>> $q->save()
=> 1
>>> $q->get('value')->numerator
=> "123"
>>> $q->get('value')->denominator
=> "10"
>>> $q->get('value')->value
=> "12.300000000"
paul121’s picture

paul121 commented
Status: Active » Needs review

Added a commit to use is_numeric() to check for valid values before populating the fraction properties.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks @paul121!

We're gonna want some tests for this, I think.

paul121’s picture

paul121 commented
Status: Needs work » Postponed

I'm hoping to re-use the existing kernel test for fraction field tests more generally in #3243485: Include fraction decimal in JSONAPI.

Marking this as postponed because adding tests will be dependent on this change.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Related issues: +#3243485: Include fraction decimal in JSONAPI

Setting this back to "Active" now that #3243485: Include fraction decimal in JSONAPI is merged.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Postponed » Active
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Active » Needs work

I rebased the MR branch and added one commit to change the property name from value to decimal, to match the change made in #3243485: Include fraction decimal in JSONAPI.

Setting this to "Needs work" so we can add tests.

paul121’s picture

paul121 commented
Status: Needs work » Needs review

Oops, accidentally committed my test to ensure the test was working. This should pass! Simple test. Do we need anything else?

  • m.stenta committed b4e61c5 on 2.x 
    Issue #3243488 by paul121, m.stenta: Allow fraction field item value to...
m.stenta’s picture

m.stenta
he/him
Primary language English
commented

Thanks @paul121! Merged! I'll tag a new release shortly...

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.