Page MenuHomePhabricator
Differential D13340

[lib] DMOperationSpec for delete entry operation
ClosedPublic
Actions

Authored by will on Sep 16 2024, 1:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 7, 2:36 PM
Unknown Object (File)
Tue, Mar 4, 6:19 PM
Unknown Object (File)
Tue, Mar 4, 5:52 PM
Unknown Object (File)
Fri, Feb 28, 5:46 AM
Unknown Object (File)
Fri, Feb 28, 5:46 AM
Unknown Object (File)
Fri, Feb 28, 5:46 AM
Unknown Object (File)
Jan 31 2025, 7:46 PM
Unknown Object (File)
Jan 23 2025, 5:16 PM
View All 52 Files
Subscribers
None

Details

Reviewers
marcin
tomek
ashoat
Commits
rCOMMd63bed70feb7: [lib] DMOperationSpec for delete entry operation
Summary

Implements the delete entry DM operation spec

Depends on D13316

Test Plan

Tested later in stack. Will test notifs before landing

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

will created this revision.Sep 16 2024, 1:16 AM
will edited the summary of this revision. (Show Details)Sep 16 2024, 1:17 AM
will added a parent revision: D13316: [lib] DMOperationSpec for create entry operation.
will added a child revision: D13341: [lib] DMOperationSpec for edit entry operation.Sep 16 2024, 1:18 AM
Harbormaster completed remote builds in B31622: Diff 44194.Sep 16 2024, 1:34 AM
will requested review of this revision.Sep 16 2024, 1:34 AM
tomek requested changes to this revision.Sep 16 2024, 3:41 AM

It is a bit confusing to see all this info being sent while deleting an entry. It might be invalid when one user edits an entry while someone else deletes it.

The proper solution would require:

  1. Modifying the payload to contain just the ID of the deleted entry without too many details
  2. Introducing a util that provides all the entry infos
  3. Introducing a separate timestamp for deleted property

This will take some time, and we're short on it, so we should search for a simpler solution.
What we can do for now, is to keep the current approach and introduce a lastUpdatedTime timestamp check.

This revision now requires changes to proceed.Sep 16 2024, 3:41 AM
will added a comment.Sep 16 2024, 5:41 AM
In D13340#375962, @tomek wrote:

It is a bit confusing to see all this info being sent while deleting an entry. It might be invalid when one user edits an entry while someone else deletes it.

The proper solution would require:

  1. Modifying the payload to contain just the ID of the deleted entry without too many details
  2. Introducing a util that provides all the entry infos
  3. Introducing a separate timestamp for deleted property

This will take some time, and we're short on it, so we should search for a simpler solution.
What we can do for now, is to keep the current approach and introduce a lastUpdatedTime timestamp check.

Same comment for edit. Did a timestamp check up the stack:
https://phab.comm.dev/D13348

will requested review of this revision.Sep 16 2024, 5:46 AM

Rerequesting review

tomek accepted this revision.Sep 16 2024, 5:57 AM
This revision is now accepted and ready to land.Sep 16 2024, 5:57 AM
Closed by commit rCOMMd63bed70feb7: [lib] DMOperationSpec for delete entry operation (authored by will). · Explain WhySep 16 2024, 2:27 PM
This revision was automatically updated to reflect the committed changes.
will added a commit: rCOMMd63bed70feb7: [lib] DMOperationSpec for delete entry operation.

Revision Contents
Changeset List

PathSize
lib/
shared/
dm-ops/
97 lines
2 lines
types/
28 lines

Diff 44247

View Options

lib/shared/dm-ops/delete-entry-spec.js

Show First 20 Lines • Show All 879 Lines • ▼ Show 20 Lines): Promise<$ReadOnlyArray<RawMessageInfo>> {
if (resultRows.length === 0) { if (resultRows.length === 0) {
return []; return [];
} }
const SQLResult = await parseMessageSQLResult(resultRows, messages, viewer); const SQLResult = await parseMessageSQLResult(resultRows, messages, viewer);
return SQLResult.map(item => item.rawMessageInfo); return SQLResult.map(item => item.rawMessageInfo);
} }
async function rawMessageInfoForRowsAndRelatedMessages(
rows: $ReadOnlyArray<Object>,
viewer?: Viewer,
): Promise<$ReadOnlyArray<RawMessageInfo>> {
const parsedResults = await parseMessageSQLResult(rows, new Map(), viewer);
const rawMessageInfoMap = new Map<
string,
RawComposableMessageInfo | RawRobotextMessageInfo,
>();
for (const message of parsedResults) {
const { rawMessageInfo } = message;
if (
rawMessageInfo.type === messageTypes.SIDEBAR_SOURCE ||
rawMessageInfo.type === messageTypes.REACTION ||
rawMessageInfo.type === messageTypes.EDIT_MESSAGE
) {
continue;
}
invariant(rawMessageInfo.id, 'rawMessageInfo.id should not be null');
rawMessageInfoMap.set(rawMessageInfo.id, rawMessageInfo);
}
const rawMessageInfos = parsedResults.map(item => item.rawMessageInfo);
const rawRelatedMessageInfos = await fetchRelatedMessages(
viewer,
rawMessageInfoMap,
);
return [...rawMessageInfos, ...rawRelatedMessageInfos];
}
export { export {
fetchCollapsableNotifs, fetchCollapsableNotifs,
fetchMessageInfos, fetchMessageInfos,
fetchMessageInfosSince, fetchMessageInfosSince,
getMessageFetchResultFromRedisMessages, getMessageFetchResultFromRedisMessages,
fetchMessageInfoForLocalID, fetchMessageInfoForLocalID,
fetchMessageInfoForEntryAction, fetchMessageInfoForEntryAction,
fetchMessageInfoByID, fetchMessageInfoByID,
fetchThreadMessagesCount, fetchThreadMessagesCount,
fetchLatestEditMessageContentByID, fetchLatestEditMessageContentByID,
fetchPinnedMessageInfos, fetchPinnedMessageInfos,
fetchRelatedMessages, fetchRelatedMessages,
rawMessageInfoForRowsAndRelatedMessages,
}; };
View Options

