Skip to content

Commit

Permalink
api: Add "followed" user-topic state, as synonym of "unmuted"
Browse files Browse the repository at this point in the history
In all the places we currently consult the user's visibility policy
for a topic, treating "followed" as "unmuted" will give it the most
accurate behavior from among what we currently implement.

In particular this makes it so that when the stream is muted, the
topic will correctly be treated as not muted.

There's still more we could do to fully implement this feature:
notably, surface in the UI what the current state is, and give the
option to set a topic to followed.  But this is an important start.

Fixes: zulip#5769
  • Loading branch information
gnprice authored and chrisbobbe committed Oct 5, 2023
1 parent 960ea99 commit 36f4466
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/action-sheets/__tests__/action-sheet-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import { makeUnreadState } from '../../unread/__tests__/unread-testlib';
import { makeMuteState } from '../../mute/__tests__/mute-testlib';
import { Role } from '../../api/permissionsTypes';
import { UserTopicVisibilityPolicy } from '../../api/modelTypes';

const buttonTitles = buttons => buttons.map(button => button.title);

Expand Down Expand Up @@ -92,6 +93,11 @@ describe('constructTopicActionButtons', () => {
expect(titles({ ...eg.plusBackgroundData, mute })).toContain('Mute topic');
});

test('show muteTopic on followed topic', () => {
const mute = makeMuteState([[eg.stream, topic, UserTopicVisibilityPolicy.Followed]]);
expect(titles({ ...eg.plusBackgroundData, mute })).toContain('Mute topic');
});

test('show resolveTopic', () => {
expect(titles({ ...eg.plusBackgroundData })).toContain('Resolve topic');
});
Expand Down
2 changes: 2 additions & 0 deletions src/action-sheets/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,7 @@ export const constructTopicActionButtons = (args: {|
break;
case UserTopicVisibilityPolicy.None:
case UserTopicVisibilityPolicy.Unmuted:
case UserTopicVisibilityPolicy.Followed:
buttons.push(muteTopic);
break;
}
Expand All @@ -677,6 +678,7 @@ export const constructTopicActionButtons = (args: {|
buttons.push(unmuteTopicInMutedStream);
break;
case UserTopicVisibilityPolicy.Unmuted:
case UserTopicVisibilityPolicy.Followed:
buttons.push(muteTopic);
break;
}
Expand Down
4 changes: 4 additions & 0 deletions src/api/modelTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,10 @@ export enum UserTopicVisibilityPolicy {
None = 0,
Muted = 1,
Unmuted = 2,
// Not in the API docs yet. But it is, uh, in the server implementation:
// https://github.com/zulip/zulip/blob/2ec8273c6/zerver/models.py#L2794-L2797
// TODO(server): delete this comment once documented
Followed = 3,
}

/**
Expand Down
19 changes: 19 additions & 0 deletions src/mute/__tests__/muteModel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ describe('getters', () => {
UserTopicVisibilityPolicy.Unmuted,
);
});

test('with topic followed', () => {
check(
makeMuteState([[eg.stream, 'topic', UserTopicVisibilityPolicy.Followed]]),
UserTopicVisibilityPolicy.Followed,
);
});
});

describe('isTopicVisibleInStream', () => {
Expand All @@ -67,6 +74,10 @@ describe('getters', () => {
test('with topic unmuted', () => {
check(makeMuteState([[eg.stream, 'topic', UserTopicVisibilityPolicy.Unmuted]]), true);
});

test('with topic followed', () => {
check(makeMuteState([[eg.stream, 'topic', UserTopicVisibilityPolicy.Followed]]), true);
});
});

describe('isTopicVisible', () => {
Expand All @@ -90,6 +101,10 @@ describe('getters', () => {
check(false, UserTopicVisibilityPolicy.Unmuted, true);
});

test('stream unmuted, topic-policy Followed', () => {
check(false, UserTopicVisibilityPolicy.Followed, true);
});

test('stream muted, topic-policy None', () => {
check(true, UserTopicVisibilityPolicy.None, false);
});
Expand All @@ -101,6 +116,10 @@ describe('getters', () => {
test('stream muted, topic-policy Unmuted', () => {
check(true, UserTopicVisibilityPolicy.Unmuted, true);
});

test('stream muted, topic-policy Followed', () => {
check(true, UserTopicVisibilityPolicy.Followed, true);
});
});
});

Expand Down
2 changes: 2 additions & 0 deletions src/mute/muteModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export function isTopicVisibleInStream(streamId: number, topic: string, mute: Mu
case UserTopicVisibilityPolicy.Muted:
return false;
case UserTopicVisibilityPolicy.Unmuted:
case UserTopicVisibilityPolicy.Followed:
return true;
}
}
Expand Down Expand Up @@ -89,6 +90,7 @@ export function isTopicVisible(
case UserTopicVisibilityPolicy.Muted:
return false;
case UserTopicVisibilityPolicy.Unmuted:
case UserTopicVisibilityPolicy.Followed:
return true;
}
}
Expand Down

0 comments on commit 36f4466

Please sign in to comment.