- 🇦🇺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 8:40am 9 January 2024 - Status changed to Needs work
12 months ago 8:14pm 9 January 2024 - 🇺🇸United States smustgrave
Recommend using MRs for faster reviews.
But either approach will need test coverage
Also solution needs to match issue summary.
- last update
12 months ago 25,991 pass, 1,830 fail - last update
12 months ago 25,986 pass, 1,840 fail - Merge request !6093Issues-3062214-incorrect-role-filter-in-views-when-added-twice-fixes → (Open) created by shalini_jha
- last update
12 months ago Composer error. Unable to continue. - 🇮🇳India guptahemant
Following test case is failing
Drupal\Tests\taxonomy\Functional\Views\TaxonomyIndexTidUiTest::testFilterGrouping
- First commit to issue fork.
- Status changed to Needs review
11 months ago 4:24pm 29 January 2024 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 4:52pm 29 January 2024 - 🇺🇸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.
- 🇮🇳India shalini_jha
Seems like some test failure , that needs to check, i am looking into it.
- 🇬🇧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.
- 🇬🇧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.
- 🇮🇳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 -