From 4a87e62dd69f32ff2a2710d45e037815cb499008 Mon Sep 17 00:00:00 2001 From: Ben Biddington Date: Tue, 23 Jul 2024 15:22:26 +1200 Subject: [PATCH 1/3] [FIX, HELP-WANTED, #6659] Add reproduction of reported behaviour See: https://github.com/signalapp/Signal-Desktop/issues/6659 --- ...sage_deletes_draft_of_new_message_tests.ts | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts diff --git a/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts b/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts new file mode 100644 index 00000000000..377d5a133d5 --- /dev/null +++ b/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts @@ -0,0 +1,144 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { Proto } from '@signalapp/mock-server'; +import { assert } from 'chai'; +import Long from 'long'; +import type { Locator } from 'playwright'; +import type { App } from '../../playwright'; +import * as durations from '../../../util/durations'; +import { Bootstrap } from '../../bootstrap'; + +const pause = process.env.PAUSE; + +const createMessage = (body: string): Proto.IDataMessage => { + return { + body, + groupV2: undefined, + timestamp: Long.fromNumber(Date.now()), + }; +}; + +// https://github.com/signalapp/Signal-Desktop/issues/6659 +describe('[6659] Editing a sent message does not delete draft of new message', function (this: Mocha.Suite) { + this.timeout(durations.MINUTE); + + let bootstrap: Bootstrap; + let app: App; + let screen: Screen; + let sentMessage: Proto.IDataMessage; + + beforeEach(async () => { + bootstrap = new Bootstrap({}); + await bootstrap.init(); + app = await bootstrap.link(); + screen = new Screen(app); + + const { phone, desktop } = bootstrap; + + sentMessage = createMessage('A B C'); + + await phone.sendRaw( + desktop, + { + dataMessage: sentMessage, + }, + { + timestamp: Number(sentMessage.timestamp), + } + ); + }); + + afterEach(async function (this: Mocha.Context) { + if (!pause) { + await bootstrap?.maybeSaveLogs(this.currentTest, app); + await app?.close(); + await bootstrap?.teardown(); + } + }); + + /* + This is the test we want to pass. + + See: `setMessageToEdit` (ts/state/ducks/conversations.ts) + + It looks to me like the draft is being completely overwritten when editing, + so not sure what the solution is. + + Skipped because I don't know how to make it pass. + + This does demonstrate an alternative way to express the tests more in user-centric + terms using `Screen`. + + */ + it.skip('[wip, fails] restores draft after editing a sent message', async () => { + await screen.openFirstConversation(); + await screen.typeMessage('Draft message'); + await screen.editMessage(sentMessage.timestamp, 'A B C D E F'); + + assert.strictEqual(await screen.draftText(), 'Draft message'); + }); + + /* + + This test demonstrates the current behaviour. Delete this once the above test passes. + + */ + it('unexpectedly clears draft after editing a sent message', async () => { + await screen.openFirstConversation(); + await screen.typeMessage('Draft message'); + await screen.editMessage(sentMessage.timestamp, 'A B C D E F'); + + assert.strictEqual(await screen.draftText(), ''); + }); +}); + +class Screen { + constructor(private app: App) {} + + public openFirstConversation = async (): Promise => { + const window = await this.app.getWindow(); + const leftPane = window.locator('#LeftPane'); + + await leftPane + .locator('.module-conversation-list__item--contact-or-conversation') + .first() + .click(); + }; + + public editMessage = async ( + timestamp: Long | null | undefined, + text: string + ): Promise => { + const page = await this.app.getWindow(); + + await page + .getByTestId(`${timestamp}`) + .locator('.module-message__buttons__menu') + .click(); + + await page.getByRole('menuitem', { name: 'Edit' }).click(); + + await this.typeMessage(text); + + await this.sendMessage(); + }; + + public typeMessage = async (text: string): Promise => { + const messageTextInput = await this.getMessageTextInput(); + await messageTextInput.fill(text); + }; + + public sendMessage = async (): Promise => { + const messageTextInput = await this.getMessageTextInput(); + await messageTextInput.press('Enter'); + }; + + public draftText = async (): Promise => { + const messageTextInput = await this.getMessageTextInput(); + return messageTextInput.textContent(); + }; + + private getMessageTextInput = (): Promise => + this.app.waitForEnabledComposer(); +} From 0b39a3162071829efd60428b71884e6b6ddb1213 Mon Sep 17 00:00:00 2001 From: Ben Biddington Date: Wed, 24 Jul 2024 08:54:11 +1200 Subject: [PATCH 2/3] Disallow editing sent messages when there is a draft present --- ts/state/selectors/message.ts | 2 +- ts/test-mock/bootstrap.ts | 9 ++ ...sage_deletes_draft_of_new_message_tests.ts | 93 +++---------------- ts/test-mock/signal-desktop-ui.ts | 71 ++++++++++++++ 4 files changed, 93 insertions(+), 82 deletions(-) create mode 100644 ts/test-mock/signal-desktop-ui.ts diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts index cb0ef8589dd..edba56a8240 100644 --- a/ts/state/selectors/message.ts +++ b/ts/state/selectors/message.ts @@ -734,7 +734,7 @@ export const getPropsForMessage = ( textAttachment, payment, canCopy: canCopy(message), - canEditMessage: canEditMessage(message), + canEditMessage: !conversation.draftText && canEditMessage(message), canDeleteForEveryone: canDeleteForEveryone(message, conversation.isMe), canDownload: canDownload(message, conversationSelector), canReact: canReact(message, ourConversationId, conversationSelector), diff --git a/ts/test-mock/bootstrap.ts b/ts/test-mock/bootstrap.ts index 5a35b1744b7..2c908a985d8 100644 --- a/ts/test-mock/bootstrap.ts +++ b/ts/test-mock/bootstrap.ts @@ -25,6 +25,7 @@ import { drop } from '../util/drop'; import type { RendererConfigType } from '../types/RendererConfig'; import { App } from './playwright'; import { CONTACT_COUNT } from './benchmarks/fixtures'; +import { SignalDesktopUI } from './signal-desktop-ui'; export { App }; @@ -347,6 +348,14 @@ export class Bootstrap { return app; } + public async signalDesktopUI(): Promise { + assert( + this.lastApp !== undefined, + 'Bootstrap has to be initialized first, see: bootstrap.init()' + ); + return new SignalDesktopUI(this.lastApp); + } + public async linkAndClose(): Promise { const app = await this.link(); diff --git a/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts b/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts index 377d5a133d5..071edf87b95 100644 --- a/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts +++ b/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts @@ -4,10 +4,10 @@ import type { Proto } from '@signalapp/mock-server'; import { assert } from 'chai'; import Long from 'long'; -import type { Locator } from 'playwright'; import type { App } from '../../playwright'; import * as durations from '../../../util/durations'; import { Bootstrap } from '../../bootstrap'; +import type { SignalDesktopUI } from '../../signal-desktop-ui'; const pause = process.env.PAUSE; @@ -25,14 +25,14 @@ describe('[6659] Editing a sent message does not delete draft of new message', f let bootstrap: Bootstrap; let app: App; - let screen: Screen; + let ui: SignalDesktopUI; let sentMessage: Proto.IDataMessage; beforeEach(async () => { bootstrap = new Bootstrap({}); await bootstrap.init(); app = await bootstrap.link(); - screen = new Screen(app); + ui = await bootstrap.signalDesktopUI(); const { phone, desktop } = bootstrap; @@ -57,88 +57,19 @@ describe('[6659] Editing a sent message does not delete draft of new message', f } }); - /* - This is the test we want to pass. - - See: `setMessageToEdit` (ts/state/ducks/conversations.ts) - - It looks to me like the draft is being completely overwritten when editing, - so not sure what the solution is. - - Skipped because I don't know how to make it pass. - - This does demonstrate an alternative way to express the tests more in user-centric - terms using `Screen`. - - */ - it.skip('[wip, fails] restores draft after editing a sent message', async () => { - await screen.openFirstConversation(); - await screen.typeMessage('Draft message'); - await screen.editMessage(sentMessage.timestamp, 'A B C D E F'); - - assert.strictEqual(await screen.draftText(), 'Draft message'); - }); - /* - This test demonstrates the current behaviour. Delete this once the above test passes. + See: `ts/components/conversation/MessageContextMenu.tsx` + + @todo: test is flaky. */ - it('unexpectedly clears draft after editing a sent message', async () => { - await screen.openFirstConversation(); - await screen.typeMessage('Draft message'); - await screen.editMessage(sentMessage.timestamp, 'A B C D E F'); + it('disallows editing sent messages when there is a draft present', async () => { + await ui.openFirstConversation(); + await ui.typeMessage('Draft message'); - assert.strictEqual(await screen.draftText(), ''); + assert.isFalse( + await ui.isShowingEditMessageMenuItem(sentMessage.timestamp) + ); }); }); - -class Screen { - constructor(private app: App) {} - - public openFirstConversation = async (): Promise => { - const window = await this.app.getWindow(); - const leftPane = window.locator('#LeftPane'); - - await leftPane - .locator('.module-conversation-list__item--contact-or-conversation') - .first() - .click(); - }; - - public editMessage = async ( - timestamp: Long | null | undefined, - text: string - ): Promise => { - const page = await this.app.getWindow(); - - await page - .getByTestId(`${timestamp}`) - .locator('.module-message__buttons__menu') - .click(); - - await page.getByRole('menuitem', { name: 'Edit' }).click(); - - await this.typeMessage(text); - - await this.sendMessage(); - }; - - public typeMessage = async (text: string): Promise => { - const messageTextInput = await this.getMessageTextInput(); - await messageTextInput.fill(text); - }; - - public sendMessage = async (): Promise => { - const messageTextInput = await this.getMessageTextInput(); - await messageTextInput.press('Enter'); - }; - - public draftText = async (): Promise => { - const messageTextInput = await this.getMessageTextInput(); - return messageTextInput.textContent(); - }; - - private getMessageTextInput = (): Promise => - this.app.waitForEnabledComposer(); -} diff --git a/ts/test-mock/signal-desktop-ui.ts b/ts/test-mock/signal-desktop-ui.ts new file mode 100644 index 00000000000..21bc047e869 --- /dev/null +++ b/ts/test-mock/signal-desktop-ui.ts @@ -0,0 +1,71 @@ +import { Locator } from 'playwright'; +import type { App } from './playwright'; + +export class SignalDesktopUI { + constructor(private app: App) {} + + public openFirstConversation = async (): Promise => { + const window = await this.app.getWindow(); + const leftPane = window.locator('#LeftPane'); + + await leftPane + .locator('.module-conversation-list__item--contact-or-conversation') + .first() + .click(); + }; + + public editMessage = async ( + timestamp: Long | null | undefined, + text: string + ): Promise => { + const editButton = await this.editMessageButton(timestamp); + + await editButton.click(); + + await this.typeMessage(text); + + await this.sendMessage(); + }; + + public isShowingEditMessageMenuItem = async ( + timestamp: Long | null | undefined + ): Promise => { + const page = await this.app.getWindow(); + + await page + .getByTestId(`${timestamp}`) + .locator('.module-message__buttons__menu') + .click(); + + const result = await page + .getByRole('menuitem', { name: 'Edit' }) + .isVisible(); + + await page.keyboard.press('Escape'); + + return result; + }; + + public typeMessage = async (text: string): Promise => { + const messageTextInput = await this.getMessageTextInput(); + await messageTextInput.fill(text); + }; + + public clearMessage = async (): Promise => { + const messageTextInput = await this.getMessageTextInput(); + await messageTextInput.clear(); + }; + + public sendMessage = async (): Promise => { + const messageTextInput = await this.getMessageTextInput(); + await messageTextInput.press('Enter'); + }; + + public messageText = async (): Promise => { + const messageTextInput = await this.getMessageTextInput(); + return messageTextInput.textContent(); + }; + + private getMessageTextInput = (): Promise => + this.app.waitForEnabledComposer(); +} From c19c52b0df789b29da1f5d04f01e02258f4ab206 Mon Sep 17 00:00:00 2001 From: Ben Biddington Date: Wed, 24 Jul 2024 16:48:37 +1200 Subject: [PATCH 3/3] Allow UI to render before asserting --- ...nt_message_deletes_draft_of_new_message_tests.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts b/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts index 071edf87b95..e401d06debe 100644 --- a/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts +++ b/ts/test-mock/messaging/defects/6659_editing_a_sent_message_deletes_draft_of_new_message_tests.ts @@ -8,6 +8,7 @@ import type { App } from '../../playwright'; import * as durations from '../../../util/durations'; import { Bootstrap } from '../../bootstrap'; import type { SignalDesktopUI } from '../../signal-desktop-ui'; +import { sleep } from '../../../util/sleep'; const pause = process.env.PAUSE; @@ -61,15 +62,23 @@ describe('[6659] Editing a sent message does not delete draft of new message', f See: `ts/components/conversation/MessageContextMenu.tsx` - @todo: test is flaky. - */ it('disallows editing sent messages when there is a draft present', async () => { await ui.openFirstConversation(); await ui.typeMessage('Draft message'); + // [!] Allow time for the menu to re-render + await sleep(100); + assert.isFalse( await ui.isShowingEditMessageMenuItem(sentMessage.timestamp) ); + + await ui.clearMessage(); + + // [!] Allow time for the menu to re-render + await sleep(100); + + assert.isTrue(await ui.isShowingEditMessageMenuItem(sentMessage.timestamp)); }); });