Skip to content

Commit

Permalink
wipref api: Add "follow" 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 "follow" 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.
  • Loading branch information
gnprice committed Oct 4, 2023
1 parent aa46752 commit 6a306d6
Show file tree
Hide file tree
Showing 5 changed files with 32 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.Follow]]);
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.Follow:
buttons.push(muteTopic);
break;
}
Expand All @@ -677,6 +678,7 @@ export const constructTopicActionButtons = (args: {|
buttons.push(unmuteTopicInMutedStream);
break;
case UserTopicVisibilityPolicy.Unmuted:
case UserTopicVisibilityPolicy.Follow:
buttons.push(muteTopic);
break;
}
Expand Down
3 changes: 3 additions & 0 deletions src/api/modelTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,9 @@ export enum UserTopicVisibilityPolicy {
None = 0,
Muted = 1,
Unmuted = 2,
// Not in the API docs yet.
// TODO(server): delete this comment once documented
Follow = 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.Follow]]),
UserTopicVisibilityPolicy.Follow,
);
});
});

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.Follow]]), true);
});
});

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

test('stream unmuted, topic-policy Follow', () => {
check(false, UserTopicVisibilityPolicy.Follow, 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 Follow', () => {
check(true, UserTopicVisibilityPolicy.Follow, 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.Follow:
return true;
}
}
Expand Down Expand Up @@ -89,6 +90,7 @@ export function isTopicVisible(
case UserTopicVisibilityPolicy.Muted:
return false;
case UserTopicVisibilityPolicy.Unmuted:
case UserTopicVisibilityPolicy.Follow:
return true;
}
}
Expand Down

0 comments on commit 6a306d6

Please sign in to comment.