Incorrect role filter in views when added twice

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

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

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

Recommend using MRs for faster reviews.

But either approach will need test coverage

Also solution needs to match issue summary.

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

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

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.

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

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

Screenshots tag removed.

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

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

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

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.

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 -

Changing status to 'Needs review' so a maintainer can review it.

I think RTBTC is okay as it has been reviewed.

I think RTBTC is okay as it has been reviewed.

There were some comments > 80 char, so I wrapped those. I also updated the summary lines in the test to remove that the test is testing for 'the correct results'. I think it is obvious that a test is testing for correct results.

This is testing the UI but there are no screenshots in the issue summary. Even on a short issue the screenshots should available from the issue summary to help reviewers/committers. Yes, I did find the screenshots but both after screenshots show 1 role filter. They do not seem correct.

I have left a question in the MR, so setting to NW for that. I updated credit too.

I do have another question. This is really testing the functionality of the ManyToOneHelper not the Roles filter. Can this problem we shown with just Views and Views UI installed.