Skip to content

Commit a5262ab

Browse files
chrisbobbechrisbobbe
committed
model: Implement edit-message methods on MessageStore
Related: #126
1 parent 1ca7f8a commit a5262ab

File tree

3 files changed

+315
-0
lines changed

3 files changed

+315
-0
lines changed

‎lib/model/message.dart

+81
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,34 @@ mixin MessageStore {
3535
/// All [Message] objects in the resulting list will be present in
3636
/// [this.messages].
3737
void reconcileMessages(List<Message> messages);
38+
39+
/// Whether the current edit request, if any, has failed.
40+
///
41+
/// Will be null if there is no current edit request
42+
/// or the message was deleted.
43+
/// Will be false if the current request hasn't failed
44+
/// and the update-message event hasn't arrived.
45+
bool? getEditMessageErrorStatus(int messageId);
46+
47+
/// Edits a message's content.
48+
///
49+
/// See also:
50+
/// * [getEditMessageErrorStatus]
51+
/// * [takeFailedMessageEdit]
52+
void editMessage({required int messageId, required String content});
53+
54+
/// Forgets the failed edit request and returns the attempted new content.
55+
///
56+
/// Should only be called when there is a failed request,
57+
/// per [getEditMessageErrorStatus].
58+
String? takeFailedMessageEdit(int messageId);
59+
}
60+
61+
class _EditMessageRequestStatus {
62+
_EditMessageRequestStatus({required this.hasError, required this.content});
63+
64+
bool hasError;
65+
final String content;
3866
}
3967

4068
class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
@@ -132,6 +160,52 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
132160
}
133161
}
134162

