From 39897c47b2483dd3c3d8c0f25d14237e8c512dc0 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Wed, 3 Jan 2024 11:11:14 -0600 Subject: [PATCH] Delete for Everyone: Don't allow unrestricted deletes in Note to Self Co-authored-by: Scott Nonnenberg --- ts/components/DeleteMessagesModal.tsx | 2 +- ts/state/selectors/message.ts | 21 ++++++++++++------- ts/state/smart/DeleteMessagesModal.tsx | 2 +- .../state/selectors/messages_test.ts | 15 ++++++++----- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/ts/components/DeleteMessagesModal.tsx b/ts/components/DeleteMessagesModal.tsx index c967d6a8e7b..e12c248270a 100644 --- a/ts/components/DeleteMessagesModal.tsx +++ b/ts/components/DeleteMessagesModal.tsx @@ -41,7 +41,7 @@ export default function DeleteMessagesModal({ : i18n('icu:DeleteMessagesModal--deleteForMe'), }); - if (canDeleteForEveryone || isMe) { + if (canDeleteForEveryone) { const tooManyMessages = messageCount > MAX_DELETE_FOR_EVERYONE; actions.push({ 'aria-disabled': tooManyMessages, diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts index 9756151e9a5..e7cc2c69f82 100644 --- a/ts/state/selectors/message.ts +++ b/ts/state/selectors/message.ts @@ -727,7 +727,7 @@ export const getPropsForMessage = ( payment, canCopy: canCopy(message), canEditMessage: canEditMessage(message), - canDeleteForEveryone: canDeleteForEveryone(message), + canDeleteForEveryone: canDeleteForEveryone(message, conversation.isMe), canDownload: canDownload(message, conversationSelector), canReact: canReact(message, ourConversationId, conversationSelector), canReply: canReply(message, ourConversationId, conversationSelector), @@ -1872,26 +1872,31 @@ export function canDeleteForEveryone( message: Pick< MessageWithUIFieldsType, 'type' | 'deletedForEveryone' | 'sent_at' | 'sendStateByConversationId' - > + >, + isMe: boolean ): boolean { return ( // Is this a message I sent? isOutgoing(message) && // Has the message already been deleted? !message.deletedForEveryone && - // Is it too old to delete? - isMoreRecentThan(message.sent_at, DAY) && + // Is it too old to delete? (we relax that requirement in Note to Self) + (isMoreRecentThan(message.sent_at, DAY) || isMe) && // Is it sent to anyone? someSendStatus(message.sendStateByConversationId, isSent) ); } export const canDeleteMessagesForEveryone = createSelector( - [getMessages, (_state, messageIds: ReadonlyArray) => messageIds], - (messagesLookup, messageIds) => { - return messageIds.every(messageId => { + [ + getMessages, + (_state, options: { messageIds: ReadonlyArray; isMe: boolean }) => + options, + ], + (messagesLookup, options) => { + return options.messageIds.every(messageId => { const message = getOwn(messagesLookup, messageId); - return message != null && canDeleteForEveryone(message); + return message != null && canDeleteForEveryone(message, options.isMe); }); } ); diff --git a/ts/state/smart/DeleteMessagesModal.tsx b/ts/state/smart/DeleteMessagesModal.tsx index 162ce795379..8913f81b5be 100644 --- a/ts/state/smart/DeleteMessagesModal.tsx +++ b/ts/state/smart/DeleteMessagesModal.tsx @@ -29,7 +29,7 @@ export function SmartDeleteMessagesModal(): JSX.Element | null { }); const canDeleteForEveryone = useSelector((state: StateType) => { - return canDeleteMessagesForEveryone(state, messageIds); + return canDeleteMessagesForEveryone(state, { messageIds, isMe }); }); const lastSelectedMessage = useSelector((state: StateType) => { return state.conversations.lastSelectedMessage; diff --git a/ts/test-electron/state/selectors/messages_test.ts b/ts/test-electron/state/selectors/messages_test.ts index bfd185acd16..21bd1d4bbfb 100644 --- a/ts/test-electron/state/selectors/messages_test.ts +++ b/ts/test-electron/state/selectors/messages_test.ts @@ -73,8 +73,9 @@ describe('state/selectors/messages', () => { type: 'incoming' as const, sent_at: Date.now() - 1000, }; + const isMe = false; - assert.isFalse(canDeleteForEveryone(message)); + assert.isFalse(canDeleteForEveryone(message, isMe)); }); it('returns false for messages that were already deleted for everyone', () => { @@ -93,8 +94,9 @@ describe('state/selectors/messages', () => { }, }, }; + const isMe = false; - assert.isFalse(canDeleteForEveryone(message)); + assert.isFalse(canDeleteForEveryone(message, isMe)); }); it('returns false for messages that were are too old to delete', () => { @@ -112,8 +114,9 @@ describe('state/selectors/messages', () => { }, }, }; + const isMe = false; - assert.isFalse(canDeleteForEveryone(message)); + assert.isFalse(canDeleteForEveryone(message, isMe)); }); it("returns false for messages that haven't been sent to anyone", () => { @@ -131,8 +134,9 @@ describe('state/selectors/messages', () => { }, }, }; + const isMe = false; - assert.isFalse(canDeleteForEveryone(message)); + assert.isFalse(canDeleteForEveryone(message, isMe)); }); it('returns true for messages that meet all criteria for deletion', () => { @@ -154,8 +158,9 @@ describe('state/selectors/messages', () => { }, }, }; + const isMe = false; - assert.isTrue(canDeleteForEveryone(message)); + assert.isTrue(canDeleteForEveryone(message, isMe)); }); });