Incorrect role filter in views when added twice

Created on 17 June 2019, over 5 years ago
Updated 4 January 2024, 12 months ago

Problem/Motivation

Additional erroneous filtering is applied when adding the same views filter twice resulting in results not appearing.

Steps to reproduce

  1. Install Drupal with standard profile
  2. Create a user with the "Content editor" role
  3. Create a user with the "Administrator" role
  4. Edit the default "Who's new" view
  5. Remove the "Last accessed" filter
  6. Add a "User: Roles" filter and select both "Content editor" and "Administrator"
  7. Add another "User: Roles" filter and expose it
  8. In the preview note all 3 users listed
  9. Select "Content editor" and press "Apply", this filter works correctly
  10. Select "Administrator" and press "Apply", there are no results.

The query at the last step is below:

SELECT "users_field_data"."created" AS "users_field_data_created", "users_field_data"."uid" AS "uid"
FROM
{users_field_data} "users_field_data"
INNER JOIN {user__roles} "user__roles" ON users_field_data.uid = user__roles.entity_id AND user__roles.deleted = '0'
LEFT JOIN {user__roles} "user__roles2" ON users_field_data.uid = user__roles2.entity_id AND user__roles2.roles_target_id != 'administrator'
WHERE (((user__roles.roles_target_id IN('content_editor', 'administrator'))) AND ((user__roles2.roles_target_id = 'administrator'))) AND ("users_field_data"."status" = '1')
ORDER BY "users_field_data_created" DESC
LIMIT 5 OFFSET 0

Note the LEFT JOIN is erroneously filtering on user__roles2.roles_target_id != 'administrator'.

Proposed resolution

Remove erroneous != filter

Remaining tasks

  • Write a failing test
  • Fix the bug.

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
Views 

Last updated about 2 hours ago

Created by

🇷🇴Romania aalin