163+
@override
164+
bool? getEditMessageErrorStatus(int messageId) =>
165+
_editMessageRequests[messageId]?.hasError;
166+
final Map<int, _EditMessageRequestStatus> _editMessageRequests = {};
167+
168+
@override
169+
void editMessage({
170+
required int messageId,
171+
required String content,
172+
}) async {
173+
if (_editMessageRequests.containsKey(messageId)) {
174+
throw StateError('message ID already in editMessageRequests');
175+
}
176+
177+
_editMessageRequests[messageId] = _EditMessageRequestStatus(
178+
hasError: false, content: content);
179+
_notifyMessageListViewsForOneMessage(messageId);
180+
try {
181+
await updateMessage(connection, messageId: messageId, content: content);
182+
// On success, we'll clear `status` from editMessageRequests
183+
// when we get the event.
184+
} catch (e) {
185+
final status = _editMessageRequests[messageId];
186+
if (status == null) {
187+
// The event actually arrived before this request failed
188+
// (can happen with network issues).
189+
// Or, the message was deleted.
190+
return;
191+
}
192+
status.hasError = true;
193+
_notifyMessageListViewsForOneMessage(messageId);
194+
}
195+
}
196+
197+
@override
198+
String? takeFailedMessageEdit(int messageId) {
199+
final status = _editMessageRequests.remove(messageId);
200+
if (status == null) {
201+
throw StateError('called takeFailedEdit, but no edit');
202+
}
203+
if (!status.hasError) {
204+
throw StateError("called takeFailedEdit, but edit hasn't failed");
205+
}
206+
return status.content;
207+
}
208+
135209
void handleUserTopicEvent(UserTopicEvent event) {
136210
for (final view in _messageListViews) {
137211
view.handleUserTopicEvent(event);
@@ -183,6 +257,12 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
183257
// The message is guaranteed to be edited.
184258
// See also: https://zulip.com/api/get-events#update_message
185259
message.editState = MessageEditState.edited;
260+
261+
// Clear the edit-message progress feedback.
262+
// This makes a rare bug where we might clear the feedback too early,
263+
// if the user raced with themself to edit the same message
264+
// from multiple clients.
265+
_editMessageRequests.remove(message.id);
186266
}
187267
if (event.renderedContent != null) {
188268
assert(message.contentType == 'text/html',
@@ -245,6 +325,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
245325
void handleDeleteMessageEvent(DeleteMessageEvent event) {
246326
for (final messageId in event.messageIds) {
247327
messages.remove(messageId);
328+
_editMessageRequests.remove(messageId);
248329
}
249330
for (final view in _messageListViews) {
250331
view.handleDeleteMessageEvent(event);

‎lib/model/store.dart

+18
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,24 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
910910
return _messages.sendMessage(destination: destination, content: content);
911911
}
912912

913+
@override
914+
bool? getEditMessageErrorStatus(int messageId) {
915+
assert(!_disposed);
916+
return _messages.getEditMessageErrorStatus(messageId);
917+
}
918+
919+
@override
920+
void editMessage({required int messageId, required String content}) {
921+
assert(!_disposed);
922+
return _messages.editMessage(messageId: messageId, content: content);
923+
}
924+
925+
@override
926+
String? takeFailedMessageEdit(int messageId) {
927+
assert(!_disposed);
928+
return _messages.takeFailedMessageEdit(messageId);
929+
}
930+
913931
static List<CustomProfileField> _sortCustomProfileFields(List<CustomProfileField> initialCustomProfileFields) {
914932
// TODO(server): The realm-wide field objects have an `order` property,
915933
// but the actual API appears to be that the fields should be shown in

‎test/model/store_test.dart

+216
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,222 @@ void main() {
589589
});
590590
});
591591

592+
group('PerAccountStore edit-message methods', () {
593+
late PerAccountStore store;
594+
late FakeApiConnection connection;
595+
late StreamMessage message;
596+
597+
Future<void> prepare() async {
598+
store = eg.store();
599+
connection = store.connection as FakeApiConnection;
600+
message = eg.streamMessage();
601+
await store.addMessage(message);
602+
}
603+
604+
test('smoke', () => awaitFakeAsync((async) async {
605+
await prepare();
606+
check(store.getEditMessageErrorStatus(message.id)).isNull();
607+
608+
connection.prepare(
609+
json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1));
610+
store.editMessage(messageId: message.id, content: 'new content');
611+
check(connection.takeRequests()).single.isA<http.Request>()
612+
..method.equals('PATCH')
613+
..url.path.equals('/api/v1/messages/${message.id}')
614+
..bodyFields.deepEquals({
615+
'content': 'new content',
616+
});
617+
618+
// Mid-request
619+
async.elapse(Duration(milliseconds: 500));
620+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
621+
622+
// Request succeeded; event hasn't arrived
623+
async.elapse(Duration(milliseconds: 500));
624+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
625+
626+
await store.handleEvent(eg.updateMessageEditEvent(message));
627+
check(store.getEditMessageErrorStatus(message.id)).isNull();
628+
}));
629+
630+
test('request fails', () => awaitFakeAsync((async) async {
631+
await prepare();
632+
check(store.getEditMessageErrorStatus(message.id)).isNull();
633+
634+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
635+
store.editMessage(messageId: message.id, content: 'new content');
636+
async.elapse(Duration(seconds: 1));
637+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
638+
}));
639+
640+
test('request fails; take failed edit', () => awaitFakeAsync((async) async {
641+
await prepare();
642+
check(store.getEditMessageErrorStatus(message.id)).isNull();
643+
644+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
645+
store.editMessage(messageId: message.id, content: 'new content');
646+
async.elapse(Duration(seconds: 1));
647+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
648+
649+
check(store.takeFailedMessageEdit(message.id)).equals('new content');
650+
check(store.getEditMessageErrorStatus(message.id)).isNull();
651+
}));
652+
653+
test('request failure after event arrival', () => awaitFakeAsync((async) async {
654+
// This can happen with network issues.
655+
656+
await prepare();
657+
check(store.getEditMessageErrorStatus(message.id)).isNull();
658+
659+
connection.prepare(
660+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
661+
store.editMessage(messageId: message.id, content: 'new content');
662+
663+
async.elapse(Duration(milliseconds: 500));
664+
await store.handleEvent(eg.updateMessageEditEvent(message));
665+
check(store.getEditMessageErrorStatus(message.id)).isNull();
666+
667+
async.elapse(Duration(milliseconds: 500));
668+
check(store.getEditMessageErrorStatus(message.id)).isNull();
669+
}));
670+
671+
test('request failure before event arrival', () => awaitFakeAsync((async) async {
672+
// This can happen with network issues.
673+
674+
await prepare();
675+
check(store.getEditMessageErrorStatus(message.id)).isNull();
676+
677+
connection.prepare(
678+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
679+
store.editMessage(messageId: message.id, content: 'new content');
680+
681+
async.elapse(Duration(seconds: 1));
682+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
683+
684+
await store.handleEvent(eg.updateMessageEditEvent(message));
685+
check(store.getEditMessageErrorStatus(message.id)).isNull();
686+
}));
687+
688+
test('request failure before event arrival; take failed edit in between', () => awaitFakeAsync((async) async {
689+
// This can happen with network issues.
690+
691+
await prepare();
692+
check(store.getEditMessageErrorStatus(message.id)).isNull();
693+
694+
connection.prepare(
695+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
696+
store.editMessage(messageId: message.id, content: 'new content');
697+
698+
async.elapse(Duration(seconds: 1));
699+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
700+
check(store.takeFailedMessageEdit(message.id)).equals('new content');
701+
702+
await store.handleEvent(eg.updateMessageEditEvent(message));
703+
check(store.getEditMessageErrorStatus(message.id)).isNull();
704+
}));
705+
706+
// TODO tests with delete-message event
707+
708+
group('takeFailedMessageEdit throws StateError', () {
709+
test('if called before request starts', () => awaitFakeAsync((async) async {
710+
await prepare();
711+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
712+
}));
713+
714+
test('if called before request failure', () => awaitFakeAsync((async) async {
715+
await prepare();
716+
connection.prepare(
717+
apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
718+
store.editMessage(messageId: message.id, content: 'new content');
719+
720+
async.elapse(Duration(milliseconds: 500));
721+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
722+
async.flushTimers();
723+
}));
724+
725+
test('if called after request success', () => awaitFakeAsync((async) async {
726+
await prepare();
727+
connection.prepare(
728+
json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1));
729+
store.editMessage(messageId: message.id, content: 'new content');
730+
731+
async.elapse(Duration(seconds: 1));
732+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
733+
}));
734+
735+
test('if called after request success and event arrival', () => awaitFakeAsync((async) async {
736+
await prepare();
737+
connection.prepare(
738+
json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1));
739+
store.editMessage(messageId: message.id, content: 'new content');
740+
741+
async.elapse(Duration(seconds: 1));
742+
await store.handleEvent(eg.updateMessageEditEvent(message));
743+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
744+
}));
745+
746+
test('if called after event arrival and request failure', () => awaitFakeAsync((async) async {
747+
// This can happen with network issues.
748+
await prepare();
749+
connection.prepare(
750+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
751+
store.editMessage(messageId: message.id, content: 'new content');
752+
753+
async.elapse(Duration(seconds: 1));
754+
await store.handleEvent(eg.updateMessageEditEvent(message));
755+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
756+
}));
757+
758+
test('if called after request failure and event arrival', () => awaitFakeAsync((async) async {
759+
// This can happen with network issues.
760+
await prepare();
761+
connection.prepare(
762+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
763+
store.editMessage(messageId: message.id, content: 'new content');
764+
765+
async.elapse(Duration(milliseconds: 500));
766+
await store.handleEvent(eg.updateMessageEditEvent(message));
767+
768+
async.elapse(Duration(milliseconds: 500));
769+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
770+
}));
771+
772+
test('if called after message-delete event and request failure', () => awaitFakeAsync((async) async {
773+
await prepare();
774+
connection.prepare(
775+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
776+
store.editMessage(messageId: message.id, content: 'new content');
777+
778+
async.elapse(Duration(seconds: 1));
779+
await store.handleEvent(eg.deleteMessageEvent([message]));
780+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
781+
}));
782+
783+
test('if called after request failure and message-delete event', () => awaitFakeAsync((async) async {
784+
await prepare();
785+
connection.prepare(
786+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
787+
store.editMessage(messageId: message.id, content: 'new content');
788+
789+
async.elapse(Duration(milliseconds: 500));
790+
await store.handleEvent(eg.deleteMessageEvent([message]));
791+
792+
async.elapse(Duration(milliseconds: 500));
793+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
794+
}));
795+
796+
test('if called a second time for the same failed request', () => awaitFakeAsync((async) async {
797+
await prepare();
798+
connection.prepare(
799+
apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
800+
store.editMessage(messageId: message.id, content: 'new content');
801+
async.elapse(Duration(seconds: 1));
802+
check(store.takeFailedMessageEdit(message.id)).equals('new content');
803+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
804+
}));
805+
});
806+
});
807+
592808
group('UpdateMachine.load', () {
593809
late TestGlobalStore globalStore;
594810
late FakeApiConnection connection;

0 commit comments

Comments
 (0)