Define typed_data support in enum-related PropType plugin definition

Created on 21 November 2024, 12 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
  • 🇫🇷France pdureau Paris
  • 🇩🇪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.

  • 🇩🇪Germany hctom
  • Pipeline finished with Success
    11 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

  • 🇫🇷France pdureau Paris
  • 🇫🇷France pdureau Paris
  • Pipeline finished with Success
    11 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
    11 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
    11 months ago
    Total: 462s
    #352945
  • 🇫🇷France pdureau Paris

    Mikael, what do you think about this proposal?

  • Pipeline finished with Success
    11 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
    11 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
    11 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
    11 months ago
    Total: 356s
    #353730
  • Pipeline finished with Success
    11 months ago
    Total: 360s
    #353731
  • 🇫🇷France pdureau Paris
  • Pipeline finished with Failed
    11 months ago
    Total: 680s
    #359471
  • Pipeline finished with Canceled
    11 months ago
    Total: 881s
    #359478
  • Pipeline finished with Failed
    11 months ago
    Total: 1209s
    #359490
  • Pipeline finished with Failed
    11 months ago
    Total: 1113s
    #359503
  • Pipeline finished with Success
    11 months ago
    Total: 1839s
    #359512
  • 🇫🇷France pdureau Paris
  • Pipeline finished with Success
    10 months ago
    Total: 1101s
    #384874
  • Pipeline finished with Success
    10 months ago
    Total: 856s
    #384903
  • Pipeline finished with Skipped
    10 months ago
    #400700
  • Pipeline finished with Success
    8 months ago
    Total: 150s
    #435903
  • Pipeline finished with Success
    8 months ago
    Total: 191s
    #437984
  • Pipeline finished with Success
    8 months ago
    Total: 148s
    #440622
  • Pipeline finished with Success
    8 months ago
    Total: 146s
    #440690
  • Pipeline finished with Success
    8 months ago
    Total: 168s
    #440711
  • Pipeline finished with Success
    8 months ago
    Total: 150s
    #440995
  • Pipeline finished with Success
    4 months ago
    Total: 301s
    #533455
  • Pipeline finished with Success
    4 months ago
    Total: 307s
    #534011
  • Pipeline finished with Success
    4 months ago
    Total: 350s
    #534025
  • Pipeline finished with Success
    about 1 month ago
    Total: 2913s
    #606610
  • Pipeline finished with Canceled
    29 days ago
    Total: 585s
    #622047
  • Pipeline finished with Success
    29 days ago
    Total: 1256s
    #622055
  • Pipeline finished with Success
    22 days ago
    Total: 350s
    #628961
  • Pipeline finished with Failed
    20 days ago
    #630902
  • Pipeline finished with Canceled
    14 days ago
    Total: 108s
    #637609
  • Pipeline finished with Success
    14 days ago
    Total: 1132s
    #637613
  • Pipeline finished with Canceled
    14 days ago
    Total: 97s
    #637887
  • Pipeline finished with Canceled
    14 days ago
    Total: 306s
    #637891
  • Pipeline finished with Canceled
    14 days ago
    Total: 878s
    #637896
  • Pipeline finished with Success
    14 days ago
    Total: 1103s
    #637902
  • Pipeline finished with Success
    13 days ago
    Total: 1139s
    #638023
  • Pipeline finished with Success
    13 days ago
    Total: 1182s
    #638418
  • Pipeline finished with Success
    13 days ago
    Total: 1384s
    #638667
  • Pipeline finished with Success
    13 days ago
    Total: 1420s
    #638829
  • Pipeline finished with Success
    13 days ago
    Total: 1402s
    #638863
  • Pipeline finished with Success
    12 days ago
    Total: 661s
    #639359
  • Pipeline finished with Failed
    8 days ago
    Total: 250s
    #643650
  • Pipeline finished with Failed
    8 days ago
    Total: 2035s
    #643653
  • Pipeline finished with Failed
    8 days ago
    Total: 762s
    #643714
  • Pipeline finished with Failed
    8 days ago
    Total: 1248s
    #643778
  • Pipeline finished with Failed
    8 days ago
    Total: 2879s
    #643799
  • Pipeline finished with Failed
    8 days ago
    Total: 1540s
    #643895
  • Pipeline finished with Failed
    8 days ago
    Total: 1462s
    #643967
  • Pipeline finished with Failed
    8 days ago
    Total: 567s
    #643983
  • Pipeline finished with Failed
    8 days ago
    Total: 625s
    #643987
  • Pipeline finished with Failed
    8 days ago
    Total: 1283s
    #643985
  • Pipeline finished with Failed
    8 days ago
    Total: 1240s
    #643994
  • Pipeline finished with Success
    7 days ago
    Total: 1521s
    #644509
  • Pipeline finished with Failed
    7 days ago
    Total: 812s
    #644558
  • Pipeline finished with Failed
    7 days ago
    Total: 978s
    #644588
  • Pipeline finished with Success
    7 days ago
    Total: 1682s
    #644635
  • Pipeline finished with Failed
    7 days ago
    Total: 2644s
    #644659
  • Pipeline finished with Success
    7 days ago
    Total: 3672s
    #644718
  • Pipeline finished with Success
    7 days ago
    Total: 907s
    #644779
  • Pipeline finished with Success
    4 days ago
    #647127
  • Pipeline finished with Success
    2 days ago
    Total: 454s
    #649160
  • Pipeline finished with Failed
    2 days ago
    Total: 403s
    #649179
  • Pipeline finished with Canceled
    2 days ago
    Total: 81s
    #649206
  • Pipeline finished with Success
    2 days ago
    Total: 347s
    #649209
  • Pipeline finished with Failed
    about 20 hours ago
    Total: 1657s
    #650704
  • Pipeline finished with Failed
    about 5 hours ago
    Total: 851s
    #651411
Production build 0.71.5 2024