Continuation Add Views EntityReference filter to be available for all entity reference fields

Created on 10 March 2023, over 1 year ago
Updated 17 July 2024, 1 day ago

Continuation of 📌 Add Views EntityReference filter to be available for all entity reference fields Closed: duplicate since there were so many comments the page was struggling to load.

Slack thread

https://drupal.slack.com/archives/C6SQM2J94/p1706633005193429

Problem/Motivation

One major piece of functionality from the D7 Entity Reference module was left out entirely in #1801304: Add Entity reference field → : the ability to render exposed views filters as a select list or autocomplete of available entities.

Proposed resolution

Create a new Views Entity Reference filter plugin to be available for all entity reference fields and migrate the existing taxonomy filters to be based on the new generic filter plugin.

How to use

  1. Add on an entity type / bundle an entity reference field, ex field_test_reference.
  2. Create a view displaying this entity type.
  3. Add a filter on the view for field_test_reference.
  4. Configure the entity selection mode and the widget display mode.
  5. Configure the filter behavior (ex: required, multiple, etc.)
  6. Finally use the filter field for filtering the results based on the selected entity from the autocomplete or select list.

Pre tasks

Remaining tasks

  • ☑ support for content entity reference
  • ☑ support for configuration entity reference
  • ☑ support for content with and without bundles
  • ☑ settings forms to configure the filter
  • ☑ display widget in select or autocomplete
  • ☑ filter values based on reference view
  • ☑ filter values based on bundles
  • ☑ sort for filter values when in bundle selection handler mode
  • ☑ argument support for when view selection handler is used
  • ☑ views configuration schema update
  • ☑ tests for general functionality
  • ☐ Address points in comment #49 (comment 534 from original ticket)
    • ☑ add test coverage for calculate dependencies
    • ☐ sort by empty on initial load (needs more info), see comment #49
    • ☐ prevent circular reference: validate no selection of an entity reference that uses itself as the filter, see comment #49
    • ☐ node titles in the select options when configuring the filter are double escaped (e.g. " shows as "), see comment #49
    • ☐ exposed filters on the chosen entity reference view are also shown on the view, see comment #49
  • ☐ get framework manager approval
  • ☑ write change record

Post tasks

User interface


Known issues

  • CANNOT REPRODUCE ATM
  • FIXED
  • FIXED fixed

API changes

None.

For 10.2

See hidden branches and comment #143

✨ Feature request
Status

Postponed

Version

11.0 🔥

Component
Views  →

Last updated about 5 hours ago

Created by

🇺🇸United States smustgrave

