[BUG] mgt-people-picker: Invalid $filter when using people-filters

[SYoungWSP]

I am trying to exclude Personal Contacts from the mgt-people-picker results in a corporate application.

The following filter works through Graph Explorer (https://developer.microsoft.com/en-us/graph/graph-explorer).

https://graph.microsoft.com/v1.0/me/people?$search=young&$filter=personType/class+eq+'Person'and+personType/subclass+eq+'OrganizationUser'$filter=personType/class+eq+'Person'and+personType/subclass+eq+'OrganizationUser'

If I add the filter to the mgt-people-picker's people-filters attribute I get no results and can see a 400 response from the Graph API call.

Graph API call made by the component:

https://graph.microsoft.com/v1.0/me/people?$search=%22young%22&$top=6&$filter=personType/class%20eq%20%27Person%27personType/class%20eq%20%27Person%27%20and%20personType/class%20eq%20%27Person%27%20and%20personType/subclass%20eq%20%27OrganizationUser%27

I notice that personType/class eq 'Person' is automatically added to the query when using the people-filters attribute so I shortened the filter to personType/subclass eq 'OrganizationUser' to avoid repetition however it looks like:

https://graph.microsoft.com/v1.0/me/people?$search=%22young%22&$top=6&$filter=personType/class%20eq%20%27Person%27personType/class%20eq%20%27Person%27%20and%20personType/subclass%20eq%20%27OrganizationUser%27

Steps to reproduce
I recreated it using Microsoft Graph Toolkit Playground (https://mgt.dev/?path=/story/components-mgt-people-picker-properties--picker-people-filters) with version 3.1.1 and then realised even the straight example has the same problem.

<mgt-people-picker people-filters="jobTitle eq 'Web Marketing Manager'">
</mgt-people-picker>

No results returned.
Request:
https://graph.microsoft.com/v1.0/me/people?$search="ale"&$top=6&$filter=personType/class eq 'Person'personType/class eq 'Person' and jobTitle eq 'Web Marketing Manager'
Response: 400
"Invalid filter clause: Syntax error at position 38 in 'personType/class eq 'Person'personType/class eq 'Person' and jobTitle eq 'Web Marketing Manager''."

Expected behavior
If personType filter specified in people-filters don't append again. Ensure 'and' is added before any people-filters to create a valid $filter.

Environment (please complete the following information):

• OS: Windows
• Browser: Edge + Firefox
• Framework: Angular
• Context: Web
• Version: 2.11.12 but recreated on 3.1.1
• Provider: Msal2Provider

[gavinbarron]

Hi @SYoungWSP.

What scopes/permissions does the user have when running those queries?

If you have only requested User.ReadBasic.All then you cannot filter on properties that are unavailable with the lower permissions. You may need to request User.Read.All instead

[SYoungWSP]

Hi @gavinbarron,

In my app I have user.read, People.Read and User.ReadBasic.All.

I'm not sure how it works in the Toolkit Playground but the error is the same there. If I try from Graph Explorer with the default permissions it works.

Thanks

[sebastienlevert]

You will need User.Read.All to be able to use this capability. It's linked to this #2624 and we really need to add docs for this.

[SYoungWSP]

Hi,

#2624 is talking about user-filters. Is it the same for people-filters that they need User.Read.All? The issue I'm encountering is that the query URL is not even formed correctly so I get a 400 response not incomplete data or a 403 or something.

Thanks

[sebastienlevert]

Oh! Wait a second! I just saw the issue.

It seems that we are double filtering and building an invalid $filter query.

https://graph.microsoft.com/v1.0/me/people?$search=%22ale%22&$top=6&$filter=personType/class eq 'Person'personType/class eq 'Person' and jobTitle eq 'Web Marketing Manager'

We can see that we're filtering 2 times on personType/class and we have an issue with string concatenation. It would be somewhere here: https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/main/packages/mgt-components/src/graph/graph.people.ts#L194

Sorry, I totally missed it the first time 😮

Would you be interested to send us a PR and we could review / help with it? Thanks!

[SYoungWSP]

Hi, I've never made a PR before but I can try. Looking at the file you linked it looks like findPeople may have the bug on this line:

microsoft-graph-toolkit/packages/mgt-components/src/graph/graph.people.ts

Line 138 in 80de496

filter += `${filter} and ${filters}`;

as it is using += but also including the existing value.

[sebastienlevert]

You're absolutely right. It seems where the issue is! Good catch!

Feel free to submit a PR, we'll gladly review it!