Skip to content

Commit

Permalink
mute: Show @-mention messages even if stream or topic muted.
Browse files Browse the repository at this point in the history
This mirrors the web app's behavior.  For the case of a topic mute,
see `messages_filtered_for_topic_mutes` in `message_list_data.js`.

For the case of a stream mute... the web implementation seems to be
more indirect and I haven't tracked down where stream muting gets
applied to the message list, but empirically it does let messages
through when they have an @-mention.

Fixes: zulip#3472
  • Loading branch information
gnprice authored and chrisbobbe committed Sep 8, 2021
1 parent 5c3ef8f commit 918ed42
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 7 deletions.
14 changes: 14 additions & 0 deletions src/chat/__tests__/narrowsSelectors-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,13 @@ describe('getShownMessagesForNarrow', () => {
test('message in a stream is muted if the topic is muted and topic matches', () => {
expect(shown(makeState({ mute: mutes }))).toEqual(false);
});

test('@-mention message is never muted', () => {
const flags = { ...eg.plusReduxState.flags, mentioned: { [message.id]: true } };
expect(shown(makeState({ flags, subscriptions: [] }))).toEqual(true);
expect(shown(makeState({ flags, subscriptions: [mutedSubscription] }))).toEqual(true);
expect(shown(makeState({ flags, mute: mutes }))).toEqual(true);
});
});

describe('stream narrow', () => {
Expand All @@ -154,6 +161,13 @@ describe('getShownMessagesForNarrow', () => {
test('message muted if topic is muted', () => {
expect(shown(makeState({ mute: mutes }))).toEqual(false);
});

test('@-mention message is never muted', () => {
const flags = { ...eg.plusReduxState.flags, mentioned: { [message.id]: true } };
expect(shown(makeState({ flags, subscriptions: [] }))).toEqual(true);
expect(shown(makeState({ flags, subscriptions: [mutedSubscription] }))).toEqual(true);
expect(shown(makeState({ flags, mute: mutes }))).toEqual(true);
});
});

describe('topic narrow', () => {
Expand Down
18 changes: 11 additions & 7 deletions src/chat/narrowsSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
getMute,
getStreams,
getOutbox,
getFlags,
} from '../directSelectors';
import { getCaughtUpForNarrow } from '../caughtup/caughtUpSelectors';
import { getAllUsersById, getOwnUserId } from '../users/userSelectors';
Expand Down Expand Up @@ -124,13 +125,17 @@ export const getShownMessagesForNarrow: Selector<$ReadOnlyArray<Message | Outbox
getMessagesForNarrow,
state => getSubscriptions(state),
state => getMute(state),
(narrow, messagesForNarrow, subscriptions, mute) =>
state => getFlags(state),
(narrow, messagesForNarrow, subscriptions, mute, flags) =>
caseNarrow(narrow, {
home: _ =>
messagesForNarrow.filter(message => {
if (message.type === 'private') {
return true;
}
if (flags.mentioned[message.id]) {
return true;
}
const streamName = streamNameOfStreamMessage(message);
return (
showStreamInHomeNarrow(streamName, subscriptions)
Expand All @@ -143,6 +148,9 @@ export const getShownMessagesForNarrow: Selector<$ReadOnlyArray<Message | Outbox
if (message.type === 'private') {
return true;
}
if (flags.mentioned[message.id]) {
return true;
}
const streamName = streamNameOfStreamMessage(message);
return !isTopicMuted(streamName, message.subject, mute);
}),
Expand All @@ -158,12 +166,8 @@ export const getShownMessagesForNarrow: Selector<$ReadOnlyArray<Message | Outbox
// In a PM narrow, no messages can be in a muted stream or topic.
pm: _ => messagesForNarrow,

// This logic isn't quite right - we want to make sure we never hide a
// message that has a mention, even if we aren't in the "Mentioned"
// narrow. (#3472) However, it's more complex to do that, and this code
// fixes the largest problem we'd had with muted mentioned messages, which
// is that they show up in the count for the "Mentions" tab, but without
// this conditional they wouldn't in the actual narrow.
// In the @-mentions narrow, all messages are mentions, which we
// always show despite stream or topic mutes.
mentioned: _ => messagesForNarrow,

// The all-PMs narrow doesn't matter here, because we don't offer a
Expand Down

0 comments on commit 918ed42

Please sign in to comment.