Skip to content

Commit ea74a55

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

File tree

3 files changed

+276
-0
lines changed

3 files changed

+276
-0
lines changed

‎lib/model/message.dart‎

Lines changed: 81 additions & 0 deletions
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‎

Lines changed: 18 additions & 0 deletions
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/message_test.dart‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import 'dart:convert';
2+
import 'dart:io';
23

34
import 'package:checks/checks.dart';
5+
import 'package:http/http.dart' as http;
46
import 'package:test/scaffolding.dart';
57
import 'package:zulip/api/model/events.dart';
68
import 'package:zulip/api/model/model.dart';
79
import 'package:zulip/api/model/submessage.dart';
10+
import 'package:zulip/api/route/messages.dart';
811
import 'package:zulip/model/message_list.dart';
912
import 'package:zulip/model/narrow.dart';
1013
import 'package:zulip/model/store.dart';
@@ -13,6 +16,7 @@ import '../api/fake_api.dart';
1316
import '../api/model/model_checks.dart';
1417
import '../api/model/submessage_checks.dart';
1518
import '../example_data.dart' as eg;
19+
import '../fake_async.dart';
1620
import '../stdlib_checks.dart';
1721
import 'message_list_test.dart';
1822
import 'store_checks.dart';
@@ -123,6 +127,179 @@ void main() {
123127
});
124128
});
125129

