[lib] Introduce delete message spec
AcceptedPublic
Actions

Authored by tomek on Fri, Mar 28, 5:45 AM.

Details

Reviewers

kamil
ashoat

Summary

This spec is mostly inspired by the one for EDIT_MESSAGE

https://linear.app/comm/issue/ENG-10326/add-a-spec-for-the-delete-message-message

Depends on D14510

Test Plan

Flow. Tested later in the stack.

Diff Detail

Repository

rCOMM Comm

Branch

delete-messages

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Fri, Mar 28, 5:45 AM

Harbormaster completed remote builds in B33863: Diff 47610.Fri, Mar 28, 6:04 AM

tomek requested review of this revision.Fri, Mar 28, 6:04 AM

ashoat accepted this revision.Fri, Mar 28, 6:45 AM

ashoat added inline comments.

lib/shared/messages/delete-message-spec.js
108–109	Interesting that you use `FUTURE_CODE_VERSION` here but you used `NEXT_CODE_VERSION` earlier in the stack. I guess we want to introduce the migrations earlier than we want the messages unshimmed? Is there a downside to introducing the unshimming at the same time?

This revision is now accepted and ready to land.Fri, Mar 28, 6:45 AM

tomek retitled this revision from [lib] Introduce a message spec to [lib] Introduce delete message spec.Thu, Apr 3, 8:16 AM

tomek added inline comments.Thu, Apr 3, 8:19 AM

lib/shared/messages/delete-message-spec.js
108–109	I think the safest approach is to use `FUTURE_CODE_VERSION` initially and then replace it with `NEXT_CODE_VERSION` in a diff that e.g. performs a migration. By taking this approach we're making sure that there's no chance to introduce an issue by landing only a part of the stack. Update the earlier diff with `FUTURE_CODE_VERSION`.

tomek added a child revision: D14534: [lib] Add a new endpoint for deleting messages.Thu, Apr 3, 8:21 AM

Revision Contents
Changeset List

Path

Size

lib/

shared/

messages/

delete-message-spec.js

75 lines

Diff 47610

View Options

lib/shared/messages/delete-message-spec.js

