Skip to content

Commit f014ec3

Browse files
gnpricegnprice
committed
stream: Track user-topic data; add getters
These getters are borrowed from muteModel.js in zulip-mobile.
1 parent 03cefca commit f014ec3

File tree

4 files changed

+323
-7
lines changed

4 files changed

+323
-7
lines changed

‎lib/model/store.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,15 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
195195
Map<String, ZulipStream> get streamsByName => _streams.streamsByName;
196196
@override
197197
Map<int, Subscription> get subscriptions => _streams.subscriptions;
198+
@override
199+
UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, String topic) =>
200+
_streams.topicVisibilityPolicy(streamId, topic);
201+
198202
final StreamStoreImpl _streams;
199203

204+
@visibleForTesting
205+
StreamStoreImpl get debugStreamStore => _streams;
206+
200207
// TODO lots more data. When adding, be sure to update handleEvent too.
201208

202209
// TODO call [RecentDmConversationsView.dispose] in [dispose]
@@ -304,6 +311,10 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
304311
assert(debugLog("server event: subscription/${event.op}"));
305312
_streams.handleSubscriptionEvent(event);
306313
notifyListeners();
314+
} else if (event is UserTopicEvent) {
315+
assert(debugLog("server event: user_topic"));
316+
_streams.handleUserTopicEvent(event);
317+
notifyListeners();
307318
} else if (event is MessageEvent) {
308319
assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}"));
309320
recentDmConversationsView.handleMessageEvent(event);

‎lib/model/stream.dart

Lines changed: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,59 @@ mixin StreamStore {
1212
Map<int, ZulipStream> get streams;
1313
Map<String, ZulipStream> get streamsByName;
1414
Map<int, Subscription> get subscriptions;
15+
UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, String topic);
1516

16-
// (This mixin will make a useful home for nontrivial getter implementations.)
17+
/// Whether this topic should appear when already focusing on its stream.
18+
///
19+
/// This is determined purely by the user's visibility policy for the topic.
20+
///
21+
/// This function is appropriate for muting calculations in UI contexts that
22+
/// are already specific to a stream: for example the stream's unread count,
23+
/// or the message list in the stream's narrow.
24+
///
25+
/// For UI contexts that are not specific to a particular stream, see
26+
/// [isTopicVisible].
27+
bool isTopicVisibleInStream(int streamId, String topic) {
28+
switch (topicVisibilityPolicy(streamId, topic)) {
29+
case UserTopicVisibilityPolicy.none:
30+
return true;
31+
case UserTopicVisibilityPolicy.muted:
32+
return false;
33+
case UserTopicVisibilityPolicy.unmuted:
34+
case UserTopicVisibilityPolicy.followed:
35+
return true;
36+
case UserTopicVisibilityPolicy.unknown:
37+
assert(false);
38+
return true;
39+
}
40+
}
41+
42+
/// Whether this topic should appear when not specifically focusing
43+
/// on this stream.
44+
///
45+
/// This takes into account the user's visibility policy for the stream
46+
/// overall, as well as their policy for this topic.
47+
///
48+
/// For UI contexts that are specific to a particular stream, see
49+
/// [isTopicVisibleInStream].
50+
bool isTopicVisible(int streamId, String topic) {
51+
switch (topicVisibilityPolicy(streamId, topic)) {
52+
case UserTopicVisibilityPolicy.none:
53+
switch (subscriptions[streamId]?.isMuted) {
54+
case false: return true;
55+
case true: return false;
56+
case null: return false; // not subscribed; treat like muted
57+
}
58+
case UserTopicVisibilityPolicy.muted:
59+
return false;
60+
case UserTopicVisibilityPolicy.unmuted:
61+
case UserTopicVisibilityPolicy.followed:
62+
return true;
63+
case UserTopicVisibilityPolicy.unknown:
64+
assert(false);
65+
return true;
66+
}
67+
}
1768
}
1869

1970
/// The implementation of [StreamStore] that does the work.
@@ -31,15 +82,30 @@ class StreamStoreImpl with StreamStore {
3182
streams.putIfAbsent(stream.streamId, () => stream);
3283
}
3384