Live updates comments and jobs are added and updated live.
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.

  • 🇦🇺Australia mstrelan

    Confirmed this issue still exists on 10.1.x, updated the issue summary with steps to reproduce in the standard issue template.

  • First commit to issue fork.
  • Assigned to shalini_jha
  • Issue was unassigned.
  • 🇮🇳India shalini_jha

    I have able to reproduce the issue and after debugging the issue i have found that if i will add one more role like "moderator" ,and selected this role in filters with content editor and administrator, then filter will work with administrator & content editor but it will not work work with moderator role,
    according to my finding here else condition is responsible when we filter this i am not sure if this is needed for this condition or not

    $join->extra = [
      [
            'field' => $this->handler->realField,
           'operator' => '!=',
           'value' => $value,
          'numeric' => !empty($this->handler->definition['numeric']),
          ],
      ];
    

    let me know if anyone have any suggestion.
    Thanks

  • 🇮🇳India shalini_jha

    According to my analysis, I have identified the root cause of the issues. In the current code, there is a specific condition handling the scenario of having multiple filters of the same type. When two identical filters are added, the code adds an extra condition to the join, resulting in a query similar to the following:

    LEFT JOIN {user__roles} "user__roles2" ON users_field_data.uid = user__roles2.entity_id AND user__roles2.roles_target_id != 'administrator'
    WHERE ((user__roles2.roles_target_id = 'content_editor'))
    

    However, there is an issue with the way the extra condition is added within a foreach loop. The code is currently picking only one key from the loop and using it consistently in the left AND condition. As a consequence, the same key is passed every time, leading to incorrect results. Specifically, when the filter's selected option matches the passed value in the query, the view does not return the expected results.

    To address this issue, I have implemented modifications to the code using two different approaches:

    Patch #1
    I addressed the issue by introducing the following changes:

    In the code, I added the selected value from the view handler.Implemented an if condition within the foreach loop to ensure that the selected value is not equal to the current loop value before adding it to the extra condition.instead of a single key from array i am taking complete array.
    Consequently, the generated query now looks like this:

    SELECT "users_field_data"."created" AS "users_field_data_created", "users_field_data"."uid" AS "uid"
    FROM
    {users_field_data} "users_field_data"
    INNER JOIN {user__roles} "user__roles" ON users_field_data.uid = user__roles.entity_id AND user__roles.deleted = '0'
    LEFT JOIN {user__roles} "user__roles2" ON users_field_data.uid = user__roles2.entity_id AND (user__roles2.deleted = '0' AND user__roles2.roles_target_id != 'all' AND user__roles2.roles_target_id != 'content_editor')
    WHERE ((user__roles.roles_target_id IN('all', 'administrator', 'content_editor'))) AND ((user__roles2.roles_target_id = 'administrator'))
    ORDER BY "users_field_data_created" DESC
    LIMIT 5 OFFSET 0
    

    Patch#2

    After further investigation, I deemed it more appropriate to simplify the logic. We will now add an if condition, ensuring that the value is passed to the extra AND condition only if it is equal to the selected value.This adjustment avoids potential issues associated with using multiple values in the join AND condition.

    In this case, the modified logic results in a cleaner query:

    SELECT "users_field_data"."created" AS "users_field_data_created", "users_field_data"."uid" AS "uid"
    FROM
    {users_field_data} "users_field_data"
    INNER JOIN {user__roles} "user__roles" ON users_field_data.uid = user__roles.entity_id AND user__roles.deleted = '0'
    LEFT JOIN {user__roles} "user__roles2" ON users_field_data.uid = user__roles2.entity_id AND (user__roles2.deleted = '0' AND user__roles2.roles_target_id = 'administrator')
    WHERE ((user__roles.roles_target_id IN('all', 'administrator', 'content_editor'))) AND ((user__roles2.roles_target_id = 'administrator'))
    ORDER BY "users_field_data_created" DESC
    LIMIT 5 OFFSET 0
    
    

    Please review and let me know
    Thanks

  • Status changed to Needs review 12 months ago
  • 🇮🇳India prashant.c Dharamshala
  • Status changed to Needs work 12 months ago
  • 🇺🇸United States smustgrave

    Recommend using MRs for faster reviews.

    But either approach will need test coverage

    Also solution needs to match issue summary.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 12 months ago
    25,991 pass, 1,830 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 12 months ago
    25,986 pass, 1,840 fail
  • 🇮🇳India shalini_jha

    I have added a MR and screen shots for the result.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 5.3 & MySQL 5.5
    last update 12 months ago
    Composer error. Unable to continue.
  • Pipeline finished with Failed
    12 months ago
    Total: 7262s
    #74673
  • 🇮🇳India guptahemant

    Following test case is failing Drupal\Tests\taxonomy\Functional\Views\TaxonomyIndexTidUiTest::testFilterGrouping

  • First commit to issue fork.
  • Pipeline finished with Failed
    11 months ago
    Total: 168s
    #84325
  • Pipeline finished with Success
    11 months ago
    Total: 591s
    #84330
  • Status changed to Needs review 11 months ago
  • The previous solution failed for the following test case:
    Drupal\Tests\taxonomy\Functional\Views\TaxonomyIndexTidUiTest::testFilterGrouping

    The logic cannot be changed to the = operator since it is used for views with group filters combined with the NOT operator.

    $join->extra[] = [
                    'field' => $this->handler->realField,
                    'operator' => '!=',
                    'value' => $value,
                    'numeric' => !empty($this->handler->definition['numeric']),
                  ];

    Fixed the issue. Please review this.

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

    Was previously tagged for tests and screenshots which still appear to be needed.

  • 🇺🇸United States tpham1023

    I was able to reproduce the issue. Here are the screenshots.

  • 🇬🇧United Kingdom oily Greater London

    Screenshots tag removed.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 128s
    #330385
  • Pipeline finished with Failed
    about 2 months ago
    Total: 121s
    #330391
  • Pipeline finished with Failed
    about 2 months ago
    Total: 504s
    #330406
  • Pipeline finished with Failed
    about 2 months ago
    Total: 393s
    #330413
  • 🇮🇳India shalini_jha

    Seems like some test failure , that needs to check, i am looking into it.

  • Pipeline finished with Canceled
    about 2 months ago
    Total: 92s
    #330454
  • Pipeline finished with Canceled
    about 2 months ago
    Total: 343s
    #330455
  • 🇬🇧United Kingdom oily Greater London

    Renamed test view in accordance with naming pattern. Removed UUID at top of the test view. Attempt to fix multiple broken tests.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 426s
    #330459
  • Pipeline finished with Success
    about 2 months ago
    Total: 1772s
    #330471
  • 🇬🇧United Kingdom oily Greater London

    Test-only test fails as it should. Pipeline is all green. Change to 'Needs Review'.

  • 🇮🇳India shalini_jha

    I have added the Test coverage for this functionality. That still needs some work and will verify properly, my work was in progress. I am assigning this to me for now.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 684s
    #330665
  • Pipeline finished with Failed
    about 2 months ago
    Total: 650s
    #330671
  • Pipeline finished with Failed
    about 2 months ago
    Total: 663s
    #330675
  • Pipeline finished with Failed
    about 2 months ago
    Total: 552s
    #330686
  • 🇮🇳India shalini_jha

    I have added test coverage for this issue and verified all filter options.
    failing test :

    1) Drupal\Tests\user\Functional\Views\DuplicateRoleFilterTest::testUserRolesFilter
    Behat\Mink\Exception\ResponseTextException: The text "User with role two" was not found anywhere in the text of the current page.
    
    /var/www/html/vendor/behat/mink/src/WebAssert.php:907
    /var/www/html/vendor/behat/mink/src/WebAssert.php:293
    /var/www/html/core/tests/Drupal/Tests/WebAssert.php:979
    /var/www/html/core/modules/user/tests/src/Functional/Views/DuplicateRoleFilterTest.php:84
    

    Now pipeline failure is also fixed so moving this to NR. Kindly review.

  • Test status - Pass
    Patch applied successfully. Issue looks fixed after applying patch.

    Steps to reproduce-
    - Install Drupal with standard profile
    - Create a user with the "Content editor" role
    - Create a user with the "Administrator" role
    - Edit the default "Who's new" view
    - Remove the "Last accessed" filter
    - Add a "User: Roles" filter and select both "Content editor" and "Administrator"
    - Add another "User: Roles" filter and expose it
    - In the preview note all 3 users listed
    - Select "Content editor" and press "Apply", this filter works correctly
    - Select "Administrator" and press "Apply", there are no results.

    Moving it to RTBC

    Attaching screenshots -

Production build 0.71.5 2024