From f1a917262fd0919de95947ba3251035e834b53f8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 9 Aug 2024 20:22:52 -0700 Subject: [PATCH 1/2] api: Drop isOwner, isAdmin, isGuest from User class These are obsoleted by [User.role] (which dates to Zulip 4.0). We never looked at these fields, but had a latent bug because we weren't keeping them up to date on RealmUserUpdateEvent. To fix the bug, just stop having the fields around at all. (Later if we find we want booleans with these handy names, we can always add getters that just inspect [User.role].) --- lib/api/model/model.dart | 9 +++------ lib/api/model/model.g.dart | 6 ------ test/api/model/model_checks.dart | 3 --- test/example_data.dart | 3 --- 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index c98fea005c..920b28fbd7 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -194,9 +194,9 @@ class User { String fullName; String dateJoined; bool isActive; // Really sometimes absent in /register, but we normalize that away; see [InitialSnapshot.realmUsers]. - bool isOwner; - bool isAdmin; - bool isGuest; + // bool isOwner; // obsoleted by [role]; ignore + // bool isAdmin; // obsoleted by [role]; ignore + // bool isGuest; // obsoleted by [role]; ignore bool? isBillingAdmin; // TODO(server-5) bool isBot; int? botType; // TODO enum @@ -240,9 +240,6 @@ class User { required this.fullName, required this.dateJoined, required this.isActive, - required this.isOwner, - required this.isAdmin, - required this.isGuest, required this.isBillingAdmin, required this.isBot, required this.botType, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 10c1c6b00c..0171278e4d 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -97,9 +97,6 @@ User _$UserFromJson(Map json) => User( fullName: json['full_name'] as String, dateJoined: json['date_joined'] as String, isActive: json['is_active'] as bool, - isOwner: json['is_owner'] as bool, - isAdmin: json['is_admin'] as bool, - isGuest: json['is_guest'] as bool, isBillingAdmin: json['is_billing_admin'] as bool?, isBot: json['is_bot'] as bool, botType: (json['bot_type'] as num?)?.toInt(), @@ -125,9 +122,6 @@ Map _$UserToJson(User instance) => { 'full_name': instance.fullName, 'date_joined': instance.dateJoined, 'is_active': instance.isActive, - 'is_owner': instance.isOwner, - 'is_admin': instance.isAdmin, - 'is_guest': instance.isGuest, 'is_billing_admin': instance.isBillingAdmin, 'is_bot': instance.isBot, 'bot_type': instance.botType, diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index c8261ba78f..71d199e864 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -8,9 +8,6 @@ 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 isOwner => has((x) => x.isOwner, 'isOwner'); - Subject get isAdmin => has((x) => x.isAdmin, 'isAdmin'); - Subject get isGuest => has((x) => x.isGuest, 'isGuest'); Subject get isBillingAdmin => has((x) => x.isBillingAdmin, 'isBillingAdmin'); Subject get isBot => has((x) => x.isBot, 'isBot'); Subject get botType => has((x) => x.botType, 'botType'); diff --git a/test/example_data.dart b/test/example_data.dart index c7820326ce..e14b7c437a 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -94,9 +94,6 @@ User user({ fullName: fullName ?? 'A user', // TODO generate example names dateJoined: '2023-04-28', isActive: isActive ?? true, - isOwner: false, - isAdmin: false, - isGuest: false, isBillingAdmin: false, isBot: isBot ?? false, botType: null, From 156c693d2b69420f374f2d6b8910d61c5b0e45fb Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 11 Aug 2024 14:02:06 -0700 Subject: [PATCH 2/2] api [nfc]: Mark all never-updated User fields final This was prompted by noticing recently that we weren't updating [User.isActive]: https://github.com/zulip/zulip-flutter/pull/816#discussion_r1689098946 After fixing that, I looked and found there were actually three other fields on User which we weren't updating -- a latent bug, since we never looked at those fields, but a bug. Fixed that in the preceding commit. Now the remaining fields that don't get updated are fields that really are immutable on a Zulip user. Mark those as final, and add a comment to help maintain the pattern. --- lib/api/model/model.dart | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 920b28fbd7..ef5297ac61 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -188,18 +188,25 @@ enum Emojiset { /// in . @JsonSerializable(fieldRename: FieldRename.snake) class User { + // When adding a field to this class: + // * If a [RealmUserUpdateEvent] can update it, be sure to add + // that case to [RealmUserUpdateEvent] and its handler. + // * If the field can never change for a given Zulip user, mark it final. + // * (If it can change but [RealmUserUpdateEvent] doesn't cover that, + // then that's a bug in the API; raise it in `#api design`.) + final int userId; String? deliveryEmail; String email; String fullName; - String dateJoined; + final String dateJoined; bool isActive; // Really sometimes absent in /register, but we normalize that away; see [InitialSnapshot.realmUsers]. // bool isOwner; // obsoleted by [role]; ignore // bool isAdmin; // obsoleted by [role]; ignore // bool isGuest; // obsoleted by [role]; ignore bool? isBillingAdmin; // TODO(server-5) - bool isBot; - int? botType; // TODO enum + final bool isBot; + final int? botType; // TODO enum int? botOwnerId; @JsonKey(unknownEnumValue: UserRole.unknown) UserRole role; @@ -214,7 +221,7 @@ class User { Map? profileData; @JsonKey(readValue: _readIsSystemBot) - bool isSystemBot; + final bool isSystemBot; static Map? _readProfileData(Map json, String key) { final value = (json[key] as Map?);