boxes: Use `user_id`s in autocomplete for user mentions. #928

prah23 · 2021-02-17T22:16:23Z

This PR adds a commit that append user_ids to the users' names in the autocomplete typeahead for user mentions. This helps in distinguishing between users having the same name, similar to the pm recipients autocomplete.

Tests updated.

Fixes #151.

neiljp

@prah23 This seems to work, but while you mentioned adding tests later on czo, you note in the commit text that tests are included - did you forget to add them?

Manually this seems to work, but once we have tests then we could ensure we exercise the code a little more. The duplicates check relies on adjacent elements being identical given the sort?

prah23 · 2021-02-22T17:25:31Z

@neiljp I haven't removed the "Tests updated." part of the commit text from the previous iteration, where I've appended user_ids to all the names in the list. I'm currently adding tests and will update you once the PR is ready. Also, I maintain a separate list of duplicate names, which I check with before appending user_ids to the final list that is passed to the typeahead, so the duplicates check does not rely on adjacency or any particular order for that matter.

The duplicates list is formed by checking if the current name in the loop exists in the sub-list of elements leading to the current name. I've run this loop on the user_names list.

neiljp · 2021-02-22T22:34:06Z

Re adjacency, I was referring to the sub-list check you indicated. If the list is sorted, can we rely upon the previous entry only? Alternatively, it might be clearer to use Counter. In any case, once we have tests then this will be easier to explore.

zulipbot · 2021-03-21T21:01:19Z

ERROR: Label "PR needs review" does not exist and was thus not removed from this pull request.

prah23 · 2021-03-21T21:02:13Z

@zulipbot add "PR needs review"

prah23 · 2021-03-21T21:03:13Z

@zulipbot remove "PR awaiting update"

neiljp

@prah23 This seems close to being merged 👍

tests/conftest.py

tests/ui_tools/test_boxes.py

tests/conftest.py

tests/ui_tools/test_boxes.py

zulipterminal/ui_tools/boxes.py

prah23

@neiljp I've addressed all your comments from the previous review. I haven't combined the tests due to the typeahead list being different (normal and silent mentions) and I've also had to explicitly add a test case, which I've explained inline. It's ready for a review.

prah23 · 2021-03-29T16:49:57Z

tests/ui_tools/test_boxes.py

+    @pytest.mark.parametrize('text', [
+        prefix + 'Human'[:index + 1] for index in range(len('Human'))
+        for prefix in ['@', '@**']
+    ] + ['@**'])


I'm having to add this prefix as an extra case this way because it moves away from the general pattern that is common for both prefixes. We're able to test only the prefix as a case in case of @** and not @ is because the only the former is specifically used only for user mentions and has only user names in the typeahead, the latter also being applicable to group names.

prah23 · 2021-03-29T20:04:37Z

@zulipbot remove "PR awaiting update"
@zulipbot add "PR needs review"

This commit adds two new users to the testing suite, both named 'Human Duplicate' as a prep commit for future tests that need such users. Tests updated (by adding test cases where relevant and extending existing parameters to include both these users in typeaheads and footer texts).

prah23 · 2021-06-07T21:36:07Z

@neiljp this has been black rebased, is up-to-date, and ready for another review.

neiljp

@prah23 As discussed this looks close to merging and really clean 👍

I just left a few comments inline.

zulipterminal/ui_tools/boxes.py

tests/ui_tools/test_boxes.py

Prior to this commit, user mentions through autocomplete used only names in the typeahead. This commit appends `user_id`s to all the multiple instances of the same name to use the **Name|User_id** format of user mentions. This is done by using collections.Counter to check the count of each user name in the users list. The user_id is appended to names that have a count greater than 1. The appended user_id would serve as a distinction between users having the same name. Tests added and updated. Fixes zulip#151.

neiljp · 2021-06-11T23:47:25Z

@prah23 Thanks for working on this - I'm going to merge this now 🎉

We could debate the best test structure, but having the feature there and tested is good and we can always iterate from there 👍

zulipbot added the size: XL [Automatic label added by zulipbot] label Feb 17, 2021

prah23 force-pushed the autocomplete_users_refinement branch from 9ab0e5b to 2bb5bae Compare February 18, 2021 20:09

zulipbot added size: S [Automatic label added by zulipbot] and removed size: XL [Automatic label added by zulipbot] labels Feb 18, 2021

neiljp reviewed Feb 21, 2021

View reviewed changes

neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Feb 21, 2021

neiljp added this to the Next Release milestone Feb 28, 2021

neiljp mentioned this pull request Mar 13, 2021

Create extended user mention syntax when needed. #151

Closed

