Skip to content

Commit 69767fb

Browse files
gnpricechrisbobbe
gnprice
authored and
chrisbobbe
committed
msglist [nfc]: Split haveSameRecipient from messagesSameDay
We'll shortly want to start doing different things for these two cases.
1 parent 8f0f27c commit 69767fb

File tree

2 files changed

+69
-63
lines changed

2 files changed

+69
-63
lines changed

‎lib/model/message_list.dart

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,9 @@ mixin _MessageSequence {
181181
final message = messages[index];
182182
final content = contents[index];
183183
bool canShareSender;
184-
if (index > 0 && canShareRecipientHeader(messages[index - 1], message)) {
184+
if (index > 0
185+
&& haveSameRecipient(messages[index - 1], message)
186+
&& messagesSameDay(messages[index - 1], message)) {
185187
assert(items.last is MessageListMessageItem);
186188
final prevMessageItem = items.last as MessageListMessageItem;
187189
assert(identical(prevMessageItem.message, messages[index - 1]));
@@ -228,7 +230,7 @@ mixin _MessageSequence {
228230
}
229231

230232
@visibleForTesting
231-
bool canShareRecipientHeader(Message prevMessage, Message message) {
233+
bool haveSameRecipient(Message prevMessage, Message message) {
232234
if (prevMessage is StreamMessage && message is StreamMessage) {
233235
if (prevMessage.streamId != message.streamId) return false;
234236
if (prevMessage.subject != message.subject) return false;
@@ -239,6 +241,7 @@ bool canShareRecipientHeader(Message prevMessage, Message message) {
239241
} else {
240242
return false;
241243
}
244+
return true;
242245

243246
// switch ((prevMessage, message)) {
244247
// case (StreamMessage(), StreamMessage()):
@@ -248,12 +251,14 @@ bool canShareRecipientHeader(Message prevMessage, Message message) {
248251
// default:
249252
// return false;
250253
// }
254+
}
251255

256+
@visibleForTesting
257+
bool messagesSameDay(Message prevMessage, Message message) {
252258
// TODO memoize [DateTime]s... also use memoized for showing date/time in msglist
253259
final prevTime = DateTime.fromMillisecondsSinceEpoch(prevMessage.timestamp * 1000);
254260
final time = DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000);
255261
if (!_sameDay(prevTime, time)) return false;
256-
257262
return true;
258263
}
259264

‎test/model/message_list_test.dart

Lines changed: 61 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -810,38 +810,38 @@ void main() async {
810810
]);
811811
});
812812

813-
group('canShareRecipientHeader', () {
814-
test('stream messages vs DMs, no share', () {
813+
group('haveSameRecipient', () {
814+
test('stream messages vs DMs, no match', () {
815815
final dmMessage = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]);
816-
final streamMessage = eg.streamMessage(timestamp: dmMessage.timestamp);
817-
check(canShareRecipientHeader(streamMessage, dmMessage)).isFalse();
818-
check(canShareRecipientHeader(dmMessage, streamMessage)).isFalse();
816+
final streamMessage = eg.streamMessage();
817+
check(haveSameRecipient(streamMessage, dmMessage)).isFalse();
818+
check(haveSameRecipient(dmMessage, streamMessage)).isFalse();
819819
});
820820

821-
test('stream messages of same day share just if same stream/topic', () {
821+
test('stream messages match just if same stream/topic', () {
822822
final stream0 = eg.stream(streamId: 123);
823823
final stream1 = eg.stream(streamId: 234);
824824
final messageAB = eg.streamMessage(stream: stream0, topic: 'foo');
825-
final messageXB = eg.streamMessage(stream: stream1, topic: 'foo', timestamp: messageAB.timestamp);
826-
final messageAX = eg.streamMessage(stream: stream0, topic: 'bar', timestamp: messageAB.timestamp);
827-
check(canShareRecipientHeader(messageAB, messageAB)).isTrue();
828-
check(canShareRecipientHeader(messageAB, messageXB)).isFalse();
829-
check(canShareRecipientHeader(messageXB, messageAB)).isFalse();
830-
check(canShareRecipientHeader(messageAB, messageAX)).isFalse();
831-
check(canShareRecipientHeader(messageAX, messageAB)).isFalse();
832-
check(canShareRecipientHeader(messageAX, messageXB)).isFalse();
833-
check(canShareRecipientHeader(messageXB, messageAX)).isFalse();
825+
final messageXB = eg.streamMessage(stream: stream1, topic: 'foo');
826+
final messageAX = eg.streamMessage(stream: stream0, topic: 'bar');
827+
check(haveSameRecipient(messageAB, messageAB)).isTrue();
828+
check(haveSameRecipient(messageAB, messageXB)).isFalse();
829+
check(haveSameRecipient(messageXB, messageAB)).isFalse();
830+
check(haveSameRecipient(messageAB, messageAX)).isFalse();
831+
check(haveSameRecipient(messageAX, messageAB)).isFalse();
832+
check(haveSameRecipient(messageAX, messageXB)).isFalse();
833+
check(haveSameRecipient(messageXB, messageAX)).isFalse();
834834
});
835835

836-
test('DMs of same day share just if same recipients', () {
836+
test('DMs match just if same recipients', () {
837837
final message0 = eg.dmMessage(from: eg.selfUser, to: []);
838-
final message01 = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser], timestamp: message0.timestamp);
839-
final message10 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], timestamp: message0.timestamp);
840-
final message02 = eg.dmMessage(from: eg.selfUser, to: [eg.thirdUser], timestamp: message0.timestamp);
841-
final message20 = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], timestamp: message0.timestamp);
842-
final message012 = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser, eg.thirdUser], timestamp: message0.timestamp);
843-
final message102 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser, eg.thirdUser], timestamp: message0.timestamp);
844-
final message201 = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser], timestamp: message0.timestamp);
838+
final message01 = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]);
839+
final message10 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
840+
final message02 = eg.dmMessage(from: eg.selfUser, to: [eg.thirdUser]);
841+
final message20 = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser]);
842+
final message012 = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser, eg.thirdUser]);
843+
final message102 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser, eg.thirdUser]);
844+
final message201 = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser]);
845845
final groups = [[message0], [message01, message10],
846846
[message02, message20], [message012, message102, message201]];
847847
for (int i0 = 0; i0 < groups.length; i0++) {
@@ -852,52 +852,52 @@ void main() async {
852852
final message1 = groups[i1][j1];
853853
check(
854854
because: 'recipients ${message0.allRecipientIds} vs ${message1.allRecipientIds}',
855-
canShareRecipientHeader(message0, message1),
855+
haveSameRecipient(message0, message1),
856856
).equals(i0 == i1);
857857
}
858858
}
859859
}
860860
}
861861
});
862+
});
862863

