From 433359844586545bbc6fd943ebf932474b6cc35c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 15:07:02 -0700 Subject: [PATCH 01/14] api: Add user-group memberships, and their events --- lib/api/model/events.dart | 77 ++++++++++++++++++++++++++++++++++- lib/api/model/events.g.dart | 80 +++++++++++++++++++++++++++++++++++++ lib/api/model/model.dart | 6 ++- lib/api/model/model.g.dart | 8 ++++ lib/model/user_group.dart | 6 +++ test/example_data.dart | 4 ++ 6 files changed, 178 insertions(+), 3 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index e472f151b7..65c1cd794f 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -34,7 +34,10 @@ sealed class Event { switch (json['op'] as String) { case 'add': return UserGroupAddEvent.fromJson(json); case 'update': return UserGroupUpdateEvent.fromJson(json); - // TODO(#1687): add_members, remove_members, add_subgroups, remove_subgroups + case 'add_members': return UserGroupAddMembersEvent.fromJson(json); + case 'remove_members': return UserGroupRemoveMembersEvent.fromJson(json); + case 'add_subgroups': return UserGroupAddSubgroupsEvent.fromJson(json); + case 'remove_subgroups': return UserGroupRemoveSubgroupsEvent.fromJson(json); case 'remove': return UserGroupRemoveEvent.fromJson(json); default: return UnexpectedEvent.fromJson(json); } @@ -280,6 +283,78 @@ class UserGroupUpdateData { Map toJson() => _$UserGroupUpdateDataToJson(this); } +/// A [UserGroupEvent] with op `add_members`: https://zulip.com/api/get-events#user_group-add_members +@JsonSerializable(fieldRename: FieldRename.snake) +class UserGroupAddMembersEvent extends UserGroupEvent { + @override + @JsonKey(includeToJson: true) + String get op => 'add_members'; + + final int groupId; + final List userIds; + + UserGroupAddMembersEvent({required super.id, required this.groupId, required this.userIds}); + + factory UserGroupAddMembersEvent.fromJson(Map json) => _$UserGroupAddMembersEventFromJson(json); + + @override + Map toJson() => _$UserGroupAddMembersEventToJson(this); +} + +/// A [UserGroupEvent] with op `remove_members`: https://zulip.com/api/get-events#user_group-remove_members +@JsonSerializable(fieldRename: FieldRename.snake) +class UserGroupRemoveMembersEvent extends UserGroupEvent { + @override + @JsonKey(includeToJson: true) + String get op => 'remove_members'; + + final int groupId; + final List userIds; + + UserGroupRemoveMembersEvent({required super.id, required this.groupId, required this.userIds}); + + factory UserGroupRemoveMembersEvent.fromJson(Map json) => _$UserGroupRemoveMembersEventFromJson(json); + + @override + Map toJson() => _$UserGroupRemoveMembersEventToJson(this); +} + +/// A [UserGroupEvent] with op `add_subgroups`: https://zulip.com/api/get-events#user_group-add_subgroups +@JsonSerializable(fieldRename: FieldRename.snake) +class UserGroupAddSubgroupsEvent extends UserGroupEvent { + @override + @JsonKey(includeToJson: true) + String get op => 'add_subgroups'; + + final int groupId; + final List directSubgroupIds; + + UserGroupAddSubgroupsEvent({required super.id, required this.groupId, required this.directSubgroupIds}); + + factory UserGroupAddSubgroupsEvent.fromJson(Map json) => _$UserGroupAddSubgroupsEventFromJson(json); + + @override + Map toJson() => _$UserGroupAddSubgroupsEventToJson(this); +} + +/// A [UserGroupEvent] with op `remove_subgroups`: https://zulip.com/api/get-events#user_group-remove_subgroups +@JsonSerializable(fieldRename: FieldRename.snake) +class UserGroupRemoveSubgroupsEvent extends UserGroupEvent { + @override + @JsonKey(includeToJson: true) + String get op => 'remove_subgroups'; + + final int groupId; + final List directSubgroupIds; + + UserGroupRemoveSubgroupsEvent({required super.id, required this.groupId, required this.directSubgroupIds}); + + factory UserGroupRemoveSubgroupsEvent.fromJson(Map json) => _$UserGroupRemoveSubgroupsEventFromJson(json); + + @override + Map toJson() => _$UserGroupRemoveSubgroupsEventToJson(this); +} + /// A [UserGroupEvent] with op `remove`: https://zulip.com/api/get-events#user_group-remove @JsonSerializable(fieldRename: FieldRename.snake) class UserGroupRemoveEvent extends UserGroupEvent { diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index bb85039555..50910d7a11 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -134,6 +134,86 @@ Map _$UserGroupUpdateDataToJson( 'deactivated': instance.deactivated, }; +UserGroupAddMembersEvent _$UserGroupAddMembersEventFromJson( + Map json, +) => UserGroupAddMembersEvent( + id: (json['id'] as num).toInt(), + groupId: (json['group_id'] as num).toInt(), + userIds: (json['user_ids'] as List) + .map((e) => (e as num).toInt()) + .toList(), +); + +Map _$UserGroupAddMembersEventToJson( + UserGroupAddMembersEvent instance, +) => { + 'id': instance.id, + 'type': instance.type, + 'op': instance.op, + 'group_id': instance.groupId, + 'user_ids': instance.userIds, +}; + +UserGroupRemoveMembersEvent _$UserGroupRemoveMembersEventFromJson( + Map json, +) => UserGroupRemoveMembersEvent( + id: (json['id'] as num).toInt(), + groupId: (json['group_id'] as num).toInt(), + userIds: (json['user_ids'] as List) + .map((e) => (e as num).toInt()) + .toList(), +); + +Map _$UserGroupRemoveMembersEventToJson( + UserGroupRemoveMembersEvent instance, +) => { + 'id': instance.id, + 'type': instance.type, + 'op': instance.op, + 'group_id': instance.groupId, + 'user_ids': instance.userIds, +}; + +UserGroupAddSubgroupsEvent _$UserGroupAddSubgroupsEventFromJson( + Map json, +) => UserGroupAddSubgroupsEvent( + id: (json['id'] as num).toInt(), + groupId: (json['group_id'] as num).toInt(), + directSubgroupIds: (json['direct_subgroup_ids'] as List) + .map((e) => (e as num).toInt()) + .toList(), +); + +Map _$UserGroupAddSubgroupsEventToJson( + UserGroupAddSubgroupsEvent instance, +) => { + 'id': instance.id, + 'type': instance.type, + 'op': instance.op, + 'group_id': instance.groupId, + 'direct_subgroup_ids': instance.directSubgroupIds, +}; + +UserGroupRemoveSubgroupsEvent _$UserGroupRemoveSubgroupsEventFromJson( + Map json, +) => UserGroupRemoveSubgroupsEvent( + id: (json['id'] as num).toInt(), + groupId: (json['group_id'] as num).toInt(), + directSubgroupIds: (json['direct_subgroup_ids'] as List) + .map((e) => (e as num).toInt()) + .toList(), +); + +Map _$UserGroupRemoveSubgroupsEventToJson( + UserGroupRemoveSubgroupsEvent instance, +) => { + 'id': instance.id, + 'type': instance.type, + 'op': instance.op, + 'group_id': instance.groupId, + 'direct_subgroup_ids': instance.directSubgroupIds, +}; + UserGroupRemoveEvent _$UserGroupRemoveEventFromJson( Map json, ) => UserGroupRemoveEvent( diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 6f0e502a39..4ab6efdcd4 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -358,8 +358,8 @@ class UserGroup { final int id; // TODO(#1687) to maintain members, also act on user deactivation: https://github.com/zulip/zulip-flutter/issues/662#issuecomment-2405845356 - // List members; // TODO(#1687) track group members - // List directSubgroupIds; // TODO(#1687) track group members + final Set members; + final Set directSubgroupIds; String name; String description; @@ -377,6 +377,8 @@ class UserGroup { UserGroup({ required this.id, + required this.members, + required this.directSubgroupIds, required this.name, required this.description, required this.isSystemGroup, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index e9cf30e5d2..93fccad3e6 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -96,6 +96,12 @@ Map _$RealmEmojiItemToJson(RealmEmojiItem instance) => UserGroup _$UserGroupFromJson(Map json) => UserGroup( id: (json['id'] as num).toInt(), + members: (json['members'] as List) + .map((e) => (e as num).toInt()) + .toSet(), + directSubgroupIds: (json['direct_subgroup_ids'] as List) + .map((e) => (e as num).toInt()) + .toSet(), name: json['name'] as String, description: json['description'] as String, isSystemGroup: json['is_system_group'] as bool, @@ -104,6 +110,8 @@ UserGroup _$UserGroupFromJson(Map json) => UserGroup( Map _$UserGroupToJson(UserGroup instance) => { 'id': instance.id, + 'members': instance.members.toList(), + 'direct_subgroup_ids': instance.directSubgroupIds.toList(), 'name': instance.name, 'description': instance.description, 'is_system_group': instance.isSystemGroup, diff --git a/lib/model/user_group.dart b/lib/model/user_group.dart index 2079b323ec..9f09fc3636 100644 --- a/lib/model/user_group.dart +++ b/lib/model/user_group.dart @@ -74,6 +74,12 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { if (data.name != null) group.name = data.name!; if (data.description != null) group.description = data.description!; if (data.deactivated != null) group.deactivated = data.deactivated!; + + case UserGroupAddMembersEvent(): + case UserGroupRemoveMembersEvent(): + case UserGroupAddSubgroupsEvent(): + case UserGroupRemoveSubgroupsEvent(): + break; // TODO(#1687): update group memberships on event } } } diff --git a/test/example_data.dart b/test/example_data.dart index 3e4da34b96..366c1734a4 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -198,6 +198,8 @@ int _lastUserGroupId = 100; UserGroup userGroup({ int? id, + Iterable? members, + Iterable? directSubgroupIds, String? name, String? description, bool isSystemGroup = false, @@ -205,6 +207,8 @@ UserGroup userGroup({ }) { return UserGroup( id: id ??= _nextUserGroupId(), + members: Set.of(members ?? []), + directSubgroupIds: Set.of(directSubgroupIds ?? []), name: name ??= 'group-$id', description: description ?? 'A group named $name', isSystemGroup: isSystemGroup, From a0cfecd0091ddd4c0adcab53e0116a98e970eeb1 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 15:11:25 -0700 Subject: [PATCH 02/14] group: Maintain user-group membership on events Fixes #1687. We'll add tests for this after an upcoming commit provides a way of accessing group membership that's more relevant for the app's needs. That way we can write the tests to that interface. --- lib/api/model/model.dart | 1 - lib/model/store.dart | 1 + lib/model/user_group.dart | 38 +++++++++++++++++++++++++++++++++----- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 4ab6efdcd4..3377cd190f 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -357,7 +357,6 @@ enum Emojiset { class UserGroup { final int id; - // TODO(#1687) to maintain members, also act on user deactivation: https://github.com/zulip/zulip-flutter/issues/662#issuecomment-2405845356 final Set members; final Set directSubgroupIds; diff --git a/lib/model/store.dart b/lib/model/store.dart index c240d47920..946a64e4f9 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -757,6 +757,7 @@ class PerAccountStore extends PerAccountStoreBase with case RealmUserUpdateEvent(): assert(debugLog("server event: realm_user/update")); + _groups.handleRealmUserUpdateEvent(event); _users.handleRealmUserEvent(event); autocompleteViewManager.handleRealmUserUpdateEvent(event); notifyListeners(); diff --git a/lib/model/user_group.dart b/lib/model/user_group.dart index 9f09fc3636..5c2ee96d72 100644 --- a/lib/model/user_group.dart +++ b/lib/model/user_group.dart @@ -57,6 +57,12 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { final Map _groups; + UserGroup? _expectGroup(int groupId) { + final group = _groups[groupId]; + // TODO(log) if group not found + return group; + } + void handleUserGroupEvent(UserGroupEvent event) { switch (event) { case UserGroupAddEvent(): @@ -66,20 +72,42 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { _groups.remove(event.groupId); case UserGroupUpdateEvent(): - final group = _groups[event.groupId]; - if (group == null) { - return; // TODO log - } + final group = _expectGroup(event.groupId); + if (group == null) return; final data = event.data; if (data.name != null) group.name = data.name!; if (data.description != null) group.description = data.description!; if (data.deactivated != null) group.deactivated = data.deactivated!; + // TODO(#1687): test handling the four membership-related events case UserGroupAddMembersEvent(): + final group = _expectGroup(event.groupId); + if (group == null) return; + group.members.addAll(event.userIds); + case UserGroupRemoveMembersEvent(): + final group = _expectGroup(event.groupId); + if (group == null) return; + group.members.removeAll(event.userIds); + case UserGroupAddSubgroupsEvent(): + final group = _expectGroup(event.groupId); + if (group == null) return; + group.directSubgroupIds.addAll(event.directSubgroupIds); + case UserGroupRemoveSubgroupsEvent(): - break; // TODO(#1687): update group memberships on event + final group = _expectGroup(event.groupId); + if (group == null) return; + group.directSubgroupIds.removeAll(event.directSubgroupIds); + } + } + + void handleRealmUserUpdateEvent(RealmUserUpdateEvent event) { + if (event.isActive == false) { + // TODO(#1687): test handling user deactivation + for (final group in _groups.values) { + group.members.remove(event.userId); + } } } } From cd47dcc1da3ae69d305df7c2122e0c37f9c96e19 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 15:25:55 -0700 Subject: [PATCH 03/14] api: Define type GroupSettingValue --- lib/api/model/model.dart | 44 ++++++++++++++++++++++++++++++++++++++ lib/api/model/model.g.dart | 18 ++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 3377cd190f..0c1f7ad3e1 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -12,6 +12,50 @@ export 'reaction.dart'; part 'model.g.dart'; +/// A Zulip "group-setting value": https://zulip.com/api/group-setting-values +sealed class GroupSettingValue { + const GroupSettingValue(); + + factory GroupSettingValue.fromJson(Object? json) { + return switch (json) { + int() => GroupSettingValueNamed.fromJson(json), + Map() => GroupSettingValueNameless.fromJson(json), + _ => throw FormatException(), + }; + } + + Object? toJson(); +} + +class GroupSettingValueNamed extends GroupSettingValue { + final int groupId; + + const GroupSettingValueNamed(this.groupId); + + factory GroupSettingValueNamed.fromJson(int json) => GroupSettingValueNamed(json); + + @override + int toJson() => groupId; +} + +@JsonSerializable(fieldRename: FieldRename.snake) +class GroupSettingValueNameless extends GroupSettingValue { + // TODO(server): The API docs say these should be "direct_member_ids" and + // "direct_subgroup_ids", but empirically they're "direct_members" + // and "direct_subgroups". Discussion: + // https://chat.zulip.org/#narrow/channel/378-api-design/topic/groups.20redesign/near/2247218 + final List directMembers; + final List directSubgroups; + + GroupSettingValueNameless({required this.directMembers, required this.directSubgroups}); + + factory GroupSettingValueNameless.fromJson(Map json) => + _$GroupSettingValueNamelessFromJson(json); + + @override + Map toJson() => _$GroupSettingValueNamelessToJson(this); +} + /// As in [InitialSnapshot.customProfileFields]. /// /// For docs, search for "custom_profile_fields:" diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 93fccad3e6..1c871a027d 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -8,6 +8,24 @@ part of 'model.dart'; // JsonSerializableGenerator // ************************************************************************** +GroupSettingValueNameless _$GroupSettingValueNamelessFromJson( + Map json, +) => GroupSettingValueNameless( + directMembers: (json['direct_members'] as List) + .map((e) => (e as num).toInt()) + .toList(), + directSubgroups: (json['direct_subgroups'] as List) + .map((e) => (e as num).toInt()) + .toList(), +); + +Map _$GroupSettingValueNamelessToJson( + GroupSettingValueNameless instance, +) => { + 'direct_members': instance.directMembers, + 'direct_subgroups': instance.directSubgroups, +}; + CustomProfileField _$CustomProfileFieldFromJson(Map json) => CustomProfileField( id: (json['id'] as num).toInt(), From 7e387dc1cad53a6fadaab757c25b4c56a9e95806 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 19 Aug 2025 13:05:44 -0700 Subject: [PATCH 04/14] group: Add selfInGroupSetting This is the way we'll be inspecting group membership for checking permissions (#814), which is the main way we'll be using group membership in general. Because this method is how most of the app will consume group membership, adding this method also lets us write natural tests for the logic that maintains group membership on events. At this stage we stick with a very straightforward implementation. We might optimize it later with more data structures. --- lib/model/user_group.dart | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/model/user_group.dart b/lib/model/user_group.dart index 5c2ee96d72..e700cab175 100644 --- a/lib/model/user_group.dart +++ b/lib/model/user_group.dart @@ -18,6 +18,10 @@ mixin UserGroupStore on PerAccountStoreBase { /// /// Consider using [activeGroups] instead. Iterable get allGroups; + + /// Whether the self-user is a (transitive) member of the given group, + /// a group-setting value. + bool selfInGroupSetting(GroupSettingValue value); } mixin ProxyUserGroupStore on UserGroupStore { @@ -30,6 +34,9 @@ mixin ProxyUserGroupStore on UserGroupStore { Iterable get activeGroups => userGroupStore.activeGroups; @override Iterable get allGroups => userGroupStore.allGroups; + @override + bool selfInGroupSetting(GroupSettingValue value) + => userGroupStore.selfInGroupSetting(value); } /// The implementation of [UserGroupStore] that does the work. @@ -55,6 +62,26 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { return _groups.values; } + @override + bool selfInGroupSetting(GroupSettingValue value) { // TODO(#1687) test + return switch (value) { + GroupSettingValueNamed() => + _selfInGroup(value.groupId), + GroupSettingValueNameless() => + value.directMembers.contains(selfUserId) + || value.directSubgroups.any(_selfInGroup), + }; + } + + bool _selfInGroup(int groupId) { + final group = _groups[groupId]; + if (group == null) return false; // TODO(log); should know all groups + // TODO(perf), TODO(#814): memoize which groups the self-user is in, + // to save doing this depth-first search on each permission check + return group.members.contains(selfUserId) + || group.directSubgroupIds.any(_selfInGroup); + } + final Map _groups; UserGroup? _expectGroup(int groupId) { From 11b09ffcf77aeb7cb214a2faebfd7386d8fda4ba Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 19 Aug 2025 15:12:39 -0700 Subject: [PATCH 05/14] group test: Test group membership, and handling events for it --- lib/model/user_group.dart | 4 +- test/model/user_group_test.dart | 88 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/lib/model/user_group.dart b/lib/model/user_group.dart index e700cab175..045db6798f 100644 --- a/lib/model/user_group.dart +++ b/lib/model/user_group.dart @@ -63,7 +63,7 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { } @override - bool selfInGroupSetting(GroupSettingValue value) { // TODO(#1687) test + bool selfInGroupSetting(GroupSettingValue value) { return switch (value) { GroupSettingValueNamed() => _selfInGroup(value.groupId), @@ -106,7 +106,6 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { if (data.description != null) group.description = data.description!; if (data.deactivated != null) group.deactivated = data.deactivated!; - // TODO(#1687): test handling the four membership-related events case UserGroupAddMembersEvent(): final group = _expectGroup(event.groupId); if (group == null) return; @@ -131,7 +130,6 @@ class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { void handleRealmUserUpdateEvent(RealmUserUpdateEvent event) { if (event.isActive == false) { - // TODO(#1687): test handling user deactivation for (final group in _groups.values) { group.members.remove(event.userId); } diff --git a/test/model/user_group_test.dart b/test/model/user_group_test.dart index f05b3c3c32..0741f53b95 100644 --- a/test/model/user_group_test.dart +++ b/test/model/user_group_test.dart @@ -2,6 +2,7 @@ import 'package:checks/checks.dart'; import 'package:test_api/scaffolding.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/store.dart'; import 'package:zulip/model/user_group.dart'; import '../api/model/model_checks.dart'; @@ -92,6 +93,93 @@ void main() { }]); }); + group('membership', () { + // These tests exercise membership via selfInGroupSetting, because that's + // the main interface the app uses to consume group membership. + + late PerAccountStore store; + + void prepare(List groups, {List? users}) { + store = eg.store(initialSnapshot: eg.initialSnapshot( + realmUsers: users, realmUserGroups: groups)); + } + + bool isMember(UserGroup group) { + return store.selfInGroupSetting(GroupSettingValueNamed(group.id)); + } + + test('initial', () { + final groups = []; + groups.add(eg.userGroup(members: [eg.selfUser.userId])); + groups.add(eg.userGroup(members: [eg.user().userId])); + groups.add(eg.userGroup(directSubgroupIds: [groups[0].id])); + groups.add(eg.userGroup(directSubgroupIds: [groups[2].id])); + groups.add(eg.userGroup(directSubgroupIds: [groups[1].id])); + + prepare(groups); + check(groups.map(isMember)).deepEquals([ + true, + false, + true, + true, + false, + ]); + }); + + test('UserGroupEvent', () async { + final groups = List.generate(4, (_) => eg.userGroup()); + prepare(groups); + check(groups.map(isMember)).deepEquals([false, false, false, false]); + + // Add a membership. + await store.handleEvent(UserGroupAddMembersEvent(id: 0, + groupId: groups[0].id, userIds: [eg.selfUser.userId])); + check(groups.map(isMember)).deepEquals([true, false, false, false]); + + // Add a chain of transitive memberships. + await store.handleEvent(UserGroupAddSubgroupsEvent(id: 0, + groupId: groups[1].id, directSubgroupIds: [groups[0].id])); + check(groups.map(isMember)).deepEquals([true, true, false, false]); + await store.handleEvent(UserGroupAddSubgroupsEvent(id: 0, + groupId: groups[2].id, directSubgroupIds: [groups[1].id])); + check(groups.map(isMember)).deepEquals([true, true, true, false]); + + // Cut the middle link of the chain. + await store.handleEvent(UserGroupRemoveSubgroupsEvent(id: 0, + groupId: groups[1].id, directSubgroupIds: [groups[0].id])); + check(groups.map(isMember)).deepEquals([true, false, false, false]); + + // Restore the middle link; cut the bottom link. + await store.handleEvent(UserGroupAddSubgroupsEvent(id: 0, + groupId: groups[1].id, directSubgroupIds: [groups[0].id])); + check(groups.map(isMember)).deepEquals([true, true, true, false]); + await store.handleEvent(UserGroupRemoveMembersEvent(id: 0, + groupId: groups[0].id, userIds: [eg.selfUser.userId])); + check(groups.map(isMember)).deepEquals([false, false, false, false]); + }); + + test('RealmUserUpdateEvent', () async { + // This test uses the membership data structure directly, because + // selfInGroupSetting would only be affected if the self-user were + // deactivated, and in that case we wouldn't be getting an event. + + final user = eg.user(); + final group = eg.userGroup(members: [user.userId]); + prepare(users: [eg.selfUser, user], [group]); + check(store.getGroup(group.id)!.members).deepEquals([user.userId]); + + // An update to a random irrelevant field has no effect. + await store.handleEvent(RealmUserUpdateEvent(id: 0, + userId: user.userId, fullName: 'New Name')); + check(store.getGroup(group.id)!.members).deepEquals([user.userId]); + + // But deactivating the user removes them from groups. + await store.handleEvent(RealmUserUpdateEvent(id: 0, + userId: user.userId, isActive: false)); + check(store.getGroup(group.id)!.members).isEmpty(); + }); + }); + test('various fields make it through', () async { final store = eg.store(initialSnapshot: eg.initialSnapshot( realmUserGroups: [ From 1637dee6dde5c1e6882d390365666f0d02ac4a4b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 19:54:29 -0700 Subject: [PATCH 06/14] group [nfc]: Add HasUserGroupStore --- lib/model/user_group.dart | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/model/user_group.dart b/lib/model/user_group.dart index 045db6798f..4fb52b6d49 100644 --- a/lib/model/user_group.dart +++ b/lib/model/user_group.dart @@ -39,6 +39,15 @@ mixin ProxyUserGroupStore on UserGroupStore { => userGroupStore.selfInGroupSetting(value); } +abstract class HasUserGroupStore extends PerAccountStoreBase with UserGroupStore, ProxyUserGroupStore { + HasUserGroupStore({required UserGroupStore groups}) + : userGroupStore = groups, super(core: groups.core); + + @protected + @override + final UserGroupStore userGroupStore; +} + /// The implementation of [UserGroupStore] that does the work. class UserGroupStoreImpl extends PerAccountStoreBase with UserGroupStore { UserGroupStoreImpl({required super.core, required List groups}) From 65a09776476b749fba878f0b667e97718486bbaf Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 19:54:52 -0700 Subject: [PATCH 07/14] realm [nfc]: Give RealmStore a reference to UserGroupStore --- lib/model/realm.dart | 14 +++++++++----- lib/model/store.dart | 7 ++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/model/realm.dart b/lib/model/realm.dart index d54e5a5467..cb3774808e 100644 --- a/lib/model/realm.dart +++ b/lib/model/realm.dart @@ -4,6 +4,7 @@ import '../api/model/events.dart'; import '../api/model/initial_snapshot.dart'; import '../api/model/model.dart'; import 'store.dart'; +import 'user_group.dart'; /// The portion of [PerAccountStore] for realm settings, server settings, /// and similar data about the whole realm or server. @@ -11,7 +12,10 @@ import 'store.dart'; /// See also: /// * [RealmStoreImpl] for the implementation of this that does the work. /// * [HasRealmStore] for an implementation useful for other substores. -mixin RealmStore on PerAccountStoreBase { +mixin RealmStore on PerAccountStoreBase, UserGroupStore { + @protected + UserGroupStore get userGroupStore; + //|////////////////////////////////////////////////////////////// // Server settings, explicitly so named. @@ -159,9 +163,9 @@ mixin ProxyRealmStore on RealmStore { /// A base class for [PerAccountStore] substores that need access to [RealmStore] /// as well as to [CorePerAccountStore]. -abstract class HasRealmStore extends PerAccountStoreBase with RealmStore, ProxyRealmStore { +abstract class HasRealmStore extends HasUserGroupStore with RealmStore, ProxyRealmStore { HasRealmStore({required RealmStore realm}) - : realmStore = realm, super(core: realm.core); + : realmStore = realm, super(groups: realm.userGroupStore); @protected @override @@ -169,9 +173,9 @@ abstract class HasRealmStore extends PerAccountStoreBase with RealmStore, ProxyR } /// The implementation of [RealmStore] that does the work. -class RealmStoreImpl extends PerAccountStoreBase with RealmStore { +class RealmStoreImpl extends HasUserGroupStore with RealmStore { RealmStoreImpl({ - required super.core, + required super.groups, required InitialSnapshot initialSnapshot, }) : serverPresencePingIntervalSeconds = initialSnapshot.serverPresencePingIntervalSeconds, diff --git a/lib/model/store.dart b/lib/model/store.dart index 946a64e4f9..af5c1b9902 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -496,15 +496,16 @@ class PerAccountStore extends PerAccountStoreBase with throw Exception("bad initial snapshot: self-user missing from user list"); } - final realm = RealmStoreImpl(core: core, initialSnapshot: initialSnapshot); + final groups = UserGroupStoreImpl(core: core, + groups: initialSnapshot.realmUserGroups); + final realm = RealmStoreImpl(groups: groups, initialSnapshot: initialSnapshot); final users = UserStoreImpl(realm: realm, initialSnapshot: initialSnapshot, userMap: userMap); final channels = ChannelStoreImpl(users: users, initialSnapshot: initialSnapshot); return PerAccountStore._( core: core, - groups: UserGroupStoreImpl(core: core, - groups: initialSnapshot.realmUserGroups), + groups: groups, realm: realm, emoji: EmojiStoreImpl(core: core, allRealmEmoji: initialSnapshot.realmEmoji), From f080dad07d0de1d6df04dd184c4d0cad0876a1a9 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 20:18:43 -0700 Subject: [PATCH 08/14] realm: Denormalize self-user role onto RealmStoreImpl --- lib/model/realm.dart | 16 ++++++++++++++++ lib/model/store.dart | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/model/realm.dart b/lib/model/realm.dart index cb3774808e..408b858d4f 100644 --- a/lib/model/realm.dart +++ b/lib/model/realm.dart @@ -177,7 +177,9 @@ class RealmStoreImpl extends HasUserGroupStore with RealmStore { RealmStoreImpl({ required super.groups, required InitialSnapshot initialSnapshot, + required User selfUser, }) : + _selfUserRole = selfUser.role, serverPresencePingIntervalSeconds = initialSnapshot.serverPresencePingIntervalSeconds, serverPresenceOfflineThresholdSeconds = initialSnapshot.serverPresenceOfflineThresholdSeconds, serverTypingStartedExpiryPeriodMilliseconds = initialSnapshot.serverTypingStartedExpiryPeriodMilliseconds, @@ -195,6 +197,13 @@ class RealmStoreImpl extends HasUserGroupStore with RealmStore { realmDefaultExternalAccounts = initialSnapshot.realmDefaultExternalAccounts, customProfileFields = _sortCustomProfileFields(initialSnapshot.customProfileFields); + /// The [User.role] of the self-user. + /// + /// The main home of this information is [UserStore]: `store.selfUser.role`. + /// We need it here for interpreting some permission settings; + /// so we denormalize it here to avoid a cycle between substores. + UserRole _selfUserRole; // ignore: unused_field // TODO(#814) + @override final int serverPresencePingIntervalSeconds; @override @@ -257,4 +266,11 @@ class RealmStoreImpl extends HasUserGroupStore with RealmStore { void handleCustomProfileFieldsEvent(CustomProfileFieldsEvent event) { customProfileFields = _sortCustomProfileFields(event.fields); } + + void handleRealmUserUpdateEvent(RealmUserUpdateEvent event) { + // Compare [UserStoreImpl.handleRealmUserEvent]. + if (event.userId == selfUserId) { + if (event.role != null) _selfUserRole = event.role!; + } + } } diff --git a/lib/model/store.dart b/lib/model/store.dart index af5c1b9902..71bf40f094 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -498,7 +498,8 @@ class PerAccountStore extends PerAccountStoreBase with final groups = UserGroupStoreImpl(core: core, groups: initialSnapshot.realmUserGroups); - final realm = RealmStoreImpl(groups: groups, initialSnapshot: initialSnapshot); + final realm = RealmStoreImpl(groups: groups, initialSnapshot: initialSnapshot, + selfUser: selfUser); final users = UserStoreImpl(realm: realm, initialSnapshot: initialSnapshot, userMap: userMap); final channels = ChannelStoreImpl(users: users, @@ -759,6 +760,7 @@ class PerAccountStore extends PerAccountStoreBase with case RealmUserUpdateEvent(): assert(debugLog("server event: realm_user/update")); _groups.handleRealmUserUpdateEvent(event); + _realm.handleRealmUserUpdateEvent(event); _users.handleRealmUserEvent(event); autocompleteViewManager.handleRealmUserUpdateEvent(event); notifyListeners(); From 76aa6b538af275b42051308eca74e13c3727fcc2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 21 Aug 2025 14:32:42 -0700 Subject: [PATCH 09/14] api: Add SupportedPermissionSettings type, and fixture --- lib/api/model/initial_snapshot.dart | 114 ++++++++++++++++++++++++++ lib/api/model/initial_snapshot.g.dart | 35 ++++++++ 2 files changed, 149 insertions(+) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index 50a9f0c243..26e1948d9d 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -109,6 +109,9 @@ class InitialSnapshot { @JsonKey(readValue: _readUsersIsActiveFallbackTrue) final List crossRealmBots; + // TODO(server): Get this API stabilized, to replace [SupportedPermissionSettings.fixture]. + // final SupportedPermissionSettings? serverSupportedPermissionSettings; + // TODO etc., etc. // If adding fields, keep them all in the order they appear in the API docs. @@ -389,3 +392,114 @@ class UnreadHuddleSnapshot { Map toJson() => _$UnreadHuddleSnapshotToJson(this); } + +/// Metadata about how to interpret the various group-based permission settings. +/// +/// This is the type that [InitialSnapshot.serverSupportedPermissionSettings] +/// would have, according to the API as it exists as of 2025-08; +/// but that API is documented as unstable and subject to change. +/// +/// For a useful value of this type, see [SupportedPermissionSettings.fixture]. +/// +/// For docs, search for "d_perm" in: https://zulip.com/api/register-queue +@JsonSerializable(fieldRename: FieldRename.snake) +class SupportedPermissionSettings { + final Map realm; + final Map stream; + final Map group; + + /// Metadata about how to interpret certain group-based permission settings, + /// including all those that this client uses, based on "current" servers. + /// + /// "Current" here means as of when this code was written, or last updated; + /// details in comments below. Naturally it'd be better to have an API to + /// get this information from the actual server. + /// + /// Effectively we're counting on it being uncommon for the metadata for a + /// given permission to ever change from one server version to the next, + /// so that the values we take from one server version usually remain valid + /// for all past and future server versions that have the corresponding + /// permission at all. + /// + /// TODO(server): Stabilize [InitialSnapshot.serverSupportedPermissionSettings] + /// or a similar API, and switch to using that. See thread: + /// https://chat.zulip.org/#narrow/channel/378-api-design/topic/server_supported_permission_settings/near/2247549 + static SupportedPermissionSettings fixture = SupportedPermissionSettings( + realm: {}, // Please go ahead and fill this in when we come to need it. + group: {}, // Please go ahead and fill this in when we come to need it. + stream: { + // From the server's Stream.stream_permission_group_settings, + // in zerver/models/streams.py. Current as of f9dc13014, 2025-08. + "can_add_subscribers_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: false, + // default_group_name=SystemGroups.NOBODY, + ), + "can_administer_channel_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: false, + // default_group_name="stream_creator_or_nobody", + ), + "can_delete_any_message_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.NOBODY, + ), + "can_delete_own_message_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.NOBODY, + ), + "can_move_messages_out_of_channel_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.NOBODY, + ), + "can_move_messages_within_channel_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.NOBODY, + ), + "can_remove_subscribers_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.ADMINISTRATORS, + ), + "can_send_message_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.EVERYONE, + ), + "can_subscribe_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: false, + // default_group_name=SystemGroups.NOBODY, + ), + "can_resolve_topics_group": PermissionSettingsItem( + // allow_nobody_group=True, + allowEveryoneGroup: true, + // default_group_name=SystemGroups.NOBODY, + ), + }, + ); + + SupportedPermissionSettings({required this.realm, required this.stream, required this.group}); + + factory SupportedPermissionSettings.fromJson(Map json) => + _$SupportedPermissionSettingsFromJson(json); + + Map toJson() => _$SupportedPermissionSettingsToJson(this); +} + +@JsonSerializable(fieldRename: FieldRename.snake) +class PermissionSettingsItem { + final bool allowEveryoneGroup; + // also other fields not yet used + + PermissionSettingsItem({required this.allowEveryoneGroup}); + + factory PermissionSettingsItem.fromJson(Map json) => + _$PermissionSettingsItemFromJson(json); + + Map toJson() => _$PermissionSettingsItemToJson(this); +} diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index d6de29713e..481b3184b7 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -355,3 +355,38 @@ Map _$UnreadHuddleSnapshotToJson( 'user_ids_string': instance.userIdsString, 'unread_message_ids': instance.unreadMessageIds, }; + +SupportedPermissionSettings _$SupportedPermissionSettingsFromJson( + Map json, +) => SupportedPermissionSettings( + realm: (json['realm'] as Map).map( + (k, e) => + MapEntry(k, PermissionSettingsItem.fromJson(e as Map)), + ), + stream: (json['stream'] as Map).map( + (k, e) => + MapEntry(k, PermissionSettingsItem.fromJson(e as Map)), + ), + group: (json['group'] as Map).map( + (k, e) => + MapEntry(k, PermissionSettingsItem.fromJson(e as Map)), + ), +); + +Map _$SupportedPermissionSettingsToJson( + SupportedPermissionSettings instance, +) => { + 'realm': instance.realm, + 'stream': instance.stream, + 'group': instance.group, +}; + +PermissionSettingsItem _$PermissionSettingsItemFromJson( + Map json, +) => PermissionSettingsItem( + allowEveryoneGroup: json['allow_everyone_group'] as bool, +); + +Map _$PermissionSettingsItemToJson( + PermissionSettingsItem instance, +) => {'allow_everyone_group': instance.allowEveryoneGroup}; From 1550173ab9f0d1eb457240c6231676a07f64b6f4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 19 Aug 2025 16:10:21 -0700 Subject: [PATCH 10/14] realm test [nfc]: Reorder tests to match implementation --- test/model/realm_test.dart | 56 +++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/test/model/realm_test.dart b/test/model/realm_test.dart index 5aecfd289d..4155974764 100644 --- a/test/model/realm_test.dart +++ b/test/model/realm_test.dart @@ -6,6 +6,34 @@ import 'package:zulip/api/model/model.dart'; import '../example_data.dart' as eg; void main() { + test('processTopicLikeServer', () { + final emptyTopicDisplayName = eg.defaultRealmEmptyTopicDisplayName; + + TopicName process(TopicName topic, int zulipFeatureLevel) { + final account = eg.selfAccount.copyWith(zulipFeatureLevel: zulipFeatureLevel); + final store = eg.store(account: account, initialSnapshot: eg.initialSnapshot( + zulipFeatureLevel: zulipFeatureLevel, + realmEmptyTopicDisplayName: emptyTopicDisplayName)); + return store.processTopicLikeServer(topic); + } + + void doCheck(TopicName topic, TopicName expected, int zulipFeatureLevel) { + check(process(topic, zulipFeatureLevel)).equals(expected); + } + + check(() => process(eg.t(''), 333)).throws(); + doCheck(eg.t('(no topic)'), eg.t('(no topic)'), 333); + doCheck(eg.t(emptyTopicDisplayName), eg.t(emptyTopicDisplayName), 333); + doCheck(eg.t('other topic'), eg.t('other topic'), 333); + + doCheck(eg.t(''), eg.t(''), 334); + doCheck(eg.t('(no topic)'), eg.t('(no topic)'), 334); + doCheck(eg.t(emptyTopicDisplayName), eg.t(''), 334); + doCheck(eg.t('other topic'), eg.t('other topic'), 334); + + doCheck(eg.t('(no topic)'), eg.t(''), 370); + }); + group('customProfileFields', () { test('update clobbers old list', () async { final store = eg.store(initialSnapshot: eg.initialSnapshot( @@ -47,32 +75,4 @@ void main() { check(store.customProfileFields.map((f) => f.id)).deepEquals([2, 0, 1]); }); }); - - test('processTopicLikeServer', () { - final emptyTopicDisplayName = eg.defaultRealmEmptyTopicDisplayName; - - TopicName process(TopicName topic, int zulipFeatureLevel) { - final account = eg.selfAccount.copyWith(zulipFeatureLevel: zulipFeatureLevel); - final store = eg.store(account: account, initialSnapshot: eg.initialSnapshot( - zulipFeatureLevel: zulipFeatureLevel, - realmEmptyTopicDisplayName: emptyTopicDisplayName)); - return store.processTopicLikeServer(topic); - } - - void doCheck(TopicName topic, TopicName expected, int zulipFeatureLevel) { - check(process(topic, zulipFeatureLevel)).equals(expected); - } - - check(() => process(eg.t(''), 333)).throws(); - doCheck(eg.t('(no topic)'), eg.t('(no topic)'), 333); - doCheck(eg.t(emptyTopicDisplayName), eg.t(emptyTopicDisplayName), 333); - doCheck(eg.t('other topic'), eg.t('other topic'), 333); - - doCheck(eg.t(''), eg.t(''), 334); - doCheck(eg.t('(no topic)'), eg.t('(no topic)'), 334); - doCheck(eg.t(emptyTopicDisplayName), eg.t(''), 334); - doCheck(eg.t('other topic'), eg.t('other topic'), 334); - - doCheck(eg.t('(no topic)'), eg.t(''), 370); - }); } From c6206a84b22fcc4ccbcd684f96caab5f33d1aa53 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 20:26:47 -0700 Subject: [PATCH 11/14] realm: Add selfHasPermissionForGroupSetting --- lib/model/realm.dart | 44 ++++++++++++++++++++++++++++++++- test/model/realm_test.dart | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/lib/model/realm.dart b/lib/model/realm.dart index 408b858d4f..02db21c56e 100644 --- a/lib/model/realm.dart +++ b/lib/model/realm.dart @@ -121,8 +121,14 @@ mixin RealmStore on PerAccountStoreBase, UserGroupStore { } return topic; } + + /// Whether the self-user has the given (group-based) permission. + bool selfHasPermissionForGroupSetting(GroupSettingValue value, + GroupSettingType type, String name); } +enum GroupSettingType { realm, stream, group } + mixin ProxyRealmStore on RealmStore { @protected RealmStore get realmStore; @@ -159,6 +165,9 @@ mixin ProxyRealmStore on RealmStore { Map get realmDefaultExternalAccounts => realmStore.realmDefaultExternalAccounts; @override List get customProfileFields => realmStore.customProfileFields; + @override + bool selfHasPermissionForGroupSetting(GroupSettingValue value, GroupSettingType type, String name) => + realmStore.selfHasPermissionForGroupSetting(value, type, name); } /// A base class for [PerAccountStore] substores that need access to [RealmStore] @@ -197,12 +206,45 @@ class RealmStoreImpl extends HasUserGroupStore with RealmStore { realmDefaultExternalAccounts = initialSnapshot.realmDefaultExternalAccounts, customProfileFields = _sortCustomProfileFields(initialSnapshot.customProfileFields); + @override + bool selfHasPermissionForGroupSetting(GroupSettingValue value, + GroupSettingType type, String name) { + // Compare web's settings_data.user_has_permission_for_group_setting. + // + // In the whole web app, there's just one caller for that function with + // a user other than the self user: stream_data.can_post_messages_in_stream, + // and only for get_current_user_and_their_bots_with_post_messages_permission, + // with only the self-user's own bots as the arguments. + // That exists for deciding whether to offer the "Generate email address" + // button, and if so then which users to offer in the dropdown; + // it's predicting whether /api/get-stream-email-address would succeed. + if (_selfUserRole == UserRole.guest) { + final config = _groupSettingConfig(type, name); + if (!config.allowEveryoneGroup) return false; + } + return selfInGroupSetting(value); + } + + /// The metadata for how to interpret the given group-based permission setting. + PermissionSettingsItem _groupSettingConfig(GroupSettingType type, String name) { + final supportedSettings = SupportedPermissionSettings.fixture; + + // Compare web's group_permission_settings.get_group_permission_setting_config. + final configGroup = switch (type) { + GroupSettingType.realm => supportedSettings.realm, + GroupSettingType.stream => supportedSettings.stream, + GroupSettingType.group => supportedSettings.group, + }; + final config = configGroup[name]; + return config!; // TODO(log) + } + /// The [User.role] of the self-user. /// /// The main home of this information is [UserStore]: `store.selfUser.role`. /// We need it here for interpreting some permission settings; /// so we denormalize it here to avoid a cycle between substores. - UserRole _selfUserRole; // ignore: unused_field // TODO(#814) + UserRole _selfUserRole; @override final int serverPresencePingIntervalSeconds; diff --git a/test/model/realm_test.dart b/test/model/realm_test.dart index 4155974764..04ceb28855 100644 --- a/test/model/realm_test.dart +++ b/test/model/realm_test.dart @@ -2,6 +2,7 @@ import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/realm.dart'; import '../example_data.dart' as eg; @@ -34,6 +35,55 @@ void main() { doCheck(eg.t('(no topic)'), eg.t(''), 370); }); + group('selfHasPermissionForGroupSetting', () { + // Most of the implementation of this is in [UserGroupStore.selfInGroupSetting], + // and is tested in more detail in user_group_test.dart . + + bool hasPermission(User selfUser, UserGroup group, String permissionName) { + final store = eg.store(selfUser: selfUser, + initialSnapshot: eg.initialSnapshot( + realmUsers: [selfUser], realmUserGroups: [group])); + return store.selfHasPermissionForGroupSetting( + GroupSettingValueNamed(group.id), + GroupSettingType.stream, permissionName); + } + + test('not in group -> no permission', () { + final selfUser = eg.user(); + final group = eg.userGroup(members: []); + check(hasPermission(selfUser, group, 'can_subscribe_group')) + .isFalse(); + }); + + test('in group -> has permission', () { + final selfUser = eg.user(); + final group = eg.userGroup(members: [selfUser.userId]); + check(hasPermission(selfUser, group, 'can_subscribe_group')) + .isTrue(); + }); + + test('guest -> no permission, despite group', () { + final selfUser = eg.user(role: UserRole.guest); + final group = eg.userGroup(members: [selfUser.userId]); + check(hasPermission(selfUser, group, 'can_subscribe_group')) + .isFalse(); + }); + + test('guest -> still has permission, if allowEveryoneGroup', () { + final selfUser = eg.user(role: UserRole.guest); + final group = eg.userGroup(members: [selfUser.userId]); + check(hasPermission(selfUser, group, 'can_send_message_group')) + .isTrue(); + }); + + test('guest not in group -> no permission, even if allowEveryoneGroup', () { + final selfUser = eg.user(role: UserRole.guest); + final group = eg.userGroup(members: []); + check(hasPermission(selfUser, group, 'can_send_message_group')) + .isFalse(); + }); + }); + group('customProfileFields', () { test('update clobbers old list', () async { final store = eg.store(initialSnapshot: eg.initialSnapshot( From 8c3b11fb1093c705698d1de912e24f2484ec28a2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 19 Aug 2025 16:02:28 -0700 Subject: [PATCH 12/14] test: Add eg.nobodyGroup --- test/example_data.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/example_data.dart b/test/example_data.dart index 366c1734a4..01ce96b372 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -216,6 +216,12 @@ UserGroup userGroup({ ); } +final UserGroup nobodyGroup = userGroup( + isSystemGroup: true, + name: 'role:nobody', description: 'Nobody', + members: [], directSubgroupIds: [], +); + RealmEmojiItem realmEmojiItem({ required String emojiCode, required String emojiName, From c3be2299383955b0d278154112b0c6e5f242902d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 15:30:22 -0700 Subject: [PATCH 13/14] api: Add channel fields canSubscribeGroup, canAddSubscribersGroup --- lib/api/model/events.dart | 3 +++ lib/api/model/events.g.dart | 2 ++ lib/api/model/model.dart | 13 ++++++++++--- lib/api/model/model.g.dart | 18 ++++++++++++++++++ lib/model/channel.dart | 4 ++++ test/example_data.dart | 9 +++++++++ 6 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 65c1cd794f..2f0f846d49 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -674,6 +674,9 @@ class ChannelUpdateEvent extends ChannelEvent { return value as int?; case ChannelPropertyName.channelPostPolicy: return ChannelPostPolicy.fromApiValue(value as int); + case ChannelPropertyName.canAddSubscribersGroup: + case ChannelPropertyName.canSubscribeGroup: + return GroupSettingValue.fromJson(value); case ChannelPropertyName.streamWeeklyTraffic: return value as int?; case null: diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 50910d7a11..6e7aadf362 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -461,6 +461,8 @@ const _$ChannelPropertyNameEnumMap = { ChannelPropertyName.inviteOnly: 'invite_only', ChannelPropertyName.messageRetentionDays: 'message_retention_days', ChannelPropertyName.channelPostPolicy: 'stream_post_policy', + ChannelPropertyName.canAddSubscribersGroup: 'can_add_subscribers_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 0c1f7ad3e1..a821474325 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -641,7 +641,8 @@ class ZulipStream { ChannelPostPolicy channelPostPolicy; // final bool isAnnouncementOnly; // deprecated for `channelPostPolicy`; ignore - // GroupSettingsValue canRemoveSubscribersGroup; // TODO(#814) + GroupSettingValue? canAddSubscribersGroup; // TODO(server-10) + GroupSettingValue? canSubscribeGroup; // TODO(server-10) // TODO(server-8): added in FL 199, was previously only on [Subscription] objects int? streamWeeklyTraffic; @@ -658,6 +659,8 @@ class ZulipStream { required this.historyPublicToSubscribers, required this.messageRetentionDays, required this.channelPostPolicy, + required this.canAddSubscribersGroup, + required this.canSubscribeGroup, required this.streamWeeklyTraffic, }); @@ -675,6 +678,8 @@ class ZulipStream { historyPublicToSubscribers: subscription.historyPublicToSubscribers, messageRetentionDays: subscription.messageRetentionDays, channelPostPolicy: subscription.channelPostPolicy, + canAddSubscribersGroup: subscription.canAddSubscribersGroup, + canSubscribeGroup: subscription.canSubscribeGroup, streamWeeklyTraffic: subscription.streamWeeklyTraffic, ); } @@ -705,8 +710,8 @@ enum ChannelPropertyName { messageRetentionDays, @JsonValue('stream_post_policy') channelPostPolicy, - // canRemoveSubscribersGroup, // TODO(#814) - // canRemoveSubscribersGroupId, // TODO(#814) handle // TODO(server-8) remove + canAddSubscribersGroup, + canSubscribeGroup, streamWeeklyTraffic; /// Get a [ChannelPropertyName] from a raw, snake-case string we recognize, else null. @@ -786,6 +791,8 @@ class Subscription extends ZulipStream { required super.historyPublicToSubscribers, required super.messageRetentionDays, required super.channelPostPolicy, + required super.canAddSubscribersGroup, + required super.canSubscribeGroup, required super.streamWeeklyTraffic, required this.desktopNotifications, required this.emailNotifications, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 1c871a027d..b5bb32da2a 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -250,6 +250,12 @@ ZulipStream _$ZulipStreamFromJson(Map json) => ZulipStream( _$ChannelPostPolicyEnumMap, json['stream_post_policy'], ), + canAddSubscribersGroup: json['can_add_subscribers_group'] == null + ? null + : GroupSettingValue.fromJson(json['can_add_subscribers_group']), + canSubscribeGroup: json['can_subscribe_group'] == null + ? null + : GroupSettingValue.fromJson(json['can_subscribe_group']), streamWeeklyTraffic: (json['stream_weekly_traffic'] as num?)?.toInt(), ); @@ -266,6 +272,8 @@ Map _$ZulipStreamToJson(ZulipStream instance) => 'history_public_to_subscribers': instance.historyPublicToSubscribers, 'message_retention_days': instance.messageRetentionDays, 'stream_post_policy': instance.channelPostPolicy, + 'can_add_subscribers_group': instance.canAddSubscribersGroup, + 'can_subscribe_group': instance.canSubscribeGroup, 'stream_weekly_traffic': instance.streamWeeklyTraffic, }; @@ -292,6 +300,12 @@ Subscription _$SubscriptionFromJson(Map json) => Subscription( _$ChannelPostPolicyEnumMap, json['stream_post_policy'], ), + canAddSubscribersGroup: json['can_add_subscribers_group'] == null + ? null + : GroupSettingValue.fromJson(json['can_add_subscribers_group']), + canSubscribeGroup: json['can_subscribe_group'] == null + ? null + : GroupSettingValue.fromJson(json['can_subscribe_group']), streamWeeklyTraffic: (json['stream_weekly_traffic'] as num?)?.toInt(), desktopNotifications: json['desktop_notifications'] as bool?, emailNotifications: json['email_notifications'] as bool?, @@ -316,6 +330,8 @@ Map _$SubscriptionToJson(Subscription instance) => 'history_public_to_subscribers': instance.historyPublicToSubscribers, 'message_retention_days': instance.messageRetentionDays, 'stream_post_policy': instance.channelPostPolicy, + 'can_add_subscribers_group': instance.canAddSubscribersGroup, + 'can_subscribe_group': instance.canSubscribeGroup, 'stream_weekly_traffic': instance.streamWeeklyTraffic, 'desktop_notifications': instance.desktopNotifications, 'email_notifications': instance.emailNotifications, @@ -475,6 +491,8 @@ const _$ChannelPropertyNameEnumMap = { ChannelPropertyName.inviteOnly: 'invite_only', ChannelPropertyName.messageRetentionDays: 'message_retention_days', ChannelPropertyName.channelPostPolicy: 'stream_post_policy', + ChannelPropertyName.canAddSubscribersGroup: 'can_add_subscribers_group', + ChannelPropertyName.canSubscribeGroup: 'can_subscribe_group', ChannelPropertyName.streamWeeklyTraffic: 'stream_weekly_traffic', }; diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 7a10d52f6b..78080ade95 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -337,6 +337,10 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore { stream.messageRetentionDays = event.value as int?; case ChannelPropertyName.channelPostPolicy: stream.channelPostPolicy = event.value as ChannelPostPolicy; + case ChannelPropertyName.canAddSubscribersGroup: + stream.canAddSubscribersGroup = event.value as GroupSettingValue; + case ChannelPropertyName.canSubscribeGroup: + stream.canSubscribeGroup = event.value as GroupSettingValue; case ChannelPropertyName.streamWeeklyTraffic: stream.streamWeeklyTraffic = event.value as int?; } diff --git a/test/example_data.dart b/test/example_data.dart index 01ce96b372..84d1abf6aa 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -443,6 +443,8 @@ ZulipStream stream({ bool? historyPublicToSubscribers, int? messageRetentionDays, ChannelPostPolicy? channelPostPolicy, + GroupSettingValue? canAddSubscribersGroup, + GroupSettingValue? canSubscribeGroup, int? streamWeeklyTraffic, }) { _checkPositive(streamId, 'stream ID'); @@ -462,6 +464,8 @@ ZulipStream stream({ historyPublicToSubscribers: historyPublicToSubscribers ?? true, messageRetentionDays: messageRetentionDays, channelPostPolicy: channelPostPolicy ?? ChannelPostPolicy.any, + canAddSubscribersGroup: canAddSubscribersGroup ?? GroupSettingValueNamed(nobodyGroup.id), + canSubscribeGroup: canSubscribeGroup ?? GroupSettingValueNamed(nobodyGroup.id), streamWeeklyTraffic: streamWeeklyTraffic, ); } @@ -500,6 +504,8 @@ Subscription subscription( historyPublicToSubscribers: stream.historyPublicToSubscribers, messageRetentionDays: stream.messageRetentionDays, channelPostPolicy: stream.channelPostPolicy, + canAddSubscribersGroup: stream.canAddSubscribersGroup, + canSubscribeGroup: stream.canSubscribeGroup, streamWeeklyTraffic: stream.streamWeeklyTraffic, desktopNotifications: desktopNotifications ?? false, emailNotifications: emailNotifications ?? false, @@ -1171,6 +1177,9 @@ ChannelUpdateEvent channelUpdateEvent( assert(value is int?); case ChannelPropertyName.channelPostPolicy: assert(value is ChannelPostPolicy); + case ChannelPropertyName.canAddSubscribersGroup: + case ChannelPropertyName.canSubscribeGroup: + assert(value is GroupSettingValue); case ChannelPropertyName.streamWeeklyTraffic: assert(value is int?); } From cf83c9ec60aa15cf5f125153ffeb5426e467b538 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Jul 2025 20:48:08 -0700 Subject: [PATCH 14/14] channel: Add selfHasContentAccess, using canSubscribeGroup et al Fixes #814. Fixes part of #1786. The general implementation of group-based permissions (#814) was added in the commits leading up to this one. This commit demonstrates implementing a given such permission. Then we also use the new permission in one place where we already want to use it, toward #1786. (There's another part of #1786 which calls for a variation: determining whether the user would be able to re-subscribe to a channel if they unsubscribe. We'll leave that for a follow-up.) We'll also be adding a more prominent bit of UI that wants this permission soon: the list of channels, #188. To exercise this method on real data, I added some debugging prints that printed a list of the channels where this returned true, and a list of the channels where it returned false. I also manually tested the change for #1786, by visiting an unsubscribed private channel (via the link in a #-mention), long-pressing the app bar to get the channel action sheet, and then (as another user, on web) adding and removing permission for the test user to subscribe themself. --- lib/model/channel.dart | 39 +++++++++++++++++++++++++++++++++++ lib/widgets/action_sheet.dart | 7 ++++--- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 78080ade95..8ef01054bd 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -5,6 +5,7 @@ import 'package:flutter/foundation.dart'; import '../api/model/events.dart'; import '../api/model/initial_snapshot.dart'; import '../api/model/model.dart'; +import 'realm.dart'; import 'store.dart'; import 'user.dart'; @@ -139,6 +140,44 @@ mixin ChannelStore on UserStore { } } + bool selfHasContentAccess(ZulipStream channel) { + // Compare web's stream_data.has_content_access. + if (channel.isWebPublic) return true; + if (channel is Subscription) return true; + // Here web calls has_metadata_access... but that always returns true, + // as its comment says. + if (selfUser.role == UserRole.guest) return false; + if (!channel.inviteOnly) return true; + return _selfHasContentAccessViaGroupPermissions(channel); + } + + bool _selfHasContentAccessViaGroupPermissions(ZulipStream channel) { + // Compare web's stream_data.has_content_access_via_group_permissions. + // TODO(#814) try to clean up this logic; perhaps record more explicitly + // what default/fallback value to use for a given group-based permission + // on older servers. + + if (channel.canAddSubscribersGroup != null + && selfHasPermissionForGroupSetting(channel.canAddSubscribersGroup!, + GroupSettingType.stream, 'can_add_subscribers_group')) { + // The behavior before this permission was introduced was equivalent to + // the "nobody" group. + // TODO(server-10): simplify + return true; + } + + if (channel.canSubscribeGroup != null + && selfHasPermissionForGroupSetting(channel.canSubscribeGroup!, + GroupSettingType.stream, 'can_subscribe_group')) { + // The behavior before this permission was introduced was equivalent to + // the "nobody" group. + // TODO(server-10): simplify + return true; + } + + return false; + } + bool hasPostingPermission({ required ZulipStream inChannel, required User user, diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index dcf82389bc..ce1be46000 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -445,10 +445,11 @@ void showChannelActionSheet(BuildContext context, { && messageListPageNarrow.streamId == channelId; final unreadCount = store.unreads.countInChannelNarrow(channelId); - final isSubscribed = store.subscriptions[channelId] != null; + final channel = store.streams[channelId]; + final isSubscribed = channel is Subscription; final buttonSections = [ - if (!isSubscribed) - // TODO(#1786) check group-based can-subscribe permission + if (!isSubscribed + && channel != null && store.selfHasContentAccess(channel)) [SubscribeButton(pageContext: pageContext, channelId: channelId)], [ if (unreadCount > 0)