Update fraction and decimal properties via setValue()

Created on 5 April 2023, over 1 year ago
Updated 19 April 2023, over 1 year ago

Problem/Motivation

I discovered an issue in the farmOS Ledger module ( πŸ› Editing an existing price quantity does not update value/unit price/total price Fixed ) which I traced to the Fraction module, where the fraction and decimal properties of Fraction fields are not being updated properly when the field value is updated. This causes them to have the old value

Steps to reproduce

See πŸ› Editing an existing price quantity does not update value/unit price/total price Fixed for a real-world example.

This demonstrates a failing automated test in PHP:

    // Starting with a $quantity entity with a Fraction value field.
    $fraction_field = $quantity->get('value');

    // Set value with a decimal.
    $fraction_field->set(0, 1.2);
    $this->assertEquals(12, $fraction_field->first()->numerator);
    $this->assertEquals(10, $fraction_field->first()->denominator);
    $this->assertEquals('12/10', $fraction_field->first()->get('fraction')->getValue()->toString());
    $this->assertEquals('1.2', $fraction_field->first()->get('decimal')->getValue());

    // Change the value.
    $fraction_field->set(0, 12.3);
    $this->assertEquals(123, $fraction_field->first()->numerator);
    $this->assertEquals(10, $fraction_field->first()->denominator);
    $this->assertEquals('123/10', $fraction_field->first()->get('fraction')->getValue()->toString());
    $this->assertEquals('12.3', $fraction_field->first()->get('decimal')->getValue());

The last two assertEquals() fail because the fraction and decimal properties are still 12/10 and 1.2, respectively (not updated with the new value).

Proposed resolution

@paul121 found an example of a core field type that uses custom computed properties like we do: TextProcessed

https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/text...

One thing that the TextProcesses class does that ours don't is override the base setValue() method: https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/text...

Notably, it only changes one line from the base method:

$this->value = $value; to $this->processed = $value;

This is because the property uses a protected $processed class property instead of the default $value class property.

Likewise, our FractionProperty class uses a $fraction class property, and our FractionDecimalProperty class uses a $decimal class property. So we need to override setValue() similarly in both of our classes to ensure it is setting the correct class property.

Remaining tasks

  • Demonstrate the issue with a failing automated test.
  • Implement the fix and show that it fixes the failing test.

User interface changes

None.

API changes

None.

Data model changes

None.

πŸ› Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States m.stenta

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024