Exclude deactivated realm emoji from autocomplete

lib/api/model/model.dart

@@ -115,15 +115,16 @@ class CustomProfileFieldExternalAccountData {
 /// in <https://zulip.com/api/register-queue>.
 @JsonSerializable(fieldRename: FieldRename.snake)
 class RealmEmojiItem {
-  final String id;
+  @JsonKey(name: 'id')
+  final String emojiCode;
   final String name;
   final String sourceUrl;
   final String? stillUrl;
   final bool deactivated;
   final int? authorId;
 
   RealmEmojiItem({
-    required this.id,
+    required this.emojiCode,
     required this.name,
     required this.sourceUrl,
     required this.stillUrl,
@@ -137,7 +138,6 @@ class RealmEmojiItem {
   Map<String, dynamic> toJson() => _$RealmEmojiItemToJson(this);
 }
 
-
 /// The name of a user setting that has a property in [UserSettings].
 ///
 /// In Zulip event-handling code (for [UserSettingsUpdateEvent]),

lib/model/autocomplete.dart

@@ -761,6 +761,11 @@ class EmojiAutocompleteResult extends ComposeAutocompleteResult {
   EmojiAutocompleteResult(this.candidate);
 
   final EmojiCandidate candidate;
+
+  @override
+  String toString() {
+    return 'EmojiAutocompleteResult(${candidate.description()})';
+  }
 }
 
 /// A result from an @-mention autocomplete interaction,

lib/model/emoji.dart

@@ -86,14 +86,22 @@ final class EmojiCandidate {
     required List<String>? aliases,
     required this.emojiDisplay,
   }) : _aliases = aliases;
+
+  /// Used for implementing [toString] and [EmojiAutocompleteResult.toString].
+  String description() {
+    final typeLabel = emojiType.name.replaceFirst(RegExp(r'Emoji$'), '');
+    return '$typeLabel $emojiCode $emojiName'
+      '${aliases.isNotEmpty ? ' $aliases' : ''}';
+  }
+
+  @override
+  String toString() {
+    return 'EmojiCandidate(${description()})';
+  }
 }
 
 /// The portion of [PerAccountStore] describing what emoji exist.
 mixin EmojiStore {
-  /// The realm's custom emoji (for [ReactionType.realmEmoji],
-  /// indexed by [Reaction.emojiCode].
-  Map<String, RealmEmojiItem> get realmEmoji;
-
   EmojiDisplay emojiDisplayFor({
     required ReactionType emojiType,
     required String emojiCode,
@@ -117,14 +125,25 @@ mixin EmojiStore {
 class EmojiStoreImpl with EmojiStore {
   EmojiStoreImpl({
     required this.realmUrl,
-    required this.realmEmoji,
+    required this.allRealmEmoji,
   }) : _serverEmojiData = null; // TODO(#974) maybe start from a hard-coded baseline
 
   /// The same as [PerAccountStore.realmUrl].
   final Uri realmUrl;
 
-  @override
-  Map<String, RealmEmojiItem> realmEmoji;
+  /// The realm's custom emoji, indexed by their [RealmEmojiItem.emojiCode],
+  /// including deactivated emoji not available for new uses.
+  ///
+  /// These are the emoji that can have [ReactionType.realmEmoji].
+  ///
+  /// For emoji available to be newly used, see [activeRealmEmoji].
+  Map<String, RealmEmojiItem> allRealmEmoji;
+
+  /// The realm's custom emoji that are available for new uses
+  /// in messages and reactions.
+  Iterable<RealmEmojiItem> get activeRealmEmoji {
+    return allRealmEmoji.values.where((emoji) => !emoji.deactivated);
+  }
 
   /// The realm-relative URL of the unique "Zulip extra emoji", :zulip:.
   static const kZulipEmojiUrl = '/static/generated/emoji/images/emoji/unicode/zulip.png';
@@ -142,7 +161,7 @@ class EmojiStoreImpl with EmojiStore {
         return UnicodeEmojiDisplay(emojiName: emojiName, emojiUnicode: parsed);
 
       case ReactionType.realmEmoji:
-        final item = realmEmoji[emojiCode];
+        final item = allRealmEmoji[emojiCode];
         if (item == null) break;
         // TODO we don't check emojiName matches the known realm emoji; is that right?
         return _tryImageEmojiDisplay(
@@ -203,10 +222,43 @@ class EmojiStoreImpl with EmojiStore {
   }
 
   List<EmojiCandidate> _generateAllCandidates() {
+    // Compare `emoji_picker.rebuild_catalog` in Zulip web;
+    // `composebox_typeahead.update_emoji_data` which receives its output;
+    // and `emoji.update_emojis` which builds part of its input.
+    //   https://github.com/zulip/zulip/blob/0f59e2e78/web/src/emoji_picker.ts#L132-L185
+    //   https://github.com/zulip/zulip/blob/0f59e2e78/web/src/composebox_typeahead.ts#L138-L163
+    //   https://github.com/zulip/zulip/blob/0f59e2e78/web/src/emoji.ts#L232-L278
+    //
+    // Behavior differences we might copy or change, TODO:
+    //  * Web has a particular ordering of Unicode emoji;
+    //    a data file groups them by category and orders within each of those,
+    //    and the code has a list of categories.
+    //    This seems useful; it'll call for expanding the server emoji data API.
+    //  * Both here and in web, the realm emoji appear in whatever order the
+    //    server returned them in; and that order appears to be random,
+    //    presumably the iteration order of some Python dict,
+    //    and to vary over time.
+    //
+    // Behavior differences that web should probably fix, TODO(web):
+    //  * Web ranks the realm's custom emoji (and the Zulip extra emoji) at the
+    //    end of the base list, as seen in the emoji picker on an empty query;
+    //    but then ranks them first, after only the six "popular" emoji,
+    //    once there's a non-empty query.
+    //  * Web gives the six "popular" emoji a set order amongst themselves,
+    //    like we do after #1112; but in web, this order appears only in the
+    //    emoji picker on an empty query, and is otherwise lost even when the
+    //    emoji are taken out of their home categories and shown instead
+    //    together at the front.
+    //
+    //    In web on an empty query, :+1: aka :like: comes first, and
+    //    :heart: aka :love: comes later (fourth); but then on the query "l",
+    //    the results begin with :love: and then :like:.  They've flipped order,
+    //    even though they're equally good prefix matches to the query.
+
     final results = <EmojiCandidate>[];
 
     final namesOverridden = {
-      for (final emoji in realmEmoji.values) emoji.name,
+      for (final emoji in activeRealmEmoji) emoji.name,
       'zulip',
     };
     // TODO(log) if _serverEmojiData missing
@@ -230,16 +282,15 @@ class EmojiStoreImpl with EmojiStore {
         aliases: aliases));
     }
 
-    for (final entry in realmEmoji.entries) {
-      final emojiName = entry.value.name;
+    for (final emoji in activeRealmEmoji) {
+      final emojiName = emoji.name;
       if (emojiName == 'zulip') {
-        // TODO does 'zulip' really override realm emoji?
-        //   (This is copied from zulip-mobile's behavior.)
+        // :zulip: overrides realm emoji; compare web's `emoji.update_emojis`.
         continue;
       }
       results.add(_emojiCandidateFor(
         emojiType: ReactionType.realmEmoji,
-        emojiCode: entry.key, emojiName: emojiName,
+        emojiCode: emoji.emojiCode, emojiName: emojiName,
         aliases: null));
     }
 
@@ -263,7 +314,7 @@ class EmojiStoreImpl with EmojiStore {
   }
 
   void handleRealmEmojiUpdateEvent(RealmEmojiUpdateEvent event) {
-    realmEmoji = event.realmEmoji;
+    allRealmEmoji = event.realmEmoji;
     _allEmojiCandidates = null;
   }
 }

lib/model/store.dart

@@ -274,7 +274,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
       customProfileFields: _sortCustomProfileFields(initialSnapshot.customProfileFields),
       emailAddressVisibility: initialSnapshot.emailAddressVisibility,
       emoji: EmojiStoreImpl(
-        realmUrl: realmUrl, realmEmoji: initialSnapshot.realmEmoji),
+        realmUrl: realmUrl, allRealmEmoji: initialSnapshot.realmEmoji),
       accountId: accountId,
       selfUserId: account.userId,
       userSettings: initialSnapshot.userSettings,
@@ -386,9 +386,6 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
   ////////////////////////////////
   // The realm's repertoire of available emoji.
 
-  @override
-  Map<String, RealmEmojiItem> get realmEmoji => _emoji.realmEmoji;
-
   @override
   EmojiDisplay emojiDisplayFor({
     required ReactionType emojiType,

test/example_data.dart

@@ -77,7 +77,7 @@ RealmEmojiItem realmEmojiItem({
 }) {
   assert(RegExp(r'^[1-9][0-9]*$').hasMatch(emojiCode));
   return RealmEmojiItem(
-    id: emojiCode,
+    emojiCode: emojiCode,
     name: emojiName,
     sourceUrl: sourceUrl ?? '/emoji/$emojiCode.png',
     stillUrl: stillUrl,

test/model/emoji_test.dart

@@ -123,6 +123,31 @@ void main() {
       return store;
     }
 
+    test('realm emoji included only when active', () {
+      final store = prepare(realmEmoji: {
+        '1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'abc', deactivated: true),
+        '2': eg.realmEmojiItem(emojiCode: '2', emojiName: 'abcd'),
+      });
+      check(store.allEmojiCandidates()).deepEquals([
+        isRealmCandidate(emojiCode: '2', emojiName: 'abcd'),
+        isZulipCandidate(),
+      ]);
+    });
+
+    test('realm emoji tolerate name collisions', () {
+      final store = prepare(realmEmoji: {
+        '1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'test', deactivated: true),
+        '2': eg.realmEmojiItem(emojiCode: '2', emojiName: 'try', deactivated: true),
+        '3': eg.realmEmojiItem(emojiCode: '3', emojiName: 'try', deactivated: true),
+        '4': eg.realmEmojiItem(emojiCode: '4', emojiName: 'try'),
+        '5': eg.realmEmojiItem(emojiCode: '5', emojiName: 'test', deactivated: true),
+      });
+      check(store.allEmojiCandidates()).deepEquals([
+        isRealmCandidate(emojiCode: '4', emojiName: 'try'),
+        isZulipCandidate(),
+      ]);
+    });
+
     test('realm emoji overrides Unicode emoji', () {
       final store = prepare(realmEmoji: {
         '1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'smiley'),
@@ -137,6 +162,18 @@ void main() {
       ]);
     });
 
+    test('deactivated realm emoji cause no override of Unicode emoji', () {
+      final store = prepare(realmEmoji: {
+        '1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'ant', deactivated: true),
+      }, unicodeEmoji: {
+        '1f41c': ['ant'],
+      });
+      check(store.allEmojiCandidates()).deepEquals([
+        isUnicodeCandidate('1f41c', ['ant']),
+        isZulipCandidate(),
+      ]);
+    });
+
     test('Unicode emoji with overridden aliases survives with remaining names', () {
       final store = prepare(realmEmoji: {
         '1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'tangerine'),

test/model/store_checks.dart

@@ -33,7 +33,6 @@ extension PerAccountStoreChecks on Subject<PerAccountStore> {
   Subject<String> get zulipVersion => has((x) => x.zulipVersion, 'zulipVersion');
   Subject<int> get maxFileUploadSizeMib => has((x) => x.maxFileUploadSizeMib, 'maxFileUploadSizeMib');
   Subject<Map<String, RealmDefaultExternalAccount>> get realmDefaultExternalAccounts => has((x) => x.realmDefaultExternalAccounts, 'realmDefaultExternalAccounts');
-  Subject<Map<String, RealmEmojiItem>> get realmEmoji => has((x) => x.realmEmoji, 'realmEmoji');
   Subject<List<CustomProfileField>> get customProfileFields => has((x) => x.customProfileFields, 'customProfileFields');
   Subject<int> get accountId => has((x) => x.accountId, 'accountId');
   Subject<Account> get account => has((x) => x.account, 'account');