fix(people picker): fixes #1292 where people picker returns no results. #1293
Conversation
|
Hey! It loads a group instead of the correct user! Cheers, |
* Add `.github/fabricbot.json` * Adding Modern TaxonomyTree to Test Harness * Delete fabricbot.json Co-authored-by: msftbot[bot] <48340428+msftbot[bot]@users.noreply.github.com> Co-authored-by: Alex Terentiev <aleksei.terentiev@gmail.com>
|
@SmarterJB hey, curious if the dynamic form works properly in 3.9.0? Based on what I can see in the commit history, the change I made simply reverted the people picker back to what was published in 3.9.0. Wondering if the issue is more with the dynamic form than the people picker? I have not used the dynamic form, but if I get a chance this week, I can try and debug to see what it may be doing. |
|
Hey! Sorry, it seems that your change has nothing to do with the behaviour it shows! Its just that because the user was not found, the dynamic form used the Title, so it seemed that it works! But the problem is somewhere else in the DynamicForm using the PeoplePicker when allowing all principalTypes! I dug into it today a little bit, and the issue is that the DynamicForm does not use the e-mail adress of the user in the So... Sorry, my comment was wrong, so your PR does not break anything, but the DynamicForm itself combined with PeoplePicker is broken still! I tried today to fix it, but it has a lot of dependencies for saving the DynamicForm if you change the way you store the userdata, so this would take a bit of time! So I think I should open another issue there! |
|
So this is how it should be, it should be the user "Josef Benda" selected! but if I change the setting to: |
* Add `.github/fabricbot.json` * Fix issue with FilePicker Tile view and selection of file * Typo Co-authored-by: msftbot[bot] <48340428+msftbot[bot]@users.noreply.github.com> Co-authored-by: Alex Terentiev <aleksei.terentiev@gmail.com>
) * Add `.github/fabricbot.json` * feat(modern taxonomy picker): ability to disallow selecting children Adds new `allowSelectingChildren` prop, which is true by default. Setting to `false` means only the top-level terms appear in search results, and their tree nodes are not expandable. Respects `anchorTermId`. Co-authored-by: msftbot[bot] <48340428+msftbot[bot]@users.noreply.github.com> Co-authored-by: Alex Terentiev <aleksei.terentiev@gmail.com> Co-authored-by: Jake Stanger <jake.stanger@core.co.uk>
…User Use webAbsoluteUrl if provided through props to ensure user
|
@SmarterJB thanks for the additional information. It would be great if you could open a separate issue please as it really seems like a different one. |
src/services/PeopleSearchService.ts
Outdated
| } | ||
|
|
||
| // Check if users need to be searched in a specific SharePoint Group | ||
| if (groupId && typeof (groupId) === 'number') { | ||
| searchBody.queryParams.SharePointGroupID = groupId; | ||
| searchBody.queryParams["SharePointGroupID"] = groupId; | ||
| } |
There was a problem hiding this comment.
Can you revert to using dot notation, please? I assume this was not related to the issue itself?
There was a problem hiding this comment.
@joelfmrodrigues in the previous version (3.9.0) it was using the [] syntax. Since SharePointGroupID is not defined on the queryParams object on line 114, the dot syntax throw an error on 127 and 132. The issue here is that you cannot pass a null value for SharePointGroupID, so we cannot include it on the queryParams object by default, which is why I believe the [] syntax is being used on 127 and 132. I'm happy to update the PR, just let me know what you think about the above. Thanks!
There was a problem hiding this comment.
Sorry, you are correct as this would throw an error :) : https://stackoverflow.com/questions/12710905/how-do-i-dynamically-assign-properties-to-an-object-in-typescript
Any chance you could add any as the type for searchBody as this should allow the dot notation to be used? If it's too much hassle, let me know and I can update after approving the PR
Many thanks a sorry for so many comments
There was a problem hiding this comment.
@joelfmrodrigues not a problem, just made the changes and added a new commit. Let me know if anything else is needed. Thanks!
|
@c-eiser13 the PR was now including all the previous changes from other PRs as pending changes - not 100% sure why GitHub did not recognise them as existing changes... Many thanks for looking into this and providing a fix! |
|
Closed as changes directly implemented into dev branch |
What's in this Pull Request?
Reverted changes previously made for 1.15 support that cause the people picker to return 0 results.
Please describe the changes in this PR. Sample description or details around bugs which are being fixed.
People picker queryParams was including a SharePointGroupID param with null value by default and the API does not allow null values for this param.