Edit-message (7/n): Implement edit-message methods on MessageStore

lib/api/route/messages.dart

@@ -267,6 +267,9 @@ class SendMessageResult {
 }
 
 /// https://zulip.com/api/update-message
+///
+/// `prev_content_sha256` is sent if the feature level is >= 379,
+/// otherwise ignored.
 Future<UpdateMessageResult> updateMessage(
   ApiConnection connection, {
   required int messageId,
@@ -276,6 +279,7 @@ Future<UpdateMessageResult> updateMessage(
   bool? sendNotificationToNewThread,
   String? content,
   int? streamId,
+  String? prevContentSha256,
 }) {
   return connection.patch('updateMessage', UpdateMessageResult.fromJson, 'messages/$messageId', {
     if (topic != null) 'topic': RawParameter(topic.apiName),
@@ -284,6 +288,8 @@ Future<UpdateMessageResult> updateMessage(
     if (sendNotificationToNewThread != null) 'send_notification_to_new_thread': sendNotificationToNewThread,
     if (content != null) 'content': RawParameter(content),
     if (streamId != null) 'stream_id': streamId,
+    // TODO(server-11) remove FL condition and its mention in the dartdoc
+    if (prevContentSha256 != null && connection.zulipFeatureLevel! >= 379) 'prev_content_sha256': RawParameter(prevContentSha256),
   });
 }
 

lib/model/message.dart

@@ -1,5 +1,7 @@
 import 'dart:convert';
 
+import 'package:crypto/crypto.dart';
+
 import '../api/model/events.dart';
 import '../api/model/model.dart';
 import '../api/route/messages.dart';
@@ -35,6 +37,40 @@ mixin MessageStore {
   /// All [Message] objects in the resulting list will be present in
   /// [this.messages].
   void reconcileMessages(List<Message> messages);
+
+  /// Whether the current edit request for the given message, if any, has failed.
+  ///
+  /// Will be null if there is no current edit request.
+  /// Will be false if the current request hasn't failed
+  /// and the update-message event hasn't arrived.
+  bool? getEditMessageErrorStatus(int messageId);
+
+  /// Edit a message's content, via a request to the server.
+  ///
+  /// Should only be called when there is no current edit request for [messageId],
+  /// i.e., [getEditMessageErrorStatus] returns null for [messageId].
+  ///
+  /// See also:
+  ///   * [getEditMessageErrorStatus]
+  ///   * [takeFailedMessageEdit]
+  void editMessage({
+    required int messageId,
+    required String originalRawContent,
+    required String newContent,
+  });
+
+  /// Forgets the failed edit request and returns the attempted new content.
+  ///
+  /// Should only be called when there is a failed request,
+  /// per [getEditMessageErrorStatus].
+  String? takeFailedMessageEdit(int messageId);
+}
+
+class _EditMessageRequestStatus {
+  _EditMessageRequestStatus({required this.hasError, required this.newContent});
+
+  bool hasError;
+  final String newContent;
 }
 
 class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
@@ -132,6 +168,60 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
     }
   }
 