lib/shared/dm-ops/dm-op-specs.js

Show First 20 Lines • Show All 879 Lines • ▼ Show 20 Lines): Promise<$ReadOnlyArray<RawMessageInfo>> {
if (resultRows.length === 0) { if (resultRows.length === 0) {
return []; return [];
} }
const SQLResult = await parseMessageSQLResult(resultRows, messages, viewer); const SQLResult = await parseMessageSQLResult(resultRows, messages, viewer);
return SQLResult.map(item => item.rawMessageInfo); return SQLResult.map(item => item.rawMessageInfo);
} }
async function rawMessageInfoForRowsAndRelatedMessages(
rows: $ReadOnlyArray<Object>,
viewer?: Viewer,
): Promise<$ReadOnlyArray<RawMessageInfo>> {
const parsedResults = await parseMessageSQLResult(rows, new Map(), viewer);
const rawMessageInfoMap = new Map<
string,
RawComposableMessageInfo | RawRobotextMessageInfo,
>();
for (const message of parsedResults) {
const { rawMessageInfo } = message;
if (
rawMessageInfo.type === messageTypes.SIDEBAR_SOURCE ||
rawMessageInfo.type === messageTypes.REACTION ||
rawMessageInfo.type === messageTypes.EDIT_MESSAGE
) {
continue;
}
invariant(rawMessageInfo.id, 'rawMessageInfo.id should not be null');
rawMessageInfoMap.set(rawMessageInfo.id, rawMessageInfo);
}
const rawMessageInfos = parsedResults.map(item => item.rawMessageInfo);
const rawRelatedMessageInfos = await fetchRelatedMessages(
viewer,
rawMessageInfoMap,
);
return [...rawMessageInfos, ...rawRelatedMessageInfos];
}
export { export {
fetchCollapsableNotifs, fetchCollapsableNotifs,
fetchMessageInfos, fetchMessageInfos,
fetchMessageInfosSince, fetchMessageInfosSince,
getMessageFetchResultFromRedisMessages, getMessageFetchResultFromRedisMessages,
fetchMessageInfoForLocalID, fetchMessageInfoForLocalID,
fetchMessageInfoForEntryAction, fetchMessageInfoForEntryAction,
fetchMessageInfoByID, fetchMessageInfoByID,
fetchThreadMessagesCount, fetchThreadMessagesCount,
fetchLatestEditMessageContentByID, fetchLatestEditMessageContentByID,
fetchPinnedMessageInfos, fetchPinnedMessageInfos,
fetchRelatedMessages, fetchRelatedMessages,
rawMessageInfoForRowsAndRelatedMessages,
}; };
View Options

lib/types/dm-ops.js

Show First 20 Lines • Show All 879 Lines • ▼ Show 20 Lines): Promise<$ReadOnlyArray<RawMessageInfo>> {
if (resultRows.length === 0) { if (resultRows.length === 0) {
return []; return [];
} }
const SQLResult = await parseMessageSQLResult(resultRows, messages, viewer); const SQLResult = await parseMessageSQLResult(resultRows, messages, viewer);
return SQLResult.map(item => item.rawMessageInfo); return SQLResult.map(item => item.rawMessageInfo);
} }
async function rawMessageInfoForRowsAndRelatedMessages(
rows: $ReadOnlyArray<Object>,
viewer?: Viewer,
): Promise<$ReadOnlyArray<RawMessageInfo>> {
const parsedResults = await parseMessageSQLResult(rows, new Map(), viewer);
const rawMessageInfoMap = new Map<
string,
RawComposableMessageInfo | RawRobotextMessageInfo,
>();
for (const message of parsedResults) {
const { rawMessageInfo } = message;
if (
rawMessageInfo.type === messageTypes.SIDEBAR_SOURCE ||
rawMessageInfo.type === messageTypes.REACTION ||
rawMessageInfo.type === messageTypes.EDIT_MESSAGE
) {
continue;
}
invariant(rawMessageInfo.id, 'rawMessageInfo.id should not be null');
rawMessageInfoMap.set(rawMessageInfo.id, rawMessageInfo);
}
const rawMessageInfos = parsedResults.map(item => item.rawMessageInfo);
const rawRelatedMessageInfos = await fetchRelatedMessages(
viewer,
rawMessageInfoMap,
);
return [...rawMessageInfos, ...rawRelatedMessageInfos];
}
export { export {
fetchCollapsableNotifs, fetchCollapsableNotifs,
fetchMessageInfos, fetchMessageInfos,
fetchMessageInfosSince, fetchMessageInfosSince,
getMessageFetchResultFromRedisMessages, getMessageFetchResultFromRedisMessages,
fetchMessageInfoForLocalID, fetchMessageInfoForLocalID,
fetchMessageInfoForEntryAction, fetchMessageInfoForEntryAction,
fetchMessageInfoByID, fetchMessageInfoByID,
fetchThreadMessagesCount, fetchThreadMessagesCount,
fetchLatestEditMessageContentByID, fetchLatestEditMessageContentByID,
fetchPinnedMessageInfos, fetchPinnedMessageInfos,
fetchRelatedMessages, fetchRelatedMessages,
rawMessageInfoForRowsAndRelatedMessages,
}; };