Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

People picker component "Single" selection mode not working, still lets you select 2 items #2919

Closed
gomezj5 opened this issue Dec 15, 2023 · 12 comments · Fixed by #2920
Closed
Labels
bug Something isn't working State: In Review

Comments

@gomezj5
Copy link

gomezj5 commented Dec 15, 2023

I'm using graph toolkit for react, and have a people picker that can show up to 5 people on the search, but accept only one selection.
An attribute seems to exist to regulate if the selection can be multiple or single.
The issue is that even if I set the attribute to "single" the picker still allows me to select 2 elements

image image image

Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

Copy link

This issue has been assigned to you, @sebastienlevert. You are listed as the author for the document associated with this issue. If this is not correct, please take the following actions.

  • Assign this issue to the correct author
  • Create a pull request to update the author field in the YAML front-matter of this topic

@sebastienlevert
Copy link
Contributor

When using your setup with your attributes, I can't get it to work as I'm getting an error from Graph :

{
    "error": {
        "code": "Request_UnsupportedQuery",
        "message": "Filter operator 'NotEqualsMatch' is not supported.",
        "innerError": {
            "date": "2023-12-18T15:06:55",
            "request-id": "d2f4fe2c-5616-489d-8b00-ce4475a6e969",
            "client-request-id": "d2f4fe2c-5616-489d-8b00-ce4475a6e969"
        }
    }
}

If you remove the ne clause, can you confirm this now works? It appears the component is not appending the right headers / count parameters to the query when used with userFilters. This seems like an issue. I'll try to move this issue to the project repo.

You can see it succeed without the ne clause in this simple repro: https://stackblitz.com/edit/mgt-8925?file=src%2FApp.tsx

If you are willing to send us a PR that would fix this issue, please feel free! https://aka.ms/mgt

Adding @gavinbarron @musale @Mnickii for awareness.

@sebastienlevert sebastienlevert transferred this issue from microsoftgraph/microsoft-graph-docs-contrib Dec 18, 2023
@sebastienlevert sebastienlevert removed their assignment Dec 18, 2023
@sebastienlevert sebastienlevert added the bug Something isn't working label Dec 18, 2023
@gomezj5
Copy link
Author

gomezj5 commented Dec 18, 2023

The idea is to not let the user select its own account, the user's own account must be excluded from the picker and thus, the reason to put an 'ne' filter. Is there any other way to do that without triggering this behavior? I think the 'selected items' collection can be manipulated by a 'selected change' event
Would be nice if the option can be added in the future but a workaround will be ok for the time being

@sebastienlevert
Copy link
Contributor

I absolutely understand the use case, which is a very value one! Now, the use of ne on this API is tricky as it requires changes to the way the API works and is where we hit a limitation / bug. This will get fixed. Have a look at the limitations / opportunities by the Advanced Query Capabilities: https://learn.microsoft.com/en-us/graph/aad-advanced-queries?tabs=http.

Now, onto your question and finding a workaround, I wasn't able to find anything that would exclude a user (as excluding means using negative keywords on the API) and wouldn't work from the API directly. You could remove the current user when selected, but for our use case, this would be too late as the item would already be selected (and visible in the selection area). Adding @gavinbarron for thoughts on this case. To me, this would be a good for 2 specific changes to the people picker component:

  • Making sure ConsistencyLevel = Eventual and $count=true is appended to any query leveraging userFilters.
  • Provide an extra attribute exclude-current-user that would automatically add the and id ne currentUserId to any query to avoid getting the current user in the list.

@gavinbarron
Copy link
Member

What version of MGT is being used? When testing a simple case single selection is limiting the number of selection available:
https://mgt.dev/?path=/story/components-mgt-people-picker-properties--single-select-mode

Even adding all the other filter options I still can only select one user with the current version of MGT.

Regarding the enhancements to allow negative filter queries, I feel like adding a extra attribute for excluding the current user is a nice convenient short cut. As to adding the consistency level and count parameters, I think that we might want to be a bit smart about those and only add them if there is an ne clause in the filter.

@gomezj5
Copy link
Author

gomezj5 commented Dec 18, 2023

Tried a simpler version:

image

Problem still persists even without the 'ne' queries
Another detail to point out is that this is happening only in react, using the @microsoft/mgt-react package
Updated mgt-react to v3+ but got the same result
Another interesting find, when the error happens (when the second element is selected) I get this from the console

image image

@sebastienlevert
Copy link
Contributor

Interesting... Can you reproduce in the stackblitz link I shared? https://stackblitz.com/edit/mgt-8925?file=src%2FApp.tsx

I tried to give a first go to supporting the advanced queries using the userFilters and it seems that it works on my end. We'll see if this makes it or not. PR here: #2920

@gomezj5
Copy link
Author

gomezj5 commented Dec 19, 2023

done it, and was able to fix the issue, so far I've learned the following

1: The issue happens only on mtg-react v2, once I've migrated to v3 the issue went away
2: Migrating to V3 does not break anything on React17 or Fluent UI8 :)
3: The 'ne' user filter works as intended with V3 without altering the results or the index

@gomezj5 gomezj5 closed this as completed Dec 19, 2023
@gomezj5
Copy link
Author

gomezj5 commented Dec 19, 2023

Another interesting find
When using mtg-react and you have a component that renders "child" components containing Personas, the user query might not trigger but if you add this to the parent component, once the child component is rendered mtg-react will make the queries and populate the personas

MgtPerson.config.useContactApis = true;

@sebastienlevert
Copy link
Contributor

Thanks for sharing your findings. I'm curious on the userFilters usage with the 'ne' operator. You are saying you can run exclude yourself from the results? We faced the issue today and created a fix to bring this feature for all our code paths using the users endpoint. Were you using this branch?

@gomezj5
Copy link
Author

gomezj5 commented Dec 19, 2023

I was using mtg-react 2.11.2 and using the following query

userFilters={userType eq 'Member' and mail ne 'myemail@contoso.com'}

mtg-react produces a query like this and sends it to graph

https://graph.microsoft.com/v1.0/users?$count=true&$search="displayName:vane" OR "mail:vane"&$filter=userType eq 'Member' and mail ne 'myemail@contoso.com'&$top=5

Still works without issues in V3+

@sebastienlevert
Copy link
Contributor

The query (once searching) works, but I assume there is an HTTP 400 on the initial load where it doesn't load anybody from the /users endpoint. Can you confirm you see this behavior? But I agree with you, the search path seems to work for me also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working State: In Review
Projects
Archived in project
3 participants