From 369ff36ce8550511cdccbb8984b3df292b1f6832 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 19 Sep 2025 17:21:45 -0700 Subject: [PATCH 1/5] channel test [nfc]: s/hasPostingPermission/selfCanSendMessage/ (oops) I meant to do this in da1d968f3, which renamed the method in the code; oops. --- test/model/channel_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart index 00dbfb10a2..9e3265cdda 100644 --- a/test/model/channel_test.dart +++ b/test/model/channel_test.dart @@ -455,7 +455,7 @@ void main() { }); }); - group('hasPostingPermission', () { + group('selfCanSendMessage', () { final testCases = [ (ChannelPostPolicy.unknown, UserRole.unknown, true), (ChannelPostPolicy.unknown, UserRole.guest, true), From 3013d4f48b740689d05fb92358ff036e17ff4dc4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 19 Sep 2025 16:51:12 -0700 Subject: [PATCH 2/5] model: Add ZulipStream.canSendMessageGroup, new in FL 333 --- lib/api/model/events.dart | 1 + lib/api/model/events.g.dart | 1 + lib/api/model/model.dart | 5 +++++ lib/api/model/model.g.dart | 9 +++++++++ lib/model/channel.dart | 2 ++ test/example_data.dart | 10 ++++++++++ 6 files changed, 28 insertions(+) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 94bf662e30..5d86935186 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -680,6 +680,7 @@ class ChannelUpdateEvent extends ChannelEvent { case ChannelPropertyName.canAddSubscribersGroup: case ChannelPropertyName.canDeleteAnyMessageGroup: case ChannelPropertyName.canDeleteOwnMessageGroup: + case ChannelPropertyName.canSendMessageGroup: case ChannelPropertyName.canSubscribeGroup: return GroupSettingValue.fromJson(value); case ChannelPropertyName.streamWeeklyTraffic: diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 398802892a..0313976018 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -465,6 +465,7 @@ const _$ChannelPropertyNameEnumMap = { ChannelPropertyName.canAddSubscribersGroup: 'can_add_subscribers_group', ChannelPropertyName.canDeleteAnyMessageGroup: 'can_delete_any_message_group', ChannelPropertyName.canDeleteOwnMessageGroup: 'can_delete_own_message_group', + ChannelPropertyName.canSendMessageGroup: 'can_send_message_group', ChannelPropertyName.canSubscribeGroup: 'can_subscribe_group', ChannelPropertyName.streamWeeklyTraffic: 'stream_weekly_traffic', }; diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index d0a80c2b4c..674c1cd8a7 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -655,6 +655,7 @@ class ZulipStream { GroupSettingValue? canAddSubscribersGroup; // TODO(server-10) GroupSettingValue? canDeleteAnyMessageGroup; // TODO(server-11) GroupSettingValue? canDeleteOwnMessageGroup; // TODO(server-11) + GroupSettingValue? canSendMessageGroup; // TODO(server-10) GroupSettingValue? canSubscribeGroup; // TODO(server-10) // TODO(server-8): added in FL 199, was previously only on [Subscription] objects @@ -676,6 +677,7 @@ class ZulipStream { required this.canAddSubscribersGroup, required this.canDeleteAnyMessageGroup, required this.canDeleteOwnMessageGroup, + required this.canSendMessageGroup, required this.canSubscribeGroup, required this.streamWeeklyTraffic, }); @@ -698,6 +700,7 @@ class ZulipStream { canAddSubscribersGroup: subscription.canAddSubscribersGroup, canDeleteAnyMessageGroup: subscription.canDeleteAnyMessageGroup, canDeleteOwnMessageGroup: subscription.canDeleteOwnMessageGroup, + canSendMessageGroup: subscription.canSendMessageGroup, canSubscribeGroup: subscription.canSubscribeGroup, streamWeeklyTraffic: subscription.streamWeeklyTraffic, ); @@ -733,6 +736,7 @@ enum ChannelPropertyName { canAddSubscribersGroup, canDeleteAnyMessageGroup, canDeleteOwnMessageGroup, + canSendMessageGroup, canSubscribeGroup, streamWeeklyTraffic; @@ -817,6 +821,7 @@ class Subscription extends ZulipStream { required super.canAddSubscribersGroup, required super.canDeleteAnyMessageGroup, required super.canDeleteOwnMessageGroup, + required super.canSendMessageGroup, required super.canSubscribeGroup, required super.streamWeeklyTraffic, required this.desktopNotifications, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index b21b2ee29e..5581dc565b 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -260,6 +260,9 @@ ZulipStream _$ZulipStreamFromJson(Map json) => ZulipStream( canDeleteOwnMessageGroup: json['can_delete_own_message_group'] == null ? null : GroupSettingValue.fromJson(json['can_delete_own_message_group']), + canSendMessageGroup: json['can_send_message_group'] == null + ? null + : GroupSettingValue.fromJson(json['can_send_message_group']), canSubscribeGroup: json['can_subscribe_group'] == null ? null : GroupSettingValue.fromJson(json['can_subscribe_group']), @@ -283,6 +286,7 @@ Map _$ZulipStreamToJson(ZulipStream instance) => 'can_add_subscribers_group': instance.canAddSubscribersGroup, 'can_delete_any_message_group': instance.canDeleteAnyMessageGroup, 'can_delete_own_message_group': instance.canDeleteOwnMessageGroup, + 'can_send_message_group': instance.canSendMessageGroup, 'can_subscribe_group': instance.canSubscribeGroup, 'stream_weekly_traffic': instance.streamWeeklyTraffic, }; @@ -320,6 +324,9 @@ Subscription _$SubscriptionFromJson(Map json) => Subscription( canDeleteOwnMessageGroup: json['can_delete_own_message_group'] == null ? null : GroupSettingValue.fromJson(json['can_delete_own_message_group']), + canSendMessageGroup: json['can_send_message_group'] == null + ? null + : GroupSettingValue.fromJson(json['can_send_message_group']), canSubscribeGroup: json['can_subscribe_group'] == null ? null : GroupSettingValue.fromJson(json['can_subscribe_group']), @@ -351,6 +358,7 @@ Map _$SubscriptionToJson(Subscription instance) => 'can_add_subscribers_group': instance.canAddSubscribersGroup, 'can_delete_any_message_group': instance.canDeleteAnyMessageGroup, 'can_delete_own_message_group': instance.canDeleteOwnMessageGroup, + 'can_send_message_group': instance.canSendMessageGroup, 'can_subscribe_group': instance.canSubscribeGroup, 'stream_weekly_traffic': instance.streamWeeklyTraffic, 'desktop_notifications': instance.desktopNotifications, @@ -515,6 +523,7 @@ const _$ChannelPropertyNameEnumMap = { ChannelPropertyName.canAddSubscribersGroup: 'can_add_subscribers_group', ChannelPropertyName.canDeleteAnyMessageGroup: 'can_delete_any_message_group', ChannelPropertyName.canDeleteOwnMessageGroup: 'can_delete_own_message_group', + ChannelPropertyName.canSendMessageGroup: 'can_send_message_group', ChannelPropertyName.canSubscribeGroup: 'can_subscribe_group', ChannelPropertyName.streamWeeklyTraffic: 'stream_weekly_traffic', }; diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 0403ba0bdf..890bd81cfe 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -405,6 +405,8 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore { stream.canDeleteAnyMessageGroup = event.value as GroupSettingValue; case ChannelPropertyName.canDeleteOwnMessageGroup: stream.canDeleteOwnMessageGroup = event.value as GroupSettingValue; + case ChannelPropertyName.canSendMessageGroup: + stream.canSendMessageGroup = event.value as GroupSettingValue; case ChannelPropertyName.canSubscribeGroup: stream.canSubscribeGroup = event.value as GroupSettingValue; case ChannelPropertyName.streamWeeklyTraffic: diff --git a/test/example_data.dart b/test/example_data.dart index 9a9eb1099e..9bc82b9d6d 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -461,9 +461,16 @@ ZulipStream stream({ GroupSettingValue? canAddSubscribersGroup, GroupSettingValue? canDeleteAnyMessageGroup, GroupSettingValue? canDeleteOwnMessageGroup, + GroupSettingValue? canSendMessageGroup, GroupSettingValue? canSubscribeGroup, int? streamWeeklyTraffic, }) { + if (channelPostPolicy == null) { + // Set a default for realmCanDeleteOwnMessageGroup, but only if we're + // not trying to test legacy behavior with channelPostPolicy. + canSendMessageGroup ??= groupSetting(members: [selfUser.userId]); + } + _checkPositive(streamId, 'stream ID'); _checkPositive(firstMessageId, 'message ID'); var effectiveStreamId = streamId ?? _nextStreamId(); @@ -485,6 +492,7 @@ ZulipStream stream({ canAddSubscribersGroup: canAddSubscribersGroup ?? GroupSettingValueNamed(nobodyGroup.id), canDeleteAnyMessageGroup: canDeleteAnyMessageGroup ?? GroupSettingValueNamed(nobodyGroup.id), canDeleteOwnMessageGroup: canDeleteOwnMessageGroup ?? GroupSettingValueNamed(nobodyGroup.id), + canSendMessageGroup: canSendMessageGroup, canSubscribeGroup: canSubscribeGroup ?? GroupSettingValueNamed(nobodyGroup.id), streamWeeklyTraffic: streamWeeklyTraffic, ); @@ -528,6 +536,7 @@ Subscription subscription( canAddSubscribersGroup: stream.canAddSubscribersGroup, canDeleteAnyMessageGroup: stream.canDeleteAnyMessageGroup, canDeleteOwnMessageGroup: stream.canDeleteOwnMessageGroup, + canSendMessageGroup: stream.canSendMessageGroup, canSubscribeGroup: stream.canSubscribeGroup, streamWeeklyTraffic: stream.streamWeeklyTraffic, desktopNotifications: desktopNotifications ?? false, @@ -1206,6 +1215,7 @@ ChannelUpdateEvent channelUpdateEvent( case ChannelPropertyName.canAddSubscribersGroup: case ChannelPropertyName.canDeleteAnyMessageGroup: case ChannelPropertyName.canDeleteOwnMessageGroup: + case ChannelPropertyName.canSendMessageGroup: case ChannelPropertyName.canSubscribeGroup: assert(value is GroupSettingValue); case ChannelPropertyName.streamWeeklyTraffic: From d7f74ea43279852e170cde85c943886dcca3d17c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 19 Sep 2025 17:01:10 -0700 Subject: [PATCH 3/5] channel: Implement modern group-based permission check for can-send-message Fixes #1862. --- lib/model/channel.dart | 17 ++++++++++++++++- test/model/channel_test.dart | 29 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 890bd81cfe..904dd3a6db 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -191,6 +191,21 @@ mixin ChannelStore on UserStore { bool selfCanSendMessage({ required ZulipStream inChannel, required DateTime byDate, + }) { + // (selfHasPermissionForGroupSetting isn't equipped to handle the old-server + // fallback logic for this specific permission; it's dynamic and depends on + // channelPostPolicy, so we do our own null check here.) + if (inChannel.canSendMessageGroup != null) { + return selfHasPermissionForGroupSetting(inChannel.canSendMessageGroup!, + GroupSettingType.stream, 'can_send_message_group'); + } else { + return _selfPassesLegacyChannelPostPolicy(inChannel: inChannel, atDate: byDate); + } + } + + bool _selfPassesLegacyChannelPostPolicy({ + required ZulipStream inChannel, + required DateTime atDate, }) { final role = selfUser.role; // We let the users with [unknown] role to send the message, then the server @@ -202,7 +217,7 @@ mixin ChannelStore on UserStore { case ChannelPostPolicy.fullMembers: { if (!role.isAtLeast(UserRole.member)) return false; if (role == UserRole.member) { - return selfHasPassedWaitingPeriod(byDate: byDate); + return selfHasPassedWaitingPeriod(byDate: atDate); } return true; } diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart index 9e3265cdda..adffc1a712 100644 --- a/test/model/channel_test.dart +++ b/test/model/channel_test.dart @@ -9,9 +9,12 @@ import 'package:zulip/model/channel.dart'; import '../api/model/model_checks.dart'; import '../example_data.dart' as eg; import '../stdlib_checks.dart'; +import 'binding.dart'; import 'test_store.dart'; void main() { + TestZulipBinding.ensureInitialized(); + group('Unified stream/sub data', () { /// Check that `streams`, `streamsByName`, and `subscriptions` all agree /// and point to the same objects where applicable. @@ -456,6 +459,32 @@ void main() { }); group('selfCanSendMessage', () { + test('in group', () { + addTearDown(testBinding.reset); + final now = testBinding.utcNow(); + + final canSendMessageGroup = eg.groupSetting(members: [eg.selfUser.userId]); + final channel = eg.stream(canSendMessageGroup: canSendMessageGroup); + final store = eg.store( + initialSnapshot: eg.initialSnapshot(streams: [channel])); + check(store.selfCanSendMessage(inChannel: channel, byDate: now)) + .isTrue(); + }); + + test('not in group', () { + addTearDown(testBinding.reset); + final now = testBinding.utcNow(); + + final canSendMessageGroup = eg.groupSetting(members: []); + final channel = eg.stream(canSendMessageGroup: canSendMessageGroup); + final store = eg.store( + initialSnapshot: eg.initialSnapshot(streams: [channel])); + check(store.selfCanSendMessage(inChannel: channel, byDate: now)) + .isFalse(); + }); + }); + + group('selfCanSendMessage, legacy', () { final testCases = [ (ChannelPostPolicy.unknown, UserRole.unknown, true), (ChannelPostPolicy.unknown, UserRole.guest, true), From 4496ca7d9e56ad7587b39e818b28f6881e51f439 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 19 Sep 2025 17:04:08 -0700 Subject: [PATCH 4/5] channel: Make channelPostPolicy optional, because deprecated --- lib/api/model/model.dart | 2 +- lib/api/model/model.g.dart | 4 ++-- lib/model/channel.dart | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 674c1cd8a7..2bc6d7d487 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -649,7 +649,7 @@ class ZulipStream { bool historyPublicToSubscribers; int? messageRetentionDays; @JsonKey(name: 'stream_post_policy') - ChannelPostPolicy channelPostPolicy; + ChannelPostPolicy? channelPostPolicy; // TODO(server-10) remove // final bool isAnnouncementOnly; // deprecated for `channelPostPolicy`; ignore GroupSettingValue? canAddSubscribersGroup; // TODO(server-10) diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 5581dc565b..70316ae1eb 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -247,7 +247,7 @@ ZulipStream _$ZulipStreamFromJson(Map json) => ZulipStream( isWebPublic: json['is_web_public'] as bool, historyPublicToSubscribers: json['history_public_to_subscribers'] as bool, messageRetentionDays: (json['message_retention_days'] as num?)?.toInt(), - channelPostPolicy: $enumDecode( + channelPostPolicy: $enumDecodeNullable( _$ChannelPostPolicyEnumMap, json['stream_post_policy'], ), @@ -311,7 +311,7 @@ Subscription _$SubscriptionFromJson(Map json) => Subscription( isWebPublic: json['is_web_public'] as bool, historyPublicToSubscribers: json['history_public_to_subscribers'] as bool, messageRetentionDays: (json['message_retention_days'] as num?)?.toInt(), - channelPostPolicy: $enumDecode( + channelPostPolicy: $enumDecodeNullable( _$ChannelPostPolicyEnumMap, json['stream_post_policy'], ), diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 904dd3a6db..31e3eb96f2 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -198,8 +198,11 @@ mixin ChannelStore on UserStore { if (inChannel.canSendMessageGroup != null) { return selfHasPermissionForGroupSetting(inChannel.canSendMessageGroup!, GroupSettingType.stream, 'can_send_message_group'); - } else { + } else if (inChannel.channelPostPolicy != null) { return _selfPassesLegacyChannelPostPolicy(inChannel: inChannel, atDate: byDate); + } else { + assert(false); // TODO(log) + return true; } } @@ -207,12 +210,13 @@ mixin ChannelStore on UserStore { required ZulipStream inChannel, required DateTime atDate, }) { + assert(inChannel.channelPostPolicy != null); final role = selfUser.role; // We let the users with [unknown] role to send the message, then the server // will decide to accept it or not based on its actual role. if (role == UserRole.unknown) return true; - switch (inChannel.channelPostPolicy) { + switch (inChannel.channelPostPolicy!) { case ChannelPostPolicy.any: return true; case ChannelPostPolicy.fullMembers: { if (!role.isAtLeast(UserRole.member)) return false; From de03f4ba8ed4ce191d611a79768d0d84e87c3378 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 19 Sep 2025 17:07:41 -0700 Subject: [PATCH 5/5] channel: Remove an unknown-role check that should be a no-op --- lib/model/channel.dart | 6 +++--- test/model/channel_test.dart | 5 ----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 31e3eb96f2..098f146df1 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -212,9 +212,9 @@ mixin ChannelStore on UserStore { }) { assert(inChannel.channelPostPolicy != null); final role = selfUser.role; - // We let the users with [unknown] role to send the message, then the server - // will decide to accept it or not based on its actual role. - if (role == UserRole.unknown) return true; + + // (Could early-return true on [UserRole.unknown], + // but pre-333 servers shouldn't be giving us an unknown role.) switch (inChannel.channelPostPolicy!) { case ChannelPostPolicy.any: return true; diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart index adffc1a712..e7e57357d6 100644 --- a/test/model/channel_test.dart +++ b/test/model/channel_test.dart @@ -486,32 +486,27 @@ void main() { group('selfCanSendMessage, legacy', () { final testCases = [ - (ChannelPostPolicy.unknown, UserRole.unknown, true), (ChannelPostPolicy.unknown, UserRole.guest, true), (ChannelPostPolicy.unknown, UserRole.member, true), (ChannelPostPolicy.unknown, UserRole.moderator, true), (ChannelPostPolicy.unknown, UserRole.administrator, true), (ChannelPostPolicy.unknown, UserRole.owner, true), - (ChannelPostPolicy.any, UserRole.unknown, true), (ChannelPostPolicy.any, UserRole.guest, true), (ChannelPostPolicy.any, UserRole.member, true), (ChannelPostPolicy.any, UserRole.moderator, true), (ChannelPostPolicy.any, UserRole.administrator, true), (ChannelPostPolicy.any, UserRole.owner, true), - (ChannelPostPolicy.fullMembers, UserRole.unknown, true), (ChannelPostPolicy.fullMembers, UserRole.guest, false), // The fullMembers/member case gets its own tests further below. // (ChannelPostPolicy.fullMembers, UserRole.member, /* complicated */), (ChannelPostPolicy.fullMembers, UserRole.moderator, true), (ChannelPostPolicy.fullMembers, UserRole.administrator, true), (ChannelPostPolicy.fullMembers, UserRole.owner, true), - (ChannelPostPolicy.moderators, UserRole.unknown, true), (ChannelPostPolicy.moderators, UserRole.guest, false), (ChannelPostPolicy.moderators, UserRole.member, false), (ChannelPostPolicy.moderators, UserRole.moderator, true), (ChannelPostPolicy.moderators, UserRole.administrator, true), (ChannelPostPolicy.moderators, UserRole.owner, true), - (ChannelPostPolicy.administrators, UserRole.unknown, true), (ChannelPostPolicy.administrators, UserRole.guest, false), (ChannelPostPolicy.administrators, UserRole.member, false), (ChannelPostPolicy.administrators, UserRole.moderator, false),