Problem/Motivation

Currently it is not possible to e.g. use List (Text) Drupal fields for Data from a field source plugin on enum component properties.
While checking the PropType plugins, I saw, that the enum plugin does not have its supported typed_data values defined.

Steps to reproduce

  • Define a component with a property of enum type
  • Use that component on an entity with a List (text) field (or even List (integer) or List (float)) and try to set the component's enum property value with the Data from a field source plugin -> the list field(s) are not available

Proposed resolution

  • Add string, float, integer as supported types to typed_data definition values on enum property plugin
  • Ensure that potential field values, that are missing in the component's enum-definition do not result in fatal errors during property validation (e.g. silently remove that value, if it does not exist in the component's property definition)

Remaining tasks

  • Create issue fork and MR to fix this issue
  • Decide how field values should be handled, when not available in component's property definition, to avoid fatal errors form property validator

User interface changes

n/a

API changes

  • enum PropType plugin will have string, float, integer entries in its typed_data definition value

Data model changes

n/a

Issue fork ui_patterns-3488879

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

hctom created an issue. See original summary.

hctom’s picture

hctom
Primary language German
commented
Title: Define typed_data support in enum-related PropType plugin definition » Define typed_data support in enum-related PropType plugin definitions
pdureau’s picture

pdureau commented
Title: Define typed_data support in enum-related PropType plugin definitions » [2.0.0-beta5] Define typed_data support in enum-related PropType plugin definitions
hctom’s picture

hctom
Primary language German
commented
Issue summary: View changes

Update to issue summary to name the right typed_data values for number fields (because number is not correct there / not available as typed data type)

hctom’s picture

hctom
Primary language German
commented
Issue summary: View changes

Removed decimal from list of typed_data values to add, because there is not List (decimal) field type in Drupal.

hctom’s picture

hctom
Primary language German
commented
Assigned: Unassigned » hctom

hctom opened merge request !272

hctom’s picture

hctom
Primary language German
commented
Assigned: hctom » Unassigned
Status: Active » Needs review

So, finally got the time to create my first draft for this:

  • Added the typed_data values to the enum* PropType plugins
  • Added some logic to ensure field source value is a valid enum option - if not, it will fall back to a valid value in that order:
    • property default value (if defined)
    • first enum option value (if property is required)
    • NULL (if property is optional)

...and another idea: Should invalid field values be logged somehow?

Looking forward to your review ;)

pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau

i will have a look

pdureau’s picture

pdureau commented
Title: [2.0.0-beta5] Define typed_data support in enum-related PropType plugin definitions » [2.0.0-rc1] Define typed_data support in enum-related PropType plugin definitions
pdureau’s picture

pdureau commented
Title: [2.0.0-rc1] Define typed_data support in enum-related PropType plugin definitions » [2.0.0-beta5] Define typed_data support in enum-related PropType plugin definitions
pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Needs review » Needs work

Hi Tom,

Tested with enum, it works well, thanks a lot

However, you did the change also for enum_set and enum_list and I am struggling to see the use cases this is covering:
What is the best way of testing integration with enum_set and enum_list ?

For information, enum_set is an array of values where:

  • every value is from the same enum
  • and each value can be found only one time

enum_list is an array of values where:

  • every value is from the same enum
  • and each value can be found many time
pdureau’s picture

pdureau commented

Following our Slack discussion, I believe we need to focus on enum prop type and create a follow-up issue for others.

Is it the opportunity to simplify the logic in EntityFieldSource::getPropValue() ?

Do you know about EnumTrait::convertValueToEnumType() ? it may be useful to send string values to numerical enums.

hctom’s picture

hctom
Primary language German
commented
Title: [2.0.0-beta5] Define typed_data support in enum-related PropType plugin definitions » [2.0.0-beta5] Define typed_data support in enum PropType plugin definition
Issue summary: View changes

Update issue title and summary to target enum property type plugins only with this ticket for now.

hctom’s picture

hctom
Primary language German
commented
Status: Needs work » Needs review

Changed code to only target enum property plugins for now, simplified the implementation of EntityFieldSource::getPropValue() a little and used EnumTrait::convertValueToEnumType() for the final (rectified) return value.

Looking forward to the new review results and please don't forget to say something about the idea to log invalid values.

pdureau’s picture

pdureau commented
Assigned: Unassigned » just_like_good_vibes

Mikael, what do you think about this proposal?

just_like_good_vibes made their first commit to this issue’s fork.

pdureau’s picture

pdureau commented
Status: Needs review » Needs work

Move the logic to ::normalize() and add an optional prop definition paramater.

Move some logic from EnumTrait to ::normalize() ? Careful because this trait is also used in sources.

just_like_good_vibes opened merge request !275

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

hctom, Pierre,
yes sorry i have continued the work from hctom but moved the logic to the prop types, which is more natural.
the code is almost ready.
i will try to finish for tomorrow morning, and also add a few more tests.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: just_like_good_vibes » pdureau
Status: Needs work » Needs review

i added some tests to the MR too,
let's discuss asap?

pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Needs review » Fixed
pdureau’s picture

pdureau commented
Status: Fixed » Closed (fixed)