Show All 32 Lines	(threadInfo, messageStore) => {
if (!thread \|\| !viewerID \|\| thread.messageIDs.length === 0) {		if (!thread \|\| !viewerID \|\| thread.messageIDs.length === 0) {
return {		return {
lastUpdatedAtLeastTime: threadInfo.creationTime,		lastUpdatedAtLeastTime: threadInfo.creationTime,
lastUpdatedAtMostTime: threadInfo.creationTime,		lastUpdatedAtMostTime: threadInfo.creationTime,
lastUpdatedTime: () => Promise.resolve(threadInfo.creationTime),		lastUpdatedTime: () => Promise.resolve(threadInfo.creationTime),
};		};
}		}

		const mostRecentMessage = messageStore.messages[thread.messageIDs[0]];
		const lastUpdatedAtMostTime =
		mostRecentMessage?.time ?? threadInfo.creationTime;

// We default to the most recent message time.		// We default to the most recent message time.
// - This default will only be relevant in the case that		// - This default will only be relevant in the case that
// getLastUpdatedTime is implemented for every message in the loop		// getLastUpdatedTime is implemented for every message in the loop
// below, and doesn't return a number for any of them. Otherwise		// below, and doesn't return a number for any of them. Otherwise
// lastUpdatedAtLeastTime will be set to the most recent message that		// lastUpdatedAtLeastTime will be set to the most recent message that
// returns a number, or whose message spec doesn't implement		// returns a number, or whose message spec doesn't implement
// getLastUpdatedTime.		// getLastUpdatedTime.
// - We default to the most recent message time because the best		// - We default to the most recent message time because the best
// alternative is the thread creation time, but this is likely very		// alternative is the thread creation time, but this is likely very
// far in the past.		// far in the past.
const mostRecentMessage = messageStore.messages[thread.messageIDs[0]];		let lastUpdatedAtLeastTime = lastUpdatedAtMostTime;
let lastUpdatedAtLeastTime =
mostRecentMessage?.time ?? threadInfo.creationTime;

const getLastUpdatedTimeParams = {		const getLastUpdatedTimeParams = {
threadInfo,		threadInfo,
viewerID,		viewerID,
getMessageAuthor: (messageID: string) =>		getMessageAuthor: (messageID: string) =>
baseGetMessageAuthor(messageID, messageStore),		baseGetMessageAuthor(messageID, messageStore),
};		};

let lastUpdatedTime: ?() => Promise<?number>;		let lastUpdatedTime: ?() => Promise<?number>;
let highestTimestamp: ?number;
for (const messageID of thread.messageIDs) {		for (const messageID of thread.messageIDs) {
const messageInfo = messageStore.messages[messageID];		const messageInfo = messageStore.messages[messageID];
if (!messageInfo) {		if (!messageInfo) {
continue;		continue;
}		}

// We call getLastUpdatedTime on the message spec. It can return either		// We call getLastUpdatedTime on the message spec. It can return either
// ?number or () => Promise<?number>. If the message spec doesn't		// ?number or () => Promise<?number>. If the message spec doesn't
// implement getLastUpdatedTime, then we default to messageInfo.time.		// implement getLastUpdatedTime, then we default to messageInfo.time.
const { getLastUpdatedTime } = messageSpecs[messageInfo.type];		const { getLastUpdatedTime } = messageSpecs[messageInfo.type];
const lastUpdatedTimeResult = getLastUpdatedTime		const lastUpdatedTimeResult = getLastUpdatedTime
? getLastUpdatedTime(messageInfo, getLastUpdatedTimeParams)		? getLastUpdatedTime(messageInfo, getLastUpdatedTimeParams)
: messageInfo.time;		: messageInfo.time;

// We only need to consider the first positive number result because
// thread.messageIDs is ordered chronologically.
if (
!highestTimestamp &&
lastUpdatedTimeResult &&
typeof lastUpdatedTimeResult === 'number'
) {
highestTimestamp = lastUpdatedTimeResult;
}

// We rely on the fact that thread.messageIDs is ordered chronologically		// We rely on the fact that thread.messageIDs is ordered chronologically
// (newest first) to chain together lastUpdatedTime. An older message's		// (newest first) to chain together lastUpdatedTime. An older message's
// lastUpdatedTime is only considered if all of the newer messages		// lastUpdatedTime is only considered if all of the newer messages
// return falsey.		// return falsey.
const prevLastUpdatedTime = lastUpdatedTime;		const prevLastUpdatedTime = lastUpdatedTime;
lastUpdatedTime = async () => {		lastUpdatedTime = async () => {
if (prevLastUpdatedTime) {		if (prevLastUpdatedTime) {
const earlierChecks = await prevLastUpdatedTime();		const earlierChecks = await prevLastUpdatedTime();
Show All 27 Lines	(threadInfo, messageStore) => {
const earlierChecks = await lastUpdatedTime();		const earlierChecks = await lastUpdatedTime();
if (earlierChecks) {		if (earlierChecks) {
return earlierChecks;		return earlierChecks;
}		}
}		}
return lastUpdatedAtLeastTime;		return lastUpdatedAtLeastTime;
};		};

const lastUpdatedAtMostTime = highestTimestamp ?? threadInfo.creationTime;

return {		return {
lastUpdatedAtLeastTime,		lastUpdatedAtLeastTime,
lastUpdatedAtMostTime,		lastUpdatedAtMostTime,
lastUpdatedTime: lastUpdatedWithFallback,		lastUpdatedTime: lastUpdatedWithFallback,
};		};
},		},
[viewerID, baseGetMessageAuthor],		[viewerID, baseGetMessageAuthor],
);		);
}		}

export { useGetLastUpdatedTimes };		export { useGetLastUpdatedTimes };

[lib] Introduce delete message specAcceptedPublicActions