Do not override existing filters in automatic update

Created on 14 February 2025, 24 days ago

Problem/Motivation

The update function _views_core_entity_reference_update_as_a_reference blindly writes the renamed filter to the filter array. This can override existing filters which is bad.

Steps to reproduce

  • Have a view with the filters field_related_articles_target_id_reference and field_related_articles_target_id.
  • Install this module.
  • Notice the absence of the old field_related_articles_target_id filter, having been overwritten by the new field_related_articles_target_id filter which is the renamed field_related_articles_target_id_reference filter.

Proposed resolution

Use \Drupal\views\ViewExecutable::generateHandlerId to generate a unique filter ID and use it throughout all the properties.

Remaining tasks

  • Implement solution
  • Implement tests
🐛 Bug report
Status

Active

Version

1.0

Component

Code

Created by

🇩🇪Germany tgauges

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

Merge Requests

Comments & Activities

  • Issue created by @tgauges
  • 🇬🇧United Kingdom scott_euser

    So your case is where you created both?

    In most cases I expect people actually want it to get maintained so rather than creating unique, maybe only make it unique if the target of the change already exists?

  • Pipeline finished with Failed
    24 days ago
    Total: 148s
    #424469
  • 🇩🇪Germany tgauges

    @scott_euser

    yes, this is for when both types of filters exist.

    \Drupal\views\ViewExecutable::generateHandlerId does exactly that: It only changes the ID if it is not unique.

  • 🇬🇧United Kingdom scott_euser

    Makes sense, thanks! I'll give it a play as soon as I can.

  • 🇬🇧United Kingdom scott_euser

    This looks fine thanks! Just needs phpcs fixes (minor) and test fixes.

    For the teset fixes, it seems the source yml in the new field you added contains the attributes of the entity reference plugin yet is marked as 'numeric' plugin which I believe is what is causing schema errors.

  • Pipeline finished with Failed
    21 days ago
    Total: 170s
    #426285
  • Pipeline finished with Success
    21 days ago
    #426303
  • 🇩🇪Germany tgauges

    Thanks for the hint regarding the numeric plugin. I fixed the issues and the MR can be reviewed.

  • 🇬🇧United Kingdom scott_euser

    Thanks! This looks good; I would resolve merge conflicts myself from your other issue, but a bit nervous I'll break things - hope you don't mind this last step.

    Thank you

  • Pipeline finished with Canceled
    20 days ago
    Total: 72s
    #427272
  • Pipeline finished with Success
    20 days ago
    Total: 175s
    #427274
  • 🇩🇪Germany tgauges

    I merged the current 1.0.x-Branch into the issue branch and updated a comment, please review :)

  • Pipeline finished with Skipped
    13 days ago
    #432970
  • 🇬🇧United Kingdom scott_euser

    Cheers thank you!

Production build 0.71.5 2024