Skip to content

Commit 18831f9

Browse files
committed
mute [nfc]: Rename getter to isTopicVisibleInStream, and flip sense
With the upcoming feature of unmuted topics in muted streams, the question "is this topic muted?" will split into several different questions that can have different answers. To prepare for that, stop having a function asking that less-specific question. For several call sites, we can cleanly write a name and jsdoc for a function that covers all of them. In the topic action sheet, the question we'll end up wanting to ask is different from those, and reflects a UI choice we're making specifically about the action sheet. So just put an explicit switch statement there.
1 parent 4bc5d61 commit 18831f9

File tree

6 files changed

+32
-23
lines changed

6 files changed

+32
-23
lines changed

src/action-sheets/index.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
} from '../utils/narrow';
3333
import { pmUiRecipientsFromKeyRecipients } from '../utils/recipient';
3434
import type { PmKeyRecipients } from '../utils/recipient';
35-
import { isTopicMuted } from '../mute/muteModel';
35+
import { getTopicVisibilityPolicy } from '../mute/muteModel';
3636
import * as api from '../api';
3737
import { showConfirmationDialog, showErrorAlert, showToast } from '../utils/info';
3838
import { doNarrow, deleteOutboxMessage, fetchSomeMessageIdForConversation } from '../actions';
@@ -648,10 +648,13 @@ export const constructTopicActionButtons = (args: {|
648648
}
649649
if (sub && !streamMuted) {
650650
// Stream subscribed and not muted.
651-
if (isTopicMuted(streamId, topic, mute)) {
652-
buttons.push(unmuteTopic);
653-
} else {
654-
buttons.push(muteTopic);
651+
switch (getTopicVisibilityPolicy(mute, streamId, topic)) {
652+
case UserTopicVisibilityPolicy.Muted:
653+
buttons.push(unmuteTopic);
654+
break;
655+
case UserTopicVisibilityPolicy.None:
656+
buttons.push(muteTopic);
657+
break;
655658
}
656659
} else if (sub && streamMuted) {
657660
// TODO(#5691): offer new "unmute topic" concept, when server supports it

src/chat/narrowsSelectors.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
caseNarrow,
2424
streamIdOfNarrow,
2525
} from '../utils/narrow';
26-
import { getMute, isTopicMuted } from '../mute/muteModel';
26+
import { getMute, isTopicVisibleInStream } from '../mute/muteModel';
2727
import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects';
2828
import * as logging from '../utils/logging';
2929
import { getStreamsById, getSubscriptionsById } from '../subscriptions/subscriptionSelectors';
@@ -121,7 +121,7 @@ export const getShownMessagesForNarrow: Selector<$ReadOnlyArray<Message | Outbox
121121
}
122122
return (
123123
sub.in_home_view
124-
&& !isTopicMuted(message.stream_id, message.subject, mute)
124+
&& isTopicVisibleInStream(message.stream_id, message.subject, mute)
125125
);
126126
}),
127127

@@ -133,7 +133,7 @@ export const getShownMessagesForNarrow: Selector<$ReadOnlyArray<Message | Outbox
133133
if (flags.mentioned[message.id]) {
134134
return true;
135135
}
136-
return !isTopicMuted(message.stream_id, message.subject, mute);
136+
return isTopicVisibleInStream(message.stream_id, message.subject, mute);
137137
}),
138138

139139
// In the starred-message view, ignore stream/topic mutes.

src/mute/__tests__/muteModel-test.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import deepFreeze from 'deep-freeze';
33

44
import fullReducer from '../../boot/reducers';
5-
import { getMute, getTopicVisibilityPolicy, isTopicMuted, reducer } from '../muteModel';
5+
import { getMute, getTopicVisibilityPolicy, isTopicVisibleInStream, reducer } from '../muteModel';
66
import { EVENT, EVENT_MUTED_TOPICS } from '../../actionConstants';
77
import * as eg from '../../__tests__/lib/exampleData';
88
import { makeMuteState, makeUserTopic } from './mute-testlib';
@@ -33,21 +33,21 @@ describe('getters', () => {
3333
});
3434
});
3535

36-
describe('isTopicMuted', () => {
36+
describe('isTopicVisibleInStream', () => {
3737
function check(state, expected) {
38-
expect(isTopicMuted(eg.stream.stream_id, 'topic', state)).toEqual(expected);
38+
expect(isTopicVisibleInStream(eg.stream.stream_id, 'topic', state)).toEqual(expected);
3939
}
4040

4141
test('with nothing for stream', () => {
42-
check(makeMuteState([]), false);
42+
check(makeMuteState([]), true);
4343
});
4444

4545
test('with nothing for topic', () => {
46-
check(makeMuteState([[eg.stream, 'other topic']]), false);
46+
check(makeMuteState([[eg.stream, 'other topic']]), true);
4747
});
4848

4949
test('with topic muted', () => {
50-
check(makeMuteState([[eg.stream, 'topic']]), true);
50+
check(makeMuteState([[eg.stream, 'topic']]), false);
5151
});
5252
});
5353
});