prah23 force-pushed the autocomplete_users_refinement branch from 2bb5bae to 8d61607 Compare March 21, 2021 19:24

zulipbot added size: XL [Automatic label added by zulipbot] and removed size: S [Automatic label added by zulipbot] labels Mar 21, 2021

prah23 force-pushed the autocomplete_users_refinement branch 3 times, most recently from 11ca5a4 to c41ed97 Compare March 21, 2021 20:54

zulipbot added PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback and removed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback labels Mar 21, 2021

zulipbot added the PR needs review PR requires feedback to proceed label Mar 21, 2021

zulipbot removed the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Mar 21, 2021

neiljp reviewed Mar 26, 2021

View reviewed changes

neiljp added PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback and removed PR needs review PR requires feedback to proceed labels Mar 26, 2021

prah23 force-pushed the autocomplete_users_refinement branch 4 times, most recently from 4b7371b to 7172b8f Compare March 29, 2021 15:42

prah23 commented Mar 29, 2021

View reviewed changes

zulipbot added PR needs review PR requires feedback to proceed and removed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback labels Mar 29, 2021

prah23 requested a review from neiljp May 1, 2021 00:03

prah23 force-pushed the autocomplete_users_refinement branch from 7172b8f to 85abbf6 Compare June 7, 2021 20:22

prah23 force-pushed the autocomplete_users_refinement branch from 85abbf6 to 7a9cb8f Compare June 7, 2021 21:07

neiljp reviewed Jun 9, 2021

View reviewed changes

zulipterminal/ui_tools/boxes.py Show resolved Hide resolved

tests/ui_tools/test_boxes.py Outdated Show resolved Hide resolved

tests/ui_tools/test_boxes.py Outdated Show resolved Hide resolved

neiljp added PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback and removed PR needs review PR requires feedback to proceed labels Jun 9, 2021

prah23 force-pushed the autocomplete_users_refinement branch from 7a9cb8f to 1d87807 Compare June 11, 2021 22:32

prah23 added PR needs review PR requires feedback to proceed and removed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback labels Jun 11, 2021

prah23 force-pushed the autocomplete_users_refinement branch from 1d87807 to aede198 Compare June 11, 2021 22:37

prah23 force-pushed the autocomplete_users_refinement branch from aede198 to ecbbc64 Compare June 11, 2021 22:38

neiljp merged commit 9280f09 into zulip:main Jun 11, 2021

neiljp removed the PR needs review PR requires feedback to proceed label Jun 11, 2021

neiljp mentioned this pull request Jul 31, 2021

TRACKING: Typeahead improvements #448

Closed

12 tasks

prah23 mentioned this pull request Feb 14, 2022

Two users with the the same name appear as the same person #1151

Open

neiljp added area: autocomplete missing feature: user A missing feature for all users, present in another Zulip client labels Feb 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boxes: Use `user_id`s in autocomplete for user mentions. #928

boxes: Use `user_id`s in autocomplete for user mentions. #928

prah23 commented Feb 17, 2021 •

edited

Loading

neiljp left a comment

prah23 commented Feb 22, 2021

neiljp commented Feb 22, 2021

zulipbot commented Mar 21, 2021

prah23 commented Mar 21, 2021

prah23 commented Mar 21, 2021

neiljp left a comment

prah23 left a comment •

edited

Loading

prah23 Mar 29, 2021 •

edited

Loading

prah23 commented Mar 29, 2021

prah23 commented Jun 7, 2021

neiljp left a comment

neiljp commented Jun 11, 2021

boxes: Use user_ids in autocomplete for user mentions. #928

boxes: Use user_ids in autocomplete for user mentions. #928

Conversation

prah23 commented Feb 17, 2021 • edited Loading

neiljp left a comment

Choose a reason for hiding this comment

prah23 commented Feb 22, 2021

neiljp commented Feb 22, 2021

zulipbot commented Mar 21, 2021

prah23 commented Mar 21, 2021

prah23 commented Mar 21, 2021

neiljp left a comment

Choose a reason for hiding this comment

prah23 left a comment • edited Loading

Choose a reason for hiding this comment

prah23 Mar 29, 2021 • edited Loading

Choose a reason for hiding this comment

prah23 commented Mar 29, 2021

prah23 commented Jun 7, 2021

neiljp left a comment

Choose a reason for hiding this comment

neiljp commented Jun 11, 2021

boxes: Use `user_id`s in autocomplete for user mentions. #928

boxes: Use `user_id`s in autocomplete for user mentions. #928

prah23 commented Feb 17, 2021 •

edited

Loading

prah23 left a comment •

edited

Loading

prah23 Mar 29, 2021 •

edited

Loading