Define typed_data support in enum-related PropType plugin definition

Created on 21 November 2024, 5 months ago

Problem/Motivation

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

Steps to reproduce

  • Define a component with a property of any 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, number, integer as supported types to typed_data definition values on enum* source plugins
  • 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 plugins will have string, integer, number entries in their typed_data definition value

Data model changes

n/a

📌 Task
Status

Active

Version

2.0

Component

Code

Created by

🇩🇪Germany hctom

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

Merge Requests

Comments & Activities

  • Issue created by @hctom
  • 🇩🇪Germany hctom

    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)

  • 🇩🇪Germany hctom

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

  • Pipeline finished with Success
    5 months ago
    Total: 461s
    #351204
  • 🇩🇪Germany hctom

    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)

    Looking forward to your review ;)

  • 🇫🇷France pdureau Paris

    i will have a look

  • Pipeline finished with Success
    5 months ago
    Total: 402s
    #352166
  • 🇫🇷France pdureau Paris

    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 ?

  • 🇫🇷France pdureau Paris

    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.

  • 🇩🇪Germany hctom

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

  • Pipeline finished with Canceled
    5 months ago
    Total: 147s
    #352942
  • 🇩🇪Germany hctom

    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.

  • Pipeline finished with Failed
    5 months ago
    Total: 462s
    #352945
  • 🇫🇷France pdureau Paris

    Mikael, what do you think about this proposal?

  • Pipeline finished with Success
    5 months ago
    Total: 335s
    #353057
  • First commit to issue fork.
  • 🇫🇷France pdureau Paris

    Move the logic to ::normalize()

  • Merge request !275temp propal → (Merged) created by just_like_good_vibes
  • Pipeline finished with Success
    5 months ago
    Total: 340s
    #353299
  • 🇫🇷France just_like_good_vibes PARIS

    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.

  • Pipeline finished with Canceled
    5 months ago
    Total: 77s
    #353729
  • 🇫🇷France just_like_good_vibes PARIS

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

  • Pipeline finished with Failed
    5 months ago
    Total: 356s
    #353730
  • Pipeline finished with Success
    5 months ago
    Total: 360s
    #353731
  • Pipeline finished with Failed
    4 months ago
    Total: 680s
    #359471
  • Pipeline finished with Canceled
    4 months ago
    Total: 881s
    #359478
  • Pipeline finished with Failed
    4 months ago
    Total: 1209s
    #359490
  • Pipeline finished with Failed
    4 months ago
    Total: 1113s
    #359503
  • Pipeline finished with Success
    4 months ago
    Total: 1839s
    #359512
  • Pipeline finished with Success
    3 months ago
    Total: 1101s
    #384874
  • Pipeline finished with Success
    3 months ago
    Total: 856s
    #384903
  • Pipeline finished with Skipped
    3 months ago
    #400700
  • Pipeline finished with Success
    about 2 months ago
    Total: 150s
    #435903
  • Pipeline finished with Success
    about 2 months ago
    Total: 191s
    #437984
  • Pipeline finished with Success
    about 1 month ago
    Total: 148s
    #440622
  • Pipeline finished with Success
    about 1 month ago
    Total: 146s
    #440690
  • Pipeline finished with Success
    about 1 month ago
    Total: 168s
    #440711
  • Pipeline finished with Success
    about 1 month ago
    Total: 150s
    #440995
Production build 0.71.5 2024