zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.29k stars 653 forks source link

Handle wildcard mentions uniformly with direct @-mentions #4911

Open gnprice opened 3 years ago

gnprice commented 3 years ago

Messages come with two different potential flags mentioned and wildcard_mentioned. See API docs: https://zulip.com/api/update-message-flags#available-flags

In several of the places where we refer to mentioned, we look at wildcard_mentioned at the same time, and the overall semantics we get is mentioned || wildcard_mentioned.

It's likely that that's actually the semantics we want every time we refer to mentioned. We should switch to doing that -- or, if we find there's a case where we don't want that semantics, we should be sure to leave a comment there making that choice quite explicit.

And in order to make that a maintainable state for the code to be in, we should make it so that the most natural way to just look up whether a message has an @-mention will get the combined semantics mentioned || wildcard_mentioned. The simplest way to do that is to make state.flags.mentioned end up having that combined semantics. That's also what the webapp effectively does. So let's do that.

Note there is an important wrinkle to get right when we do so: we need to be sure to have correct behavior when the message is edited so that the flags change. (See more-detailed discussion: https://github.com/zulip/zulip-mobile/pull/3542#issuecomment-529702087 ) We'll want to be sure to have tests for that case.

There was a previous draft of this as 7fa8ef7486a958804ebe872d3d485358a866b97a, in #3542. Feel free to borrow from there, but that commit is small and needed some changes, so it might be simplest to implement this from scratch.

timabbott commented 3 years ago

It is worth mentioning that for email and push notifications, we do have settings that control whether wildcard mentions should trigger emacs/push notifications: https://zulip.com/help/pm-mention-alert-notifications#wildcard-mentions

But UI logic for displaying whether a mention occurred should use that mentioned || wildcard_mentioned logic.