compose: Show error dialog on failure of edit-message request

I'm not finding the discussion where we said we wanted this (for
consistency with the send-message UX), but I think we did.

Error-feedback logic copied from the tap-"Send" button
(_SendButtonState._send) modulo the error dialog's title, "Message
not saved" vs. "Message not sent".

Author: chrisbobbe

lib/model/message.dart

@@ -352,6 +352,7 @@ class MessageStoreImpl extends HasRealmStore with MessageStore, _OutboxMessageSt
       }
       status.hasError = true;
       _notifyMessageListViewsForOneMessage(messageId);
+      rethrow;
     }
   }
 

lib/widgets/compose_box.dart

@@ -1774,7 +1774,7 @@ class _EditMessageBanner extends _Banner {
   Color getBackgroundColor(DesignVariables designVariables) =>
     designVariables.bannerBgIntInfo;
 
-  void _handleTapSave (BuildContext pageContext) {
+  void _handleTapSave (BuildContext pageContext) async {
     final store = PerAccountStoreWidget.of(pageContext);
     final controller = composeBoxState.controller;
     if (controller is! EditMessageComposeBoxController) return; // TODO(log)
@@ -1800,10 +1800,24 @@ class _EditMessageBanner extends _Banner {
     final messageId = controller.messageId;
     final newContent = controller.content.textNormalized;
     composeBoxState.endEditInteraction();
-    store.editMessage(
-      messageId: messageId,
-      originalRawContent: originalRawContent,
-      newContent: newContent);
+
+    try {
+      await store.editMessage(
+        messageId: messageId,
+        originalRawContent: originalRawContent,
+        newContent: newContent);
+    } on ApiRequestException catch (e) {
+      if (!pageContext.mounted) return;
+      final zulipLocalizations = ZulipLocalizations.of(pageContext);
+      final message = switch (e) {
+        ZulipApiException() => zulipLocalizations.errorServerMessage(e.message),
+        _ => e.message,
+      };
+      showErrorDialog(context: pageContext,
+        title: zulipLocalizations.errorMessageEditNotSaved,
+        message: message);
+      return;
+    }
   }
 
   @override

test/model/message_test.dart

@@ -7,6 +7,7 @@ import 'package:crypto/crypto.dart';
 import 'package:fake_async/fake_async.dart';
 import 'package:http/http.dart' as http;
 import 'package:test/scaffolding.dart';
+import 'package:zulip/api/exception.dart';
 import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/model/submessage.dart';
@@ -682,8 +683,8 @@ void main() {
       check(store.getEditMessageErrorStatus(message.id)).isNull();
 
       connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
-      unawaited(store.editMessage(messageId: message.id,
-        originalRawContent: 'old content', newContent: 'new content'));
+      unawaited(check(store.editMessage(messageId: message.id,
+        originalRawContent: 'old content', newContent: 'new content')).throws<ZulipApiException>());
       checkNotifiedOnce();
       async.elapse(Duration(seconds: 1));
       check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
@@ -695,8 +696,8 @@ void main() {
       check(store.getEditMessageErrorStatus(message.id)).isNull();
 
       connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
-      unawaited(store.editMessage(messageId: message.id,
-        originalRawContent: 'old content', newContent: 'new content'));
+      unawaited(check(store.editMessage(messageId: message.id,
+        originalRawContent: 'old content', newContent: 'new content')).throws<ZulipApiException>());
       checkNotifiedOnce();
       async.elapse(Duration(seconds: 1));
       check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
@@ -760,8 +761,8 @@ void main() {
 
       connection.prepare(
         httpException: const SocketException('failed'), delay: Duration(seconds: 1));
-      unawaited(store.editMessage(messageId: message.id,
-        originalRawContent: 'old content', newContent: 'new content'));
+      unawaited(check(store.editMessage(messageId: message.id,
+        originalRawContent: 'old content', newContent: 'new content')).throws<NetworkException>());
       checkNotifiedOnce();
 
       async.elapse(Duration(seconds: 1));
@@ -781,8 +782,8 @@ void main() {
 
       connection.prepare(
         httpException: const SocketException('failed'), delay: Duration(seconds: 1));
-      unawaited(store.editMessage(messageId: message.id,
-        originalRawContent: 'old content', newContent: 'new content'));
+      unawaited(check(store.editMessage(messageId: message.id,
+        originalRawContent: 'old content', newContent: 'new content')).throws<NetworkException>());
       checkNotifiedOnce();
 
       async.elapse(Duration(seconds: 1));
@@ -801,8 +802,8 @@ void main() {
       check(store.getEditMessageErrorStatus(message.id)).isNull();
 
       connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
-      unawaited(store.editMessage(messageId: message.id,
-        originalRawContent: 'old content', newContent: 'new content'));
+      unawaited(check(store.editMessage(messageId: message.id,
+        originalRawContent: 'old content', newContent: 'new content')).throws<ZulipApiException>());
       checkNotifiedOnce();
       async.elapse(Duration(seconds: 1));
       check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();

test/widgets/action_sheet_test.dart

@@ -1780,6 +1780,12 @@ void main() {
         await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e
       }
 
+      Future<void> takeErrorDialogAndPump(WidgetTester tester) async {
+        final errorDialog = checkErrorDialog(tester, expectedTitle: 'Message not saved');
+        await tester.tap(find.byWidget(errorDialog));
+        await tester.pump();
+      }
+
       group('present/absent appropriately', () {
         /// Test whether the edit-message button is visible, given params.
         ///
@@ -1870,6 +1876,7 @@ void main() {
               connection.prepare(apiException: eg.apiBadRequest());
               await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save'));
               await tester.pump(Duration.zero);
+              await takeErrorDialogAndPump(tester);
             } else if (errorStatus == false) {
               // We're testing the request-in-progress state. Prepare a delay,
               // tap Save, and wait through only part of the delay.

test/widgets/compose_box_test.dart

@@ -1748,6 +1748,12 @@ void main() {
       await tester.pump(); // message list updates
     }
 
+    Future<void> takeErrorDialogAndPump(WidgetTester tester) async {
+      final errorDialog = checkErrorDialog(tester, expectedTitle: 'Message not saved');
+      await tester.tap(find.byWidget(errorDialog));
+      await tester.pump();
+    }
+
     /// Check that the compose box is in the "Preparing…" state,
     /// awaiting the fetch-raw-content request.
     Future<void> checkAwaitingRawMessageContent(WidgetTester tester) async {
@@ -1770,6 +1776,7 @@ void main() {
       await tester.tap(
         find.widgetWithText(ZulipWebUiKitButton, 'Save'), warnIfMissed: false);
       await tester.pump(Duration.zero);
+      checkNoDialog(tester);
       check(connection.lastRequest).equals(lastRequest);
     }
 
@@ -1788,6 +1795,7 @@ void main() {
       connection.prepare(apiException: eg.apiBadRequest());
       await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save'));
       await tester.pump(Duration.zero);
+      await takeErrorDialogAndPump(tester);
       await tester.tap(find.text('EDIT NOT SAVED'));
       await tester.pump();
       connection.takeRequests();
@@ -1966,6 +1974,7 @@ void main() {
         await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save'));
         connection.takeRequests();
         await tester.pump(Duration.zero);
+        await takeErrorDialogAndPump(tester);
         checkNotInEditingMode(tester, narrow: narrow);
         check(find.text('EDIT NOT SAVED')).findsOne();
 

test/widgets/message_list_test.dart

@@ -10,6 +10,7 @@ import 'package:flutter/rendering.dart';
 import 'package:flutter_checks/flutter_checks.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:http/http.dart' as http;
+import 'package:zulip/api/exception.dart';
 import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/initial_snapshot.dart';
 import 'package:zulip/api/model/model.dart';
@@ -2292,12 +2293,14 @@ void main() {
         messages: [message]);
 
       connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
-      unawaited(store.editMessage(messageId: message.id,
+      unawaited(check(store.editMessage(messageId: message.id,
         originalRawContent: 'foo',
-        newContent: 'bar'));
+        newContent: 'bar')).throws<ZulipApiException>());
       await tester.pump(Duration.zero);
       checkEditInProgress(tester);
       await tester.pump(Duration(seconds: 1));
+      // (the error dialog is tested elsewhere;
+      // it's triggered in the "Save" tap handler, not store.editMessage)
       checkEditFailed(tester);
 
       connection.prepare(json: GetMessageResult(