Problem/Motivation

FractionItem does not define the static method mainPropertyName() and falls back to the FieldItemBase value of 'value'. The Fraction field does not have a column named 'value', and fails a hasData() call.

Steps to reproduce

  1. Create a CommerceEntityTrait field with a custom fraction field type.
  2. Save the trait to an entity type.
  3. Try to load the configuration page on the entity type.

The page will fail to load. The Commerce ConfigurableFieldManager will run a call to hasData() on that field, which will fail because the column 'value' cannot be found in the base table.

Proposed resolution

Add the 'numerator' column as the fields main property value.
Return NULL from the mainPropertyName() static method.

Remaining tasks

See merge request.

Issue fork fraction-3217975

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

alexandersluiter created an issue. See original summary.

alexandersluiter opened merge request !2

alexandersluiter opened merge request !3

alexandersluiter closed merge request !2

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Version: 8.x-1.3 » 2.x-dev

Thanks for opening this @alexandersluiter - and for the merge request!

I wasn't aware of this method (getMainProperty()) - I'm curious where is is used, apart from Commerce, as well as more detail about how Commerce uses it.

This might be a bit tricky - `numerator` might not be the best solution. It depends on where this "main property" is used.

Without a denominator, the numerator field is sort of meaningless. So if the numerator is loaded by itself and used for something - I'm worried that might lead to issues.

The Commerce ConfigurableFieldManager will run a call to hasData() on that field, which will fail because the column 'value' cannot be found in the base table.

Is this actually an issue with Commerce's ConfigurableFieldManager::hasData() logic I wonder?

Haven't dug too deep, but I found this older issue that sounds similar/related: #3072156: ConfigurableFieldManager::hasData() doesn't work for address field types

alexandersluiter’s picture

alexandersluiter commented
Related issues: +#3217962: Invalid mainPropertyName value

I've updated the merge request to return a NULL value as the FieldItemInterace allows for it. It should not have any unintended consequences now. It also fixes the issue that the Commerce ConfigurableFieldManager exposes. I'm not sure what else uses it, outside of Commerce, however, the interface exists and the method is used at least once in a fairy large module project. The Interval project had the same issue and settled on returning NULL.

https://www.drupal.org/project/interval/issues/3217962

alexandersluiter’s picture

alexandersluiter commented
Issue summary: View changes

m.stenta’s picture

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

Thanks @alexandersluiter! I merged this into the 2.x branch (this is where all new development is happening, with support for Drupal 8 and 9).

I'm hoping to tag 2.0.0 in the near future.

alexandersluiter’s picture

alexandersluiter commented
Status: Fixed » Active

Awesome! Thank you!

m.stenta’s picture

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

Status: Fixed » Closed (fixed)

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