From 6714098388c86b7260cad7fb2057160928135ad7 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 6 Aug 2024 14:33:47 +0430 Subject: [PATCH 1/8] api: Add `realmWaitingPeriodThreshold` to `InitialSnapshot` This is necessary in the next commit(s) where we need to determine if a user has become a full member. For how to determine if a user is a full member, see: https://zulip.com/api/roles-and-permissions#determining-if-a-user-is-a-full-member --- lib/api/model/initial_snapshot.dart | 9 +++++++++ lib/api/model/initial_snapshot.g.dart | 3 +++ test/example_data.dart | 2 ++ 3 files changed, 14 insertions(+) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index 6f45d1a050..6be0cadda9 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -64,6 +64,14 @@ class InitialSnapshot { final List? userTopics; // TODO(server-6) + /// The number of days until a user's account is treated as a full member. + /// + /// Search for "realm_waiting_period_threshold" in https://zulip.com/api/register-queue. + /// + /// For how to determine if a user is a full member, see: + /// https://zulip.com/api/roles-and-permissions#determining-if-a-user-is-a-full-member + final int realmWaitingPeriodThreshold; + final Map realmDefaultExternalAccounts; final int maxFileUploadSizeMib; @@ -117,6 +125,7 @@ class InitialSnapshot { required this.streams, required this.userSettings, required this.userTopics, + required this.realmWaitingPeriodThreshold, required this.realmDefaultExternalAccounts, required this.maxFileUploadSizeMib, required this.serverEmojiDataUrl, diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index 80cf0f6cc8..75472c1d7f 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -58,6 +58,8 @@ InitialSnapshot _$InitialSnapshotFromJson(Map json) => userTopics: (json['user_topics'] as List?) ?.map((e) => UserTopicItem.fromJson(e as Map)) .toList(), + realmWaitingPeriodThreshold: + (json['realm_waiting_period_threshold'] as num).toInt(), realmDefaultExternalAccounts: (json['realm_default_external_accounts'] as Map).map( (k, e) => MapEntry( @@ -106,6 +108,7 @@ Map _$InitialSnapshotToJson(InitialSnapshot instance) => 'streams': instance.streams, 'user_settings': instance.userSettings, 'user_topics': instance.userTopics, + 'realm_waiting_period_threshold': instance.realmWaitingPeriodThreshold, 'realm_default_external_accounts': instance.realmDefaultExternalAccounts, 'max_file_upload_size_mib': instance.maxFileUploadSizeMib, 'server_emoji_data_url': instance.serverEmojiDataUrl?.toString(), diff --git a/test/example_data.dart b/test/example_data.dart index 1ba04f626f..f3189a3481 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -824,6 +824,7 @@ InitialSnapshot initialSnapshot({ List? streams, UserSettings? userSettings, List? userTopics, + int? realmWaitingPeriodThreshold, Map? realmDefaultExternalAccounts, int? maxFileUploadSizeMib, Uri? serverEmojiDataUrl, @@ -857,6 +858,7 @@ InitialSnapshot initialSnapshot({ emojiset: Emojiset.google, ), userTopics: userTopics, + realmWaitingPeriodThreshold: realmWaitingPeriodThreshold ?? 0, realmDefaultExternalAccounts: realmDefaultExternalAccounts ?? {}, maxFileUploadSizeMib: maxFileUploadSizeMib ?? 25, serverEmojiDataUrl: serverEmojiDataUrl From bb867b45ce5d02d1c6a24b680aaf3923dbcf1101 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 6 Aug 2024 14:50:23 +0430 Subject: [PATCH 2/8] store: Add `realmWaitingPeriodThreshold` to `PerAccountStore` --- lib/model/store.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/model/store.dart b/lib/model/store.dart index ddce0217b1..28aa777adb 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -268,6 +268,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess globalStore: globalStore, connection: connection, realmUrl: realmUrl, + realmWaitingPeriodThreshold: initialSnapshot.realmWaitingPeriodThreshold, maxFileUploadSizeMib: initialSnapshot.maxFileUploadSizeMib, realmDefaultExternalAccounts: initialSnapshot.realmDefaultExternalAccounts, customProfileFields: _sortCustomProfileFields(initialSnapshot.customProfileFields), @@ -310,6 +311,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess required GlobalStore globalStore, required this.connection, required this.realmUrl, + required this.realmWaitingPeriodThreshold, required this.maxFileUploadSizeMib, required this.realmDefaultExternalAccounts, required this.customProfileFields, @@ -373,6 +375,8 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess Uri? tryResolveUrl(String reference) => _tryResolveUrl(realmUrl, reference); String get zulipVersion => account.zulipVersion; + /// For docs, please see [InitialSnapshot.realmWaitingPeriodThreshold]. + final int realmWaitingPeriodThreshold; // TODO(#668): update this realm setting final int maxFileUploadSizeMib; // No event for this. final Map realmDefaultExternalAccounts; List customProfileFields; From a06b1d5e42a43528e0b0503e143102c8ac15740c Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Mon, 12 Aug 2024 22:49:58 +0430 Subject: [PATCH 3/8] test [nfc]: Have `eg.user` accept `dateJoined` and `role` --- test/example_data.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index f3189a3481..c7739b1928 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -110,8 +110,10 @@ User user({ String? deliveryEmail, String? email, String? fullName, + String? dateJoined, bool? isActive, bool? isBot, + UserRole? role, String? avatarUrl, Map? profileData, }) { @@ -122,13 +124,13 @@ User user({ deliveryEmail: effectiveDeliveryEmail, email: email ?? effectiveDeliveryEmail, fullName: fullName ?? 'A user', // TODO generate example names - dateJoined: '2023-04-28', + dateJoined: dateJoined ?? '2024-02-24T11:18+00:00', isActive: isActive ?? true, isBillingAdmin: false, isBot: isBot ?? false, botType: null, botOwnerId: null, - role: UserRole.member, + role: role ?? UserRole.member, timezone: 'UTC', avatarUrl: avatarUrl, avatarVersion: 0, From 6b4c5693504a0a6c8c789a34adcdea0739ba539b Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Sat, 10 Aug 2024 23:43:53 +0430 Subject: [PATCH 4/8] model: Add `hasPassedWaitingPeriod` helper method to `PerAccountStore` Also add `UserRole.isAtLeast` method. --- lib/api/model/model.dart | 5 +++++ lib/model/store.dart | 22 ++++++++++++++++++++++ test/model/store_test.dart | 22 ++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index d732b18b5f..bd66976b86 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -303,6 +303,11 @@ enum UserRole{ final int? apiValue; int? toJson() => apiValue; + + bool isAtLeast(UserRole threshold) { + // Roles with more privilege have lower [apiValue]. + return apiValue! <= threshold.apiValue!; + } } /// As in `streams` in the initial snapshot. diff --git a/lib/model/store.dart b/lib/model/store.dart index 28aa777adb..85bb355813 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -434,6 +434,28 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess final TypingStatus typingStatus; + /// Whether [user] has passed the realm's waiting period to be a full member. + /// + /// See: + /// https://zulip.com/api/roles-and-permissions#determining-if-a-user-is-a-full-member + /// + /// To determine if a user is a full member, callers must also check that the + /// user's role is at least [UserRole.member]. + bool hasPassedWaitingPeriod(User user, {required DateTime byDate}) { + // [User.dateJoined] is in UTC. For logged-in users, the format is: + // YYYY-MM-DDTHH:mm+00:00, which includes the timezone offset for UTC. + // For logged-out spectators, the format is: YYYY-MM-DD, which doesn't + // include the timezone offset. In the later case, [DateTime.parse] will + // interpret it as the client's local timezone, which could lead to + // incorrect results; but that's acceptable for now because the app + // doesn't support viewing as a spectator. + // + // See the related discussion: + // https://chat.zulip.org/#narrow/channel/412-api-documentation/topic/provide.20an.20explicit.20format.20for.20.60realm_user.2Edate_joined.60/near/1980194 + final dateJoined = DateTime.parse(user.dateJoined); + return byDate.difference(dateJoined).inDays >= realmWaitingPeriodThreshold; + } + //////////////////////////////// // Streams, topics, and stuff about them. diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 6819dcfb6a..10f00dd0cf 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -247,6 +247,28 @@ void main() { }); }); + group('PerAccountStore.hasPassedWaitingPeriod', () { + final store = eg.store(initialSnapshot: + eg.initialSnapshot(realmWaitingPeriodThreshold: 2)); + + final testCases = [ + ('2024-11-25T10:00+00:00', DateTime.utc(2024, 11, 25 + 0, 10, 00), false), + ('2024-11-25T10:00+00:00', DateTime.utc(2024, 11, 25 + 1, 10, 00), false), + ('2024-11-25T10:00+00:00', DateTime.utc(2024, 11, 25 + 2, 09, 59), false), + ('2024-11-25T10:00+00:00', DateTime.utc(2024, 11, 25 + 2, 10, 00), true), + ('2024-11-25T10:00+00:00', DateTime.utc(2024, 11, 25 + 1000, 07, 00), true), + ]; + + for (final (String dateJoined, DateTime currentDate, bool hasPassedWaitingPeriod) in testCases) { + test('user joined at $dateJoined ${hasPassedWaitingPeriod ? 'has' : "hasn't"} ' + 'passed waiting period by $currentDate', () { + final user = eg.user(dateJoined: dateJoined); + check(store.hasPassedWaitingPeriod(user, byDate: currentDate)) + .equals(hasPassedWaitingPeriod); + }); + } + }); + group('PerAccountStore.handleEvent', () { // Mostly this method just dispatches to ChannelStore and MessageStore etc., // and so most of the tests live in the test files for those From 25c088ad961c98b8ceed77ee53797a9d1cd71759 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Sat, 2 Nov 2024 07:43:29 +0430 Subject: [PATCH 5/8] model: Add `hasPostingPermission` helper method to `PerAccountStore` --- lib/model/store.dart | 24 ++++++++++++ test/model/store_test.dart | 76 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/lib/model/store.dart b/lib/model/store.dart index 85bb355813..acb9d9bcae 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -474,6 +474,30 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess final ChannelStoreImpl _channels; + bool hasPostingPermission({ + required ZulipStream inChannel, + required User user, + required DateTime byDate, + }) { + final role = user.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) { + case ChannelPostPolicy.any: return true; + case ChannelPostPolicy.fullMembers: { + if (!role.isAtLeast(UserRole.member)) return false; + return role == UserRole.member + ? hasPassedWaitingPeriod(user, byDate: byDate) + : true; + } + case ChannelPostPolicy.moderators: return role.isAtLeast(UserRole.moderator); + case ChannelPostPolicy.administrators: return role.isAtLeast(UserRole.administrator); + case ChannelPostPolicy.unknown: return true; + } + } + //////////////////////////////// // Messages, and summaries of messages. diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 10f00dd0cf..ac9e64aa2a 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -269,6 +269,82 @@ void main() { } }); + group('PerAccountStore.hasPostingPermission', () { + 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), + (ChannelPostPolicy.administrators, UserRole.administrator, true), + (ChannelPostPolicy.administrators, UserRole.owner, true), + ]; + + for (final (ChannelPostPolicy policy, UserRole role, bool canPost) in testCases) { + test('"${role.name}" user ${canPost ? 'can' : "can't"} post in channel ' + 'with "${policy.name}" policy', () { + final store = eg.store(); + final actual = store.hasPostingPermission( + inChannel: eg.stream(channelPostPolicy: policy), user: eg.user(role: role), + // [byDate] is not actually relevant for these test cases; for the + // ones which it is, they're practiced below. + byDate: DateTime.now()); + check(actual).equals(canPost); + }); + } + + group('"member" user posting in a channel with "fullMembers" policy', () { + PerAccountStore localStore({required int realmWaitingPeriodThreshold}) => + eg.store(initialSnapshot: eg.initialSnapshot( + realmWaitingPeriodThreshold: realmWaitingPeriodThreshold)); + + User memberUser({required String dateJoined}) => eg.user( + role: UserRole.member, dateJoined: dateJoined); + + test('a "full" member -> can post in the channel', () { + final store = localStore(realmWaitingPeriodThreshold: 3); + final hasPermission = store.hasPostingPermission( + inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers), + user: memberUser(dateJoined: '2024-11-25T10:00+00:00'), + byDate: DateTime.utc(2024, 11, 28, 10, 00)); + check(hasPermission).isTrue(); + }); + + test('not a "full" member -> cannot post in the channel', () { + final store = localStore(realmWaitingPeriodThreshold: 3); + final actual = store.hasPostingPermission( + inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers), + user: memberUser(dateJoined: '2024-11-25T10:00+00:00'), + byDate: DateTime.utc(2024, 11, 28, 09, 59)); + check(actual).isFalse(); + }); + }); + }); + group('PerAccountStore.handleEvent', () { // Mostly this method just dispatches to ChannelStore and MessageStore etc., // and so most of the tests live in the test files for those From 4de8c23fe9f9669b093d18ed352f844930ad52ca Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Sat, 19 Oct 2024 07:05:04 +0430 Subject: [PATCH 6/8] test [nfc]: Add `streams` and `eg.selfUser` to `store` in several tests Adding these is necessary for the next commit(s), otherwise these tests will fail. --- test/widgets/action_sheet_test.dart | 2 +- test/widgets/autocomplete_test.dart | 2 + test/widgets/compose_box_test.dart | 57 +++++++++++++++++++---------- test/widgets/message_list_test.dart | 2 + 4 files changed, 42 insertions(+), 21 deletions(-) diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 28dc1956d3..3dade81b48 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -46,7 +46,7 @@ Future setupToMessageActionSheet(WidgetTester tester, { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - await store.addUser(eg.user(userId: message.senderId)); + await store.addUsers([eg.selfUser, eg.user(userId: message.senderId)]); if (message is StreamMessage) { final stream = eg.stream(streamId: message.streamId); await store.addStream(stream); diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index 18e3881891..34d0885967 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -77,10 +77,12 @@ Future setupToTopicInput(WidgetTester tester, { addTearDown(testBinding.reset); await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + await store.addUser(eg.selfUser); final connection = store.connection as FakeApiConnection; // prepare message list data final stream = eg.stream(); + await store.addStream(stream); final message = eg.streamMessage(stream: stream, sender: eg.selfUser); connection.prepare(json: GetMessagesResult( anchor: message.id, diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 666e1941f4..7e9ecf3d5e 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -39,14 +39,22 @@ void main() { final contentInputFinder = find.byWidgetPredicate( (widget) => widget is TextField && widget.controller is ComposeContentController); - Future> prepareComposeBox(WidgetTester tester, - {required Narrow narrow, List users = const []}) async { + Future> prepareComposeBox(WidgetTester tester, { + required Narrow narrow, + List users = const [], + List streams = const [], + }) async { + if (narrow case ChannelNarrow(:var streamId) || TopicNarrow(: var streamId)) { + assert(streams.any((stream) => stream.streamId == streamId), + 'Add a channel with "streamId" the same as of $narrow.streamId to the store.'); + } addTearDown(testBinding.reset); await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); await store.addUsers([eg.selfUser, ...users]); + await store.addStreams(streams); connection = store.connection as FakeApiConnection; final controllerKey = GlobalKey(); @@ -205,22 +213,25 @@ void main() { } testWidgets('_StreamComposeBox', (tester) async { + final channel = eg.stream(); final key = await prepareComposeBox(tester, - narrow: ChannelNarrow(eg.stream().streamId)); + narrow: ChannelNarrow(channel.streamId), streams: [channel]); checkComposeBoxTextFields(tester, controllerKey: key, expectTopicTextField: true); }); testWidgets('_FixedDestinationComposeBox', (tester) async { + final channel = eg.stream(); final key = await prepareComposeBox(tester, - narrow: TopicNarrow.ofMessage(eg.streamMessage())); + narrow: TopicNarrow(channel.streamId, 'topic'), streams: [channel]); checkComposeBoxTextFields(tester, controllerKey: key, expectTopicTextField: false); }); }); group('ComposeBox typing notices', () { - const narrow = TopicNarrow(123, 'some topic'); + final channel = eg.stream(); + final narrow = TopicNarrow(channel.streamId, 'some topic'); void checkTypingRequest(TypingOp op, SendableNarrow narrow) => checkSetTypingStatusRequests(connection.takeRequests(), [(op, narrow)]); @@ -232,7 +243,7 @@ void main() { } testWidgets('smoke TopicNarrow', (tester) async { - await prepareComposeBox(tester, narrow: narrow); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await checkStartTyping(tester, narrow); @@ -254,9 +265,9 @@ void main() { }); testWidgets('smoke ChannelNarrow', (tester) async { - const narrow = ChannelNarrow(123); + final narrow = ChannelNarrow(channel.streamId); final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic'); - await prepareComposeBox(tester, narrow: narrow); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await enterTopic(tester, narrow: narrow, topic: destinationNarrow.topic); await checkStartTyping(tester, destinationNarrow); @@ -267,7 +278,7 @@ void main() { }); testWidgets('clearing text sends a "typing stopped" notice', (tester) async { - await prepareComposeBox(tester, narrow: narrow); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await checkStartTyping(tester, narrow); @@ -277,7 +288,7 @@ void main() { }); testWidgets('hitting send button sends a "typing stopped" notice', (tester) async { - await prepareComposeBox(tester, narrow: narrow); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await checkStartTyping(tester, narrow); @@ -295,13 +306,14 @@ void main() { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + await store.addStream(channel); connection = store.connection as FakeApiConnection; await tester.pumpWidget(const ZulipApp()); await tester.pump(); final navigator = await ZulipApp.navigator; unawaited(navigator.push(MaterialAccountWidgetRoute( - accountId: eg.selfAccount.id, page: const ComposeBox(narrow: narrow)))); + accountId: eg.selfAccount.id, page: ComposeBox(narrow: narrow)))); await tester.pumpAndSettle(); } @@ -317,9 +329,9 @@ void main() { }); testWidgets('for content input, unfocusing sends a "typing stopped" notice', (tester) async { - const narrow = ChannelNarrow(123); + final narrow = ChannelNarrow(channel.streamId); final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic'); - await prepareComposeBox(tester, narrow: narrow); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await enterTopic(tester, narrow: narrow, topic: destinationNarrow.topic); await checkStartTyping(tester, destinationNarrow); @@ -331,7 +343,7 @@ void main() { }); testWidgets('selection change sends a "typing started" notice', (tester) async { - final controllerKey = await prepareComposeBox(tester, narrow: narrow); + final controllerKey = await prepareComposeBox(tester, narrow: narrow, streams: [channel]); final composeBoxController = controllerKey.currentState!; await checkStartTyping(tester, narrow); @@ -352,7 +364,7 @@ void main() { }); testWidgets('unfocusing app sends a "typing stopped" notice', (tester) async { - await prepareComposeBox(tester, narrow: narrow); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await checkStartTyping(tester, narrow); @@ -382,7 +394,8 @@ void main() { addTearDown(TypingNotifier.debugReset); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - await prepareComposeBox(tester, narrow: const TopicNarrow(123, 'some topic')); + await prepareComposeBox(tester, narrow: const TopicNarrow(123, 'some topic'), + streams: [eg.stream(streamId: 123)]); await tester.enterText(contentInputFinder, 'hello world'); @@ -447,8 +460,10 @@ void main() { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); - final narrow = ChannelNarrow(eg.stream().streamId); - final controllerKey = await prepareComposeBox(tester, narrow: narrow); + final channel = eg.stream(); + final narrow = ChannelNarrow(channel.streamId); + final controllerKey = await prepareComposeBox(tester, + narrow: narrow, streams: [channel]); final composeBoxController = controllerKey.currentState!; // (When we check that the send button looks disabled, it should be because @@ -507,8 +522,10 @@ void main() { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); - final narrow = ChannelNarrow(eg.stream().streamId); - final controllerKey = await prepareComposeBox(tester, narrow: narrow); + final channel = eg.stream(); + final narrow = ChannelNarrow(channel.streamId); + final controllerKey = await prepareComposeBox(tester, + narrow: narrow, streams: [channel]); final composeBoxController = controllerKey.currentState!; // (When we check that the send button looks disabled, it should be because diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 8e23e9f88d..444f78c4c9 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -103,6 +103,7 @@ void main() { testWidgets('MessageListPageState.narrow', (tester) async { final stream = eg.stream(); await setupMessageListPage(tester, narrow: ChannelNarrow(stream.streamId), + streams: [stream], messages: [eg.streamMessage(stream: stream, content: "

