api [nfc]: Comment on why to keep an existing implementation

Author: chrisbobbe

lib/api/model/model.dart

@@ -872,6 +872,14 @@ enum MessageEditState {
   /// but for purposes of [Message.editState], we want to ignore such renames.
   /// This method identifies topic moves that should be ignored in that context.
   static bool topicMoveWasResolveOrUnresolve(TopicName topic, TopicName prevTopic) {
+    // Implemented to match web; see analyze_edit_history in zulip/zulip's
+    // web/src/message_list_view.ts.
+    //
+    // Also, this is a hot codepath (decoding messages, a high-volume type of
+    // data we get from the server), so we avoid calling [canonicalize] and
+    // using [TopicName.resolvedTopicPrefixRegexp], to be performance-sensitive.
+    // Discussion:
+    //   https://github.com/zulip/zulip-flutter/pull/1242#discussion_r1917592157
     if (topic.apiName.startsWith(TopicName.resolvedTopicPrefix)
         && topic.apiName.substring(TopicName.resolvedTopicPrefix.length) == prevTopic.apiName) {
       return true;

test/api/model/model_test.dart

@@ -325,10 +325,35 @@ void main() {
         ]);
       });
 
+      // Technically the topic *was* unresolved, so MessageEditState.none
+      // would be valid and preferable -- if it didn't need more intense
+      // computation than we're comfortable with in a hot codepath, i.e.,
+      // a regex test instead of a simple `startsWith` / `substring` check.
+      // See comment on the implementation, and discussion:
+      //   https://github.com/zulip/zulip-flutter/pull/1242#discussion_r1917592157
       test('Unresolving topic with a weird prefix -> moved', () {
           checkEditState(MessageEditState.moved,
             [{'prev_topic': '✔ ✔old_topic', 'topic': 'old_topic'}]);
       });
+
+      // Similar reasoning as in the previous test.
+      // Also, Zulip doesn't produce topics with a weird resolved-topic prefix,
+      // so this case can only be produced by unusual input in an
+      // edit/move-topic UI. A "moved" marker seems like a fine response
+      // in that circumstance.
+      test('Resolving topic with a weird prefix -> moved', () {
+          checkEditState(MessageEditState.moved,
+            [{'prev_topic': 'old_topic', 'topic': '✔ ✔old_topic'}]);
+      });
+
+      // Similar reasoning as the previous test, including that this case had to
+      // involve unusual input in an edit/move-topic UI.
+      // Here the computation burden would have come from calling
+      // [TopicName.canonicalize].
+      test('Topic was resolved but with changed case -> moved', () {
+        checkEditState(MessageEditState.moved,
+          [{'prev_topic': 'old ToPiC', 'topic': '✔ OLD tOpIc'}]);
+      });
     });
   });
 }