From a1070e88a616efca84ed2f556cf517b15cbae3c6 Mon Sep 17 00:00:00 2001 From: Komyyy Date: Mon, 17 Feb 2025 16:36:03 +0900 Subject: [PATCH 1/4] message_list: make current time testable --- lib/widgets/message_list.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index e25445e514..ab11dac0a0 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -5,6 +5,7 @@ import 'package:intl/intl.dart' hide TextDirection; import '../api/model/model.dart'; import '../generated/l10n/zulip_localizations.dart'; +import '../model/binding.dart'; import '../model/message_list.dart'; import '../model/narrow.dart'; import '../model/store.dart'; @@ -1370,7 +1371,7 @@ class DateText extends StatelessWidget { formatHeaderDate( zulipLocalizations, DateTime.fromMillisecondsSinceEpoch(timestamp * 1000), - now: DateTime.now())); + now: ZulipBinding.instance.utcNow().toLocal())); } } From 8741f480f783d2a013b12aa0cb4e926fd42ed8e7 Mon Sep 17 00:00:00 2001 From: Komyyy Date: Mon, 17 Feb 2025 16:39:25 +0900 Subject: [PATCH 2/4] compose_box: make current time testable --- lib/widgets/compose_box.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 2f47f05f44..74172e3963 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -2016,7 +2016,7 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM case TopicNarrow(:final streamId): final channel = store.streams[streamId]; if (channel == null || !store.hasPostingPermission(inChannel: channel, - user: store.selfUser, byDate: DateTime.now())) { + user: store.selfUser, byDate: ZulipBinding.instance.utcNow())) { return _ErrorBanner(getLabel: (zulipLocalizations) => zulipLocalizations.errorBannerCannotPostInChannelLabel); } From 8a7a80abf6ef0a6ef9fc076f97da47eec57d9731 Mon Sep 17 00:00:00 2001 From: Komyyy Date: Mon, 17 Feb 2025 19:43:48 +0900 Subject: [PATCH 3/4] per_account_store: reduce redundant arguments --- lib/model/store.dart | 9 ++++---- lib/widgets/compose_box.dart | 3 +-- test/model/store_test.dart | 42 ++++++++++++++++++++---------------- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/lib/model/store.dart b/lib/model/store.dart index 18a09e32ce..6c299b5bd3 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -21,6 +21,7 @@ import '../log.dart'; import '../notifications/receive.dart'; import 'actions.dart'; import 'autocomplete.dart'; +import 'binding.dart'; import 'database.dart'; import 'emoji.dart'; import 'localizations.dart'; @@ -656,7 +657,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor /// /// 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}) { + bool hasPassedWaitingPeriod(User user) { // [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 @@ -668,7 +669,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor // 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; + final now = ZulipBinding.instance.utcNow(); + return now.difference(dateJoined).inDays >= realmWaitingPeriodThreshold; } /// The given user's real email address, if known, for displaying in the UI. @@ -719,7 +721,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor 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 @@ -731,7 +732,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor case ChannelPostPolicy.fullMembers: { if (!role.isAtLeast(UserRole.member)) return false; return role == UserRole.member - ? hasPassedWaitingPeriod(user, byDate: byDate) + ? hasPassedWaitingPeriod(user) : true; } case ChannelPostPolicy.moderators: return role.isAtLeast(UserRole.moderator); diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 74172e3963..47961e6d60 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -2015,8 +2015,7 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM case ChannelNarrow(:final streamId): case TopicNarrow(:final streamId): final channel = store.streams[streamId]; - if (channel == null || !store.hasPostingPermission(inChannel: channel, - user: store.selfUser, byDate: ZulipBinding.instance.utcNow())) { + if (channel == null || !store.hasPostingPermission(inChannel: channel, user: store.selfUser)) { return _ErrorBanner(getLabel: (zulipLocalizations) => zulipLocalizations.errorBannerCannotPostInChannelLabel); } diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 0b303b53e2..69de5a37d9 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:io'; import 'package:checks/checks.dart'; +import 'package:clock/clock.dart'; import 'package:fake_async/fake_async.dart'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; @@ -478,9 +479,11 @@ void main() { 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); + withClock(Clock.fixed(currentDate), () { + final user = eg.user(dateJoined: dateJoined); + check(store.hasPassedWaitingPeriod(user)) + .equals(hasPassedWaitingPeriod); + }); }); } }); @@ -524,11 +527,10 @@ void main() { test('"${role.name}" user ${canPost ? 'can' : "can't"} post in channel ' 'with "${policy.name}" policy', () { final store = eg.store(); + // we don't use `withClock` because current time is not actually relevant for + // these test cases; for the ones which it is, they're practiced below. 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()); + inChannel: eg.stream(channelPostPolicy: policy), user: eg.user(role: role)); check(actual).equals(canPost); }); } @@ -542,21 +544,23 @@ void main() { 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(); + withClock(Clock.fixed(DateTime.utc(2024, 11, 28, 10, 00)), () { + final store = localStore(realmWaitingPeriodThreshold: 3); + final hasPermission = store.hasPostingPermission( + inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers), + user: memberUser(dateJoined: '2024-11-25T10:00+00: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(); + withClock(Clock.fixed(DateTime.utc(2024, 11, 28, 09, 59)), () { + final store = localStore(realmWaitingPeriodThreshold: 3); + final actual = store.hasPostingPermission( + inChannel: eg.stream(channelPostPolicy: ChannelPostPolicy.fullMembers), + user: memberUser(dateJoined: '2024-11-25T10:00+00:00')); + check(actual).isFalse(); + }); }); }); }); From 80ac1f34d5bfa0da1fa086d353537e5871f0adf0 Mon Sep 17 00:00:00 2001 From: Komyyy Date: Mon, 17 Feb 2025 19:46:29 +0900 Subject: [PATCH 4/4] message_list: reduce redundant arguments --- lib/widgets/message_list.dart | 14 +++++++------- test/widgets/message_list_test.dart | 8 +++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index ab11dac0a0..431643dcae 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1370,19 +1370,19 @@ class DateText extends StatelessWidget { ), formatHeaderDate( zulipLocalizations, - DateTime.fromMillisecondsSinceEpoch(timestamp * 1000), - now: ZulipBinding.instance.utcNow().toLocal())); + DateTime.fromMillisecondsSinceEpoch(timestamp * 1000))); } } @visibleForTesting String formatHeaderDate( ZulipLocalizations zulipLocalizations, - DateTime dateTime, { - required DateTime now, -}) { - assert(!dateTime.isUtc && !now.isUtc, - '`dateTime` and `now` need to be in local time.'); + DateTime dateTime, +) { + assert(!dateTime.isUtc, + '`dateTime` need to be in local time.'); + + final now = ZulipBinding.instance.utcNow().toLocal(); if (dateTime.year == now.year && dateTime.month == now.month && diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 81cc384ef8..d0e1f43e06 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:checks/checks.dart'; +import 'package:clock/clock.dart'; import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; @@ -1421,7 +1422,6 @@ void main() { group('formatHeaderDate', () { final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - final now = DateTime.parse("2023-01-10 12:00"); final testCases = [ ("2023-01-10 12:00", zulipLocalizations.today), ("2023-01-10 00:00", zulipLocalizations.today), @@ -1436,8 +1436,10 @@ void main() { ]; for (final (dateTime, expected) in testCases) { test('$dateTime returns $expected', () { - check(formatHeaderDate(zulipLocalizations, DateTime.parse(dateTime), now: now)) - .equals(expected); + withClock(Clock.fixed(DateTime.parse("2023-01-10 12:00")), () { + check(formatHeaderDate(zulipLocalizations, DateTime.parse(dateTime))) + .equals(expected); + }); }); } });