34-
final streamsByName = streams.map(
35-
(_, stream) => MapEntry(stream.name, stream));
85+
final topicVisibility = <int, Map<String, UserTopicVisibilityPolicy>>{};
86+
for (final item in initialSnapshot.userTopics ?? const <UserTopicItem>[]) {
87+
if (_warnInvalidVisibilityPolicy(item.visibilityPolicy)) {
88+
// Not a value we expect. Keep it out of our data structures. // TODO(log)
89+
continue;
90+
}
91+
final forStream = topicVisibility.putIfAbsent(item.streamId, () => {});
92+
forStream[item.topicName] = item.visibilityPolicy;
93+
}
3694

37-
return StreamStoreImpl._(streams: streams, streamsByName: streamsByName,
38-
subscriptions: subscriptions);
95+
return StreamStoreImpl._(
96+
streams: streams,
97+
streamsByName: streams.map((_, stream) => MapEntry(stream.name, stream)),
98+
subscriptions: subscriptions,
99+
topicVisibility: topicVisibility,
100+
);
39101
}
40102

41-
StreamStoreImpl._({required this.streams, required this.streamsByName,
42-
required this.subscriptions});
103+
StreamStoreImpl._({
104+
required this.streams,
105+
required this.streamsByName,
106+
required this.subscriptions,
107+
required this.topicVisibility,
108+
});
43109

44110
@override
45111
final Map<int, ZulipStream> streams;
@@ -48,6 +114,21 @@ class StreamStoreImpl with StreamStore {
48114
@override
49115
final Map<int, Subscription> subscriptions;
50116

117+
final Map<int, Map<String, UserTopicVisibilityPolicy>> topicVisibility;
118+
119+
@override
120+
UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, String topic) {
121+
return topicVisibility[streamId]?[topic] ?? UserTopicVisibilityPolicy.none;
122+
}
123+
124+
static bool _warnInvalidVisibilityPolicy(UserTopicVisibilityPolicy visibilityPolicy) {
125+
if (visibilityPolicy == UserTopicVisibilityPolicy.unknown) {
126+
// Not a value we expect. Keep it out of our data structures. // TODO(log)
127+
return true;
128+
}
129+
return false;
130+
}
131+
51132
void handleStreamEvent(StreamEvent event) {
52133
switch (event) {
53134
case StreamCreateEvent():
@@ -124,4 +205,24 @@ class StreamStoreImpl with StreamStore {
124205
// We don't currently store the data these would update; that's #374.
125206
}
126207
}
208+
209+
void handleUserTopicEvent(UserTopicEvent event) {
210+
UserTopicVisibilityPolicy visibilityPolicy = event.visibilityPolicy;
211+
if (_warnInvalidVisibilityPolicy(visibilityPolicy)) {
212+
visibilityPolicy = UserTopicVisibilityPolicy.none;
213+
}
214+
if (visibilityPolicy == UserTopicVisibilityPolicy.none) {
215+
// This is the "zero value" for this type, which our data structure
216+
// represents by leaving the topic out entirely.
217+
final forStream = topicVisibility[event.streamId];
218+
if (forStream == null) return;
219+
forStream.remove(event.topicName);
220+
if (forStream.isEmpty) {
221+
topicVisibility.remove(event.streamId);
222+
}
223+
} else {
224+
final forStream = topicVisibility.putIfAbsent(event.streamId, () => {});
225+
forStream[event.topicName] = visibilityPolicy;
226+
}
227+
}
127228
}

‎test/model/stream_test.dart

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
import 'package:checks/checks.dart';
33
import 'package:test/scaffolding.dart';
44
import 'package:zulip/api/model/events.dart';
5+
import 'package:zulip/api/model/initial_snapshot.dart';
56
import 'package:zulip/api/model/model.dart';
7+
import 'package:zulip/model/store.dart';
68
import 'package:zulip/model/stream.dart';
79