863-
test('messages to same recipient share just if same day', () {
864-
// These timestamps will differ depending on the timezone of the
865-
// environment where the tests are run, in order to give the same results
866-
// in the code under test which is also based on the ambient timezone.
867-
// TODO(dart): It'd be great if tests could control the ambient timezone,
868-
// so as to exercise cases like where local time falls back across midnight.
869-
int timestampFromLocalTime(String date) => DateTime.parse(date).millisecondsSinceEpoch ~/ 1000;
870-
871-
const t111a = '2021-01-01 00:00:00';
872-
const t111b = '2021-01-01 12:00:00';
873-
const t111c = '2021-01-01 23:59:58';
874-
const t111d = '2021-01-01 23:59:59';
875-
const t112a = '2021-01-02 00:00:00';
876-
const t112b = '2021-01-02 00:00:01';
877-
const t121 = '2021-02-01 00:00:00';
878-
const t211 = '2022-01-01 00:00:00';
879-
final groups = [[t111a, t111b, t111c, t111d], [t112a, t112b], [t121], [t211]];
880-
881-
final stream = eg.stream();
882-
for (int i0 = 0; i0 < groups.length; i0++) {
883-
for (int i1 = i0; i1 < groups.length; i1++) {
884-
for (int j0 = 0; j0 < groups[i0].length; j0++) {
885-
for (int j1 = (i0 == i1) ? j0 : 0; j1 < groups[i1].length; j1++) {
886-
final time0 = groups[i0][j0];
887-
final time1 = groups[i1][j1];
888-
check(because: 'times $time0, $time1', canShareRecipientHeader(
889-
eg.streamMessage(stream: stream, topic: 'foo', timestamp: timestampFromLocalTime(time0)),
890-
eg.streamMessage(stream: stream, topic: 'foo', timestamp: timestampFromLocalTime(time1)),
891-
)).equals(i0 == i1);
892-
check(because: 'times $time0, $time1', canShareRecipientHeader(
893-
eg.dmMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time0)),
894-
eg.dmMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time1)),
895-
)).equals(i0 == i1);
896-
}
864+
test('messagesSameDay', () {
865+
// These timestamps will differ depending on the timezone of the
866+
// environment where the tests are run, in order to give the same results
867+
// in the code under test which is also based on the ambient timezone.
868+
// TODO(dart): It'd be great if tests could control the ambient timezone,
869+
// so as to exercise cases like where local time falls back across midnight.
870+
int timestampFromLocalTime(String date) => DateTime.parse(date).millisecondsSinceEpoch ~/ 1000;
871+
872+
const t111a = '2021-01-01 00:00:00';
873+
const t111b = '2021-01-01 12:00:00';
874+
const t111c = '2021-01-01 23:59:58';
875+
const t111d = '2021-01-01 23:59:59';
876+
const t112a = '2021-01-02 00:00:00';
877+
const t112b = '2021-01-02 00:00:01';
878+
const t121 = '2021-02-01 00:00:00';
879+
const t211 = '2022-01-01 00:00:00';
880+
final groups = [[t111a, t111b, t111c, t111d], [t112a, t112b], [t121], [t211]];
881+
882+
final stream = eg.stream();
883+
for (int i0 = 0; i0 < groups.length; i0++) {
884+
for (int i1 = i0; i1 < groups.length; i1++) {
885+
for (int j0 = 0; j0 < groups[i0].length; j0++) {
886+
for (int j1 = (i0 == i1) ? j0 : 0; j1 < groups[i1].length; j1++) {
887+
final time0 = groups[i0][j0];
888+
final time1 = groups[i1][j1];
889+
check(because: 'times $time0, $time1', messagesSameDay(
890+
eg.streamMessage(stream: stream, topic: 'foo', timestamp: timestampFromLocalTime(time0)),
891+
eg.streamMessage(stream: stream, topic: 'foo', timestamp: timestampFromLocalTime(time1)),
892+
)).equals(i0 == i1);
893+
check(because: 'times $time0, $time1', messagesSameDay(
894+
eg.dmMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time0)),
895+
eg.dmMessage(from: eg.selfUser, to: [], timestamp: timestampFromLocalTime(time1)),
896+
)).equals(i0 == i1);
897897
}
898898
}
899899
}
900-
});
900+
}
901901
});
902902
}
903903

@@ -948,7 +948,8 @@ void checkInvariants(MessageListView model) {
948948
for (int j = 0; j < model.messages.length; j++) {
949949
bool isFirstInBlock = false;
950950
if (j == 0
951-
|| !canShareRecipientHeader(model.messages[j-1], model.messages[j])) {
951+
|| !haveSameRecipient(model.messages[j-1], model.messages[j])
952+
|| !messagesSameDay(model.messages[j-1], model.messages[j])) {
952953
check(model.items[i++]).isA<MessageListRecipientHeaderItem>()
953954
.message.identicalTo(model.messages[j]);
954955
isFirstInBlock = true;

0 commit comments

Comments
 (0)