+  @override
+  bool? getEditMessageErrorStatus(int messageId) =>
+    _editMessageRequests[messageId]?.hasError;
+
+  final Map<int, _EditMessageRequestStatus> _editMessageRequests = {};
+
+  @override
+  void editMessage({
+    required int messageId,
+    required String originalRawContent,
+    required String newContent,
+  }) async {
+    if (_editMessageRequests.containsKey(messageId)) {
+      throw StateError('an edit request is already in progress');
+    }
+
+    _editMessageRequests[messageId] = _EditMessageRequestStatus(
+      hasError: false, newContent: newContent);
+    _notifyMessageListViewsForOneMessage(messageId);
+    try {
+      await updateMessage(connection,
+        messageId: messageId,
+        content: newContent,
+        prevContentSha256: sha256.convert(utf8.encode(originalRawContent)).toString());
+      // On success, we'll clear the status from _editMessageRequests
+      // when we get the event.
+    } catch (e) {
+      // TODO(log) if e is something unexpected
+
+      final status = _editMessageRequests[messageId];
+      if (status == null) {
+        // The event actually arrived before this request failed
+        // (can happen with network issues).
+        // Or, the message was deleted.
+        return;
+      }
+      status.hasError = true;
+      _notifyMessageListViewsForOneMessage(messageId);
+    }
+  }
+
+  @override
+  String? takeFailedMessageEdit(int messageId) {
+    final status = _editMessageRequests.remove(messageId);
+    _notifyMessageListViewsForOneMessage(messageId);
+    if (status == null) {
+      throw StateError('called takeFailedMessageEdit, but no edit');
+    }
+    if (!status.hasError) {
+      throw StateError("called takeFailedMessageEdit, but edit hasn't failed");
+    }
+    return status.newContent;
+  }
+
   void handleUserTopicEvent(UserTopicEvent event) {
     for (final view in _messageListViews) {
       view.handleUserTopicEvent(event);
@@ -183,6 +273,12 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
       // The message is guaranteed to be edited.
       // See also: https://zulip.com/api/get-events#update_message
       message.editState = MessageEditState.edited;
+
+      // Clear the edit-message progress feedback.
+      // This makes a rare bug where we might clear the feedback too early,
+      // if the user raced with themself to edit the same message
+      // from multiple clients.
+      _editMessageRequests.remove(message.id);
     }
     if (event.renderedContent != null) {
       assert(message.contentType == 'text/html',
@@ -245,6 +341,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
   void handleDeleteMessageEvent(DeleteMessageEvent event) {
     for (final messageId in event.messageIds) {
       messages.remove(messageId);
+      _editMessageRequests.remove(messageId);
     }
     for (final view in _messageListViews) {
       view.handleDeleteMessageEvent(event);

lib/model/store.dart

@@ -737,11 +737,36 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
   void unregisterMessageList(MessageListView view) =>
     _messages.unregisterMessageList(view);
   @override
+  Future<void> sendMessage({required MessageDestination destination, required String content}) {
+    assert(!_disposed);
+    return _messages.sendMessage(destination: destination, content: content);
+  }
+  @override
   void reconcileMessages(List<Message> messages) {
     _messages.reconcileMessages(messages);
     // TODO(#649) notify [unreads] of the just-fetched messages
     // TODO(#650) notify [recentDmConversationsView] of the just-fetched messages
   }
+  @override
+  bool? getEditMessageErrorStatus(int messageId) {
+    assert(!_disposed);
+    return _messages.getEditMessageErrorStatus(messageId);
+  }
+  @override
+  void editMessage({
+    required int messageId,
+    required String originalRawContent,
+    required String newContent,
+  }) {
+    assert(!_disposed);
+    return _messages.editMessage(messageId: messageId,
+      originalRawContent: originalRawContent, newContent: newContent);
+  }
+  @override
+  String? takeFailedMessageEdit(int messageId) {
+    assert(!_disposed);
+    return _messages.takeFailedMessageEdit(messageId);
+  }
 
   @override
   Set<MessageListView> get debugMessageListViews => _messages.debugMessageListViews;
@@ -904,12 +929,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
     }
   }
 
-  @override
-  Future<void> sendMessage({required MessageDestination destination, required String content}) {
-    assert(!_disposed);
-    return _messages.sendMessage(destination: destination, content: content);
-  }
-
   static List<CustomProfileField> _sortCustomProfileFields(List<CustomProfileField> initialCustomProfileFields) {
     // TODO(server): The realm-wide field objects have an `order` property,
     //   but the actual API appears to be that the fields should be shown in

test/api/route/messages_test.dart

@@ -455,6 +455,7 @@ void main() {
       bool? sendNotificationToNewThread,
       String? content,
       int? streamId,
+      String? prevContentSha256,
       required Map<String, String> expected,
     }) async {
       final result = await updateMessage(connection,
@@ -465,14 +466,43 @@ void main() {
         sendNotificationToNewThread: sendNotificationToNewThread,
         content: content,
         streamId: streamId,
+        prevContentSha256: prevContentSha256,
       );
       check(connection.lastRequest).isA<http.Request>()
         ..method.equals('PATCH')
         ..url.path.equals('/api/v1/messages/$messageId')
-        ..bodyFields.deepEquals(expected);
+        ..bodyFields.deepEquals(expected)
+        ..bodyFields.length.equals(expected.length);
       return result;
     }
 
+    test('pure content change', () {
+      return FakeApiConnection.with_((connection) async {
+        connection.prepare(json: UpdateMessageResult().toJson());
+        await checkUpdateMessage(connection,
+          messageId: eg.streamMessage().id,
+          content: 'asdf',
+          prevContentSha256: '34a780ad578b997db55b260beb60b501f3e04d30ba1a51fcf43cd8dd1241780d',
+          expected: {
+            'content': 'asdf',
+            'prev_content_sha256': '34a780ad578b997db55b260beb60b501f3e04d30ba1a51fcf43cd8dd1241780d',
+          });
+      });
+    });
+
+    test('pure content change (legacy, before prev_content_sha256)', () {
+      return FakeApiConnection.with_(zulipFeatureLevel: 378, (connection) async {
+        connection.prepare(json: UpdateMessageResult().toJson());
+        await checkUpdateMessage(connection,
+          messageId: eg.streamMessage().id,
+          content: 'asdf',
+          prevContentSha256: '34a780ad578b997db55b260beb60b501f3e04d30ba1a51fcf43cd8dd1241780d',
+          expected: {
+            'content': 'asdf',
+          });
+      });
+    });
+
     test('topic/content change', () {
       // A separate test exercises `streamId`;
       // the API doesn't allow changing channel and content at the same time.