Group messages by status, grouping everything delivered+ together

Co-authored-by: Scott Nonnenberg <scott@signal.org>
This commit is contained in:
Fedor Indutny 2022-03-22 16:10:29 -07:00 committed by GitHub
parent 9835cbbda0
commit 468920cce8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 73 additions and 6 deletions

View File

@ -5,6 +5,7 @@ import { assert } from 'chai';
import { times } from 'lodash';
import { v4 as uuid } from 'uuid';
import { MINUTE, SECOND } from '../../util/durations';
import type { MaybeMessageTimelineItemType } from '../../util/timelineUtil';
import {
ScrollAnchor,
areMessagesInSameGroup,
@ -14,18 +15,20 @@ import {
describe('<Timeline> utilities', () => {
describe('areMessagesInSameGroup', () => {
const defaultNewer = {
const defaultNewer: MaybeMessageTimelineItemType = {
type: 'message' as const,
data: {
author: { id: uuid() },
timestamp: new Date(1998, 10, 21, 12, 34, 56, 123).valueOf(),
status: 'delivered',
},
};
const defaultOlder = {
const defaultOlder: MaybeMessageTimelineItemType = {
...defaultNewer,
data: {
...defaultNewer.data,
timestamp: defaultNewer.data.timestamp - MINUTE,
status: 'delivered',
},
};
@ -115,7 +118,56 @@ describe('<Timeline> utilities', () => {
assert.isFalse(areMessagesInSameGroup(defaultOlder, true, defaultNewer));
});
it('returns true if the everything above works out', () => {
it("returns false if they don't have matching sent status (and not delivered)", () => {
const older = {
...defaultOlder,
data: { ...defaultOlder.data, status: 'sent' as const },
};
assert.isFalse(areMessagesInSameGroup(older, false, defaultNewer));
});
it("returns false if newer is deletedForEveryone and older isn't", () => {
const newer = {
...defaultNewer,
data: { ...defaultNewer.data, deletedForEveryone: true },
};
assert.isFalse(areMessagesInSameGroup(defaultOlder, false, newer));
});
it("returns true if older is deletedForEveryone and newer isn't", () => {
const older = {
...defaultOlder,
data: { ...defaultOlder.data, deletedForEveryone: true },
};
assert.isTrue(areMessagesInSameGroup(older, false, defaultNewer));
});
it('returns true if both are deletedForEveryone', () => {
const older = {
...defaultOlder,
data: { ...defaultOlder.data, deletedForEveryone: true },
};
const newer = {
...defaultNewer,
data: { ...defaultNewer.data, deletedForEveryone: true },
};
assert.isTrue(areMessagesInSameGroup(older, false, newer));
});
it('returns true if they have delivered status or above', () => {
const older = {
...defaultOlder,
data: { ...defaultOlder.data, status: 'read' as const },
};
assert.isTrue(areMessagesInSameGroup(older, false, defaultNewer));
});
it('returns true if everything above works out', () => {
assert.isTrue(areMessagesInSameGroup(defaultOlder, false, defaultNewer));
});
});

View File

@ -8,6 +8,7 @@ import { WidthBreakpoint } from '../components/_util';
import { MINUTE } from './durations';
import { missingCaseError } from './missingCaseError';
import { isSameDay } from './timestamp';
import type { LastMessageStatus } from '../model-types.d';
const COLLAPSE_WITHIN = 3 * MINUTE;
@ -32,15 +33,17 @@ export enum UnreadIndicatorPlacement {
JustBelow,
}
type MessageTimelineItemDataType = Readonly<{
export type MessageTimelineItemDataType = Readonly<{
author: { id: string };
deletedForEveryone?: boolean;
reactions?: ReadonlyArray<unknown>;
status?: LastMessageStatus;
timestamp: number;
}>;
// This lets us avoid passing a full `MessageType`. That's useful for tests and for
// documentation.
type MaybeMessageTimelineItemType = Readonly<
export type MaybeMessageTimelineItemType = Readonly<
| undefined
| TimelineItemType
| { type: 'message'; data: MessageTimelineItemDataType }
@ -51,6 +54,10 @@ const getMessageTimelineItemData = (
): undefined | MessageTimelineItemDataType =>
timelineItem?.type === 'message' ? timelineItem.data : undefined;
function isDelivered(status?: LastMessageStatus) {
return status === 'delivered' || status === 'read' || status === 'viewed';
}
export function areMessagesInSameGroup(
olderTimelineItem: MaybeMessageTimelineItemType,
unreadIndicator: boolean,
@ -70,12 +77,20 @@ export function areMessagesInSameGroup(
return false;
}
// We definitely don't want to group if we transition from non-deleted to deleted, since
// deleted messages don't show status.
if (newerMessage.deletedForEveryone && !olderMessage.deletedForEveryone) {
return false;
}
return Boolean(
!olderMessage.reactions?.length &&
olderMessage.author.id === newerMessage.author.id &&
newerMessage.timestamp >= olderMessage.timestamp &&
newerMessage.timestamp - olderMessage.timestamp < COLLAPSE_WITHIN &&
isSameDay(olderMessage.timestamp, newerMessage.timestamp)
isSameDay(olderMessage.timestamp, newerMessage.timestamp) &&
(olderMessage.status === newerMessage.status ||
(isDelivered(newerMessage.status) && isDelivered(olderMessage.status)))
);
}