Skip to content

Commit

Permalink
msglist: Handle updated events in MessageListView (zulip#118).
Browse files Browse the repository at this point in the history
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 zulip#118.
  • Loading branch information
oxling committed Jul 26, 2023
1 parent 2465701 commit a045cc7
Show file tree
Hide file tree
Showing 6 changed files with 278 additions and 6 deletions.
8 changes: 4 additions & 4 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,13 @@ class Subscription {
sealed class Message {
final String? avatarUrl;
final String client;
final String content;
String content;
final String contentType;

// final List<MessageEditHistory> editHistory; // TODO handle
final int id;
final bool isMeMessage;
final int? lastEditTimestamp;
bool isMeMessage;
int? lastEditTimestamp;

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

// final List<TopicLink> topicLinks; // TODO handle
// final string type; // handled by runtime type of object
final List<String> flags; // TODO enum
List<String> flags; // TODO enum
final String? matchContent;
final String? matchSubject;

Expand Down
66 changes: 66 additions & 0 deletions lib/model/message_list.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import 'package:collection/collection.dart';
import 'package:flutter/foundation.dart';

import '../api/model/events.dart';
import '../api/model/model.dart';
import '../api/route/messages.dart';
import 'content.dart';
Expand Down Expand Up @@ -86,6 +88,70 @@ class MessageListView extends ChangeNotifier {
notifyListeners();
}

_applyChangesToMessage(UpdateMessageEvent event, Message message) {
//In earlier server versions, omitting the userId indicates that this is a rendering-only update.
//That means this change was initiated by the server, not the user.
bool isRenderingOnly = (event.renderingOnly != null && event.renderingOnly == true) ||
event.userId == null;

if (event.editTimestamp != null && !isRenderingOnly) {
//Only update the timestamp if this was a user-led update,
//not a server-only update
message.lastEditTimestamp = event.editTimestamp;
}

if (!event.flags.equals(message.flags)) {
message.flags = event.flags;
}

if (event.renderedContent != null) {
assert(message.contentType == 'text/html', "Expected message to have html contentType. Instead, got ${message.contentType}");
message.content = event.renderedContent!;
}

if (event.isMeMessage != null) {
message.isMeMessage = event.isMeMessage!;
}

}

//This is almost directly copied from package:collection/src/algorithms.dart.
//The way that package was set up doesn't allow us to search
//for a message ID among a bunch of message objects - this is a quick
//modification of that method to work here for us.
@visibleForTesting
int findMessageWithId(int messageId) {
var min = 0;
var max = messages.length;

while (min < max) {
var mid = min + ((max - min) >> 1);
Message message = messages[mid];
var comp = message.id.compareTo(messageId);
if (comp == 0) return mid;
if (comp < 0) {
min = mid + 1;
} else {
max = mid;
}
}
return -1;
}

void maybeUpdateMessage(UpdateMessageEvent event) {
int idx = findMessageWithId(event.messageId);

if (idx == -1) {
return;
}

Message message = messages[idx];
_applyChangesToMessage(event, message);

contents[idx] = parseContent(message.content);
notifyListeners();
}

/// Called when the app is reassembled during debugging, e.g. for hot reload.
///
/// This will redo from scratch any computations we can, such as parsing
Expand Down
4 changes: 3 additions & 1 deletion lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ class PerAccountStore extends ChangeNotifier {
}
} else if (event is UpdateMessageEvent) {
assert(debugLog("server event: update_message ${event.messageId}"));
// TODO handle
for (final view in _messageListViews) {
view.maybeUpdateMessage(event);
}
} else if (event is DeleteMessageEvent) {
assert(debugLog("server event: delete_message ${event.messageIds}"));
// TODO handle
Expand Down
3 changes: 3 additions & 0 deletions test/api/model/model_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ extension MessageChecks on Subject<Message> {
}

Subject<List<String>> get flags => has((e) => e.flags, 'flags');
Subject<String> get content => has((e) => e.content, 'content');
Subject<bool> get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage');
Subject<int?> get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp');

// TODO accessors for other fields
}
Expand Down
3 changes: 2 additions & 1 deletion test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ StreamMessage streamMessage({
String? topic,
String? content,
String? contentMarkdown,
List<String>? flags,
}) {
final effectiveStream = stream ?? _stream();
// The use of JSON here is convenient in order to delegate parts of the data
Expand All @@ -140,7 +141,7 @@ StreamMessage streamMessage({
..._messagePropertiesFromContent(content, contentMarkdown),
'display_recipient': effectiveStream.name,
'stream_id': effectiveStream.streamId,
'flags': [],
'flags': flags ?? [],
'id': id ?? 1234567, // TODO generate example IDs
'subject': topic ?? 'example topic',
'timestamp': 1678139636,
Expand Down
200 changes: 200 additions & 0 deletions test/model/message_list_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
import 'package:checks/checks.dart';
import 'package:test/scaffolding.dart';
import 'package:zulip/api/model/events.dart';
import 'package:zulip/api/model/model.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/message_list.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/model/store.dart';
import '../api/fake_api.dart';
import '../api/model/model_checks.dart';
import '../model/binding.dart';
import '../model/test_store.dart';
import '../example_data.dart' as eg;

const int userId = 1;
const int streamId = 2;

Future<PerAccountStore> setupStore(ZulipStream stream) async {
await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot());
PerAccountStore store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id);
store.addUser(eg.user(userId: userId));
store.addStream(stream);
return store;
}

Future<MessageListView> messageListViewWithMessages(List<Message> messages, PerAccountStore store, Narrow narrow) async {
MessageListView messageList = MessageListView.init(store: store, narrow: narrow);

final connection = store.connection as FakeApiConnection;

connection.prepare(json: GetMessagesResult(
anchor: messages.first.id,
foundNewest: true,
foundOldest: true,
foundAnchor: true,
historyLimited: false,
messages: messages,
).toJson());

await messageList.fetch();

check(messageList.messages.length).equals(messages.length);

return messageList;
}

void main() async {
TestZulipBinding.ensureInitialized();
const narrow = StreamNarrow(streamId);

final ZulipStream stream = eg.stream(streamId: streamId);
PerAccountStore store = await setupStore(stream);

group('update message tests', () {

test('find message in message list returns index of message', () async {
Message m1 = eg.streamMessage(id: 792, stream: stream);
Message m2 = eg.streamMessage(id: 793, stream: stream);
Message m3 = eg.streamMessage(id: 794, stream: stream);

MessageListView messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow);

int idx = messageList.findMessageWithId(793);
check(idx).equals(1);

idx = messageList.findMessageWithId(999);
check(idx).equals(-1);
});

test('update events are correctly applied to message when it is in the stream', () async {
String oldContent = "<p>Hello, world</p>";
String newContent = "<p>Hello, edited</p>";
int newTimestamp = 99999;

List<String> oldFlags = [];
List<String> newFlags = ["starred"];

Message mockMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent, flags: oldFlags);
MessageListView messageList = await messageListViewWithMessages([mockMessage], store, narrow);

UpdateMessageEvent updateEvent = UpdateMessageEvent(
id: 1,
messageId: mockMessage.id,
messageIds: [mockMessage.id],
flags: newFlags,
renderedContent: newContent,
editTimestamp: newTimestamp,
isMeMessage: true,
userId: userId
);

Message message = messageList.messages[0];
check(message)
..content.equals(oldContent)
..flags.deepEquals(oldFlags)
..isMeMessage.equals(false);

bool listenersNotified = false;

messageList.addListener(() { listenersNotified = true; });
messageList.maybeUpdateMessage(updateEvent);

Message updatedMessage = messageList.messages[0];
check(updatedMessage).identicalTo(message);
check(listenersNotified).equals(true);

check(message)
..content.equals(newContent)
..lastEditTimestamp.equals(newTimestamp)
..flags.equals(newFlags)
..isMeMessage.equals(true);
});

test('update event is ignored when message is not in the message list', () async {
String oldContent = "<p>Hello, world</p>";
String newContent = "<p>Hello, edited</p>";
int newTimestamp = 99999;

Message mockMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent);
MessageListView messageList = await messageListViewWithMessages([mockMessage], store, narrow);

UpdateMessageEvent updateEvent = UpdateMessageEvent(
id: 1,
messageId: 972,
messageIds: [972],
flags: mockMessage.flags,
renderedContent: newContent,
editTimestamp: newTimestamp,
userId: userId,
);

Message message = messageList.messages[0];
check(message).content.equals(oldContent);

bool listenersNotified = false;

messageList.addListener(() { listenersNotified = true; });
messageList.maybeUpdateMessage(updateEvent);

Message updatedMessage = messageList.messages[0];

check(listenersNotified).equals(false);
check(updatedMessage).identicalTo(message);
check(message).content.equals(oldContent);

});
test('rendering-only update does not change timestamp', () async {
String oldContent = "<p>Hello, world</p>";
String newContent = "<p>Hello, world</p> <div>Some link preview</div>";
int newTimestamp = 99999;

Message mockMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent);
MessageListView messageList = await messageListViewWithMessages([mockMessage], store, narrow);

UpdateMessageEvent updateEvent = UpdateMessageEvent(
id: 1,
messageId: 972,
messageIds: [972],
flags: mockMessage.flags,
renderedContent: newContent,
editTimestamp: newTimestamp,
renderingOnly: true,
userId: null,
);

Message message = messageList.messages[0];
messageList.maybeUpdateMessage(updateEvent);
check(message)
..content.equals(newContent)
..lastEditTimestamp.isNull();
});

test('rendering-only update does not change timestamp (for old server versions)', () async {
String oldContent = "<p>Hello, world</p>";
String newContent = "<p>Hello, world</p> <div>Some link preview</div>";
int newTimestamp = 99999;

Message mockMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent);
MessageListView messageList = await messageListViewWithMessages([mockMessage], store, narrow);

UpdateMessageEvent updateEvent = UpdateMessageEvent(
id: 1,
messageId: 972,
messageIds: [972],
flags: mockMessage.flags,
renderedContent: newContent,
editTimestamp: newTimestamp,
userId: null,
);

Message message = messageList.messages[0];
messageList.maybeUpdateMessage(updateEvent);
check(message)
..content.equals(newContent)
..lastEditTimestamp.isNull();
});


});
}

0 comments on commit a045cc7

Please sign in to comment.