Skip to content

Commit 0b91ab9

Browse files
oxlingoxling
committed
msglist: Handle updated events in MessageListView (#118).
Processes an UpdateMessageEvent and hands it off to the MessageListView to update, if the message is visible in the MessageListView. This completes the changes required for issue #118.
1 parent 2465701 commit 0b91ab9

File tree

6 files changed

+291
-6
lines changed

6 files changed

+291
-6
lines changed

‎lib/api/model/model.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,13 @@ class Subscription {
250250
sealed class Message {
251251
final String? avatarUrl;
252252
final String client;
253-
final String content;
253+
String content;
254254
final String contentType;
255255

256256
// final List<MessageEditHistory> editHistory; // TODO handle
257257
final int id;
258-
final bool isMeMessage;
259-
final int? lastEditTimestamp;
258+
bool isMeMessage;
259+
int? lastEditTimestamp;
260260

261261
// final List<Reaction> reactions; // TODO handle
262262
final int recipientId;
@@ -271,7 +271,7 @@ sealed class Message {
271271

272272
// final List<TopicLink> topicLinks; // TODO handle
273273
// final string type; // handled by runtime type of object
274-
final List<String> flags; // TODO enum
274+
List<String> flags; // TODO enum
275275
final String? matchContent;
276276
final String? matchSubject;
277277

‎lib/model/message_list.dart

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import 'package:collection/collection.dart';
12
import 'package:flutter/foundation.dart';
23

4+
import '../api/model/events.dart';
35
import '../api/model/model.dart';
46
import '../api/route/messages.dart';
57
import 'content.dart';
@@ -86,6 +88,70 @@ class MessageListView extends ChangeNotifier {
8688
notifyListeners();
8789
}
8890

91+
_applyChangesToMessage(UpdateMessageEvent event, Message message) {
92+
//In earlier server versions, omitting the userId indicates that this is a rendering-only update.
93+
//That means this change was initiated by the server, not the user.
94+
final isRenderingOnly = (event.renderingOnly != null && event.renderingOnly == true) ||
95+
event.userId == null;
96+
97+
if (event.editTimestamp != null && !isRenderingOnly) {
98+
//Only update the timestamp if this was a user-led update,
99+
//not a server-only update
100+
message.lastEditTimestamp = event.editTimestamp;
101+
}
102+
103+
if (!event.flags.equals(message.flags)) {
104+
message.flags = event.flags;
105+
}
106+
107+
if (event.renderedContent != null) {
108+
assert(message.contentType == 'text/html', "Expected message to have html contentType. Instead, got ${message.contentType}");
109+
message.content = event.renderedContent!;
110+
}
111+
112+
if (event.isMeMessage != null) {
113+
message.isMeMessage = event.isMeMessage!;
114+
}
115+
116+
}
117+
118+
//This is almost directly copied from package:collection/src/algorithms.dart.
119+
//The way that package was set up doesn't allow us to search
120+
//for a message ID among a bunch of message objects - this is a quick
121+
//modification of that method to work here for us.
122+
@visibleForTesting
123+
int findMessageWithId(int messageId) {
124+
var min = 0;
125+
var max = messages.length;
126+
127+
while (min < max) {
128+
var mid = min + ((max - min) >> 1);
129+
final message = messages[mid];
130+
var comp = message.id.compareTo(messageId);
131+
if (comp == 0) return mid;
132+
if (comp < 0) {
133+
min = mid + 1;
134+
} else {
135+
max = mid;
136+
}
137+
}
138+
return -1;
139+
}
140+
141+
void maybeUpdateMessage(UpdateMessageEvent event) {
142+
final idx = findMessageWithId(event.messageId);
143+
144+
if (idx == -1) {
145+
return;
146+
}
147+
148+
final message = messages[idx];
149+
_applyChangesToMessage(event, message);
150+
151+
contents[idx] = parseContent(message.content);
152+
notifyListeners();
153+
}
154+
89155
/// Called when the app is reassembled during debugging, e.g. for hot reload.
90156
///
91157
/// This will redo from scratch any computations we can, such as parsing

‎lib/model/store.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,9 @@ class PerAccountStore extends ChangeNotifier {
272272
}
273273
} else if (event is UpdateMessageEvent) {
274274
assert(debugLog("server event: update_message ${event.messageId}"));
275-
// TODO handle
275+
for (final view in _messageListViews) {
276+
view.maybeUpdateMessage(event);
277+
}
276278
} else if (event is DeleteMessageEvent) {
277279
assert(debugLog("server event: delete_message ${event.messageIds}"));
278280
// TODO handle

‎test/api/model/model_checks.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ extension MessageChecks on Subject<Message> {
99
}
1010

1111
Subject<List<String>> get flags => has((e) => e.flags, 'flags');
12+
Subject<String> get content => has((e) => e.content, 'content');
13+
Subject<bool> get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage');
14+
Subject<int?> get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp');
1215

1316
// TODO accessors for other fields
1417
}

‎test/example_data.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ StreamMessage streamMessage({
127127
String? topic,
128128
String? content,
129129
String? contentMarkdown,
130+
List<String>? flags,
130131
}) {
131132
final effectiveStream = stream ?? _stream();
132133
// The use of JSON here is convenient in order to delegate parts of the data
@@ -140,7 +141,7 @@ StreamMessage streamMessage({
140141
..._messagePropertiesFromContent(content, contentMarkdown),
141142
'display_recipient': effectiveStream.name,
142143
'stream_id': effectiveStream.streamId,
143-
'flags': [],
144+
'flags': flags ?? [],
144145
'id': id ?? 1234567, // TODO generate example IDs
145146
'subject': topic ?? 'example topic',
146147
'timestamp': 1678139636,

‎test/model/message_list_test.dart

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:test/scaffolding.dart';
3+
import 'package:zulip/api/model/events.dart';
4+
import 'package:zulip/api/model/model.dart';
5+
import 'package:zulip/api/route/messages.dart';
6+
import 'package:zulip/model/message_list.dart';
7+
import 'package:zulip/model/narrow.dart';
8+
import 'package:zulip/model/store.dart';
9+
import '../api/fake_api.dart';
10+
import '../api/model/model_checks.dart';
11+
import '../model/binding.dart';
12+
import '../model/test_store.dart';
13+
import '../example_data.dart' as eg;
14+
15+
const int userId = 1;
16+
const int streamId = 2;
17+
18+
Future<PerAccountStore> setupStore(ZulipStream stream) async {
19+
await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot());
20+
21+
final store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id);
22+
store.addUser(eg.user(userId: userId));
23+
store.addStream(stream);
24+
return store;
25+
}
26+
27+
Future<MessageListView> messageListViewWithMessages(List<Message> messages, PerAccountStore store, Narrow narrow) async {
28+
final messageList = MessageListView.init(store: store, narrow: narrow);
29+
30+
final connection = store.connection as FakeApiConnection;
31+
32+
connection.prepare(json: GetMessagesResult(
33+
anchor: messages.first.id,
34+
foundNewest: true,
35+
foundOldest: true,
36+
foundAnchor: true,
37+
historyLimited: false,
38+
messages: messages,
39+
).toJson());
40+
41+
await messageList.fetch();
42+
43+
check(messageList.messages.length).equals(messages.length);
44+
45+
return messageList;
46+
}
47+
48+
void main() async {
49+
TestZulipBinding.ensureInitialized();
50+
51+
const narrow = StreamNarrow(streamId);
52+
final stream = eg.stream(streamId: streamId);
53+
54+
group('update message tests', () {
55+
56+
test('find message in message list returns index of message', () async {
57+
addTearDown(TestZulipBinding.instance.reset);
58+
59+
final store = await setupStore(stream);
60+
61+
final m1 = eg.streamMessage(id: 792, stream: stream);
62+
final m2 = eg.streamMessage(id: 793, stream: stream);
63+
final m3 = eg.streamMessage(id: 794, stream: stream);
64+
65+
final messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow);
66+
67+
var idx = messageList.findMessageWithId(793);
68+
check(idx).equals(1);
69+
70+
idx = messageList.findMessageWithId(999);
71+
check(idx).equals(-1);
72+
});
73+
74+
test('update events are correctly applied to message when it is in the stream', () async {
75+
addTearDown(TestZulipBinding.instance.reset);
76+
final store = await setupStore(stream);
77+
78+
const oldContent = "<p>Hello, world</p>";
79+
const newContent = "<p>Hello, edited</p>";
80+
const newTimestamp = 99999;
81+
82+
List<String> oldFlags = [];
83+
List<String> newFlags = ["starred"];
84+
85+
final mockMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent, flags: oldFlags);
86+
final messageList = await messageListViewWithMessages([mockMessage], store, narrow);
87+
88+
final updateEvent = UpdateMessageEvent(
89+
id: 1,
90+
messageId: mockMessage.id,
91+
messageIds: [mockMessage.id],
92+
flags: newFlags,
93+
renderedContent: newContent,
94+
editTimestamp: newTimestamp,
95+
isMeMessage: true,
96+
userId: userId
97+
);
98+
99+
final message = messageList.messages[0];
100+
check(message)
101+
..content.equals(oldContent)
102+
..flags.deepEquals(oldFlags)
103+
..isMeMessage.equals(false);
104+
105+
var listenersNotified = false;
106+
107+
messageList.addListener(() { listenersNotified = true; });
108+
messageList.maybeUpdateMessage(updateEvent);
109+
110+
final updatedMessage = messageList.messages[0];
111+
check(updatedMessage).identicalTo(message);
112+
check(listenersNotified).equals(true);
113+
114+
check(message)
115+
..content.equals(newContent)
116+
..lastEditTimestamp.equals(newTimestamp)
117+
..flags.equals(newFlags)
118+
..isMeMessage.equals(true);
119+
});
120+
121+
test('update event is ignored when message is not in the message list', () async {
122+
addTearDown(TestZulipBinding.instance.reset);
123+
final store = await setupStore(stream);
124+
125+
const oldContent = "<p>Hello, world</p>";
126+
const newContent = "<p>Hello, edited</p>";
127+
const newTimestamp = 99999;
128+
129+
final mockMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent);
130+
final messageList = await messageListViewWithMessages([mockMessage], store, narrow);
131+
132+
final updateEvent = UpdateMessageEvent(
133+
id: 1,
134+
messageId: 972,
135+
messageIds: [972],
136+
flags: mockMessage.flags,
137+
renderedContent: newContent,
138+
editTimestamp: newTimestamp,
139+
userId: userId,
140+
);
141+
142+
final message = messageList.messages[0];
143+
check(message).content.equals(oldContent);
144+
145+
var listenersNotified = false;
146+
147+
messageList.addListener(() { listenersNotified = true; });
148+
messageList.maybeUpdateMessage(updateEvent);
149+
150+
check(listenersNotified).equals(false);
151+
check(message).content.equals(oldContent);
152+
153+
});
154+
test('rendering-only update does not change timestamp', () async {
155+
addTearDown(TestZulipBinding.instance.reset);
156+
final store = await setupStore(stream);
157+
158+
const oldContent = "<p>Hello, world</p>";
159+
const newContent = "<p>Hello, world</p> <div>Some link preview</div>";
160+
const newTimestamp = 99999;
161+
162+
final mockMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent);
163+
final messageList = await messageListViewWithMessages([mockMessage], store, narrow);
164+
165+
final updateEvent = UpdateMessageEvent(
166+
id: 1,
167+
messageId: 972,
168+
messageIds: [972],
169+
flags: mockMessage.flags,
170+
renderedContent: newContent,
171+
editTimestamp: newTimestamp,
172+
renderingOnly: true,
173+
userId: null,
174+
);
175+
176+
final message = messageList.messages[0];
177+
messageList.maybeUpdateMessage(updateEvent);
178+
check(message)
179+
..content.equals(newContent)
180+
..lastEditTimestamp.isNull();
181+
});
182+
183+
test('rendering-only update does not change timestamp (for old server versions)', () async {
184+
addTearDown(TestZulipBinding.instance.reset);
185+
final store = await setupStore(stream);
186+
187+
const oldContent = "<p>Hello, world</p>";
188+
const newContent = "<p>Hello, world</p> <div>Some link preview</div>";
189+
const newTimestamp = 99999;
190+
191+
final mockMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent);
192+
final messageList = await messageListViewWithMessages([mockMessage], store, narrow);
193+
194+
final updateEvent = UpdateMessageEvent(
195+
id: 1,
196+
messageId: 972,
197+
messageIds: [972],
198+
flags: mockMessage.flags,
199+
renderedContent: newContent,
200+
editTimestamp: newTimestamp,
201+
userId: null,
202+
);
203+
204+
final message = messageList.messages[0];
205+
messageList.maybeUpdateMessage(updateEvent);
206+
check(message)
207+
..content.equals(newContent)
208+
..lastEditTimestamp.isNull();
209+
});
210+
211+
212+
});
213+
}

0 commit comments

Comments
 (0)