Skip to content

Commit 45b4b05

Browse files
chrisbobbechrisbobbe
committed
model: Implement edit-message methods on MessageStore
Related: #126
1 parent 2683df0 commit 45b4b05

File tree

3 files changed

+272
-0
lines changed

3 files changed

+272
-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

+173
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,179 @@ 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+
async.elapse(Duration(milliseconds: 500));
619+
// Mid-request
620+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
621+
622+
async.elapse(Duration(milliseconds: 500));
623+
// Request has succeeded; event hasn't arrived
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('takeFailedMessageEdit throws StateError when nothing to take', () => awaitFakeAsync((async) async {
654+
await prepare();
655+
check(store.getEditMessageErrorStatus(message.id)).isNull();
656+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
657+
}));
658+
659+
test('request failure after event arrival', () => awaitFakeAsync((async) async {
660+
// This can happen with network issues.
661+
662+
await prepare();
663+
check(store.getEditMessageErrorStatus(message.id)).isNull();
664+
665+
connection.prepare(
666+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
667+
store.editMessage(messageId: message.id, content: 'new content');
668+
669+
async.elapse(Duration(milliseconds: 500));
670+
await store.handleEvent(eg.updateMessageEditEvent(message));
671+
check(store.getEditMessageErrorStatus(message.id)).isNull();
672+
673+
async.elapse(Duration(milliseconds: 500));
674+
check(store.getEditMessageErrorStatus(message.id)).isNull();
675+
}));
676+
677+
test('request failure before event arrival', () => awaitFakeAsync((async) async {
678+
// This can happen with network issues.
679+
680+
await prepare();
681+
check(store.getEditMessageErrorStatus(message.id)).isNull();
682+
683+
connection.prepare(
684+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
685+
store.editMessage(messageId: message.id, content: 'new content');
686+
687+
async.elapse(Duration(seconds: 1));
688+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
689+
690+
await store.handleEvent(eg.updateMessageEditEvent(message));
691+
check(store.getEditMessageErrorStatus(message.id)).isNull();
692+
}));
693+
694+
test('request failure before event arrival; take failed edit in between', () => awaitFakeAsync((async) async {
695+
// This can happen with network issues.
696+
697+
await prepare();
698+
check(store.getEditMessageErrorStatus(message.id)).isNull();
699+
700+
connection.prepare(
701+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
702+
store.editMessage(messageId: message.id, content: 'new content');
703+
704+
async.elapse(Duration(seconds: 1));
705+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
706+
check(store.takeFailedMessageEdit(message.id)).equals('new content');
707+
708+
await store.handleEvent(eg.updateMessageEditEvent(message)); // no error
709+
check(store.getEditMessageErrorStatus(message.id)).isNull();
710+
}));
711+
712+
test('request fails, then message deleted', () => awaitFakeAsync((async) async {
713+
await prepare();
714+
check(store.getEditMessageErrorStatus(message.id)).isNull();
715+
716+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
717+
store.editMessage(messageId: message.id, content: 'new content');
718+
async.elapse(Duration(seconds: 1));
719+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
720+
721+
await store.handleEvent(eg.deleteMessageEvent([message])); // no error
722+
check(store.getEditMessageErrorStatus(message.id)).isNull();
723+
}));
724+
725+
test('message deleted while request in progress; we get failure response', () => awaitFakeAsync((async) async {
726+
await prepare();
727+
check(store.getEditMessageErrorStatus(message.id)).isNull();
728+
729+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
730+
store.editMessage(messageId: message.id, content: 'new content');
731+
732+
async.elapse(Duration(milliseconds: 500));
733+
// Mid-request
734+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
735+
736+
await store.handleEvent(eg.deleteMessageEvent([message]));
737+
check(store.getEditMessageErrorStatus(message.id)).isNull();
738+
739+
async.elapse(Duration(milliseconds: 500));
740+
// Request failure, but status has already been cleared
741+
check(store.getEditMessageErrorStatus(message.id)).isNull();
742+
}));
743+
744+
test('message deleted while request in progress but we get success response', () => awaitFakeAsync((async) async {
745+
await prepare();
746+
check(store.getEditMessageErrorStatus(message.id)).isNull();
747+
748+
connection.prepare(
749+
json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1));
750+
store.editMessage(messageId: message.id, content: 'new content');
751+
752+
async.elapse(Duration(milliseconds: 500));
753+
// Mid-request
754+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
755+
756+
await store.handleEvent(eg.deleteMessageEvent([message]));
757+
check(store.getEditMessageErrorStatus(message.id)).isNull();
758+
759+
async.elapse(Duration(milliseconds: 500));
760+
// Request success
761+
check(store.getEditMessageErrorStatus(message.id)).isNull();
762+
}));
763+
});
764+
592765
group('UpdateMachine.load', () {
593766
late TestGlobalStore globalStore;
594767
late FakeApiConnection connection;

0 commit comments

Comments
 (0)