src/mute/muteModel.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,19 @@ export function getTopicVisibilityPolicy(
3636
return mute.get(streamId)?.get(topic) ?? UserTopicVisibilityPolicy.None;
3737
}
3838

39-
export function isTopicMuted(streamId: number, topic: string, mute: MuteState): boolean {
39+
/**
40+
* Whether this topic should appear when already focusing on its stream.
41+
*
42+
* This is false if the user's visibility policy for the topic is Muted,
43+
* and true if the policy is None.
44+
*/
45+
export function isTopicVisibleInStream(streamId: number, topic: string, mute: MuteState): boolean {
4046
const policy = getTopicVisibilityPolicy(mute, streamId, topic);
4147
switch (policy) {
4248
case UserTopicVisibilityPolicy.None:
43-
return false;
44-
case UserTopicVisibilityPolicy.Muted:
4549
return true;
50+
case UserTopicVisibilityPolicy.Muted:
51+
return false;
4652
}
4753
}
4854

src/topics/topicSelectors.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { getTopics } from '../directSelectors';
66
import { getUnread, getUnreadCountForTopic } from '../unread/unreadModel';
77
import { NULL_ARRAY } from '../nullObjects';
88
import { isStreamNarrow, streamIdOfNarrow } from '../utils/narrow';
9-
import { getMute, isTopicMuted } from '../mute/muteModel';
9+
import { getMute, isTopicVisibleInStream } from '../mute/muteModel';
1010
import { getSubscriptionsById } from '../subscriptions/subscriptionSelectors';
1111

1212
export const getTopicsForNarrow: Selector<$ReadOnlyArray<string>, Narrow> = createSelector(
@@ -42,7 +42,7 @@ export const getTopicsForStream: Selector<?$ReadOnlyArray<TopicExtended>, number
4242
const streamMuted = subscription ? !subscription.in_home_view : true;
4343

4444
return topicList.map(({ name, max_id }): TopicExtended => {
45-
const isMuted = streamMuted || isTopicMuted(streamId, name, mute);
45+
const isMuted = streamMuted || !isTopicVisibleInStream(streamId, name, mute);
4646
const unreadCount = getUnreadCountForTopic(unread, streamId, name);
4747
return { name, max_id, isMuted, unreadCount };
4848
});

src/unread/unreadSelectors.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createSelector } from 'reselect';
44
import type { Narrow, Selector } from '../types';
55
import type { UnreadStreamItem } from './UnreadCards';
66
import { caseInsensitiveCompareFunc } from '../utils/misc';
7-
import { getMute, isTopicMuted } from '../mute/muteModel';
7+
import { getMute, isTopicVisibleInStream } from '../mute/muteModel';
88
import { getOwnUserId } from '../users/userSelectors';
99
import { getSubscriptionsById, getStreamsById } from '../subscriptions/subscriptionSelectors';
1010
import { caseNarrow } from '../utils/narrow';
@@ -27,7 +27,7 @@ export const getUnreadByStream: Selector<{| [number]: number |}> = createSelecto
2727
for (const [streamId, streamData] of unreadStreams.entries()) {
2828
let total = 0;
2929
for (const [topic, msgIds] of streamData) {
30-
if (!isTopicMuted(streamId, topic, mute)) {
30+
if (isTopicVisibleInStream(streamId, topic, mute)) {
3131
total += msgIds.size;
3232
}
3333
}
@@ -167,7 +167,7 @@ export const getUnreadStreamsAndTopics: Selector<$ReadOnlyArray<UnreadStreamItem
167167
let totalUnread = 0;
168168
const topics = [];
169169
for (const [topic, msgIds] of streamData) {
170-
if (isTopicMuted(streamId, topic, mute)) {
170+
if (!isTopicVisibleInStream(streamId, topic, mute)) {
171171
continue;
172172
}
173173
totalUnread += msgIds.size;
@@ -228,7 +228,7 @@ export const getUnreadCountForNarrow: Selector<number, Narrow> = createSelector(
228228
unread.streams
229229
.get(streamId)
230230
?.entrySeq()
231-
.filterNot(([topic, _]) => isTopicMuted(streamId, topic, mute))
231+
.filter(([topic, _]) => isTopicVisibleInStream(streamId, topic, mute))
232232
.map(([_, msgIds]) => msgIds.size)
233233
.reduce((s, x) => s + x, 0) ?? 0,
234234

0 commit comments

Comments
 (0)