From 7dce4e84aa495c341480dd8af2520da643da3c89 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 01/12] api: Mark User.isBillingAdmin as required, relying on server 5+, FL 73+ See "Feature level 73" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/model/model.dart | 2 +- lib/api/model/model.g.dart | 2 +- test/api/model/model_checks.dart | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index ae4fb7b5a3..bf059794fc 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -225,7 +225,7 @@ class User { // bool isOwner; // obsoleted by [role]; ignore // bool isAdmin; // obsoleted by [role]; ignore // bool isGuest; // obsoleted by [role]; ignore - bool? isBillingAdmin; // TODO(server-5) + bool isBillingAdmin; final bool isBot; final int? botType; // TODO enum int? botOwnerId; diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 453d204737..04003f2e24 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -101,7 +101,7 @@ User _$UserFromJson(Map json) => User( fullName: json['full_name'] as String, dateJoined: json['date_joined'] as String, isActive: json['is_active'] as bool, - isBillingAdmin: json['is_billing_admin'] as bool?, + isBillingAdmin: json['is_billing_admin'] as bool, isBot: json['is_bot'] as bool, botType: (json['bot_type'] as num?)?.toInt(), botOwnerId: (json['bot_owner_id'] as num?)?.toInt(), diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index f882233993..4885c13161 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -9,7 +9,7 @@ extension UserChecks on Subject { Subject get fullName => has((x) => x.fullName, 'fullName'); Subject get dateJoined => has((x) => x.dateJoined, 'dateJoined'); Subject get isActive => has((x) => x.isActive, 'isActive'); - Subject get isBillingAdmin => has((x) => x.isBillingAdmin, 'isBillingAdmin'); + Subject get isBillingAdmin => has((x) => x.isBillingAdmin, 'isBillingAdmin'); Subject get isBot => has((x) => x.isBot, 'isBot'); Subject get botType => has((x) => x.botType, 'botType'); Subject get botOwnerId => has((x) => x.botOwnerId, 'botOwnerId'); From 9176452a66371748b447654b01f64f752b5e9853 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 02/12] api: Drop is_cross_realm_bot fallback for User.isSystemBot, relying on server 5+, FL 83+ See "Feature level 83" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/model/model.dart | 12 +++--------- lib/api/model/model.g.dart | 2 +- test/api/model/model_test.dart | 1 - 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index bf059794fc..034c7d29cf 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -241,7 +241,9 @@ class User { @JsonKey(readValue: _readProfileData) Map? profileData; - @JsonKey(readValue: _readIsSystemBot) + // This field is absent in `realm_users` and `realm_non_active_users`, + // which contain no system bots; it's present in `cross_realm_bots`. + @JsonKey(defaultValue: false) final bool isSystemBot; static Map? _readProfileData(Map json, String key) { @@ -253,14 +255,6 @@ class User { return (value != null && value.isNotEmpty) ? value : null; } - static bool _readIsSystemBot(Map json, String key) { - // This field is absent in `realm_users` and `realm_non_active_users`, - // which contain no system bots; it's present in `cross_realm_bots`. - return (json[key] as bool?) - ?? (json['is_cross_realm_bot'] as bool?) // TODO(server-5): renamed to `is_system_bot` - ?? false; - } - User({ required this.userId, required this.deliveryEmail, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 04003f2e24..b8eb56a774 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -121,7 +121,7 @@ User _$UserFromJson(Map json) => User( ProfileFieldUserData.fromJson(e as Map), ), ), - isSystemBot: User._readIsSystemBot(json, 'is_system_bot') as bool, + isSystemBot: json['is_system_bot'] as bool? ?? false, ); Map _$UserToJson(User instance) => { diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index 7ace2326ee..c28d2fbb05 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -62,7 +62,6 @@ void main() { test('is_system_bot', () { check(mkUser({}).isSystemBot).isFalse(); - check(mkUser({'is_cross_realm_bot': true}).isSystemBot).isTrue(); check(mkUser({'is_system_bot': true}).isSystemBot).isTrue(); }); }); From 9f15154d9d1d44377eedf2553eba1afce7f3ab68 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 03/12] TODO api: Add TODO to modernize GetServerSettingsResult.zulipMergeBase Marked as TODO as this might change in the future. See https://github.com/zulip/zulip-flutter/pull/904#discussion_r1747943940. See "Feature level 88" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/route/realm.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/api/route/realm.dart b/lib/api/route/realm.dart index a43c2e9921..b37d0cc995 100644 --- a/lib/api/route/realm.dart +++ b/lib/api/route/realm.dart @@ -36,7 +36,8 @@ class GetServerSettingsResult { final int zulipFeatureLevel; final String zulipVersion; - final String? zulipMergeBase; // TODO(server-5) + // TODO(server-5): Modernize this once we get to #267. + final String? zulipMergeBase; final bool pushNotificationsEnabled; final bool isIncompatible; From 4fa341a9b056ae6cef5e1134a18be4f36ad3290d Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 04/12] api: Mark InitialSnapshot.zulipMergeBase as required, relying on server 5+, FL 88+ See "Feature level 88" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/model/initial_snapshot.dart | 2 +- lib/api/model/initial_snapshot.g.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index a9efdabdd5..dc2299bb9d 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -18,7 +18,7 @@ class InitialSnapshot { final int lastEventId; final int zulipFeatureLevel; final String zulipVersion; - final String? zulipMergeBase; // TODO(server-5) + final String zulipMergeBase; final List alertWords; diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index c33c457226..1d35e647fd 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -15,7 +15,7 @@ InitialSnapshot _$InitialSnapshotFromJson( lastEventId: (json['last_event_id'] as num).toInt(), zulipFeatureLevel: (json['zulip_feature_level'] as num).toInt(), zulipVersion: json['zulip_version'] as String, - zulipMergeBase: json['zulip_merge_base'] as String?, + zulipMergeBase: json['zulip_merge_base'] as String, alertWords: (json['alert_words'] as List) .map((e) => e as String) .toList(), From 100d25e365d8015c8a7027a433696c2c17132e89 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 05/12] api: Mark {InitialSnapshot,PerAccountStore}.userSettings as required, relying on server 5+, FL 89+ See "Feature level 89" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/model/initial_snapshot.dart | 8 +------- lib/api/model/initial_snapshot.g.dart | 6 +++--- lib/model/emoji.dart | 4 ++-- lib/model/store.dart | 8 ++++---- lib/widgets/emoji_reaction.dart | 2 +- test/model/store_checks.dart | 2 +- test/model/store_test.dart | 8 ++++---- 7 files changed, 16 insertions(+), 22 deletions(-) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index dc2299bb9d..fa978ccc50 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -66,13 +66,7 @@ class InitialSnapshot { final List streams; - // Servers pre-5.0 don't have `user_settings`, and instead provide whatever - // user settings they support at toplevel in the initial snapshot. Since we're - // likely to desupport pre-5.0 servers before wide release, we prefer to - // ignore the toplevel fields and use `user_settings` where present instead, - // even at the expense of functionality with pre-5.0 servers. - // TODO(server-5) remove pre-5.0 comment - final UserSettings? userSettings; // TODO(server-5) + final UserSettings userSettings; final List? userTopics; // TODO(server-6) diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index 1d35e647fd..7f6821d108 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -70,9 +70,9 @@ InitialSnapshot _$InitialSnapshotFromJson( streams: (json['streams'] as List) .map((e) => ZulipStream.fromJson(e as Map)) .toList(), - userSettings: json['user_settings'] == null - ? null - : UserSettings.fromJson(json['user_settings'] as Map), + userSettings: UserSettings.fromJson( + json['user_settings'] as Map, + ), userTopics: (json['user_topics'] as List?) ?.map((e) => UserTopicItem.fromJson(e as Map)) .toList(), diff --git a/lib/model/emoji.dart b/lib/model/emoji.dart index 0923fdab79..7f2df6f9c5 100644 --- a/lib/model/emoji.dart +++ b/lib/model/emoji.dart @@ -18,9 +18,9 @@ sealed class EmojiDisplay { EmojiDisplay({required this.emojiName}); - EmojiDisplay resolve(UserSettings? userSettings) { // TODO(server-5) + EmojiDisplay resolve(UserSettings userSettings) { if (this is TextEmojiDisplay) return this; - if (userSettings?.emojiset == Emojiset.text) { + if (userSettings.emojiset == Emojiset.text) { return TextEmojiDisplay(emojiName: emojiName); } return this; diff --git a/lib/model/store.dart b/lib/model/store.dart index b3ce59206a..35a952e36c 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -644,7 +644,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor //////////////////////////////// // Data attached to the self-account on the realm. - final UserSettings? userSettings; // TODO(server-5) + final UserSettings userSettings; @override Map get savedSnippets => _savedSnippets.savedSnippets; @@ -884,11 +884,11 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor } switch (event.property!) { case UserSettingName.twentyFourHourTime: - userSettings?.twentyFourHourTime = event.value as bool; + userSettings.twentyFourHourTime = event.value as bool; case UserSettingName.displayEmojiReactionUsers: - userSettings?.displayEmojiReactionUsers = event.value as bool; + userSettings.displayEmojiReactionUsers = event.value as bool; case UserSettingName.emojiset: - userSettings?.emojiset = event.value as Emojiset; + userSettings.emojiset = event.value as Emojiset; } notifyListeners(); diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 1f5dc557ec..b400940ec6 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -121,7 +121,7 @@ class ReactionChipsList extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final displayEmojiReactionUsers = store.userSettings?.displayEmojiReactionUsers ?? false; + final displayEmojiReactionUsers = store.userSettings.displayEmojiReactionUsers ?? false; final showNames = displayEmojiReactionUsers && reactions.total <= 3; return Wrap(spacing: 4, runSpacing: 4, crossAxisAlignment: WrapCrossAlignment.center, diff --git a/test/model/store_checks.dart b/test/model/store_checks.dart index 93e24dffdd..e5f61c05a1 100644 --- a/test/model/store_checks.dart +++ b/test/model/store_checks.dart @@ -55,7 +55,7 @@ extension PerAccountStoreChecks on Subject { Subject get accountId => has((x) => x.accountId, 'accountId'); Subject get account => has((x) => x.account, 'account'); Subject get selfUserId => has((x) => x.selfUserId, 'selfUserId'); - Subject get userSettings => has((x) => x.userSettings, 'userSettings'); + Subject get userSettings => has((x) => x.userSettings, 'userSettings'); Subject> get savedSnippets => has((x) => x.savedSnippets, 'savedSnippets'); Subject> get streams => has((x) => x.streams, 'streams'); Subject> get streamsByName => has((x) => x.streamsByName, 'streamsByName'); diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 68f9503fce..3736064f84 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -821,14 +821,14 @@ void main() { await preparePoll(); // Pick some arbitrary event and check it gets processed on the store. - check(store.userSettings!.twentyFourHourTime).isFalse(); + check(store.userSettings.twentyFourHourTime).isFalse(); connection.prepare(json: GetEventsResult(events: [ UserSettingsUpdateEvent(id: 2, property: UserSettingName.twentyFourHourTime, value: true), ], queueId: null).toJson()); updateMachine.debugAdvanceLoop(); async.elapse(Duration.zero); - check(store.userSettings!.twentyFourHourTime).isTrue(); + check(store.userSettings.twentyFourHourTime).isTrue(); })); void checkReload(FutureOr Function() prepareError, { @@ -858,14 +858,14 @@ void main() { // The new UpdateMachine updates the new store. updateMachine.debugPauseLoop(); updateMachine.poll(); - check(store.userSettings!.twentyFourHourTime).isFalse(); + check(store.userSettings.twentyFourHourTime).isFalse(); connection.prepare(json: GetEventsResult(events: [ UserSettingsUpdateEvent(id: 2, property: UserSettingName.twentyFourHourTime, value: true), ], queueId: null).toJson()); updateMachine.debugAdvanceLoop(); async.elapse(Duration.zero); - check(store.userSettings!.twentyFourHourTime).isTrue(); + check(store.userSettings.twentyFourHourTime).isTrue(); }); } From d209cf73f1ef14fbf095e93ac9c640bcb4c39884 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 22 Aug 2024 15:04:59 -0400 Subject: [PATCH 06/12] api: Mark WebAuthPayload.userId as required, relying on server 5+, FL 108+. See "Feature level 108" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/model/web_auth.dart | 13 ++++--------- lib/widgets/login.dart | 3 +-- test/api/model/web_auth_test.dart | 15 +-------------- 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/lib/api/model/web_auth.dart b/lib/api/model/web_auth.dart index 490c4b79db..5029a8e39f 100644 --- a/lib/api/model/web_auth.dart +++ b/lib/api/model/web_auth.dart @@ -7,7 +7,7 @@ import 'package:flutter/foundation.dart'; class WebAuthPayload { final Uri realm; final String email; - final int? userId; // TODO(server-5) new in FL 108 + final int userId; final String otpEncryptedApiKey; WebAuthPayload._({ @@ -25,7 +25,7 @@ class WebAuthPayload { queryParameters: { 'realm': String realmStr, 'email': String email, - // 'user_id' handled below + 'user_id': String userIdStr, 'otp_encrypted_api_key': String otpEncryptedApiKey, }, ) @@ -33,13 +33,8 @@ class WebAuthPayload { final Uri? realm = Uri.tryParse(realmStr); if (realm == null) throw const FormatException(); - // TODO(server-5) require in queryParameters (new in FL 108) - final userIdStr = url.queryParameters['user_id']; - int? userId; - if (userIdStr != null) { - userId = int.tryParse(userIdStr, radix: 10); - if (userId == null) throw const FormatException(); - } + final userId = int.tryParse(userIdStr, radix: 10); + if (userId == null) throw const FormatException(); if (!RegExp(r'^[0-9a-fA-F]{64}$').hasMatch(otpEncryptedApiKey)) { throw const FormatException(); diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index bdba3eac0d..615855e59a 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -309,8 +309,7 @@ class _LoginPageState extends State { if (payload.realm.origin != widget.serverSettings.realmUrl.origin) throw Error(); final apiKey = payload.decodeApiKey(_otp!); await _tryInsertAccountAndNavigate( - // TODO(server-5): Rely on userId from payload. - userId: payload.userId ?? await _getUserId(payload.email, apiKey), + userId: payload.userId, email: payload.email, apiKey: apiKey, ); diff --git a/test/api/model/web_auth_test.dart b/test/api/model/web_auth_test.dart index 01a670103f..7f8c326ed6 100644 --- a/test/api/model/web_auth_test.dart +++ b/test/api/model/web_auth_test.dart @@ -23,19 +23,6 @@ void main() { check(payload.decodeApiKey(otp)).equals(eg.selfAccount.apiKey); }); - // TODO(server-5) remove - test('legacy: no userId', () { - final queryParams = {...wellFormed.queryParameters}..remove('user_id'); - final payload = WebAuthPayload.parse( - wellFormed.replace(queryParameters: queryParams)); - check(payload) - ..otpEncryptedApiKey.equals(encryptedApiKey) - ..email.equals('self@example') - ..userId.isNull() - ..realm.equals(Uri.parse('https://chat.example/')); - check(payload.decodeApiKey(otp)).equals(eg.selfAccount.apiKey); - }); - test('parse fails when an expected field is missing', () { final queryParams = {...wellFormed.queryParameters}..remove('email'); final input = wellFormed.replace(queryParameters: queryParams); @@ -93,6 +80,6 @@ void main() { extension WebAuthPayloadChecks on Subject { Subject get otpEncryptedApiKey => has((x) => x.otpEncryptedApiKey, 'otpEncryptedApiKey'); Subject get email => has((x) => x.email, 'email'); - Subject get userId => has((x) => x.userId, 'userId'); + Subject get userId => has((x) => x.userId, 'userId'); Subject get realm => has((x) => x.realm, 'realm'); } From 7a4c2292c8c769e9398d31509ecdd6c58e176f67 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 22 Aug 2024 14:42:37 -0400 Subject: [PATCH 07/12] api: Mark UpdateMessageEvent fields as required, relying on server 5+, FL 114+ See "Feature level 114" from Zulip API changelog: https://zulip.com/api/changelog See also: https://zulip.com/api/get-events#update_message Signed-off-by: Zixuan James Li --- lib/api/model/events.dart | 6 +++--- lib/api/model/events.g.dart | 6 +++--- lib/model/message.dart | 4 +--- test/api/model/events_checks.dart | 6 +++--- test/example_data.dart | 6 +++--- test/model/message_test.dart | 14 ++------------ 6 files changed, 15 insertions(+), 27 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 4a5f9268e0..7b5bb241e3 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -797,13 +797,13 @@ class UpdateMessageEvent extends Event { @JsonKey(includeToJson: true) String get type => 'update_message'; - final int? userId; // TODO(server-5) - final bool? renderingOnly; // TODO(server-5) + final int userId; + final bool renderingOnly; final int messageId; final List messageIds; final List flags; - final int? editTimestamp; // TODO(server-5) + final int editTimestamp; // final String? streamName; // ignore diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 2203b2d9df..d13cca4df8 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -501,8 +501,8 @@ Map _$MessageEventToJson(MessageEvent instance) => UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => UpdateMessageEvent( id: (json['id'] as num).toInt(), - userId: (json['user_id'] as num?)?.toInt(), - renderingOnly: json['rendering_only'] as bool?, + userId: (json['user_id'] as num).toInt(), + renderingOnly: json['rendering_only'] as bool, messageId: (json['message_id'] as num).toInt(), messageIds: (json['message_ids'] as List) .map((e) => (e as num).toInt()) @@ -510,7 +510,7 @@ UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => flags: (json['flags'] as List) .map((e) => $enumDecode(_$MessageFlagEnumMap, e)) .toList(), - editTimestamp: (json['edit_timestamp'] as num?)?.toInt(), + editTimestamp: (json['edit_timestamp'] as num).toInt(), moveData: UpdateMessageMoveData.tryParseFromJson( UpdateMessageEvent._readMoveData(json, 'move_data') as Map, diff --git a/lib/model/message.dart b/lib/model/message.dart index 9e9e45ca6a..cf85ed4fb6 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -373,9 +373,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes } void _handleUpdateMessageEventTimestamp(UpdateMessageEvent event) { - // TODO(server-5): Cut this fallback; rely on renderingOnly from FL 114 - final isRenderingOnly = event.renderingOnly ?? (event.userId == null); - if (event.editTimestamp == null || isRenderingOnly) { + if (event.renderingOnly) { // A rendering-only update gets omitted from the message edit history, // and [Message.lastEditTimestamp] is the last timestamp of that history. // So on a rendering-only update, the timestamp doesn't get updated. diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index 59d01d67ac..fb02535168 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -43,12 +43,12 @@ extension MessageEventChecks on Subject { } extension UpdateMessageEventChecks on Subject { - Subject get userId => has((e) => e.userId, 'userId'); - Subject get renderingOnly => has((e) => e.renderingOnly, 'renderingOnly'); + Subject get userId => has((e) => e.userId, 'userId'); + Subject get renderingOnly => has((e) => e.renderingOnly, 'renderingOnly'); Subject get messageId => has((e) => e.messageId, 'messageId'); Subject> get messageIds => has((e) => e.messageIds, 'messageIds'); Subject> get flags => has((e) => e.flags, 'flags'); - Subject get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp'); + Subject get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp'); Subject get moveData => has((e) => e.moveData, 'moveData'); Subject get origContent => has((e) => e.origContent, 'origContent'); Subject get origRenderedContent => has((e) => e.origRenderedContent, 'origRenderedContent'); diff --git a/test/example_data.dart b/test/example_data.dart index 851f3a18e2..2c215b6e4b 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -839,8 +839,8 @@ DeleteMessageEvent deleteMessageEvent(List messages) { UpdateMessageEvent updateMessageEditEvent( Message origMessage, { - int? userId = -1, // null means null; default is [selfUser.userId] - bool? renderingOnly = false, + int? userId, + bool renderingOnly = false, int? messageId, List? flags, int? editTimestamp, @@ -851,7 +851,7 @@ UpdateMessageEvent updateMessageEditEvent( messageId ??= origMessage.id; return UpdateMessageEvent( id: 0, - userId: userId == -1 ? selfUser.userId : userId, + userId: userId ?? selfUser.userId, renderingOnly: renderingOnly, messageId: messageId, messageIds: [messageId], diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 762cc41452..730bdd9e60 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -930,16 +930,14 @@ void main() { ..content.not((it) => it.equals(updateEvent.renderedContent!)); }); - // TODO(server-5): Cut legacy case for rendering-only message update - Future checkRenderingOnly({required bool legacy}) async { + test('rendering-only update does not change timestamp', () async { final originalMessage = eg.streamMessage( lastEditTimestamp: 78492, content: "

Hello, world

"); final updateEvent = eg.updateMessageEditEvent(originalMessage, renderedContent: "

Hello, world

Some link preview
", editTimestamp: 99999, - renderingOnly: legacy ? null : true, - userId: null, + renderingOnly: true, ); await prepare(); await prepareMessages([originalMessage]); @@ -954,14 +952,6 @@ void main() { // ... edit timestamp is not. ..lastEditTimestamp.equals(originalMessage.lastEditTimestamp) ..lastEditTimestamp.not((it) => it.equals(updateEvent.editTimestamp)); - } - - test('rendering-only update does not change timestamp', () async { - await checkRenderingOnly(legacy: false); - }); - - test('rendering-only update does not change timestamp (for old server versions)', () async { - await checkRenderingOnly(legacy: true); }); group('Handle message edit state update', () { From c514b59f631698c39e98aa4eba4a7ee9c9aac1f6 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 22 Aug 2024 15:15:50 -0400 Subject: [PATCH 08/12] FIXME LINK api/notif [nfc]: Cut comment on FcmMessageChannelRecipient.streamId, relying on server 5+, FL 115+. See "Feature level 115" from Zulip API changelog: https://zulip.com/api/changelog See also: https://github.com/zulip/zulip/issues/ 18067 Signed-off-by: Zixuan James Li --- lib/api/notifications.dart | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/api/notifications.dart b/lib/api/notifications.dart index 6d028aa267..9c5df3c749 100644 --- a/lib/api/notifications.dart +++ b/lib/api/notifications.dart @@ -177,13 +177,9 @@ sealed class FcmMessageRecipient { @JsonSerializable(fieldRename: FieldRename.snake, createToJson: false) @_IntConverter() class FcmMessageChannelRecipient extends FcmMessageRecipient { - // Sending the stream ID in notifications is new in Zulip Server 5. - // But handling the lack of it would add complication, and we don't strictly - // need to -- we intend (#268) to cut pre-server-5 support before beta release. - // TODO(server-5): cut comment final int streamId; - // Current servers (as of 2023) always send the stream name. But + // Current servers (as of 2025) always send the stream name. But // future servers might not, once clients get the name from local data. // So might as well be ready. @JsonKey(name: 'stream') From 662ccaeae4595a599592a96d53e0ca4c74578a90 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 09/12] TODO api: Mark GetServerSettingsResult.realmWebPublicAccessEnabled as required, relying on server 5+, FL 116+ See https://github.com/zulip/zulip-flutter/pull/904#discussion_r1747943940. See "Feature level 116" from Zulip API changelog: https://zulip.com/api/changelog Signed-off-by: Zixuan James Li --- lib/api/route/realm.dart | 2 +- lib/api/route/realm.g.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/route/realm.dart b/lib/api/route/realm.dart index b37d0cc995..d80fc69286 100644 --- a/lib/api/route/realm.dart +++ b/lib/api/route/realm.dart @@ -51,7 +51,7 @@ class GetServerSettingsResult { final String realmName; final String realmIcon; final String realmDescription; - final bool? realmWebPublicAccessEnabled; // TODO(server-5) + final bool realmWebPublicAccessEnabled; GetServerSettingsResult({ required this.authenticationMethods, diff --git a/lib/api/route/realm.g.dart b/lib/api/route/realm.g.dart index cb7e94b48c..7df36c7644 100644 --- a/lib/api/route/realm.g.dart +++ b/lib/api/route/realm.g.dart @@ -33,7 +33,7 @@ GetServerSettingsResult _$GetServerSettingsResultFromJson( realmName: json['realm_name'] as String, realmIcon: json['realm_icon'] as String, realmDescription: json['realm_description'] as String, - realmWebPublicAccessEnabled: json['realm_web_public_access_enabled'] as bool?, + realmWebPublicAccessEnabled: json['realm_web_public_access_enabled'] as bool, ); Map _$GetServerSettingsResultToJson( From a3b054b28031b3b58d6a7603a4b7a60de09c3e0a Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 10/12] api: Drop legacy support for prev_subject and topic from edit history, relying on server 5+, FL 118+ "topic" is no longer optional when the topic was edited, we also don't need to expect "prev_subject" as opposed to "prev_topic". See "Feature level 118" from Zulip API changelog: https://zulip.com/api/changelog See also: https://zulip.com/api/get-messages#response Signed-off-by: Zixuan James Li --- lib/api/model/model.dart | 15 ++++++--------- test/api/model/model_test.dart | 10 ---------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 034c7d29cf..7956941302 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -1116,18 +1116,15 @@ enum MessageEditState { continue; } - // TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers - final prevTopicStr = (entry['prev_topic'] ?? entry['prev_subject']) as String?; + final prevTopicStr = entry['prev_topic'] as String?; final prevTopic = prevTopicStr == null ? null : TopicName.fromJson(prevTopicStr); final topicStr = entry['topic'] as String?; final topic = topicStr == null ? null : TopicName.fromJson(topicStr); - if (prevTopic != null) { - // TODO(server-5) pre-5.0 servers do not have the 'topic' field - if (topic == null) { - hasMoved = true; - } else { - hasMoved |= !topicMoveWasResolveOrUnresolve(topic, prevTopic); - } + if (topic != null || prevTopic != null) { + // Crunchy-shell validation: Both are present if the topic was edited + topic as TopicName; + prevTopic as TopicName; + hasMoved |= !topicMoveWasResolveOrUnresolve(topic, prevTopic); } } diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index c28d2fbb05..e3fa66a44c 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -286,16 +286,6 @@ void main() { checkEditState(MessageEditState.edited, [{'prev_content': 'old_content'}]); }); - - test("'prev_topic' present without the 'topic' field -> moved", () { - checkEditState(MessageEditState.moved, - [{'prev_topic': 'old_topic'}]); - }); - - test("'prev_subject' present from a pre-5.0 server -> moved", () { - checkEditState(MessageEditState.moved, - [{'prev_subject': 'old_topic'}]); - }); }); group('topic resolved in edit history', () { From bdb35bbbc5442b82101c1e385969f53cb32bd6e7 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 11/12] api: Remove sender_id fallback for UnreadDmSnapshot.otherUserId, relying on server 5+, FL 119+ See "Feature level 119" from Zulip API changelog: https://zulip.com/api/changelog See also: https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/register.3A.20When.20was.20.60unread_msgs.2Epms.5B.5D.2Eother_user_id.60.20added.3F Signed-off-by: Zixuan James Li --- lib/api/model/initial_snapshot.dart | 6 ------ lib/api/model/initial_snapshot.g.dart | 4 +--- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index fa978ccc50..b7f19a362d 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -334,15 +334,9 @@ class UnreadMessagesSnapshot { /// An item in [UnreadMessagesSnapshot.dms]. @JsonSerializable(fieldRename: FieldRename.snake) class UnreadDmSnapshot { - @JsonKey(readValue: _readOtherUserId) final int otherUserId; final List unreadMessageIds; - // TODO(server-5): Simplify away. - static dynamic _readOtherUserId(Map json, String key) { - return json[key] ?? json['sender_id']; - } - UnreadDmSnapshot({ required this.otherUserId, required this.unreadMessageIds, diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index 7f6821d108..a93645747f 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -303,9 +303,7 @@ Map _$UnreadMessagesSnapshotToJson( UnreadDmSnapshot _$UnreadDmSnapshotFromJson(Map json) => UnreadDmSnapshot( - otherUserId: - (UnreadDmSnapshot._readOtherUserId(json, 'other_user_id') as num) - .toInt(), + otherUserId: (json['other_user_id'] as num).toInt(), unreadMessageIds: (json['unread_message_ids'] as List) .map((e) => (e as num).toInt()) .toList(), From 556d5ec0cf05b9e59b74b02ce9245e23f47f64e0 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 23 Aug 2024 15:07:28 -0400 Subject: [PATCH 12/12] api: Remove legacy getMessageCompat, relying on server 5+, FL 120+ See "Feature level 120" from Zulip API changelog: https://zulip.com/api/changelog See also commit 631f4d68. Fixes: #268 Signed-off-by: Zixuan James Li --- lib/api/route/messages.dart | 51 ------------- lib/widgets/actions.dart | 15 ++-- test/api/route/messages_test.dart | 122 ------------------------------ 3 files changed, 9 insertions(+), 179 deletions(-) diff --git a/lib/api/route/messages.dart b/lib/api/route/messages.dart index 05364951cd..28f9c89907 100644 --- a/lib/api/route/messages.dart +++ b/lib/api/route/messages.dart @@ -1,69 +1,18 @@ import 'package:json_annotation/json_annotation.dart'; import '../core.dart'; -import '../exception.dart'; import '../model/model.dart'; import '../model/narrow.dart'; part 'messages.g.dart'; -/// Convenience function to get a single message from any server. -/// -/// This encapsulates a server-feature check. -/// -/// Gives null if the server reports that the message doesn't exist. -// TODO(server-5) Simplify this away; just use getMessage. -Future getMessageCompat(ApiConnection connection, { - required int messageId, - bool? applyMarkdown, - required bool allowEmptyTopicName, -}) async { - final useLegacyApi = connection.zulipFeatureLevel! < 120; - if (useLegacyApi) { - final response = await getMessages(connection, - narrow: [ApiNarrowMessageId(messageId)], - anchor: NumericAnchor(messageId), - numBefore: 0, - numAfter: 0, - applyMarkdown: applyMarkdown, - allowEmptyTopicName: allowEmptyTopicName, - - // Hard-code this param to `true`, as the new single-message API - // effectively does: - // https://chat.zulip.org/#narrow/stream/378-api-design/topic/.60client_gravatar.60.20in.20.60messages.2F.7Bmessage_id.7D.60/near/1418337 - clientGravatar: true, - ); - return response.messages.firstOrNull; - } else { - try { - final response = await getMessage(connection, - messageId: messageId, - applyMarkdown: applyMarkdown, - allowEmptyTopicName: allowEmptyTopicName, - ); - return response.message; - } on ZulipApiException catch (e) { - if (e.code == 'BAD_REQUEST') { - // Servers use this code when the message doesn't exist, according to - // the example in the doc. - return null; - } - rethrow; - } - } -} - /// https://zulip.com/api/get-message -/// -/// This binding only supports feature levels 120+. -// TODO(server-5) remove FL 120+ mention in doc, and the related `assert` Future getMessage(ApiConnection connection, { required int messageId, bool? applyMarkdown, required bool allowEmptyTopicName, }) { assert(allowEmptyTopicName, '`allowEmptyTopicName` should only be true'); - assert(connection.zulipFeatureLevel! >= 120); return connection.get('getMessage', GetMessageResult.fromJson, 'messages/$messageId', { if (applyMarkdown != null) 'apply_markdown': applyMarkdown, 'allow_empty_topic_name': allowEmptyTopicName, diff --git a/lib/widgets/actions.dart b/lib/widgets/actions.dart index 4d96727666..4daf349459 100644 --- a/lib/widgets/actions.dart +++ b/lib/widgets/actions.dart @@ -206,18 +206,21 @@ abstract final class ZulipAction { // On final failure or success, auto-dismiss the snackbar. final zulipLocalizations = ZulipLocalizations.of(context); try { - fetchedMessage = await getMessageCompat(PerAccountStoreWidget.of(context).connection, + fetchedMessage = (await getMessage(PerAccountStoreWidget.of(context).connection, messageId: messageId, applyMarkdown: false, allowEmptyTopicName: true, - ); - if (fetchedMessage == null) { - errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist; - } + )).message; } catch (e) { switch (e) { case ZulipApiException(): - errorMessage = e.message; + if (e.code == 'BAD_REQUEST') { + // Servers use this code when the message doesn't exist, according + // to the example in the doc. + errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist; + } else { + errorMessage = e.message; + } // TODO specific messages for common errors, like network errors // (support with reusable code) default: diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart index f00bf4428f..2ebabe2677 100644 --- a/test/api/route/messages_test.dart +++ b/test/api/route/messages_test.dart @@ -15,118 +15,6 @@ import '../fake_api.dart'; import 'route_checks.dart'; void main() { - group('getMessageCompat', () { - Future checkGetMessageCompat(FakeApiConnection connection, { - required bool expectLegacy, - required int messageId, - bool? applyMarkdown, - required bool allowEmptyTopicName, - }) async { - final result = await getMessageCompat(connection, - messageId: messageId, - applyMarkdown: applyMarkdown, - allowEmptyTopicName: allowEmptyTopicName, - ); - if (expectLegacy) { - check(connection.lastRequest).isA() - ..method.equals('GET') - ..url.path.equals('/api/v1/messages') - ..url.queryParameters.deepEquals({ - 'narrow': jsonEncode([ApiNarrowMessageId(messageId)]), - 'anchor': messageId.toString(), - 'num_before': '0', - 'num_after': '0', - if (applyMarkdown != null) 'apply_markdown': applyMarkdown.toString(), - 'allow_empty_topic_name': allowEmptyTopicName.toString(), - 'client_gravatar': 'true', - }); - } else { - check(connection.lastRequest).isA() - ..method.equals('GET') - ..url.path.equals('/api/v1/messages/$messageId') - ..url.queryParameters.deepEquals({ - if (applyMarkdown != null) 'apply_markdown': applyMarkdown.toString(), - 'allow_empty_topic_name': allowEmptyTopicName.toString(), - }); - } - return result; - } - - test('modern; message found', () { - return FakeApiConnection.with_((connection) async { - final message = eg.streamMessage(); - final fakeResult = GetMessageResult(message: message); - connection.prepare(json: fakeResult.toJson()); - final result = await checkGetMessageCompat(connection, - expectLegacy: false, - messageId: message.id, - applyMarkdown: true, - allowEmptyTopicName: true, - ); - check(result).isNotNull().jsonEquals(message); - }); - }); - - test('modern; message not found', () { - return FakeApiConnection.with_((connection) async { - final message = eg.streamMessage(); - connection.prepare( - apiException: eg.apiBadRequest(message: 'Invalid message(s)')); - final result = await checkGetMessageCompat(connection, - expectLegacy: false, - messageId: message.id, - applyMarkdown: true, - allowEmptyTopicName: true, - ); - check(result).isNull(); - }); - }); - - test('legacy; message found', () { - return FakeApiConnection.with_(zulipFeatureLevel: 119, (connection) async { - final message = eg.streamMessage(); - final fakeResult = GetMessagesResult( - anchor: message.id, - foundNewest: false, - foundOldest: false, - foundAnchor: true, - historyLimited: false, - messages: [message], - ); - connection.prepare(json: fakeResult.toJson()); - final result = await checkGetMessageCompat(connection, - expectLegacy: true, - messageId: message.id, - applyMarkdown: true, - allowEmptyTopicName: true, - ); - check(result).isNotNull().jsonEquals(message); - }); - }); - - test('legacy; message not found', () { - return FakeApiConnection.with_(zulipFeatureLevel: 119, (connection) async { - final message = eg.streamMessage(); - final fakeResult = GetMessagesResult( - anchor: message.id, - foundNewest: false, - foundOldest: false, - foundAnchor: false, - historyLimited: false, - messages: [], - ); - connection.prepare(json: fakeResult.toJson()); - final result = await checkGetMessageCompat(connection, - expectLegacy: true, - messageId: message.id, - applyMarkdown: true, - allowEmptyTopicName: true, - ); - check(result).isNull(); - }); - }); - }); - group('getMessage', () { Future checkGetMessage( FakeApiConnection connection, { @@ -186,16 +74,6 @@ void main() { expected: {'allow_empty_topic_name': 'true'}); }); }); - - test('Throws assertion error when FL <120', () { - return FakeApiConnection.with_(zulipFeatureLevel: 119, (connection) async { - connection.prepare(json: fakeResult.toJson()); - check(() => getMessage(connection, - messageId: 1, - allowEmptyTopicName: true, - )).throws(); - }); - }); }); test('ApiNarrow.toJson', () {