zulip-flutter
zulip-flutter copied to clipboard
autocomplete: Add "recent senders criterion" to user-mention autocomplete
Besides recent activity in DMs, activity in the current topic/stream is also considered when suggesting users in @-mention autocomplete.
Notes
- This is the second PR in the series of PRs #608 is divided into.
- This PR is stacked on top of #693 specifically the commits are shared up to and including:
autocomplete: Add "recent DM conversations" criterion.
Fixes part of: #228
Let's start applying the "buddy review" system on this PR, now that the start of the official GSoC period is imminent and it seems like everyone's actively engaged again.
@rajveermalviya, would you do the first review on this PR? Some quick bits of context to note:
-
The first few commits are part of #693, as mentioned in the description, and are getting reviewed there. So please focus on the commits that come after those.
-
The last round of review on the predecessor PR started here (and came as several separate comments): https://github.com/zulip/zulip-flutter/pull/608#pullrequestreview-2056029568
Some of that is about what's now in #693; some of it is about what's added in this PR.
Meanwhile we'll proceed with maintainer review for the first PR in this series, #693, because after the previous rounds of review (on #608) that one is further along.
As a reminder since we're just starting to use this system: when you get a buddy review request, please prioritize it right after important bugs and your regressions, ahead of most of your own PRs. (In particular I don't think anyone is currently working on an issue that would come ahead of doing buddy reviews.)
Thanks, @rajveermalviya for the review! Pushed the revision. Please have a look.
@hackerkid Putting this back in your queue after we discussed today how mentor review is meant to work :slightly_smiling_face:
(#713 would be a good PR to review first, though — it's smaller, and also this one is stacked atop #693 which is still in progress, so there's no rush for this one to get through review before #693 is ready.)
@hackerkid Now that #693 is merged, this PR is now ready for the mentor review!
@sm-sayedi
These data structures are used to keep track of user messages in topics and streams.
Can you also please expand the commit message to explicitly mention what MessageTracker and RecentSender does?
Thanks @hackerkid for the review! Revision pushed. Also added a few comments in the previous review thread. PTAL.
Thanks @chrisbobbe for the helpful comments. Revision pushed. Added #692 comment and #692 comment. PTAL.
Note: This PR is rebased on top of #777 because of the build errors mentioned in #771, so the first two commits are from #777.
Ah and there's this from Greg to be resolved: https://github.com/zulip/zulip-flutter/pull/692#discussion_r1662911244
Thanks @gnprice for the last review. Revision pushed. A few comments below. PTAL.
Thanks @gnprice for the review. Updates pushed. Please have a look.
Thank you @gnprice for the review and the added commits; they're really helpful. New revision pushed. PTAL.
Thanks @gnprice for the review. Revision pushed.
Looks good — merging! Thanks again for all your work on this. On to #828, the last stage, so that we can get this feature out to users :slightly_smiling_face:
I added some small NFC commits on top; please take a look through those:
73c827cd8 recent_senders [nfc]: Rename some foosByBar that mean foosInBar
A map of "foos by bar" is a map where each key is a "bar", and each value is a "foo" -- the phrase can be read as an abbreviation of "foos indexed by bar" or "foos, where each foo is indexed by its respective bar".
So in this function messagesByUser is accurately named because
a key represents a user and a value represents some messages.
But these other variables should be e.g. topicsInStream, not
topicsByStream. That one isn't a map where the key is a stream
and the value is a topic or topics; rather it's a collection of
topics (and more data about them) which is the collection
specifically of topics in the stream we're acting on.
eca8af9dd recent_senders [nfc]: Add and revise some docs
dbb2dcfbf recent_senders [nfc]: Write "sorted ascending"