Live updates comments and jobs are added and updated live.
  • Needs framework manager review

    It is used to alert the framework manager core committer(s) that an issue significantly impacts (or has the potential to impact) multiple subsystems or represents a significant change or addition in architecture or public APIs, and their signoff is needed (see the governance policy draft for more information). If an issue significantly impacts only one subsystem, use Needs subsystem maintainer review instead, and make sure the issue component is set to the correct subsystem.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @smustgrave
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • 🇦🇲Armenia Murz Yerevan, Armenia
  • 🇺🇸United States jhedstrom Portland, OR
  • 🇺🇸United States smustgrave

    Moving over credit. Last comment from @Lendude that need to be addressed

    @catch asked me to look at this again, mostly the test coverage. The test coverage seems mostly good, but ran into some other issues.

    calculateDependencies is untested and should be since this is different than the Term plugin
    The 'sort by' field is empty on initial adding of the filter (the field widget does something ajaxy when selecting bundles, the views widget does not, might be it?)
    Grouped autocomplete filters don't work/save (but don't work for taxonomy ref filters either), please find the related issue or open it (should be one I think)
    Description "as a Reference filter" feels a bit vague, did we get UX feedback on this?
    Should we add this new filter for taxonomy terms too, or should we skip those since it is unclear what the difference is now when have to chose between the two different filters
    I can create a circular reference by creating an Entity reference View that uses itself as the filter, not sure what goes wrong (mostly getting an empty filter after saving), but that sounds iffy
    Maybe related : Managed to click my way into "Warning: Undefined array key "reference_views" in Drupal\views\Plugin\views\filter\EntityReference->submitExtraOptionsForm() (line 440 of /app/drupal/core/modules/views/src/Plugin/views/filter/EntityReference.php)"
    Node titles in the select element are double escaped
    Exposed filters on the chosen entity reference view are also shown on the view

  • 🇪🇸Spain nuez Madrid, Spain
  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪
  • 🇳🇱Netherlands seanB Netherlands
  • 🇨🇭Switzerland Berdir Switzerland
  • 🇬🇧United Kingdom rlmumford Manchester
  • 🇮🇳India yogeshmpawar 🇮🇳 Pune(MH)
  • 🇺🇸United States xjm
  • 🇳🇿New Zealand RoSk0 Wellington
  • 🇳🇱Netherlands Lendude Amsterdam
  • 🇺🇸United States dww
  • @smustgrave opened merge request.
  • 🇺🇸United States smustgrave

    Copied over MR from original issue to here.

    Link to original MR https://git.drupalcode.org/project/drupal/-/merge_requests/3086#note_150528

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Tag from original issue.

  • 🇺🇸United States alison

    (fixing comment number in issue summary)

    Sidenote: I recommend editing #49 to get those list bullets back in, for readability.

  • 🇧🇪Belgium DieterHolvoet Brussels

    I found another issue when trying to use the new entity_reference filter on a user reference field.

    Steps to reproduce

    1. Enable the entity_reference filter for the node uid field

    /**
     * Implements hook_views_data_alter().
     */
    function my_module_views_data_alter(array &$data): void
    {
        $data['node_field_data']['uid']['filter']['id'] = 'entity_reference';
    }
    

    2. Add the Author filter (category Node) to a node view, changing Selection type to Select list.
    3. Open the view. The dropdown has no options.

    The reason this happens is because target_bundles is set to an empty array. The reason target_bundles is set to an empty array is because validateExtraOptionsForm failed, namely the following line:

    $subform_state->setValue([
      'settings',
    ], $subform_options);
    

    And that fails because SubformState::getValues fails: $this->getParents('#parents') returns ['reference_default:user'] and the array returned by parent::getValues() does not have a reference_default:user key, it does have an options key. That's as far as I got while debugging.

  • 🇺🇸United States JonMcL Brooklyn, NY

    @DieterHolvoet: I just happened to bump up against a similar situation. In my case it's a custom entity type that has no bundles.

    I think the issue is actually in DefaultSelection, which I do not think is being touched by this patch. I suspect that only users of this patch would be trying to use DefaultSelection with a bundle-less entity, so maybe that is why it hasn't been an issue before? I'm not entirely sure.

    In DefaultSelection::buildEntityQuery, there is this snippet:

        // If 'target_bundles' is NULL, all bundles are referenceable, no further
        // conditions are needed.
        if (is_array($configuration['target_bundles'])) {
          // If 'target_bundles' is an empty array, no bundle is referenceable,
          // force the query to never return anything and bail out early.
          if ($configuration['target_bundles'] === []) {
            $query->condition($entity_type->getKey('id'), NULL, '=');
            return $query;
          }
          else {
            $query->condition($entity_type->getKey('bundle'), $configuration['target_bundles'], 'IN');
          }
        }
    

    And in DefaultSelection::buildConfigurationForm, there is this condition:

        if ($entity_type->hasKey('bundle')) {
          // ..code removed for comment..
        }
        else {
          $form['target_bundles'] = [
            '#type' => 'value',
            '#value' => [],
          ];
        }
    

    So I think that '#value' => []needs to become '#value' => null, but I am not entirely sure that is the best solution or what other impact that change would have elsewhere. Alternatively, EntityReference::buildExtraOptionsForm could be modified to set target_bundles to null if the entity type does have have bundles.

    As a workaround, I ended up writing a custom EntityReferenceSelection plugin that extends DefaultSelection and changes target_bundles to null, when the entity has no bundles, inside ::buildEntityQuery. A little messy, but gets the job done (for now).

  • 🇺🇸United States AaronBauman Philadelphia

    Every instance of self::WIDGET_SELECT_LIMIT and self::ALL_VALUE should be static::.

    The use of self prevents any extending classes from overriding these uses, for example via replacing the filter class via hook_views_plugins_filter_alter().

    The use of static accommodates this use.

  • 🇺🇸United States JonMcL Brooklyn, NY

    Per my update in #55, I think something like this is needed in EntityReference::validateExtraOptionsForm. This will be to handle entities with no bundles and possibly for selecting no bundles on the subform.

    
        // If no checkboxes were checked for 'target_bundles', store NULL ("all
        // bundles are referenceable") rather than empty array ("no bundle is
        // referenceable" - typically happens when all referenceable bundles have
        // been deleted or when an entity type has no bundles).
        if ($subform_options['target_bundles'] === []) {
          $subform_options['target_bundles'] = NULL;
        }
    
        // Store the sub handler options in sub_handler_settings.
        $form_state->setValue(['options', 'sub_handler_settings'], $subform_options);
    
    
  • First commit to issue fork.
  • last update about 1 year ago
    Custom Commands Failed
  • First commit to issue fork.
  • 19:15
    18:08
    Running
  • last update about 1 year ago
    29,401 pass, 2 fail
  • 🇺🇸United States smustgrave

    Good to see this issue moving.again

  • 🇷🇴Romania vasike Ramnicu Valcea

    @smustgrave please do ...
    and thanks a lot

  • 🇺🇸United States smustgrave

    Was afraid of that. Think a new MR for 11.x will have to be started.

    here's the MR before it got messed up.

  • 🇺🇸United States smustgrave

    @vasike if you're working on this issue would recommend you start the branch so you have control of it.

  • 🇷🇴Romania vasike Ramnicu Valcea

    @smustgrave
    it seems there is an issue with with 11.x ... and 11.0.x it doesn't seem "a real one" ...

    so in ... a hurry ... we lost the track ... if i may say so .. :(

  • Assigned to BramDriesen
  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    I had this issue as well somewhere else because the issue fork got created before the branch existed and is thus not in sync with all other branches. Let me see if I can fix it.

  • last update about 1 year ago
    29,403 pass, 2 fail
  • Issue was unassigned.
  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    There we go. If anyone is wondering how I usually fix this here is the process.

    1. Start gitpod with the "old" 10.1.x branch
    2. Checkout 10.1.x
    3. Git pull
    4. Git fetch origin
    5. Git fetch --all
    6. Git checkout 11.x
    7. Git checkout -b 3347343-11-x
    8. -- do the stuff like patch applying, making changes, fixing conflicts --
    9. Git commit
    10. git push --set-upstream drupal-3347343 HEAD <<< this command changes with each issue fork, it's the last command from the "show commands" section.
  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    Also @vasike

    i just noticed something ... for drupal.org
    i tried to select new people for credits (including you) ... but it didn''t update the commit message ... hmm

    You don't have to include that in your commits. All commits will be squashed anyway when a maintainer commits this and sets the final commit message. Only maintainers of the module (or core in this case) can change the checkboxes.

  • 🇺🇸United States smustgrave

    I’ve been told issue must be committed to 11.x first and then backported. Didn’t mean to cause a road bump

    Thanks for resolving!

  • 🇷🇴Romania vasike Ramnicu Valcea

    @BramDriesen very thank you
    p.s. this is ... anyway ... witchery ... imho

  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    No problem, let's all be wizards 🧙‍♂️

  • last update about 1 year ago
    29,404 pass, 2 fail
  • last update about 1 year ago
    29,405 pass
  • Status changed to Needs review about 1 year ago
  • last update about 1 year ago
    29,404 pass, 2 fail
  • 🇷🇴Romania vasike Ramnicu Valcea

    Updates on MR:
    - rebased - using 11.x
    - Fix for entities with no bundle - DefaultSelection NULL value - as User
    - Complete - Workaround for https://www.drupal.org/project/drupal/issues/2651418 🐛 Non-array values for #ajax RTBC
    to support sub-items (for Entities with some fieldset/tree options) - as User
    - Use static for const, instead of self ... as suggested.
    - Fix tests - remove duplicate (early check) + fix the check for Filter options.

    Todo:
    - Fix https://www.drupal.org/project/drupal/issues/2651418 🐛 Non-array values for #ajax RTBC - it will cleanup code in the MR.
    - Entity with no bundle test - if really needed?!

  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    Unrelated. Will hide that patch, not sure if that stops it from being retested or not.

  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    Tested out on 11.x standard profile

    Added an entity reference field to the Article content type, pointing to basic pages
    Created a Test view
    Added the entity reference filter
    Configured and verified it filtered correctly.

    One part that was odd to me, won't hold it up, but when adding the filter it let me select the content types to filter by vs using the field settings.

    But did notice a bug
    I had 2 Articles, 1 with an entity reference value and 1 without
    When I use the filter it shows duplicates of the 1 article with a reference. Imagine it would show any number of duplicates based on number of articles.

  • 🇷🇴Romania vasike Ramnicu Valcea

    @smustgrave

    One part that was odd to me, won't hold it up, but when adding the filter it let me select the content types to filter by vs using the field settings.

    The "Reference selector" it's not a "field setting" ... but a "field instance setting"
    Which means a field could be used with different "Reference type" ... on different bundles.
    And i think it was discussed and "decided" in the original issue.

    But did notice a bug
    I had 2 Articles, 1 with an entity reference value and 1 without
    When I use the filter it shows duplicates of the 1 article with a reference. Imagine it would show any number of duplicates based on number of articles.

    I can't replicate this. Are there other "elements" in your install? Maybe multilingual or something?

  • last update about 1 year ago
    29,566 pass
  • Status changed to Needs review about 1 year ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    @smustgrave checked again on duplicates

    the only scenario is when we have multiple valued reference field and also multiple values selected for the filter.
    Which is "normal" ... and with Aggregation we could get rid of duplicates.

    So i can't see / replicate the bug.

    Let's try NR ... so maybe there will find out is close to RTBC or ... not.

  • last update about 1 year ago
    29,569 pass
  • 🇷🇴Romania vasike Ramnicu Valcea

    small update - FilterEntityReferenceTest for kernel file and class name ... as it's about the filter ... not field.

  • 🇺🇸United States smustgrave

    Tagging for subsystem maintainer as it seems this feature will require aggregation now.

  • 🇷🇴Romania vasike Ramnicu Valcea

    @smustgrave

    Tagging for subsystem maintainer as it seems this feature will require aggregation now.

    This is usual ... the "same results" you'll get also for a List field with multiple values and filter options selected ...
    So it's not about this filter in particular ... but filter multiple values for multiple valued field ...
    just saying

  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    Tested this again

    Create a simple view of the Article content type with a filter for reference filter.
    The filter is exposed and I see it in the preview.
    The dropdown looks correct
    But when I save the page and check the view page the filter does not appear.

  • 🇷🇴Romania vasike Ramnicu Valcea

    @smustgrave do you mean no filter at all?
    Not select input?

    Or no select options.

    This sound critical ... but could you, please, detail ... maybe screenshots for some steps?
    Thanks

  • Status changed to RTBC about 1 year ago
  • 🇺🇸United States smustgrave

    Actually not sure what I did wrong. Retested again and the filter appears now.

    Think this is good for committer review.

    Also not seeing the duplicates, didn't need aggregation.

  • last update about 1 year ago
    29,573 pass
  • last update about 1 year ago
    29,578 pass
  • last update about 1 year ago
    29,808 pass
  • last update about 1 year ago
    29,809 pass
  • last update about 1 year ago
    29,809 pass
  • last update about 1 year ago
    29,812 pass
  • last update about 1 year ago
    29,818 pass
  • last update about 1 year ago
    29,821 pass
  • last update about 1 year ago
    29,822 pass
  • last update about 1 year ago
    29,829 pass
  • last update 12 months ago
    29,834 pass
  • last update 12 months ago
    29,886 pass
  • last update 12 months ago
    Custom Commands Failed
  • last update 12 months ago
    Custom Commands Failed
  • last update 12 months ago
    Custom Commands Failed
  • First commit to issue fork.
  • last update 12 months ago
    Custom Commands Failed
  • last update 12 months ago
    Custom Commands Failed
  • last update 12 months ago
    Custom Commands Failed
  • last update 12 months ago
    Custom Commands Failed
  • last update 12 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • 🇧🇪Belgium DieterHolvoet Brussels

    In #2429699 comment #449 📌 Add Views EntityReference filter to be available for all entity reference fields Closed: duplicate , @Berdir answered the following when someone asked if the patch supports fields which reference configuration entities:

    No, that's impossible, content and config entities can't be joined in a relationship, config entities are not stored in a way that allows an SQL JOIN to work.

    That doesn't seem to be the case though. If I replace if ($target_entity_type instanceof ContentEntityTypeInterface) { with if ($target_entity_type instanceof EntityTypeInterface) { in core_field_views_data, it seems to work. My use case: I have an entity reference field on a node type, referencing webforms (a config entity type). After applying the patch and the change I just mentioned, I can add an entity reference filter listing all webforms, without issues.

    Should we change this so it also works with config entities? I don't see why not.

  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • Status changed to Needs work 11 months ago
  • The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • 🇮🇱Israel israelshmueli

    This problem also exists in Drupal 9.
    I created a patch against 9.5.x based on 2 files from Drupal 11 commits in gitlab

    This patch allowed me to add working exposed filter out of entity reference field that references content (nodes).
    The probleme does not exist when the field references taxonomy terms.

    The views filter should be of type "field_name as a Reference filter"

  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    Custom Commands Failed
  • last update 11 months ago
    30,136 pass, 1 fail
  • last update 11 months ago
    30,141 pass
  • Status changed to Needs review 11 months ago
  • last update 11 months ago
    30,341 pass
  • 🇷🇴Romania vasike Ramnicu Valcea

    Updates on MR:
    - Fix the CSpell Failures
    - And update to Do not limit filters available to Content Entities, use EntityTypeInterface ... as mentioned on #86
    But separate filter from relationships condition, as indeed they are not "compatible".
    In this way also we have aligned the filter available and Views Filter (getReferencedEntityType(): EntityTypeInterface)

    let's try some review.

  • Status changed to RTBC 11 months ago
  • 🇺🇸United States smustgrave

    Tried this out on a site we are developing.

    The reference fields appeared correctly as Reference filters
    Rendered as expected
    Filtering is working perfectly.

    Fingers crossed to getting in for 10.2

  • last update 11 months ago
    30,341 pass
  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    The to-do list in the summary could use an update as well. 🙂

  • last update 11 months ago
    30,341 pass
  • last update 10 months ago
    30,341 pass
  • last update 10 months ago
    30,341 pass
  • last update 10 months ago
    30,155 pass
  • 🇺🇸United States recrit

    attached a static patch for builds. This represents the MR 4053 at commit 1bc7bda7.

  • Status changed to Needs work 10 months ago
  • 🇬🇧United Kingdom catch

    If we're adding support for config entities here, then we need the equivalent test coverage to confirm it works, so marking needs work and 'needs tests' for that. Another option would be to move that change to a follow-up, although the test coverage shouldn't be too bad to add.

    Haven't done an in-depth review of the rest of the patch yet, just noticed this but added a couple of additional points from a quick look.

  • 🇷🇴Romania amateescu
    +++ b/core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php
    @@ -200,7 +200,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
    -        '#value' => [],
    +        '#value' => NULL,
    

    This change is not correct, a NULL value for the target_bundles setting means "all bundles are referenceable", while an empty array (the current value in HEAD) means "no bundles are referenceable", so this is a behavior change in the default selection plugin...

  • 🇺🇸United States recrit

    @amateescu I agree the change seems odd since it will affect all entity selection handlers.
    Here's an audit for the change to "core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php":

    • This commit introduced the change in the original MR 3629 before it was converted over to the current MR 4053- commit.
    • It appears there was some discussion in #55 that suggested setting it to NULL to fix an issue with a custom entity type. Then it was committed in #73.
    • This changed was not in the patch/MR on the original issue 📌 Add Views EntityReference filter to be available for all entity reference fields Closed: duplicate .
  • Pipeline finished with Success
    9 months ago
    #41517
  • 🇷🇴Romania vasike Ramnicu Valcea

    about the change

    +++ b/core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php
    @@ -200,7 +200,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
    -        '#value' => [],
    +        '#value' => NULL,
    

    This is needed in order to get the "bundless" entities - to work.
    Without it. there are no "options" for the Referenced entities.

    So the question, do we still need to keep this and make it work for entities without bundles.
    or this change "won't" harm.

  • Pipeline finished with Failed
    9 months ago
    #41968
  • Pipeline finished with Failed
    9 months ago
    #42109
  • Pipeline finished with Success
    9 months ago
    #42118
  • Pipeline finished with Success
    9 months ago
    #42159
  • Status changed to Needs review 9 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    Updated the MR with some tests for Filter of Config Entities References.

    The. node type config was used for Entity Reference Testing Field.

  • 🇺🇸United States smustgrave

    Agree with #91 if the issue summary bullets could be looked at.

  • Status changed to Needs work 9 months ago
  • 🇺🇸United States smustgrave

    Just for issue summary and tags update/cleanup.

  • Status changed to Needs review 8 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    Updates:

    - Updated summary
    - Change record draft available
    - Remove tags: Needs issue summary update, Needs tests
    - Add related issue, as the MR code contains this issue reference https://www.drupal.org/project/drupal/issues/2651418 🐛 Non-array values for #ajax RTBC

  • Status changed to Needs work 8 months ago
  • Pipeline finished with Failed
    8 months ago
    #56242
  • Pipeline finished with Success
    8 months ago
    #56273
  • Pipeline finished with Failed
    8 months ago
    #56277
  • Pipeline finished with Failed
    7 months ago
    #63793
  • Pipeline finished with Failed
    7 months ago
    #64186
  • Pipeline finished with Success
    7 months ago
    #64190
  • 🇷🇴Romania vasike Ramnicu Valcea

    vasike → changed the visibility of the branch 3347343-continuation-add-views to hidden.

  • Status changed to Needs review 7 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    Appear to have 2 open threads from @catch that need to be addressed.

    Thanks

  • Pipeline finished with Canceled
    6 months ago
    #74117
  • Pipeline finished with Failed
    6 months ago
    #74121
  • 🇫🇮Finland sthomen

    I'm not sure if I've missed something obvious, but this change seems to cause views to pull in the refercenced entity with an inner join. This does not happen with the default NumericFilter where it becomes a left join. I had a cursory look at the code but couldn't find out where/why this happens, but perhaps someone more versed in views can have a look.

  • Pipeline finished with Success
    6 months ago
    #75011
  • 🇷🇴Romania vasike Ramnicu Valcea

    There was an attempt to cleanup the code about required "fields" for the selection handler options
    https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs?co...
    And it "works" in the UI without issues.

    But it seems the tests fails.
    Something it's broken in the Views/Modal forms validations - or at least for required fields/elements.
    And the extra code (setRequiredViaStatesOnChildren) helps for tests to pass.

    Unfortunately ... there is no "FunctionalJavascript" tests on Filters with extra options (buildExtraOptionsForm)
    or at least i couldn't see such tests.
    That could help us here ... i'm also not aware on the other core issues related with those things.

    So for now i just reverted the change to previous "state".

    I still think the code could be cleaned up for both Filter plugin and its test.

    Questions:
    TaxonomyIndexTid / TaxonomyIndexTidDepth plugin filters do not have "FunctionalJavascript" tests, only "Functional" tests
    Could we consider the same here. So maybe we don't need the "extra code" for this new filter?

    @sthomen do you mean this change mentioned. could you please check again? as the change was reverted.
    thanks

  • 🇫🇮Finland sthomen

    @vasike My apologies, I was just being dumb. Please ignore my comment.

  • Pipeline finished with Failed
    6 months ago
    #77431
  • Pipeline finished with Success
    6 months ago
    #77435
  • Pipeline finished with Failed
    6 months ago
    #77447
  • Pipeline finished with Failed
    6 months ago
    #77453
  • Pipeline finished with Failed
    6 months ago
    #77458
  • Pipeline finished with Success
    6 months ago
    #77463
  • Status changed to Needs review 6 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    There is a second attempt to clean up the MR code on both Views Filter plugin and its tests.

    For Views Filter plugin the cleanup also tries to address/solve the 2 open threads
    1. A todo about conditional required - removed the code, i think this "todo" will have small chances to be chased and solved soon
    And, for now, it's not used (needed)
    Also renamed the method for this setRequiredViaStatesOnChildren -> removeRequiredOfSubformChildrens
    2. A "todo" about the extra options form validation. It seems the original issue has a related issue linked ... which it was missed when this new one was created
    https://www.drupal.org/project/drupal/issues/3163740 →
    So i cleaned up the code there and added the related issue url.
    3. Re-arrange some code - to be easier to maintain/read

    For tests code
    I tried to cleanup - use similar code with existing code on filters ... with the same goal - - to be easier to maintain/read (toghteter with other related tests).

    i hope we got closer to an end here

  • 🇷🇴Romania vasike Ramnicu Valcea

    forgot to link the related issue

  • Status changed to Needs work 6 months ago
  • 🇬🇧United Kingdom catch

    I needed this on a client site, and noticed that the entity reference filter doesn't show up for base fields.

    This is easily solved with the following additional change:

    diff --git a/core/modules/views/src/EntityViewsData.php b/core/modules/views/src/EntityViewsData.php
    index eb964521af..19e898f12c 100644
    --- a/core/modules/views/src/EntityViewsData.php
    +++ b/core/modules/views/src/EntityViewsData.php
    @@ -645,7 +645,7 @@ protected function processViewsDataForEntityReference($table, FieldDefinitionInt
             ];
             $views_field['field']['id'] = 'field';
             $views_field['argument']['id'] = 'numeric';
    -        $views_field['filter']['id'] = 'numeric';
    +        $views_field['filter']['id'] = 'entity_reference';
             $views_field['sort']['id'] = 'standard';
           }
           else {
    
    

    However, changing the filter type (as opposed to adding an extra field in the views data), is not compatible with the current approach taken here.

    I am wondering why the approach was changed to add an extra field, it adds a lot of noise to the UI when the 99.99% use case will be to use this filter anyway. I don't see how this can be applied to base fields without either changing the approach, or refactoring EntityViewsData to allow field types to produce multiple fields, or hard coding this reference type in there somewhere.

    Also added a couple of review points to the MR.

    Apart from those issues, this looks close and it would be great to close it before this continuation issue is as long as the original.

  • 🇬🇧United Kingdom scott_euser

    I am wondering why the approach was changed to add an extra field, it adds a lot of noise to the UI when the 99.99% use case will be to use this filter anyway. I don't see how this can be applied to base fields without either changing the approach, or refactoring EntityViewsData to allow field types to produce multiple fields, or hard coding this reference type in there somewhere.

    That happened way back here after chatting with lendude at Drupal Dev Days.
    https://www.drupal.org/project/drupal/issues/2429699#comment-14472189 📌 Add Views EntityReference filter to be available for all entity reference fields Closed: duplicate

    If I remember correctly, without this MR you can set up the reference field to eg find all node IDs >= 1000, but switching to an entity reference you lose the more math related options like that. While it seems like edge case, it is possible users are using the field as is to do such things. So a separate field seemed the only way to not break existing sites.

  • 🇷🇴Romania vasike Ramnicu Valcea

    Some thoughts about the change https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs?co...

    Why, imho, NULL value will not harm at all (there).

    1. It's a form value element for entities that doesn't have a bundle key

        if ($entity_type->hasKey('bundle')) {
           ....
        }
        else {
          $form['target_bundles'] = [
            '#type' => 'value',
            '#value' => NULL,
          ];
        }
    

    2. defaultConfiguration has NULL value

    3. The field settings will save actually NULL value for entities without bundle
    @see validateConfigurationForm
    Result

    uuid: 0918c28c-2a5d-49b0-8c33-ee7855dd23a9
    langcode: en
    status: true
    dependencies:
      config:
        - field.storage.node.field_type
        - node.type.article
    id: node.article.field_type
    field_name: field_type
    entity_type: node
    bundle: article
    label: Type
    description: ''
    required: false
    translatable: false
    default_value: {  }
    default_value_callback: ''
    settings:
      handler: 'default:node_type'
      handler_settings:
        target_bundles: null
        auto_create: false
    field_type: entity_reference

    4. And of course the "docs" about NULL vs [] "results"

          // For the 'target_bundles' setting, a NULL value is equivalent to "allow
          // entities from any bundle to be referenced" and an empty array value is
          // equivalent to "no entities from any bundle can be referenced".

    So my conclusions are:
    1. The Change to NULL should not break anything
    2. DefaultSelection could be improved for "bundless" entities
    3. Subforms and validations not working "great" on Views Filters.

    @catch what do think?
    thanks

  • Pipeline finished with Failed
    6 months ago
    #82726
  • Pipeline finished with Failed
    6 months ago
    #82731
  • 🇷🇴Romania vasike Ramnicu Valcea

    MR updated ... trying new approach to make it work for entities with no bundle.

    As part of the conclusions from #112, about DefaultSelection
    I noticed that the configuration form validation "imagines" that we have a field settings form.
    Which means the validations there won't work in another forms.

    So i moved the validations from form validation to elements validations - for portability.
    And it should work.

    p.s. i still "believe" in things i shared on #112

    Unfortunately ... the MR started failing ... PHPStan ... and the last one looks odd ....

    If there's anyone that could help with this error?
    thank you

  • 🇺🇸United States smustgrave

    That's a tough one, applied locally and even phpstorm isn't picking up on that. Usually phpstorm is good with those kind of pickups.

  • Pipeline finished with Pending
    6 months ago
    #82830
  • Pipeline finished with Failed
    6 months ago
    #82849
  • 🇷🇴Romania vasike Ramnicu Valcea

    Fixed the PHPStan errors.
    However now we have the "CSpell" CI issue ...
    https://www.drupal.org/project/drupal/issues/3401988 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active

  • 🇺🇸United States DamienMcKenna NH, USA

    Re #111, could this be handled as a backwards compatibility break in a major release? Alternatively, I wonder about creating a small contrib module to switch the definition..

  • 🇷🇴Romania vasike Ramnicu Valcea

    @DamienMcKenna with all my respect, I would say that, this is "out of box" issue.

    2 things
    - Views is in Core
    - EntityReference is in core

    What's not in core ... is Views and EntityReference "real integration"
    This is just a small part to involve more people to help with Views ... is far from "core" ... yet ... imho

    But thanks for your thoughts.

  • 🇺🇸United States DamienMcKenna NH, USA

    @vasike: Apologies that I wasn't clear, I was talking about to the suggestion in #110 to simply change core's field specification to use "entity_reference" instead of "numeric"; could this change be introduced as a breaking change, maybe with documentation on how to use the field as a number?

  • 🇺🇸United States smustgrave

    With regards to 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active there is a work around posted to the ticket but I'm not super clear how to best do it.

  • 🇬🇧United Kingdom scott_euser

    Re #111 / #116 perhaps best is to:

    1. get this in using current approach of additional filter

    Then in follow-up (because all the below is a lot of work in itself and big scope increase):

    1. deprecate use of original filter math options (ideally specifically the ones nk longer available
    2. add change record
    3. add update hook to convert the additional filter from this issue back to original name

    If that's at all possible / interesting perhaps that discussion can be had in a postponed issue to avoid this issue growing too big again

  • First commit to issue fork.
  • Pipeline finished with Failed
    6 months ago
    Total: 284s
    #82882
  • Pipeline finished with Canceled
    6 months ago
    Total: 78s
    #82885
  • 🇪🇸Spain fjgarlin

    This issue was hit by the issue mentioned in #119, but also the fact that the branch 11.x wasn't even present in the fork (I think this is another core issue).

    I brought 11.x to the fork and the MR should now run. I have no more context on the issue so not sure if it should be back to "Needs review" or still "Needs work".

  • Pipeline finished with Success
    6 months ago
    #82886
  • Status changed to Needs review 6 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea
  • Pipeline finished with Failed
    6 months ago
    Total: 597s
    #82991
  • Pipeline finished with Success
    6 months ago
    #83322
  • 🇺🇸United States smustgrave

    @catch

    I don't understand why this is being done - it adds clutter to the views UI, and the current entity reference filter just looks broken when exposed. When it's not exposed, will there even be a difference?

    What are you seeing that appears broken exactly?

  • 🇬🇧United Kingdom catch

    @smustgrave I mean that if you expose an entity reference filter in HEAD, then you get an empty text field with no indication of what to put in it, which to me looks broken and I doubt people are using it much as-is. Given that, I think we should consider applying the new behaviour to the existing filter, not adding a separate one alongside. This would also help with applying it to base fields, which the MR currently does not.

  • 🇨🇭Switzerland Berdir Switzerland

    There are ways to use it and I've done it before. For example in combination with grouped filters, which obviously doesn't scale, but is a decent workaround when you have for example a config entity reference with only a handful options.

  • 🇬🇧United Kingdom scott_euser

    Re #125 you could be using the filter without exposing it though, eg all node IDs >= 1000. Ie, not useful for a visitor but possibly useful for a site builder?

    As per my response in #111 that's why chats with @lendude suggested that would be the most realistic way to get this in core.

    I would also prefer to not have the extra filters in an ideal world and replace instead if you think we can get away with just a change record but I am concerned this then never gets in as many of us have spent chunks of time on this over the years and the goal-posts changing makes it tough of course.

    Would you be the one signing off on this eventually or should we seek review from a different core committer as well?

    Asking first because, if we go through route, it's probably a fair amount of work to change the MR but also then possibly further extend the MR to need update hooks (and upgrade tests?). So I think we should probably agree scope and ensure agreed scope has a realistic path to actually making it in before actioning a change of approach... Or agree to keep the current scope

  • 🇺🇸United States smustgrave

    Would love if this doesn't get re-scoped at this point of it. If it could be improved in follow ups +1 for that.

  • 🇬🇧United Kingdom catch

    @scott_euser I'll try to get a second and if possible third or more opinion from other core committers so this doesn't get turned around again.

    @Berdir same question as #127 - is that exposed to site visitors, or exposed at all?

  • 🇷🇴Romania vasike Ramnicu Valcea

    I open a "thread" on slack
    https://drupal.slack.com/archives/C6SQM2J94/p1706633005193429

    Also added to summary

    And I'm wondering if it's not more efficient for discussions than to "fill-up" issue comments

    p.s. maybe it could be a way for drupal.org to have a slack url about discussion on some issues.

    just thinking aloud

    sorry for the noise.

  • Status changed to Needs work 6 months ago
  • 🇬🇧United Kingdom scott_euser

    Updated issue summary to reflect need to add support for base fields into this as per message from @catch in Slack.

    Also moving back to Needs Work since there are other items in the list that are not yet completed as well.

  • Pipeline finished with Failed
    6 months ago
    Total: 522s
    #86737
  • 🇬🇧United Kingdom scott_euser

    Support for base fields added, but needs tests (updated issue summary).

    A good example is 'uid' field in default Files view, so an easy way to try this locally:

    • Before update: admin/structure/views/view/files only User ID available
    • After update: admin/structure/views/view/files only User ID and User ID as a reference available
  • Pipeline finished with Failed
    6 months ago
    Total: 672s
    #86760
  • 🇷🇴Romania vasike Ramnicu Valcea
  • Pipeline finished with Failed
    6 months ago
    Total: 561s
    #86808
  • 🇬🇧United Kingdom scott_euser

    Problem at the moment is HandlerAllTest::testHandlers() fails on Comment 'Parent ID' base field, its getting allowed to use the new 'entity_reference' filter handler, but then something inside the new filter handler does not like that as a target. I ran out of time at the moment in case anyone wants to pick this up, otherwise I will try to pick it up myself again later.

    I suspect sorting that issue will sort out the tests (unless there are further issues with supporting base field).

  • Pipeline finished with Success
    6 months ago
    Total: 634s
    #87309
  • Pipeline finished with Success
    6 months ago
    #87326
  • 🇬🇧United Kingdom scott_euser

    Okay tests passing now + new test added for base field reference.

    So just comments from #49 left to address.

  • Pipeline finished with Failed
    5 months ago
    Total: 179s
    #87819
  • Pipeline finished with Failed
    5 months ago
    Total: 568s
    #87830
  • Pipeline finished with Failed
    5 months ago
    Total: 513s
    #87842
  • Pipeline finished with Failed
    5 months ago
    Total: 169s
    #87856
  • Pipeline finished with Failed
    5 months ago
    Total: 555s
    #87872
  • Pipeline finished with Success
    5 months ago
    Total: 805s
    #87899
  • Status changed to Needs review 5 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea
  • 🇺🇸United States Deasly

    I have this merge request running in a patch on my Drupal 10.1 site. It was running fine until recent updates made to it. Now Composer wont patch it into core using "cweagans/composer-patches". Am i the only one with this issue? What should I do to get this working again the way it was? "https://git.drupalcode.org/project/drupal/-/merge_requests/4053.patch" I'm probably not the only one.

    Any thoughts on how to proceed would be appreciated as its currently blocking my monthly maintenance. Thanks.

  • 🇬🇧United Kingdom scott_euser

    Hi @Deasly.

    The merge request is meant to target Drupal 11 and it needs to continue to do so in order to eventually get in.

    In the future, best to be downloading the patch file into your project and committing it, so you reference the local file, see here for details → . It also talks about how you can do a git diff to make your own patch. The challenge here is you need to find the commit id of a version in the merge request that you are happy with then the commit ID eg of latest 10.2 Drupal (which I am guessing you are trying to update to). Then its `git diff new-commit-id old-commit-id > patch-file-name-here.patch. Will see if I can add a bit more to that doc page to help make that more clear.

    Hope that helps!

  • 🇬🇧United Kingdom scott_euser

    scott_euser → changed the visibility of the branch 10.1.x to hidden.

  • Pipeline finished with Canceled
    5 months ago
    #88280
  • 🇬🇧United Kingdom scott_euser

    scott_euser → changed the visibility of the branch 10.2.x to hidden.

  • 🇬🇧United Kingdom scott_euser

    scott_euser → changed the visibility of the branch 11.x to hidden.

  • Pipeline finished with Failed
    5 months ago
    Total: 171s
    #88282
  • 🇬🇧United Kingdom scott_euser

    Sorry for the noise here, not sure an easier way to help others out that want to use this in 10.2.x as I realise its a challenge to resolve conflicts as soon as the 11.x MR starts to become incompatible with 10.2.x like it is.

    I created a branch which people should be able to use the diff from to create a local patch BUT please bear in mind the code is just cherry-picked from the Drupal 11 Merge Request code and its therefore very possible that code is referenced that only exists in Drupal 11 and does not exist in Drupal 10 which can lead to errors, so do use with caution.

  • Pipeline finished with Success
    5 months ago
    Total: 527s
    #89367
  • 🇬🇧United Kingdom scott_euser

    Regarding #49 I have updated the issue summary with outstanding items. Details below:

    calculateDependencies is untested and should be since this is different than the Term plugin

    Added tests mimicking the coverage that TaxonomyIndexTidFilterTest::testConfigDependency() provides.

    The 'sort by' field is empty on initial adding of the filter (the field widget does something ajaxy when selecting bundles, the views widget does not, might be it?)

    I could not spot a 'sort by' when using a field widget on an entity reference field. Is it possible to provide further steps?

    Grouped autocomplete filters don't work/save (but don't work for taxonomy ref filters either), please find the related issue or open it (should be one I think)

    Here is the related issue I think 🐛 Grouped exposed taxonomy filter fails validation for autocomplete widget Needs work .

    Description "as a Reference filter" feels a bit vague, did we get UX feedback on this?

    Perhaps this should be dealt with in the deprecation dance when the old filter gets removed. Added as a follow-up task.

    Should we add this new filter for taxonomy terms too, or should we skip those since it is unclear what the difference is now when have to chose between the two different filters

    We have this as a follow-up now 📌 Convert TaxonomyIndexTid to use new EntityReference filter Active .

    I can create a circular reference by creating an Entity reference View that uses itself as the filter, not sure what goes wrong (mostly getting an empty filter after saving), but that sounds iffy
    Maybe related : Managed to click my way into "Warning: Undefined array key "reference_views" in Drupal\views\Plugin\views\filter\EntityReference->submitExtraOptionsForm() (line 440 of /app/drupal/core/modules/views/src/Plugin/views/filter/EntityReference.php)"

    Needs work - added to issue summary as a task

    Node titles in the select element are double escaped

    Needs work (I can see this works in TaxonomyIndexTid, so its not a wider problem) - added to issue summary as a task

    Exposed filters on the chosen entity reference view are also shown on the view

    Needs work - added to issue summary as a task

  • 🇺🇸United States veggieryangrace

    veggieryangrace → changed the visibility of the branch 10.2.x to active.

  • 🇺🇸United States smustgrave

    smustgrave → changed the visibility of the branch 3347343-10-2-x--do-not-merge to hidden.

  • 🇺🇸United States smustgrave

    smustgrave → changed the visibility of the branch 10.2.x to hidden.

  • 🇺🇸United States smustgrave

    Just to avoid any confusion I hid the other MRs but left a small note in the summary.

    Tested out the latest changes

    Standard Profile install
    Added a content reference to the Article content type, using basic page as the target
    Created a few basic page nodes
    Created a few article referencing those basic pages
    Updated the content view to use Content (field_content) as a Reference filter
    I was able to setup as a dropdown no problem
    Filters worked as expected
    No errors in logs.

    +1 RTBC from me but will leave in review for additional eyes.

  • Pipeline finished with Canceled
    5 months ago
    Total: 52s
    #100585
  • Pipeline finished with Success
    5 months ago
    Total: 631s
    #100589
  • Pipeline finished with Success
    5 months ago
    Total: 573s
    #100697
  • 🇷🇴Romania vasike Ramnicu Valcea

    New commit to remove a "@todo" from code ... about ajax property on Entity Reference Selection plugin.
    From related issue https://www.drupal.org/project/drupal/issues/2651418 🐛 Non-array values for #ajax RTBC

    please check the latest comments there.

  • 🇺🇸United States seanr

    I'm a little confused by this thread (it's quite long and convoluted), but is there a patch/MR that works against D10? I've got a client yelping about this. :D

  • 🇬🇧United Kingdom scott_euser

    Under the branches listed at the top, click to show hidden branches and see '3347343-10-2-x--do-not-merge' - is has a port of the 11x merge request for 10x sites.

  • 🇺🇸United States smustgrave

    FYI been using this for about a week on client side on our content view, have a dropdown for a taxonomy field and user reference field.

  • Status changed to Needs work 4 months ago
  • The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Pipeline finished with Failed
    4 months ago
    #118375
  • Pipeline finished with Failed
    4 months ago
    Total: 186s
    #118408
  • Pipeline finished with Failed
    4 months ago
    Total: 171s
    #118437
  • Pipeline finished with Failed
    4 months ago
    Total: 179s
    #118464
  • Status changed to Needs review 4 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    Solved conflicts and also new CS issues
    However it seems there are other things going on for CI
    As the new Validatable config job ... which, of course it fails

    Probably this should be covered by a child issue
    https://www.drupal.org/project/drupal/issues/2869792 🌱 [meta] Add constraints to all config entity types Active
    ... which seems ... missing
    "Not ideal ... to be polite"

    I put the issue for Review ... just to gain some attention
    As the feeling is that it could become stalled ... as original ... for other years.

    p.s. also linked those issues ... maybe there we could have more awareness

  • First commit to issue fork.
  • 🇰🇬Kyrgyzstan elaman

    Not an input to the issue, just a patch for 10.2.x

  • Status changed to Needs work 4 months ago
  • The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Pipeline finished with Failed
    4 months ago
    Total: 468s
    #119043
  • Pipeline finished with Success
    4 months ago
    Total: 641s
    #119062
  • Status changed to Needs review 4 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea

    MR green again

    Also I hid the latest patch ... needs-review-queue-bot doesn't like it https://www.drupal.org/files/issues/2024-03-14/3347343-nr-bot.txt →

  • 🇫🇷France perraudeau France

    perraudeau → changed the visibility of the branch 3347343-10-x to hidden.

  • 🇫🇷France perraudeau France

    perraudeau → changed the visibility of the branch 10.1.x to active.

  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 160s
    #138189
  • Pipeline finished with Failed
    4 months ago
    Total: 627s
    #138190
  • Pipeline finished with Failed
    4 months ago
    Total: 182s
    #138204
  • Pipeline finished with Failed
    4 months ago
    Total: 631s
    #138208
  • Pipeline finished with Success
    4 months ago
    Total: 574s
    #138218
  • 🇺🇸United States smustgrave

    Thanks everyone for keeping this one going.

    Wonder if any of the remaining tasks can be scratched off? Maybe can start reaching out to the sub-maintainer and framework managers next week for sign off.

  • 🇬🇧United Kingdom scott_euser

    These are the remaining tasks from the issue summary (other than getting framework manager approval):

    ☐ sort by empty on initial load (needs more info), see comment #49

    ☐ prevent circular reference: validate no selection of an entity reference that uses itself as the filter, see comment #49

    ☐ node titles in the select options when configuring the filter are double escaped (e.g. " shows as "), see comment #49

    ☐ exposed filters on the chosen entity reference view are also shown on the view, see comment #49

    To get this over the line, could we consider all of these back-end minor edge cases? None of them stop the site builder from achieving the primary task and none of them affect the front-end functionality for the visitor. So perhaps we could consider them as follow-ups? They would be easier to achieve in isolation later.

  • 🇺🇸United States smustgrave

    Personally I think it would be nice to get this in now and improve in follow ups. If it's functional and covers most scenarios that is.

  • 🇪🇸Spain rodrigoaguilera Barcelona

    For folks that use patches, the patch from #156 doesn't apply to 10.2.5 so here is one without the phpstan-baseline changes that applies to both.

  • Status changed to Needs work 3 months ago
  • The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Pipeline finished with Success
    3 months ago
    Total: 1079s
    #149064
  • 🇺🇸United States recrit

    Attaching a static patch of MR 4053 for builds to use. This patch applies to 11.x and 10.2.5.

  • 🇺🇸United States recrit

    I had a custom selection plugin that extended Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection. Since the plugin ID was not "views", the detection to remove the element validation (unset($sub_handler_settings['view']['#element_validate']); would not run, resulting in silent form error and the filter config values not saving.

    I changed views handler detection to class based instead of plugin id to support any plugin that extends the ViewsSelection class.

  • Pipeline finished with Success
    3 months ago
    Total: 1551s
    #151411
  • Status changed to Needs review 3 months ago
  • 🇦🇺Australia acbramley

    Please stop uploading patches to this issue. It is causing problems with the NR bot and making this even more tedious to try and get over the line.

    I completely agree with #164 - let's get this in and polish it further in follow ups if needed before we get to another 500 comment issue.

  • 🇺🇸United States JonMcL Brooklyn, NY

    JonMcL → changed the visibility of the branch 3347343-10-2-x--do-not-merge to active.

  • 🇺🇸United States JonMcL Brooklyn, NY

    JonMcL → changed the visibility of the branch 3347343-10-2-x--do-not-merge to hidden.

  • Pipeline finished with Success
    2 months ago
    Total: 568s
    #166211
  • Status changed to Needs work 2 months ago
  • The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • 🇺🇸United States swilmes

    swilmes → changed the visibility of the branch 3347343-10-2-x--do-not-merge to active.

  • Pipeline finished with Success
    about 2 months ago
    Total: 540s
    #180068
  • Status changed to Needs review about 2 months ago
  • 🇷🇴Romania vasike Ramnicu Valcea
  • First commit to issue fork.
  • Pipeline finished with Failed
    about 2 months ago
    Total: 191s
    #181596
  • Status changed to Needs work about 2 months ago
  • The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Status changed to Needs review about 2 months ago
  • 🇬🇧United Kingdom catch
  • 🇬🇧United Kingdom scott_euser

    scott_euser → changed the visibility of the branch 10.3.x to hidden.

  • 🇬🇧United Kingdom scott_euser

    scott_euser → changed the visibility of the branch 10.1.x to hidden.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 190s
    #200873
  • Pipeline finished with Failed
    about 1 month ago
    Total: 184s
    #200875
  • Pipeline finished with Failed
    about 1 month ago
    Total: 181s
    #200882
  • 🇺🇸United States mortona2k Seattle

    @scotteuser - Hey I tried to update the 10.2 branch for 10.3 yesterday, but I couldn't figure out a good way to do it. First I tried to rebase, but gave up after a bunch of conflict resolutions. Then I tried merging, most of the important changes were merged no problem, and I tried to check for other relevant changes but it's a big list and hard to verify. When I completed the merge, it was about 80% right with some extra stuff.

    Do you have any notes on how to do these kinds of branch updates in a sane way?

  • 🇬🇧United Kingdom scott_euser

    I definitely had a bit of hair-pulling as well, what I ultimately did was take the merge request diff as a patch and ran the apply patch via phpstorm which applied most files. Then I manually redid the views.views.inc as it could not manage that. So yeah, no sane way I could find yet either :)

  • 🇬🇧United Kingdom scott_euser
    --------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------
     77 | ERROR | [x] Parameter $options has null default value, but is not marked
        |       |     as nullable.
        |       |     (SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue.NullabilityTypeMissing)
    

    I would have thought that https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs?co... would fix this. Maybe I did the wrong thing, would appreciate if anyone has any tips. It seems there is a giant inheritdoc chain and this MR is just surfacing a deeper issue elsewhere.