Skip to content

Commit 6e19c53

Browse files
committed
message: Mutate messages for flags events here, instead of in msglists
Like a recent commit did for message edits, and one before that did for reactions, this fixes the part of 455 that's concerned with update-message-flags events. Like in the commit that dealt with message edits, this fixes the symptom of a dropped update when there are zero message lists open. In this case I'm *pretty* sure there wasn't a user-facing symptom with the double-processing situation when the message is in two or more open message lists. That's because I believe we don't give different behavior for a flag that's present multiple times in the `flags` list, vs. just once.
1 parent 42fbe0f commit 6e19c53

File tree

3 files changed

+47
-32
lines changed

3 files changed

+47
-32
lines changed

lib/model/message.dart

+27-2
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,33 @@ class MessageStoreImpl with MessageStore {
133133
}
134134

135135
void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) {
136-
for (final view in _messageListViews) {
137-
view.handleUpdateMessageFlagsEvent(event); // TODO update mainly in [messages] instead
136+
final isAdd = switch (event) {
137+
UpdateMessageFlagsAddEvent() => true,
138+
UpdateMessageFlagsRemoveEvent() => false,
139+
};
140+
141+
if (isAdd && (event as UpdateMessageFlagsAddEvent).all) {
142+
for (final message in messages.values) {
143+
message.flags.add(event.flag);
144+
}
145+
146+
for (final view in _messageListViews) {
147+
if (view.messages.isEmpty) continue;
148+
view.notifyListenersUnconditionally();
149+
}
150+
} else {
151+
for (final messageId in event.messages) {
152+
final message = messages[messageId];
153+
if (message == null) continue; // a message we don't know about yet
154+
155+
isAdd
156+
? message.flags.add(event.flag)
157+
: message.flags.remove(event.flag);
158+
159+
for (final view in _messageListViews) {
160+
view.notifyListenersIfMessagePresent(messageId);
161+
}
162+
}
138163
}
139164
}
140165

lib/model/message_list.dart

+9-29
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,16 @@ class MessageListView with ChangeNotifier, _MessageSequence {
444444
notifyListeners();
445445
}
446446

447+
/// Notify listeners.
448+
///
449+
/// Compare with [notifyListenersIfMessagePresent].
450+
void notifyListenersUnconditionally() {
451+
notifyListeners();
452+
}
453+
447454
/// Notify listeners if the given message is present in this view.
455+
///
456+
/// Compare with [notifyListenersUnconditionally].
448457
void notifyListenersIfMessagePresent(int messageId) {
449458
final index = _findMessageWithId(messageId);
450459
if (index != -1) {
@@ -460,35 +469,6 @@ class MessageListView with ChangeNotifier, _MessageSequence {
460469
}
461470
}
462471

463-
void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) {
464-
final isAdd = switch (event) {
465-
UpdateMessageFlagsAddEvent() => true,
466-
UpdateMessageFlagsRemoveEvent() => false,
467-
};
468-
469-
bool didUpdateAny = false;
470-
if (isAdd && (event as UpdateMessageFlagsAddEvent).all) {
471-
for (final message in messages) {
472-
message.flags.add(event.flag);
473-
didUpdateAny = true;
474-
}
475-
} else {
476-
for (final messageId in event.messages) {
477-
final index = _findMessageWithId(messageId);
478-
if (index != -1) {
479-
final message = messages[index];
480-
isAdd ? message.flags.add(event.flag) : message.flags.remove(event.flag);
481-
didUpdateAny = true;
482-
}
483-
}
484-
}
485-
if (!didUpdateAny) {
486-
return;
487-
}
488-
489-
notifyListeners();
490-
}
491-
492472
/// Called when the app is reassembled during debugging, e.g. for hot reload.
493473
///
494474
/// This will redo from scratch any computations we can, such as parsing

test/model/message_list_test.dart

+11-1
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,17 @@ void main() {
418418
doCheckMessageAfterFetch:
419419
(messageSubject) => messageSubject.content.endsWith('<p>edited</p>'),
420420
),
421-
// TODO(#455) message flags
421+
(
422+
eventName: 'UpdateMessageFlagsEvent',
423+
mkEvent: (message) => UpdateMessageFlagsAddEvent(
424+
id: 1,
425+
flag: MessageFlag.starred,
426+
messages: [message.id],
427+
all: false,
428+
),
429+
doCheckMessageAfterFetch:
430+
(messageSubject) => messageSubject.flags.contains(MessageFlag.starred),
431+
),
422432
]) {
423433
test('$eventName is applied even when message not in any msglists', () async {
424434
final stream = eg.stream();

0 commit comments

Comments
 (0)