a message

")]); final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); check(state.narrow).equals(ChannelNarrow(stream.streamId)); @@ -111,6 +112,7 @@ void main() { testWidgets('composeBoxController finds compose box', (tester) async { final stream = eg.stream(); await setupMessageListPage(tester, narrow: ChannelNarrow(stream.streamId), + streams: [stream], messages: [eg.streamMessage(stream: stream, content: "

a message

")]); final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); check(state.composeBoxController).isNotNull(); From 388b83b2dfab438b76de405f8a6b1a8684b8d425 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Thu, 24 Oct 2024 23:14:15 +0430 Subject: [PATCH 7/8] compose test [nfc]: Add a parent test group This will make it ready for the next commit(s) where we add another sub-group test. --- test/widgets/compose_box_test.dart | 126 +++++++++++++++-------------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 7e9ecf3d5e..4c0193b9cc 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -580,7 +580,7 @@ void main() { }); }); - group('compose box in DMs with deactivated users', () { + group('error banner', () { Finder contentFieldFinder() => find.descendant( of: find.byType(ComposeBox), matching: find.byType(TextField)); @@ -607,81 +607,83 @@ void main() { checkBanner(isShown: !isShown); } - Future changeUserStatus(WidgetTester tester, - {required User user, required bool isActive}) async { - await store.handleEvent(RealmUserUpdateEvent(id: 1, - userId: user.userId, isActive: isActive)); - await tester.pump(); - } + group('in DMs with deactivated users', () { + Future changeUserStatus(WidgetTester tester, + {required User user, required bool isActive}) async { + await store.handleEvent(RealmUserUpdateEvent(id: 1, + userId: user.userId, isActive: isActive)); + await tester.pump(); + } - DmNarrow dmNarrowWith(User otherUser) => DmNarrow.withUser(otherUser.userId, - selfUserId: eg.selfUser.userId); + DmNarrow dmNarrowWith(User otherUser) => DmNarrow.withUser(otherUser.userId, + selfUserId: eg.selfUser.userId); - DmNarrow groupDmNarrowWith(List otherUsers) => DmNarrow.withOtherUsers( - otherUsers.map((u) => u.userId), selfUserId: eg.selfUser.userId); + DmNarrow groupDmNarrowWith(List otherUsers) => DmNarrow.withOtherUsers( + otherUsers.map((u) => u.userId), selfUserId: eg.selfUser.userId); - group('1:1 DMs', () { - testWidgets('compose box replaced with a banner', (tester) async { - final deactivatedUser = eg.user(isActive: false); - await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser), - users: [deactivatedUser]); - checkComposeBox(isShown: false); - }); + group('1:1 DMs', () { + testWidgets('compose box replaced with a banner', (tester) async { + final deactivatedUser = eg.user(isActive: false); + await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser), + users: [deactivatedUser]); + checkComposeBox(isShown: false); + }); - testWidgets('active user becomes deactivated -> ' - 'compose box is replaced with a banner', (tester) async { - final activeUser = eg.user(isActive: true); - await prepareComposeBox(tester, narrow: dmNarrowWith(activeUser), - users: [activeUser]); - checkComposeBox(isShown: true); + testWidgets('active user becomes deactivated -> ' + 'compose box is replaced with a banner', (tester) async { + final activeUser = eg.user(isActive: true); + await prepareComposeBox(tester, narrow: dmNarrowWith(activeUser), + users: [activeUser]); + checkComposeBox(isShown: true); - await changeUserStatus(tester, user: activeUser, isActive: false); - checkComposeBox(isShown: false); - }); + await changeUserStatus(tester, user: activeUser, isActive: false); + checkComposeBox(isShown: false); + }); - testWidgets('deactivated user becomes active -> ' - 'banner is replaced with the compose box', (tester) async { - final deactivatedUser = eg.user(isActive: false); - await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser), - users: [deactivatedUser]); - checkComposeBox(isShown: false); + testWidgets('deactivated user becomes active -> ' + 'banner is replaced with the compose box', (tester) async { + final deactivatedUser = eg.user(isActive: false); + await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser), + users: [deactivatedUser]); + checkComposeBox(isShown: false); - await changeUserStatus(tester, user: deactivatedUser, isActive: true); - checkComposeBox(isShown: true); + await changeUserStatus(tester, user: deactivatedUser, isActive: true); + checkComposeBox(isShown: true); + }); }); - }); - group('group DMs', () { - testWidgets('compose box replaced with a banner', (tester) async { - final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)]; - await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers), - users: deactivatedUsers); - checkComposeBox(isShown: false); - }); + group('group DMs', () { + testWidgets('compose box replaced with a banner', (tester) async { + final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)]; + await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers), + users: deactivatedUsers); + checkComposeBox(isShown: false); + }); - testWidgets('at least one user becomes deactivated -> ' - 'compose box is replaced with a banner', (tester) async { - final activeUsers = [eg.user(isActive: true), eg.user(isActive: true)]; - await prepareComposeBox(tester, narrow: groupDmNarrowWith(activeUsers), - users: activeUsers); - checkComposeBox(isShown: true); + testWidgets('at least one user becomes deactivated -> ' + 'compose box is replaced with a banner', (tester) async { + final activeUsers = [eg.user(isActive: true), eg.user(isActive: true)]; + await prepareComposeBox(tester, narrow: groupDmNarrowWith(activeUsers), + users: activeUsers); + checkComposeBox(isShown: true); - await changeUserStatus(tester, user: activeUsers[0], isActive: false); - checkComposeBox(isShown: false); - }); + await changeUserStatus(tester, user: activeUsers[0], isActive: false); + checkComposeBox(isShown: false); + }); - testWidgets('all deactivated users become active -> ' - 'banner is replaced with the compose box', (tester) async { - final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)]; - await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers), - users: deactivatedUsers); - checkComposeBox(isShown: false); + testWidgets('all deactivated users become active -> ' + 'banner is replaced with the compose box', (tester) async { + final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)]; + await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers), + users: deactivatedUsers); + checkComposeBox(isShown: false); - await changeUserStatus(tester, user: deactivatedUsers[0], isActive: true); - checkComposeBox(isShown: false); + await changeUserStatus(tester, user: deactivatedUsers[0], isActive: true); + checkComposeBox(isShown: false); - await changeUserStatus(tester, user: deactivatedUsers[1], isActive: true); - checkComposeBox(isShown: true); + await changeUserStatus(tester, user: deactivatedUsers[1], isActive: true); + checkComposeBox(isShown: true); + }); }); }); }); From 157418c26f003b783036dd5184364434cee7d197 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Thu, 24 Oct 2024 23:17:36 +0430 Subject: [PATCH 8/8] compose: Replace compose box with a banner when cannot post in a channel Fixes: #674 --- assets/l10n/app_en.arb | 4 + lib/widgets/compose_box.dart | 50 +++++++---- test/widgets/compose_box_test.dart | 138 +++++++++++++++++++++++++---- 3 files changed, 159 insertions(+), 33 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index c14d06fa22..ce6d8e7e68 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -216,6 +216,10 @@ "@errorBannerDeactivatedDmLabel": { "description": "Label text for error banner when sending a message to one or multiple deactivated users." }, + "errorBannerCannotPostInChannelLabel": "You do not have permission to post in this channel.", + "@errorBannerCannotPostInChannelLabel": { + "description": "Error-banner text replacing the compose box when you do not have permission to send a message to the channel." + }, "composeBoxAttachFilesTooltip": "Attach files", "@composeBoxAttachFilesTooltip": { "description": "Tooltip for compose box icon to attach a file to the message." diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index d1f3b93583..2060f420da 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1158,26 +1158,8 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox super.dispose(); } - Widget? _errorBanner(BuildContext context) { - if (widget.narrow case DmNarrow(:final otherRecipientIds)) { - final store = PerAccountStoreWidget.of(context); - final hasDeactivatedUser = otherRecipientIds.any((id) => - !(store.users[id]?.isActive ?? true)); - if (hasDeactivatedUser) { - return _ErrorBanner(label: ZulipLocalizations.of(context) - .errorBannerDeactivatedDmLabel); - } - } - return null; - } - @override Widget build(BuildContext context) { - final errorBanner = _errorBanner(context); - if (errorBanner != null) { - return _ComposeBoxContainer(child: errorBanner); - } - return _ComposeBoxLayout( contentController: _contentController, contentFocusNode: _contentFocusNode, @@ -1215,8 +1197,40 @@ class ComposeBox extends StatelessWidget { } } + Widget? _errorBanner(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final selfUser = store.users[store.selfUserId]!; + switch (narrow) { + case ChannelNarrow(:final streamId): + case TopicNarrow(:final streamId): + final channel = store.streams[streamId]; + if (channel == null || !store.hasPostingPermission(inChannel: channel, + user: selfUser, byDate: DateTime.now())) { + return _ErrorBanner(label: + ZulipLocalizations.of(context).errorBannerCannotPostInChannelLabel); + } + case DmNarrow(:final otherRecipientIds): + final hasDeactivatedUser = otherRecipientIds.any((id) => + !(store.users[id]?.isActive ?? true)); + if (hasDeactivatedUser) { + return _ErrorBanner(label: + ZulipLocalizations.of(context).errorBannerDeactivatedDmLabel); + } + case CombinedFeedNarrow(): + case MentionsNarrow(): + case StarredMessagesNarrow(): + return null; + } + return null; + } + @override Widget build(BuildContext context) { + final errorBanner = _errorBanner(context); + if (errorBanner != null) { + return _ComposeBoxContainer(child: errorBanner); + } + final narrow = this.narrow; switch (narrow) { case ChannelNarrow(): diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 4c0193b9cc..008b0f39c8 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -41,6 +41,8 @@ void main() { Future> prepareComposeBox(WidgetTester tester, { required Narrow narrow, + User? selfUser, + int? realmWaitingPeriodThreshold, List users = const [], List streams = const [], }) async { @@ -49,16 +51,19 @@ void main() { 'Add a channel with "streamId" the same as of $narrow.streamId to the store.'); } addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + selfUser ??= eg.selfUser; + final selfAccount = eg.account(user: selfUser); + await testBinding.globalStore.add(selfAccount, eg.initialSnapshot( + realmWaitingPeriodThreshold: realmWaitingPeriodThreshold)); - store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + store = await testBinding.globalStore.perAccount(selfAccount.id); - await store.addUsers([eg.selfUser, ...users]); + await store.addUsers([selfUser, ...users]); await store.addStreams(streams); connection = store.connection as FakeApiConnection; final controllerKey = GlobalKey(); - await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id, child: Column( // This positions the compose box at the bottom of the screen, // simulating the layout of the message list page. @@ -303,9 +308,12 @@ void main() { Future prepareComposeBoxWithNavigation(WidgetTester tester) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final selfUser = eg.selfUser; + final selfAccount = eg.account(user: selfUser); + await testBinding.globalStore.add(selfAccount, eg.initialSnapshot()); - store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + store = await testBinding.globalStore.perAccount(selfAccount.id); + await store.addUser(selfUser); await store.addStream(channel); connection = store.connection as FakeApiConnection; @@ -313,7 +321,7 @@ void main() { await tester.pump(); final navigator = await ZulipApp.navigator; unawaited(navigator.push(MaterialAccountWidgetRoute( - accountId: eg.selfAccount.id, page: ComposeBox(narrow: narrow)))); + accountId: selfAccount.id, page: ComposeBox(narrow: narrow)))); await tester.pumpAndSettle(); } @@ -581,7 +589,9 @@ void main() { }); group('error banner', () { - Finder contentFieldFinder() => find.descendant( + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + + Finder inputFieldFinder() => find.descendant( of: find.byType(ComposeBox), matching: find.byType(TextField)); @@ -590,24 +600,26 @@ void main() { matching: find.widgetWithIcon(IconButton, icon)); void checkComposeBoxParts({required bool areShown}) { - check(contentFieldFinder().evaluate().length).equals(areShown ? 1 : 0); + final inputFieldCount = inputFieldFinder().evaluate().length; + areShown ? check(inputFieldCount).isGreaterThan(0) : check(inputFieldCount).equals(0); check(attachButtonFinder(Icons.attach_file).evaluate().length).equals(areShown ? 1 : 0); check(attachButtonFinder(Icons.image).evaluate().length).equals(areShown ? 1 : 0); check(attachButtonFinder(Icons.camera_alt).evaluate().length).equals(areShown ? 1 : 0); } - void checkBanner({required bool isShown}) { - final bannerTextFinder = find.text(GlobalLocalizations.zulipLocalizations - .errorBannerDeactivatedDmLabel); - check(bannerTextFinder.evaluate().length).equals(isShown ? 1 : 0); + void checkBannerWithLabel(String label, {required bool isShown}) { + check(find.text(label).evaluate().length).equals(isShown ? 1 : 0); } - void checkComposeBox({required bool isShown}) { + void checkComposeBoxIsShown(bool isShown, {required String bannerLabel}) { checkComposeBoxParts(areShown: isShown); - checkBanner(isShown: !isShown); + checkBannerWithLabel(bannerLabel, isShown: !isShown); } group('in DMs with deactivated users', () { + void checkComposeBox({required bool isShown}) => checkComposeBoxIsShown(isShown, + bannerLabel: zulipLocalizations.errorBannerDeactivatedDmLabel); + Future changeUserStatus(WidgetTester tester, {required User user, required bool isActive}) async { await store.handleEvent(RealmUserUpdateEvent(id: 1, @@ -686,5 +698,101 @@ void main() { }); }); }); + + group('in channel/topic narrow according to channel post policy', () { + void checkComposeBox({required bool isShown}) => checkComposeBoxIsShown(isShown, + bannerLabel: zulipLocalizations.errorBannerCannotPostInChannelLabel); + + final narrowTestCases = [ + ('channel', const ChannelNarrow(1)), + ('topic', const TopicNarrow(1, 'topic')), + ]; + + for (final (String narrowType, Narrow narrow) in narrowTestCases) { + testWidgets('compose box is shown in $narrowType narrow', (tester) async { + await prepareComposeBox(tester, + narrow: narrow, + selfUser: eg.user(role: UserRole.administrator), + streams: [eg.stream(streamId: 1, + channelPostPolicy: ChannelPostPolicy.moderators)]); + checkComposeBox(isShown: true); + }); + + testWidgets('error banner is shown in $narrowType narrow', (tester) async { + await prepareComposeBox(tester, + narrow: narrow, + selfUser: eg.user(role: UserRole.moderator), + streams: [eg.stream(streamId: 1, + channelPostPolicy: ChannelPostPolicy.administrators)]); + checkComposeBox(isShown: false); + }); + } + + testWidgets('user loses privilege -> compose box is replaced with the banner', (tester) async { + final selfUser = eg.user(role: UserRole.administrator); + await prepareComposeBox(tester, + narrow: const ChannelNarrow(1), + selfUser: selfUser, + streams: [eg.stream(streamId: 1, + channelPostPolicy: ChannelPostPolicy.administrators)]); + checkComposeBox(isShown: true); + + await store.handleEvent(RealmUserUpdateEvent(id: 1, + userId: selfUser.userId, role: UserRole.moderator)); + await tester.pump(); + checkComposeBox(isShown: false); + }); + + testWidgets('user gains privilege -> banner is replaced with the compose box', (tester) async { + final selfUser = eg.user(role: UserRole.guest); + await prepareComposeBox(tester, + narrow: const ChannelNarrow(1), + selfUser: selfUser, + streams: [eg.stream(streamId: 1, + channelPostPolicy: ChannelPostPolicy.moderators)]); + checkComposeBox(isShown: false); + + await store.handleEvent(RealmUserUpdateEvent(id: 1, + userId: selfUser.userId, role: UserRole.administrator)); + await tester.pump(); + checkComposeBox(isShown: true); + }); + + testWidgets('channel policy becomes stricter -> compose box is replaced with the banner', (tester) async { + final selfUser = eg.user(role: UserRole.guest); + final channel = eg.stream(streamId: 1, + channelPostPolicy: ChannelPostPolicy.any); + + await prepareComposeBox(tester, + narrow: const ChannelNarrow(1), + selfUser: selfUser, + streams: [channel]); + checkComposeBox(isShown: true); + + await store.handleEvent(eg.channelUpdateEvent(channel, + property: ChannelPropertyName.channelPostPolicy, + value: ChannelPostPolicy.fullMembers)); + await tester.pump(); + checkComposeBox(isShown: false); + }); + + testWidgets('channel policy becomes less strict -> banner is replaced with the compose box', (tester) async { + final selfUser = eg.user(role: UserRole.moderator); + final channel = eg.stream(streamId: 1, + channelPostPolicy: ChannelPostPolicy.administrators); + + await prepareComposeBox(tester, + narrow: const ChannelNarrow(1), + selfUser: selfUser, + streams: [channel]); + checkComposeBox(isShown: false); + + await store.handleEvent(eg.channelUpdateEvent(channel, + property: ChannelPropertyName.channelPostPolicy, + value: ChannelPostPolicy.moderators)); + await tester.pump(); + checkComposeBox(isShown: true); + }); + }); }); }