Skip to content

Commit 3fc2562

Browse files
committed
Remove unknown from UserTopicVisibilityPolicy
1 parent bbc7ce1 commit 3fc2562

File tree

15 files changed

+117
-116
lines changed

15 files changed

+117
-116
lines changed

lib/api/model/events.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,7 @@ class UserTopicEvent extends Event {
10511051
final int streamId;
10521052
final TopicName topicName;
10531053
final int lastUpdated;
1054-
final UserTopicVisibilityPolicy visibilityPolicy;
1054+
final UserTopicVisibilityPolicy? visibilityPolicy;
10551055

10561056
UserTopicEvent({
10571057
required super.id,

lib/api/model/events.g.dart

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/api/model/initial_snapshot.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,7 @@ class UserTopicItem {
370370
final int streamId;
371371
final TopicName topicName;
372372
final int lastUpdated;
373-
@JsonKey(unknownEnumValue: UserTopicVisibilityPolicy.unknown)
374-
final UserTopicVisibilityPolicy visibilityPolicy;
373+
final UserTopicVisibilityPolicy? visibilityPolicy;
375374

376375
UserTopicItem({
377376
required this.streamId,

lib/api/model/initial_snapshot.g.dart

Lines changed: 1 addition & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/api/model/model.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -899,8 +899,8 @@ enum UserTopicVisibilityPolicy {
899899
none(apiValue: 0),
900900
muted(apiValue: 1),
901901
unmuted(apiValue: 2), // TODO(server-7) newly added
902-
followed(apiValue: 3), // TODO(server-8) newly added
903-
unknown(apiValue: null); // TODO(#1074) remove this
902+
followed(apiValue: 3); // TODO(server-8) newly added
903+
//Removed unknown (#1074)
904904

905905
const UserTopicVisibilityPolicy({required this.apiValue});
906906

lib/api/route/channels.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ Future<void> updateUserTopic(ApiConnection connection, {
119119
required TopicName topic,
120120
required UserTopicVisibilityPolicy visibilityPolicy,
121121
}) {
122-
assert(visibilityPolicy != UserTopicVisibilityPolicy.unknown);
123122
assert(connection.zulipFeatureLevel! >= 170);
124123
return connection.post('updateUserTopic', (_) {}, 'user_topics', {
125124
'stream_id': streamId,

lib/model/channel.dart

Lines changed: 98 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ mixin ChannelStore on UserStore {
1919
@protected
2020
UserStore get userStore;
2121

22+
2223
/// All known channels/streams, indexed by [ZulipStream.streamId].
2324
///
2425
/// The same [ZulipStream] objects also appear in [streamsByName].
@@ -45,6 +46,16 @@ mixin ChannelStore on UserStore {
4546
/// All the channel folders, including archived ones, indexed by ID.
4647
Map<int, ChannelFolder> get channelFolders;
4748

49+
static bool _warnInvalidVisibilityPolicy(
50+
UserTopicVisibilityPolicy? visibilityPolicy
51+
) {
52+
if (visibilityPolicy == null) {
53+
// Not a value we expect. Keep it out of our data structures. // TODO(log)
54+
return true;
55+
}
56+
return false;
57+
}
58+
4859
static int compareChannelsByName(ZulipStream a, ZulipStream b) {
4960
// A user gave feedback wanting zulip-flutter to match web in putting
5061
// emoji-prefixed channels first; see #1202.
@@ -118,11 +129,22 @@ mixin ChannelStore on UserStore {
118129
UserTopicVisibilityEffect willChangeIfTopicVisibleInStream(UserTopicEvent event) {
119130
final streamId = event.streamId;
120131
final topic = event.topicName;
132+
133+
final UserTopicVisibilityPolicy? visibilityPolicy = event.visibilityPolicy;
134+
if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
135+
return UserTopicVisibilityEffect.none;
136+
}
137+
138+
final UserTopicVisibilityPolicy policy = visibilityPolicy!;
139+
121140
return UserTopicVisibilityEffect._fromBeforeAfter(
122141
_isTopicVisibleInStream(topicVisibilityPolicy(streamId, topic)),
123-
_isTopicVisibleInStream(event.visibilityPolicy));
142+
_isTopicVisibleInStream(policy),
143+
);
124144
}
125145

146+
147+
126148
static bool _isTopicVisibleInStream(UserTopicVisibilityPolicy policy) {
127149
switch (policy) {
128150
case UserTopicVisibilityPolicy.none:
@@ -132,9 +154,6 @@ mixin ChannelStore on UserStore {
132154
case UserTopicVisibilityPolicy.unmuted:
133155
case UserTopicVisibilityPolicy.followed:
134156
return true;
135-
case UserTopicVisibilityPolicy.unknown:
136-
assert(false);
137-
return true;
138157
}
139158
}
140159

@@ -155,11 +174,23 @@ mixin ChannelStore on UserStore {
155174
UserTopicVisibilityEffect willChangeIfTopicVisible(UserTopicEvent event) {
156175
final streamId = event.streamId;
157176
final topic = event.topicName;
177+
178+
final UserTopicVisibilityPolicy? visibilityPolicy = event.visibilityPolicy;
179+
if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
180+
return UserTopicVisibilityEffect.none;
181+
}
182+
183+
final UserTopicVisibilityPolicy policy = visibilityPolicy!;
184+
158185
return UserTopicVisibilityEffect._fromBeforeAfter(
159186
_isTopicVisible(streamId, topicVisibilityPolicy(streamId, topic)),
160-
_isTopicVisible(streamId, event.visibilityPolicy));
187+
_isTopicVisible(streamId, policy),
188+
);
161189
}
162190

191+
192+
193+
163194
bool _isTopicVisible(int streamId, UserTopicVisibilityPolicy policy) {
164195
switch (policy) {
165196
case UserTopicVisibilityPolicy.none:
@@ -173,9 +204,6 @@ mixin ChannelStore on UserStore {
173204
case UserTopicVisibilityPolicy.unmuted:
174205
case UserTopicVisibilityPolicy.followed:
175206
return true;
176-
case UserTopicVisibilityPolicy.unknown:
177-
assert(false);
178-
return true;
179207
}
180208
}
181209

@@ -320,26 +348,32 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
320348
required InitialSnapshot initialSnapshot,
321349
}) {
322350
final subscriptions = Map.fromEntries(initialSnapshot.subscriptions.map(
323-
(subscription) => MapEntry(subscription.streamId, subscription)));
351+
(subscription) => MapEntry(subscription.streamId, subscription)));
324352

325353
final streams = Map<int, ZulipStream>.of(subscriptions);
326354
for (final stream in initialSnapshot.streams) {
327355
streams.putIfAbsent(stream.streamId, () => stream);
328356
}
329357

330-
final channelFolders = Map.fromEntries((initialSnapshot.channelFolders ?? [])
331-
.map((channelFolder) => MapEntry(channelFolder.id, channelFolder)));
358+
final channelFolders = Map.fromEntries(
359+
(initialSnapshot.channelFolders ?? [])
360+
.map((channelFolder) => MapEntry(channelFolder.id, channelFolder)));
332361

333362
final topicVisibility = <int, TopicKeyedMap<UserTopicVisibilityPolicy>>{};
334363
for (final item in initialSnapshot.userTopics) {
335-
if (_warnInvalidVisibilityPolicy(item.visibilityPolicy)) {
336-
// Not a value we expect. Keep it out of our data structures. // TODO(log)
364+
final UserTopicVisibilityPolicy? visibilityPolicy = item.visibilityPolicy;
365+
if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
337366
continue;
338367
}
339-
final forStream = topicVisibility.putIfAbsent(item.streamId, () => makeTopicKeyedMap());
340-
forStream[item.topicName] = item.visibilityPolicy;
368+
369+
final UserTopicVisibilityPolicy policy = visibilityPolicy!;
370+
371+
final forStream =
372+
topicVisibility.putIfAbsent(item.streamId, () => makeTopicKeyedMap());
373+
forStream[item.topicName] = policy;
341374
}
342375

376+
343377
return ChannelStoreImpl._(
344378
users: users,
345379
streams: streams,
@@ -369,33 +403,30 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
369403
final Map<int, ChannelFolder> channelFolders;
370404

371405
@override
372-
Map<int, TopicKeyedMap<UserTopicVisibilityPolicy>> get debugTopicVisibility => topicVisibility;
406+
Map<int, TopicKeyedMap<UserTopicVisibilityPolicy>> get debugTopicVisibility =>
407+
topicVisibility;
373408

374409
final Map<int, TopicKeyedMap<UserTopicVisibilityPolicy>> topicVisibility;
375410

376411
@override
377-
UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic) {
412+
UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId,
413+
TopicName topic) {
378414
return topicVisibility[streamId]?[topic] ?? UserTopicVisibilityPolicy.none;
379415
}
380416

381-
static bool _warnInvalidVisibilityPolicy(UserTopicVisibilityPolicy visibilityPolicy) {
382-
if (visibilityPolicy == UserTopicVisibilityPolicy.unknown) {
383-
// Not a value we expect. Keep it out of our data structures. // TODO(log)
384-
return true;
385-
}
386-
return false;
387-
}
388417

389418
void handleChannelEvent(ChannelEvent event) {
390419
switch (event) {
391420
case ChannelCreateEvent():
392421
assert(event.streams.every((stream) =>
393-
!streams.containsKey(stream.streamId)
394-
&& !streamsByName.containsKey(stream.name)));
395-
streams.addEntries(event.streams.map((stream) => MapEntry(stream.streamId, stream)));
396-
streamsByName.addEntries(event.streams.map((stream) => MapEntry(stream.name, stream)));
397-
// (Don't touch `subscriptions`. If the user is subscribed to the stream,
398-
// details will come in a later `subscription` event.)
422+
!streams.containsKey(stream.streamId)
423+
&& !streamsByName.containsKey(stream.name)));
424+
streams.addEntries(
425+
event.streams.map((stream) => MapEntry(stream.streamId, stream)));
426+
streamsByName.addEntries(
427+
event.streams.map((stream) => MapEntry(stream.name, stream)));
428+
// (Don't touch `subscriptions`. If the user is subscribed to the stream,
429+
// details will come in a later `subscription` event.)
399430

400431
case ChannelDeleteEvent():
401432
for (final channelId in event.channelIds) {
@@ -404,7 +435,7 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
404435
assert(channelId == channel.streamId);
405436
assert(identical(channel, streamsByName[channel.name]));
406437
assert(subscriptions[channelId] == null
407-
|| identical(subscriptions[channelId], channel));
438+
|| identical(subscriptions[channelId], channel));
408439
streamsByName.remove(channel.name);
409440
subscriptions.remove(channelId);
410441
}
@@ -432,7 +463,8 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
432463
case ChannelPropertyName.name:
433464
final streamName = stream.name;
434465
assert(streamName == event.name);
435-
assert(identical(streams[stream.streamId], streamsByName[streamName]));
466+
assert(identical(
467+
streams[stream.streamId], streamsByName[streamName]));
436468
stream.name = event.value as String;
437469
streamsByName.remove(streamName);
438470
streamsByName[stream.name] = stream;
@@ -503,30 +535,30 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
503535
assert(identical(streamsByName[subscription.name], subscription));
504536
switch (event.property) {
505537
case SubscriptionProperty.color:
506-
subscription.color = event.value as int;
538+
subscription.color = event.value as int;
507539
case SubscriptionProperty.isMuted:
508-
// TODO(#1255) update [MessageListView] if affected
509-
subscription.isMuted = event.value as bool;
540+
// TODO(#1255) update [MessageListView] if affected
541+
subscription.isMuted = event.value as bool;
510542
case SubscriptionProperty.pinToTop:
511-
subscription.pinToTop = event.value as bool;
543+
subscription.pinToTop = event.value as bool;
512544
case SubscriptionProperty.desktopNotifications:
513-
subscription.desktopNotifications = event.value as bool;
545+
subscription.desktopNotifications = event.value as bool;
514546
case SubscriptionProperty.audibleNotifications:
515-
subscription.audibleNotifications = event.value as bool;
547+
subscription.audibleNotifications = event.value as bool;
516548
case SubscriptionProperty.pushNotifications:
517-
subscription.pushNotifications = event.value as bool;
549+
subscription.pushNotifications = event.value as bool;
518550
case SubscriptionProperty.emailNotifications:
519-
subscription.emailNotifications = event.value as bool;
551+
subscription.emailNotifications = event.value as bool;
520552
case SubscriptionProperty.wildcardMentionsNotify:
521553
subscription.wildcardMentionsNotify = event.value as bool;
522554
case SubscriptionProperty.unknown:
523-
// unrecognized property; do nothing
555+
// unrecognized property; do nothing
524556
return;
525557
}
526558

527559
case SubscriptionPeerAddEvent():
528560
case SubscriptionPeerRemoveEvent():
529-
// We don't currently store the data these would update; that's #374.
561+
// We don't currently store the data these would update; that's #374.
530562
}
531563
}
532564

@@ -541,10 +573,13 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
541573
final channelFolder = channelFolders[event.channelFolderId];
542574
if (channelFolder == null) return; // TODO(log)
543575

544-
if (change.name != null) channelFolder.name = change.name!;
545-
if (change.description != null) channelFolder.description = change.description!;
546-
if (change.renderedDescription != null) channelFolder.renderedDescription = change.renderedDescription!;
547-
if (change.isArchived != null) channelFolder.isArchived = change.isArchived!;
576+
if (change.name != null) channelFolder.name = change.name!;
577+
if (change.description != null)
578+
channelFolder.description = change.description!;
579+
if (change.renderedDescription != null)
580+
channelFolder.renderedDescription = change.renderedDescription!;
581+
if (change.isArchived != null)
582+
channelFolder.isArchived = change.isArchived!;
548583

549584
case ChannelFolderReorderEvent():
550585
final order = event.order;
@@ -558,27 +593,35 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
558593
}
559594

560595
void handleUserTopicEvent(UserTopicEvent event) {
561-
UserTopicVisibilityPolicy visibilityPolicy = event.visibilityPolicy;
562-
if (_warnInvalidVisibilityPolicy(visibilityPolicy)) {
563-
visibilityPolicy = UserTopicVisibilityPolicy.none;
596+
final UserTopicVisibilityPolicy? visibilityPolicy = event.visibilityPolicy;
597+
if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
598+
final forStream = topicVisibility[event.streamId];
599+
if (forStream == null) return;
600+
forStream.remove(event.topicName);
601+
if (forStream.isEmpty) {
602+
topicVisibility.remove(event.streamId);
603+
}
604+
return;
564605
}
565-
if (visibilityPolicy == UserTopicVisibilityPolicy.none) {
566-
// This is the "zero value" for this type, which our data structure
567-
// represents by leaving the topic out entirely.
606+
607+
final UserTopicVisibilityPolicy policy = visibilityPolicy!;
608+
609+
if (policy == UserTopicVisibilityPolicy.none) {
568610
final forStream = topicVisibility[event.streamId];
569611
if (forStream == null) return;
570612
forStream.remove(event.topicName);
571613
if (forStream.isEmpty) {
572614
topicVisibility.remove(event.streamId);
573615
}
574616
} else {
575-
final forStream = topicVisibility.putIfAbsent(event.streamId, () => makeTopicKeyedMap());
576-
forStream[event.topicName] = visibilityPolicy;
617+
final forStream =
618+
topicVisibility.putIfAbsent(event.streamId, () => makeTopicKeyedMap());
619+
forStream[event.topicName] = policy;
577620
}
578621
}
579622
}
580623

581-
/// A [Map] with [TopicName] keys and [V] values.
624+
/// A [Map] with [TopicName] keys and [V] values.
582625
///
583626
/// When one of these is created by [makeTopicKeyedMap],
584627
/// key equality is done case-insensitively; see there.

0 commit comments

Comments
 (0)