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

fix: get user by display name #4171

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

elzody
Copy link
Contributor

@elzody elzody commented Oct 22, 2024

Summary

When mentioning a user within a document from Collabora, it seems to search by the user's display name. Thus, if the display name has any whitespace, it will fail to mention the user. The problem is that the backend tries to use the mentioned user string (display name in this case) as the UID, which is not the same. The solution is to first search by the display name to get the proper user object, then use the UID from that to generate the notification and send it to the user.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

@elzody elzody added 3. to review Ready to be reviewed php Pull requests that update Php code labels Oct 22, 2024
@elzody elzody self-assigned this Oct 22, 2024
@elzody
Copy link
Contributor Author

elzody commented Oct 22, 2024

/backport to stable30

@elzody
Copy link
Contributor Author

elzody commented Oct 22, 2024

/backport to stable29

@elzody
Copy link
Contributor Author

elzody commented Oct 22, 2024

/backport to stable28

@elzody
Copy link
Contributor Author

elzody commented Oct 22, 2024

/backport to stable27

Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes with a suggestion.

lib/Controller/MentionController.php Outdated Show resolved Hide resolved
@@ -39,14 +41,20 @@ public function mention(int $fileId, string $mention): DataResponse {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}

$userFolder = $this->rootFolder->getUserFolder($mention);
$userResults = $this->userManager->searchDisplayName($mention, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a design flaw of the post message API that is not returning us a real unique identifier. Worth to file upstream to further discuss, but fine as a workaround with me now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File upstream where exactly? Should we rather have Collabora return the UID rather than display name?

@juliusknorr juliusknorr added the bug Something isn't working label Nov 5, 2024
@blizzz
Copy link
Member

blizzz commented Nov 5, 2024

What comes to my mind only now is that it probably does not respect various sharing settings, e.g. auto completion of users. Unless the "mention" is filtered somewhere before. Maybe the autocomplete logic (OCP\Collaboration\AutoComplete\IManager) would be better?

Signed-off-by: Elizabeth Danzberger <[email protected]>
@elzody elzody force-pushed the fix/mention-display-name-whitespace branch from 95e362a to 51f0e57 Compare November 6, 2024 18:16
@elzody
Copy link
Contributor Author

elzody commented Nov 6, 2024

What comes to my mind only now is that it probably does not respect various sharing settings, e.g. auto completion of users.

I think this is already done here in the front end, unless I'm mistaken:

const result = await axios.get(generateOcsUrl('core/autocomplete/get'), {
params: { search: text },
})

@blizzz

@elzody elzody merged commit 1974530 into main Nov 6, 2024
67 checks passed
@elzody elzody deleted the fix/mention-display-name-whitespace branch November 6, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Ready to be reviewed bug Something isn't working php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mention notification request fails with 500 status code
3 participants