130+
group('edit-message methods', () {
131+
late PerAccountStore store;
132+
late FakeApiConnection connection;
133+
late StreamMessage message;
134+
135+
Future<void> prepare() async {
136+
store = eg.store();
137+
connection = store.connection as FakeApiConnection;
138+
message = eg.streamMessage();
139+
await store.addMessage(message);
140+
}
141+
142+
test('smoke', () => awaitFakeAsync((async) async {
143+
await prepare();
144+
check(store.getEditMessageErrorStatus(message.id)).isNull();
145+
146+
connection.prepare(
147+
json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1));
148+
store.editMessage(messageId: message.id, content: 'new content');
149+
check(connection.takeRequests()).single.isA<http.Request>()
150+
..method.equals('PATCH')
151+
..url.path.equals('/api/v1/messages/${message.id}')
152+
..bodyFields.deepEquals({
153+
'content': 'new content',
154+
});
155+
156+
async.elapse(Duration(milliseconds: 500));
157+
// Mid-request
158+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
159+
160+
async.elapse(Duration(milliseconds: 500));
161+
// Request has succeeded; event hasn't arrived
162+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
163+
164+
await store.handleEvent(eg.updateMessageEditEvent(message));
165+
check(store.getEditMessageErrorStatus(message.id)).isNull();
166+
}));
167+
168+
test('request fails', () => awaitFakeAsync((async) async {
169+
await prepare();
170+
check(store.getEditMessageErrorStatus(message.id)).isNull();
171+
172+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
173+
store.editMessage(messageId: message.id, content: 'new content');
174+
async.elapse(Duration(seconds: 1));
175+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
176+
}));
177+
178+
test('request fails; take failed edit', () => awaitFakeAsync((async) async {
179+
await prepare();
180+
check(store.getEditMessageErrorStatus(message.id)).isNull();
181+
182+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
183+
store.editMessage(messageId: message.id, content: 'new content');
184+
async.elapse(Duration(seconds: 1));
185+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
186+
187+
check(store.takeFailedMessageEdit(message.id)).equals('new content');
188+
check(store.getEditMessageErrorStatus(message.id)).isNull();
189+
}));
190+
191+
test('takeFailedMessageEdit throws StateError when nothing to take', () => awaitFakeAsync((async) async {
192+
await prepare();
193+
check(store.getEditMessageErrorStatus(message.id)).isNull();
194+
check(() => store.takeFailedMessageEdit(message.id)).throws<StateError>();
195+
}));
196+
197+
test('request failure after event arrival', () => awaitFakeAsync((async) async {
198+
// This can happen with network issues.
199+
200+
await prepare();
201+
check(store.getEditMessageErrorStatus(message.id)).isNull();
202+
203+
connection.prepare(
204+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
205+
store.editMessage(messageId: message.id, content: 'new content');
206+
207+
async.elapse(Duration(milliseconds: 500));
208+
await store.handleEvent(eg.updateMessageEditEvent(message));
209+
check(store.getEditMessageErrorStatus(message.id)).isNull();
210+
211+
async.elapse(Duration(milliseconds: 500));
212+
check(store.getEditMessageErrorStatus(message.id)).isNull();
213+
}));
214+
215+
test('request failure before event arrival', () => awaitFakeAsync((async) async {
216+
// This can happen with network issues.
217+
218+
await prepare();
219+
check(store.getEditMessageErrorStatus(message.id)).isNull();
220+
221+
connection.prepare(
222+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
223+
store.editMessage(messageId: message.id, content: 'new content');
224+
225+
async.elapse(Duration(seconds: 1));
226+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
227+
228+
await store.handleEvent(eg.updateMessageEditEvent(message));
229+
check(store.getEditMessageErrorStatus(message.id)).isNull();
230+
}));
231+
232+
test('request failure before event arrival; take failed edit in between', () => awaitFakeAsync((async) async {
233+
// This can happen with network issues.
234+
235+
await prepare();
236+
check(store.getEditMessageErrorStatus(message.id)).isNull();
237+
238+
connection.prepare(
239+
httpException: const SocketException('failed'), delay: Duration(seconds: 1));
240+
store.editMessage(messageId: message.id, content: 'new content');
241+
242+
async.elapse(Duration(seconds: 1));
243+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
244+
check(store.takeFailedMessageEdit(message.id)).equals('new content');
245+
246+
await store.handleEvent(eg.updateMessageEditEvent(message)); // no error
247+
check(store.getEditMessageErrorStatus(message.id)).isNull();
248+
}));
249+
250+
test('request fails, then message deleted', () => awaitFakeAsync((async) async {
251+
await prepare();
252+
check(store.getEditMessageErrorStatus(message.id)).isNull();
253+
254+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
255+
store.editMessage(messageId: message.id, content: 'new content');
256+
async.elapse(Duration(seconds: 1));
257+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue();
258+
259+
await store.handleEvent(eg.deleteMessageEvent([message])); // no error
260+
check(store.getEditMessageErrorStatus(message.id)).isNull();
261+
}));
262+
263+
test('message deleted while request in progress; we get failure response', () => awaitFakeAsync((async) async {
264+
await prepare();
265+
check(store.getEditMessageErrorStatus(message.id)).isNull();
266+
267+
connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1));
268+
store.editMessage(messageId: message.id, content: 'new content');
269+
270+
async.elapse(Duration(milliseconds: 500));
271+
// Mid-request
272+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
273+
274+
await store.handleEvent(eg.deleteMessageEvent([message]));
275+
check(store.getEditMessageErrorStatus(message.id)).isNull();
276+
277+
async.elapse(Duration(milliseconds: 500));
278+
// Request failure, but status has already been cleared
279+
check(store.getEditMessageErrorStatus(message.id)).isNull();
280+
}));
281+
282+
test('message deleted while request in progress but we get success response', () => awaitFakeAsync((async) async {
283+
await prepare();
284+
check(store.getEditMessageErrorStatus(message.id)).isNull();
285+
286+
connection.prepare(
287+
json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1));
288+
store.editMessage(messageId: message.id, content: 'new content');
289+
290+
async.elapse(Duration(milliseconds: 500));
291+
// Mid-request
292+
check(store.getEditMessageErrorStatus(message.id)).isNotNull().isFalse();
293+
294+
await store.handleEvent(eg.deleteMessageEvent([message]));
295+
check(store.getEditMessageErrorStatus(message.id)).isNull();
296+
297+
async.elapse(Duration(milliseconds: 500));
298+
// Request success
299+
check(store.getEditMessageErrorStatus(message.id)).isNull();
300+
}));
301+
});
302+
126303
group('handleMessageEvent', () {
127304
test('from empty', () async {
128305
await prepare();

0 commit comments

Comments
 (0)