diff --git a/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.stories.tsx b/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.stories.tsx index c25201bb9a..534963d5b1 100644 --- a/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.stories.tsx +++ b/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.stories.tsx @@ -110,7 +110,6 @@ WithLocalRoomRetry.args = { export const WithMessageRejected = Template.bind({}); WithMessageRejected.args = { state: RoomStatusBarState.MessageRejected, - onResendAllClick: undefined, harms: ["org.matrix.msc4387.harassment"], }; @@ -123,6 +122,7 @@ WithMessageRejectedCanRetryInTime.args = { onResendAllClick: undefined, canRetryInSeconds: 5, harms: [], + isResending: false, }; /** @@ -132,6 +132,7 @@ export const WithMessageRejectedCanRetry = Template.bind({}); WithMessageRejectedCanRetry.args = { state: RoomStatusBarState.MessageRejected, harms: [], + isResending: false, }; /** @@ -151,15 +152,7 @@ export const WithMessageRejectedWithKnownHarm = Template.bind({}); WithMessageRejectedWithKnownHarm.args = { state: RoomStatusBarState.MessageRejected, harms: ["org.matrix.msc4387.spam"], -}; - -/** - * Rendered when a message was rejected by the server, and we use the generic message. - */ -export const WithMessageRejectedWithUnknownHarm = Template.bind({}); -WithMessageRejectedWithUnknownHarm.args = { - state: RoomStatusBarState.MessageRejected, - harms: ["any.old.harm"], + isResending: false, }; /** @@ -170,4 +163,5 @@ WithMessageRejectedWithServerMessage.args = { state: RoomStatusBarState.MessageRejected, harms: ["any.old.harm"], serverError: "OurServer rejects this content", + isResending: false, }; diff --git a/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.tsx b/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.tsx index 71e2f52e06..2532623d1c 100644 --- a/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.tsx +++ b/packages/shared-components/src/room/RoomStatusBar/RoomStatusBarView.tsx @@ -18,22 +18,22 @@ export interface RoomStatusBarViewActions { /** * Called when the user clicks on the 'resend all' button in the 'unsent messages' bar. */ - onResendAllClick?: () => void; + onResendAllClick: () => void; /** * Called when the user clicks on the 'cancel all' button in the 'unsent messages' bar. */ - onDeleteAllClick?: () => void; + onDeleteAllClick: () => void; /** * Called when the user clicks on the 'Retry' button in the 'failed to start chat' bar. */ - onRetryRoomCreationClick?: () => void; + onRetryRoomCreationClick: () => void; /** * Called when the user clicks on the 'Review Terms and Conditions' button. */ - onTermsAndConditionsClicked?: () => void; + onTermsAndConditionsClicked: () => void; } export enum RoomStatusBarState { @@ -72,13 +72,20 @@ export interface RoomStatusBarLocalRoomError { state: RoomStatusBarState.LocalRoomFailed; } -export interface RoomStatusBarMessageRejected { +export interface RoomStatusBarMessageRejectedRetryable { state: RoomStatusBarState.MessageRejected; canRetryInSeconds?: number; isResending: boolean; harms: string[]; serverError?: string; } +export interface RoomStatusBarMessageRejectedUnretryable { + state: RoomStatusBarState.MessageRejected; + harms: string[]; + serverError?: string; +} + +type RoomStatusBarMessageRejected = RoomStatusBarMessageRejectedRetryable | RoomStatusBarMessageRejectedUnretryable; export type RoomStatusBarViewSnapshot = | RoomStatusBarNoConnection @@ -197,10 +204,14 @@ function RoomStatusBarViewMessageRejected({ ); let subtitleText: string; - if (onResendAllClick) { - subtitleText = _t("room|status_bar|select_messages_to_retry"); - } else if (!onResendAllClick && snapshot.canRetryInSeconds !== undefined) { - subtitleText = _t("room|status_bar|message_rejected|can_retry_in", { count: snapshot.canRetryInSeconds }); + const canRetry = "isResending" in snapshot; + const isResending = "isResending" in snapshot && snapshot.isResending; + if (canRetry) { + if (snapshot.canRetryInSeconds !== undefined) { + subtitleText = _t("room|status_bar|message_rejected|can_retry_in", { count: snapshot.canRetryInSeconds }); + } else { + subtitleText = _t("room|status_bar|select_messages_to_retry"); + } } else { subtitleText = _t("room|status_bar|message_rejected|cannot_retry"); } @@ -210,30 +221,25 @@ function RoomStatusBarViewMessageRejected({ role="status" type="critical" actions={ - snapshot.isResending ? ( + isResending ? ( ) : ( <> - {onDeleteAllClick && ( - - )} - {(onResendAllClick || snapshot.canRetryInSeconds) && ( + + {canRetry && ( - )} - {vm.onResendAllClick && ( - - )} + + ) } diff --git a/playwright/e2e/timeline/safety-error.spec.ts b/playwright/e2e/timeline/safety-error.spec.ts index 080a8aa56e..294f928538 100644 --- a/playwright/e2e/timeline/safety-error.spec.ts +++ b/playwright/e2e/timeline/safety-error.spec.ts @@ -24,67 +24,95 @@ test.describe("Safety error rendering", () => { }, }); - test("should show a safety rejection of a message with no harms", { tag: ["@screenshot"] }, async ({ page, app, room, user }) => { - await page.route("**/_matrix/client/v3/**/send/**", async (route) => { - await route.fulfill({ json: {errcode: MatrixSafetyErrorCode.name, error: "Server provided error"}, status: 400 }); - }); - await app.viewRoomById(room.roomId); - const composer = app.getComposerField(); - await composer.fill("Hello!"); - await composer.press("Enter"); - const statusBar = page.getByRole("alert", { name: new RegExp(/.*Message rejected.*/)}); - await expect(statusBar).toMatchScreenshot("message-no-harms.png"); - }); + test( + "should show a safety rejection of a message with no harms", + { tag: ["@screenshot"] }, + async ({ page, app, room, user }) => { + await page.route("**/_matrix/client/v3/**/send/**", async (route) => { + await route.fulfill({ + json: { errcode: MatrixSafetyErrorCode.name, error: "Server provided error" }, + status: 400, + }); + }); + await app.viewRoomById(room.roomId); + const composer = app.getComposerField(); + await composer.fill("Hello!"); + await composer.press("Enter"); + const statusBar = page.getByRole("status", { name: new RegExp(/.*Message rejected.*/) }); + await expect(statusBar).toMatchScreenshot("message-no-harms.png"); + }, + ); - test("should show a safety rejection of a message with only unknown harms", { tag: ["@screenshot"] }, async ({ page, app, room, user }) => { - await page.route("**/_matrix/client/v3/**/send/**", async (route) => { - await route.fulfill({ json: {errcode: MatrixSafetyErrorCode.name, error: "Server provided error", harms: ["org.example.unknown-harm"]}, status: 400 }); - }); - await app.viewRoomById(room.roomId); - const composer = app.getComposerField(); - await composer.fill("Hello!"); - await composer.press("Enter"); - const statusBar = page.getByRole("alert", { name: new RegExp(/.*Message rejected.*/)}); - await expect(statusBar).toMatchScreenshot("message-no-harms.png"); - }); + test( + "should show a safety rejection of a message with only unknown harms", + { tag: ["@screenshot"] }, + async ({ page, app, room, user }) => { + await page.route("**/_matrix/client/v3/**/send/**", async (route) => { + await route.fulfill({ + json: { + errcode: MatrixSafetyErrorCode.name, + error: "Server provided error", + harms: ["org.example.unknown-harm"], + }, + status: 400, + }); + }); + await app.viewRoomById(room.roomId); + const composer = app.getComposerField(); + await composer.fill("Hello!"); + await composer.press("Enter"); + const statusBar = page.getByRole("status", { name: new RegExp(/.*Message rejected.*/) }); + await expect(statusBar).toMatchScreenshot("message-no-harms.png"); + }, + ); - test("should show a simple rejection of a message with spam harm", { tag: ["@screenshot"] }, async ({ page, app, room, user }) => { - await page.route("**/_matrix/client/v3/**/send/**", async (route) => { - await route.fulfill({ json: {errcode: MatrixSafetyErrorCode.name, error: "Ignored error", harms: ["org.matrix.msc4387.spam"]}, status: 400 }); - }); - await app.viewRoomById(room.roomId); - const composer = app.getComposerField(); - await composer.fill("Hello!"); - await composer.press("Enter"); - const statusBar = page.getByRole("alert", { name: new RegExp(/.*Message rejected.*/)}); - await expect(statusBar).toMatchScreenshot("message-spam.png"); - }); - test("should show a simple rejection of a message with spam harm with expiry", { tag: ["@screenshot"] }, async ({ page, app, room, user }) => { - await page.route("**/_matrix/client/v3/**/send/**", async (route) => { - await route.fulfill({ json: {errcode: MatrixSafetyErrorCode.name, error: "Ignored error", harms: ["org.matrix.msc4387.spam"], expiry: Date.now() + 1000}, status: 400 }); - }); - await app.viewRoomById(room.roomId); - const composer = app.getComposerField(); - await composer.fill("Hello!"); - await composer.press("Enter"); - const statusBar = page.getByRole("alert", { name: new RegExp(/.*Message rejected.*/)}); - await expect(statusBar).toMatchScreenshot("message-spam-expiry.png"); - // Permit a retry - await page.unroute("**/_matrix/client/v3/**/send/**"); - await statusBar.getByRole("button", { name: "Retry"}).click({ timeout: 1100 }); - await expect(statusBar).not.toBeVisible(); - }); - test("should show a simple rejection of an upload", { }, async ({ page, app, room, user }) => { - await page.route("**/_matrix/media/v3/upload**", async (route) => { - await route.fulfill({ json: {errcode: MatrixSafetyErrorCode.name, error: "Ignored error", harms: ["org.matrix.msc4387.spam"], expiry: Date.now() + 1000}, status: 400 }); - }); - await app.viewRoomById(room.roomId); - // Start waiting for file chooser before clicking. Note no await. - const fileChooserPromise = page.waitForEvent('filechooser'); - await page.getByRole("button", { name: "Attachment"}).click(); - const fileChooser = await fileChooserPromise; - await fileChooser.setFiles("playwright/sample-files/riot.png"); - await page.getByRole("button", { name: "Upload"}).click(); - await expect(page.getByRole("dialog", { name: "Upload Failed" })).toBeVisible(); - }); -}); \ No newline at end of file + test( + "should show a simple rejection of a message with spam harm", + { tag: ["@screenshot"] }, + async ({ page, app, room, user }) => { + await page.route("**/_matrix/client/v3/**/send/**", async (route) => { + await route.fulfill({ + json: { + errcode: MatrixSafetyErrorCode.name, + error: "Ignored error", + harms: ["org.matrix.msc4387.spam"], + }, + status: 400, + }); + }); + await app.viewRoomById(room.roomId); + const composer = app.getComposerField(); + await composer.fill("Hello!"); + await composer.press("Enter"); + const statusBar = page.getByRole("status", { name: new RegExp(/.*Message rejected.*/) }); + await expect(statusBar).toMatchScreenshot("message-spam.png"); + }, + ); + test( + "should show a simple rejection of a message with spam harm with expiry", + { tag: ["@screenshot"] }, + async ({ page, app, room, user }) => { + await page.route("**/_matrix/client/v3/**/send/**", async (route) => { + await route.fulfill({ + json: { + errcode: MatrixSafetyErrorCode.name, + error: "Ignored error", + harms: ["org.matrix.msc4387.spam"], + expiry: Date.now() + 1000, + }, + status: 400, + }); + }); + await app.viewRoomById(room.roomId); + const composer = app.getComposerField(); + await composer.fill("Hello!"); + await composer.press("Enter"); + const statusBar = page.getByRole("status", { name: new RegExp(/.*Message rejected.*/) }); + await expect(statusBar).toMatchScreenshot("message-spam-expiry.png"); + // Permit a retry + await page.unroute("**/_matrix/client/v3/**/send/**"); + await statusBar.getByRole("button", { name: "Retry all" }).click({ timeout: 1500 }); + await expect(statusBar).not.toBeVisible(); + }, + ); +}); diff --git a/playwright/snapshots/timeline/safety-error.spec.ts/message-no-harms-linux.png b/playwright/snapshots/timeline/safety-error.spec.ts/message-no-harms-linux.png new file mode 100644 index 0000000000..c6f98f73fb Binary files /dev/null and b/playwright/snapshots/timeline/safety-error.spec.ts/message-no-harms-linux.png differ diff --git a/playwright/snapshots/timeline/safety-error.spec.ts/message-spam-expiry-linux.png b/playwright/snapshots/timeline/safety-error.spec.ts/message-spam-expiry-linux.png new file mode 100644 index 0000000000..5e41dc2ca6 Binary files /dev/null and b/playwright/snapshots/timeline/safety-error.spec.ts/message-spam-expiry-linux.png differ diff --git a/playwright/snapshots/timeline/safety-error.spec.ts/message-spam-linux.png b/playwright/snapshots/timeline/safety-error.spec.ts/message-spam-linux.png new file mode 100644 index 0000000000..008df0ac5e Binary files /dev/null and b/playwright/snapshots/timeline/safety-error.spec.ts/message-spam-linux.png differ diff --git a/src/viewmodels/room/RoomStatusBar.ts b/src/viewmodels/room/RoomStatusBar.ts index 939a69c109..2d11165c35 100644 --- a/src/viewmodels/room/RoomStatusBar.ts +++ b/src/viewmodels/room/RoomStatusBar.ts @@ -93,12 +93,18 @@ export class RoomStatusBarViewModel }; } if (safetyError) { + const canRetry = !!safetyError.expiry; return { state: RoomStatusBarState.MessageRejected, harms: [...safetyError.harms], serverError: safetyError.error, - isResending, - canRetryInSeconds: safetyError.expiry && Math.ceil((safetyError.expiry.getTime() - Date.now()) / 1000), + ...(canRetry + ? { + isResending, + canRetryInSeconds: + safetyError.expiry && Math.ceil((safetyError.expiry.getTime() - Date.now()) / 1000), + } + : undefined), }; } return { @@ -187,9 +193,10 @@ export class RoomStatusBarViewModel } if ( this.snapshot.current.state === RoomStatusBarState.MessageRejected && + "canRetryInSeconds" in this.snapshot.current && this.snapshot.current.canRetryInSeconds ) { - this.timeout = setTimeout(() => this.setSnapshot, this.snapshot.current.canRetryInSeconds * 1000); + this.timeout = setTimeout(() => this.setSnapshot(), this.snapshot.current.canRetryInSeconds * 1000); } }