Skip to content

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Apr 5, 2019

Requires #1676 and #1677 (I will rebase on master the last two commits of this pull request once those two pull requests are merged).

In regular rooms only the current participants are candidates in mentions, but in file rooms the candidates are all the users with access to the file, even if they are not currently participants of the room. However, as the users with access to the file are a superset of the current participants those participants were returned twice. Therefore, now candidates for mentions in file rooms are got only from the users with access to the file instead of in addition to the current participants.

How to test:

  • In the Files app, share a file with another user
  • Open the Chat tab for that file
  • As the other user, open the Chat tab for the file (so the user joins the room for the file)
  • As the original user, type @ in the Chat tab to show the candidate mentions

Result with this pull request:

The other user appears once in the candidate mentions

Result without this pull request:

The other user appears twice in the candidate mentions

@danxuliu danxuliu added 2. developing bug feature: chat 💬 Chat and system messages feature: talk-sidebar ⬅️ Sidebar integration of Talk into other apps like sharing and documents labels Apr 5, 2019
@danxuliu danxuliu added this to the 6.0.0 milestone Apr 5, 2019
@danxuliu
Copy link
Member Author

danxuliu commented Apr 5, 2019

/backport to stable15

Copy link
Member

@Ivansss Ivansss left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@nickvergessen
Copy link
Member

#1676 and #1677 are in, time to rebase

danxuliu added 2 commits April 8, 2019 16:46
In regular rooms only the current participants are candidates in
mentions, but in file rooms the candidates are all the users with access
to the file, even if they are not currently participants of the room.
However, as the users with access to the file are a superset of the
current participants those participants were returned twice. Therefore,
now candidates for mentions in file rooms are got only from the users
with access to the file instead of in addition to the current
participants.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu force-pushed the fix-duplicated-candidate-mentions-in-file-rooms branch from ed03ac4 to 475abd0 Compare April 8, 2019 14:46
@danxuliu
Copy link
Member Author

danxuliu commented Apr 8, 2019

Rebased.

@nickvergessen nickvergessen merged commit c5f4b86 into master Apr 9, 2019
@nickvergessen nickvergessen deleted the fix-duplicated-candidate-mentions-in-file-rooms branch April 9, 2019 07:35
@backportbot-nextcloud
Copy link

backport to stable15 in #1693

* @since 13.0.0
*/
public function search($search, $limit, $offset, ISearchResult $searchResult) {
if ($this->room->getObjectType() === 'file') {
Copy link
Member

Choose a reason for hiding this comment

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

btw jsut for the record:
I did it the other way around, so normal users are listed before users which are not in the room yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I did not think on that. Then maybe it would be better to restore the previous code and explicitly remove duplicated elements?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug feature: chat 💬 Chat and system messages feature: talk-sidebar ⬅️ Sidebar integration of Talk into other apps like sharing and documents

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants