Views contextual filter: "allow multiple" doesn't work for user roles filter.

I confirm the issue. It does not depend on display type. Same for block or page display. Any ideas, patches?

Looks like need to be fixed parameter $force_int for unpackArgumentValue().

Result of local testing attached.

Despite that, the patch is working, but maybe need to check how to fix properly $this->definition['numeric'], now it has TRUE value even for the string context filter?

added screenshot with testing OR with AND condition in the contextual filter.

The issue was with the Roleid plugin definition. Previously role id is used to be a numeric value but it's not numeric anymore.

The patch at #8 worked perfectly for me.
Its RTBC 1 for the patch at #8.

Let more people give it a try and see for themselves before moving issue status to RTBC.

Nice find, and thanks for the work on this.

You should never (hardly ever?) have to use an alter hook inside core, since we should just be supplying the correct data to begin with.

In this case, the change should be made in \Drupal\user\UserViewsData

That currently says:

$data['user__roles']['roles_target_id']['argument'] = [
      'id' => 'user__roles_rid',
      'name table' => 'role',
      'name field' => 'name',
      'empty field name' => $this->t('No role'),
      'zero is null' => TRUE,
      'numeric' => TRUE,
    ];

So that 'numeric' seems wrong, and we should change it there.

Also, this needs an automated test.

Thanks, @Lendude

I will create a new patch for this.

The issue was with the Roleid plugin definition. Previously role id is used to be a numeric value but it's not numeric anymore.

Here is the updated patch.

+++ b/core/modules/user/src/UserViewsData.php
@@ -233,7 +233,7 @@ public function getViewsData() {
-      'numeric' => TRUE,
+      'numeric' => FALSE,

If anything, we should just leave this out and not set it too FALSE.

But looking at this some more, the better fix would probably be to not have \Drupal\user\Plugin\views\argument\RolesRid extend ManyToOne but use \Drupal\views\Plugin\views\argument\StringArgument and 'many to one' set to TRUE. But that might be too big of a change?

But don't bother rolling any of that into a patch unless we add some test coverage first, because there might be some details we are overlooking here.

This handler only has a Unit test, I feel we need a functional test for this

Replacing ManyToOne with StringArgument is working fine except the AND(role1,role2) condition.
Passing role as (role1,role2) should work as AND statement in the query however its been treated as OR statement. Looks like the AND clause is completely missing.

The codes block from the query function in StringArgument class at line number 239 is causing the issue.

if (count($this->value) > 1) {
      $operator = 'IN';
      $argument = $this->value;
    }
    else {
      $operator = '=';
    }

Passing more than one arguments in contextual filter always replace $operator with IN statement. I am strugling to find a way to have AND clause added here any help or direction will be appreciated.

@Lendude
Adding a few functional test cases to evaluate if the patch works with all the scenarios. Patch is passing on local.

\Drupal\views\Plugin\views\argument\StringArgument and 'many to one' set to TRUE. But that might be too big of a change?

I think I can review a few things in that direction.

Besides, I think we can switch the issue to 9.2.x branch.

Above test failures where caused by a broken HEAD. This was fixed in #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest. Ordered retest and put this issue back to RTBC per #16.

Hello I want to display like this image with the contextual filters of drupal but I can not display the title description and number of module of each course at once

#16 looks great 👍🏻

@mohit_aghera great stuff!

The fix seems to do the trick, so disregard my comment in #13, lets go with this.

1. +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_user_role_contextual_filter.yml
   @@ -0,0 +1,219 @@
   +  entity_reference:
   +    display_plugin: entity_reference
   +    id: entity_reference
   

   Any reason to add and use the entity reference display? Or could we just use the default display?

2. +++ b/core/modules/views/tests/src/Functional/Plugin/ContextualFiltersStringTest.php
   @@ -0,0 +1,109 @@
   + * Test contextual filters with allow multiple values setting for user roles.
   

   maybe quote 'allow multiple values'?

3. +++ b/core/modules/views/tests/src/Functional/Plugin/ContextualFiltersStringTest.php
   @@ -0,0 +1,109 @@
   +    $this->assertSession()->pageTextContains('user1');
   +    $this->assertSession()->pageTextContains('user3');
   +    $this->assertSession()->pageTextContains('user5');
   

   In all these cases we want to check for all users. We need a pageTextNotContains for the users we expect to be hidden. The way it is setup now, it could be showing all the users in every case and the test would still pass

Thanks for the review @Lendude.
1. I think "entity_reference" display might have came through when I cloned some views while writing test case.
It is not necessary. Removed it.
Though there is a difference in a format like "Entity Reference list" won't be available in the default view. However, we can safely ignore it because title will still be displayed and we want to compare the title only, irrespective of format.

Test cases are still passing with relevant changes.

2. Fixed the code

3. Added assertions to validate that titles shouldn't exist which doesn't specify criteria.

Looks great, thanks!

Committed and pushed eb2ed2340f to 9.3.x and 392faadb1d to 9.2.x. Thanks!

Nice to have this fixed and tested.