810
import '../example_data.dart' as eg;
@@ -96,4 +98,196 @@ void main() {
9698
check(store.subscriptions[stream.streamId]!.isMuted).isTrue();
9799
});
98100
});
101+
102+
group('topic visibility', () {
103+
final stream1 = eg.stream(streamId: 1, name: 'stream 1');
104+
final stream2 = eg.stream(streamId: 2, name: 'stream 2');
105+
106+
group('getter topicVisibilityPolicy', () {
107+
test('with nothing for stream', () {
108+
final store = eg.store();
109+
check(store.topicVisibilityPolicy(stream1.streamId, 'topic'))
110+
.equals(UserTopicVisibilityPolicy.none);
111+
});
112+
113+
test('with nothing for topic', () {
114+
final store = eg.store()
115+
..addUserTopic(stream1, 'other topic', UserTopicVisibilityPolicy.muted);
116+
check(store.topicVisibilityPolicy(stream1.streamId, 'topic'))
117+
.equals(UserTopicVisibilityPolicy.none);
118+
});
119+
120+
test('with topic present', () {
121+
final store = eg.store();
122+
for (final policy in [
123+
UserTopicVisibilityPolicy.muted,
124+
UserTopicVisibilityPolicy.unmuted,
125+
UserTopicVisibilityPolicy.followed,
126+
]) {
127+
store.addUserTopic(stream1, 'topic', policy);
128+
check(store.topicVisibilityPolicy(stream1.streamId, 'topic'))
129+
.equals(policy);
130+
}
131+
});
132+
});
133+
134+
group('isTopicVisible/InStream', () {
135+
test('with policy none, stream not muted', () {
136+
final store = eg.store()..addStream(stream1)
137+
..addSubscription(eg.subscription(stream1));
138+
check(store.isTopicVisibleInStream(stream1.streamId, 'topic')).isTrue();
139+
check(store.isTopicVisible (stream1.streamId, 'topic')).isTrue();
140+
});
141+
142+
test('with policy none, stream muted', () {
143+
final store = eg.store()..addStream(stream1)
144+
..addSubscription(eg.subscription(stream1, isMuted: true));
145+
check(store.isTopicVisibleInStream(stream1.streamId, 'topic')).isTrue();
146+
check(store.isTopicVisible (stream1.streamId, 'topic')).isFalse();
147+
});
148+
149+
test('with policy none, stream unsubscribed', () {
150+
final store = eg.store()..addStream(stream1);
151+
check(store.isTopicVisibleInStream(stream1.streamId, 'topic')).isTrue();
152+
check(store.isTopicVisible (stream1.streamId, 'topic')).isFalse();
153+
});
154+
155+
test('with policy muted', () {
156+
final store = eg.store()..addStream(stream1)
157+
..addSubscription(eg.subscription(stream1))
158+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
159+
check(store.isTopicVisibleInStream(stream1.streamId, 'topic')).isFalse();
160+
check(store.isTopicVisible (stream1.streamId, 'topic')).isFalse();
161+
});
162+
163+
test('with policy unmuted', () {
164+
final store = eg.store()..addStream(stream1)
165+
..addSubscription(eg.subscription(stream1, isMuted: true))
166+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.unmuted);
167+
check(store.isTopicVisibleInStream(stream1.streamId, 'topic')).isTrue();
168+
check(store.isTopicVisible (stream1.streamId, 'topic')).isTrue();
169+
});
170+
171+
test('with policy followed', () {
172+
final store = eg.store()..addStream(stream1)
173+
..addSubscription(eg.subscription(stream1, isMuted: true))
174+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.followed);
175+
check(store.isTopicVisibleInStream(stream1.streamId, 'topic')).isTrue();
176+
check(store.isTopicVisible (stream1.streamId, 'topic')).isTrue();
177+
});
178+
});
179+
180+
UserTopicItem makeUserTopicItem(
181+
ZulipStream stream, String topic, UserTopicVisibilityPolicy policy) {
182+
return UserTopicItem(
183+
streamId: stream.streamId,
184+
topicName: topic,
185+
lastUpdated: 1234567890,
186+
visibilityPolicy: policy,
187+
);
188+
}
189+
190+
void compareTopicVisibility(PerAccountStore store, List<UserTopicItem> expected) {
191+
final expectedStore = eg.store(initialSnapshot: eg.initialSnapshot(
192+
userTopics: expected,
193+
));
194+
check(store.debugStreamStore.topicVisibility)
195+
.deepEquals(expectedStore.debugStreamStore.topicVisibility);
196+
}
197+
198+
test('data structure', () {
199+
final store = eg.store(initialSnapshot: eg.initialSnapshot(
200+
streams: [stream1, stream2],
201+
userTopics: [
202+
makeUserTopicItem(stream1, 'topic 1', UserTopicVisibilityPolicy.muted),
203+
makeUserTopicItem(stream1, 'topic 2', UserTopicVisibilityPolicy.unmuted),
204+
makeUserTopicItem(stream2, 'topic 3', UserTopicVisibilityPolicy.unknown),
205+
makeUserTopicItem(stream2, 'topic 4', UserTopicVisibilityPolicy.followed),
206+
]));
207+
check(store.debugStreamStore.topicVisibility).deepEquals({
208+
stream1.streamId: {
209+
'topic 1': UserTopicVisibilityPolicy.muted,
210+
'topic 2': UserTopicVisibilityPolicy.unmuted,
211+
},
212+
stream2.streamId: {
213+
// 'topic 3' -> unknown treated as no policy
214+
'topic 4': UserTopicVisibilityPolicy.followed,
215+
}
216+
});
217+
});
218+
219+
group('events', () {
220+
test('add with new stream', () {
221+
final store = eg.store()
222+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
223+
compareTopicVisibility(store, [
224+
makeUserTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.muted),
225+
]);
226+
});
227+
228+
test('add in existing stream', () {
229+
final store = eg.store()
230+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted)
231+
..addUserTopic(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted);
232+
compareTopicVisibility(store, [
233+
makeUserTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.muted),
234+
makeUserTopicItem(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted),
235+
]);
236+
});
237+
238+
test('update existing policy', () {
239+
final store = eg.store()
240+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted)
241+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.unmuted);
242+
compareTopicVisibility(store, [
243+
makeUserTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.unmuted),
244+
]);
245+
});
246+
247+
test('remove, with others in stream', () {
248+
final store = eg.store()
249+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted)
250+
..addUserTopic(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted)
251+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.none);
252+
compareTopicVisibility(store, [
253+
makeUserTopicItem(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted),
254+
]);
255+
});
256+
257+
test('remove, as last in stream', () {
258+
final store = eg.store()
259+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted)
260+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.none);
261+
compareTopicVisibility(store, [
262+
]);
263+
});
264+
265+
test('treat unknown enum value as removing', () {
266+
final store = eg.store()
267+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted)
268+
..addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.unknown);
269+
compareTopicVisibility(store, [
270+
]);
271+
});
272+
});
273+
274+
test('smoke', () {
275+
final stream = eg.stream();
276+
final store = eg.store(initialSnapshot: eg.initialSnapshot(
277+
streams: [stream],
278+
userTopics: [
279+
makeUserTopicItem(stream, 'topic 1', UserTopicVisibilityPolicy.muted),
280+
makeUserTopicItem(stream, 'topic 2', UserTopicVisibilityPolicy.unmuted),
281+
makeUserTopicItem(stream, 'topic 3', UserTopicVisibilityPolicy.followed),
282+
]));
283+
check(store.topicVisibilityPolicy(stream.streamId, 'topic 1'))
284+
.equals(UserTopicVisibilityPolicy.muted);
285+
check(store.topicVisibilityPolicy(stream.streamId, 'topic 2'))
286+
.equals(UserTopicVisibilityPolicy.unmuted);
287+
check(store.topicVisibilityPolicy(stream.streamId, 'topic 3'))
288+
.equals(UserTopicVisibilityPolicy.followed);
289+
check(store.topicVisibilityPolicy(stream.streamId, 'topic 4'))
290+
.equals(UserTopicVisibilityPolicy.none);
291+
});
292+
});
99293
}

‎test/model/test_store.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,14 @@ extension PerAccountStoreTestExtension on PerAccountStore {
8484
void addSubscriptions(List<Subscription> subscriptions) {
8585
handleEvent(SubscriptionAddEvent(id: 1, subscriptions: subscriptions));
8686
}
87+
88+
void addUserTopic(ZulipStream stream, String topic, UserTopicVisibilityPolicy visibilityPolicy) {
89+
handleEvent(UserTopicEvent(
90+
id: 1,
91+
streamId: stream.streamId,
92+
topicName: topic,
93+
lastUpdated: 1234567890,
94+
visibilityPolicy: visibilityPolicy,
95+
));
96+
}
8797
}

0 commit comments

Comments
 (0)