Change the title of VerificationRequestDialog when a request is cancelled (#30879)

* Test that VerificationRequestDialog updates when phase changes

* Change the title of VerificationRequestDialog when a request is cancelled

Part of implementing
https://github.com/element-hq/element-meta/issues/2898 but split out as
a separate change because it involves making VerificationRequestDialog
listen for changes to the verificationRequest so it can update based on
changes to phase.

src/components/views/dialogs/VerificationRequestDialog.tsx

@@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import React from "react";
-import { type VerificationRequest } from "matrix-js-sdk/src/crypto-api";
+import { VerificationPhase, VerificationRequestEvent, type VerificationRequest } from "matrix-js-sdk/src/crypto-api";
 import { type User } from "matrix-js-sdk/src/matrix";
 
 import { MatrixClientPeg } from "../../../MatrixClientPeg";
@@ -23,7 +23,17 @@ interface IProps {
 }
 
 interface IState {
+    // The VerificationRequest that is ongoing. This can be replaced if a
+    // promise was supplied in the props and it completes.
     verificationRequest?: VerificationRequest;
+
+    // What phase the VerificationRequest is at. This is part of
+    // verificationRequest but we have it as independent state because we need
+    // to update when it changes.
+    //
+    // We listen to the `Change` event on verificationRequest and update phase
+    // when that fires.
+    phase?: VerificationPhase;
 }
 
 export default class VerificationRequestDialog extends React.Component<IProps, IState> {
@@ -31,22 +41,51 @@ export default class VerificationRequestDialog extends React.Component<IProps, I
         super(props);
         this.state = {
             verificationRequest: this.props.verificationRequest,
+            phase: this.props.verificationRequest?.phase,
         };
     }
 
     public componentDidMount(): void {
+        // Listen to when the verificationRequest changes, so we can keep our
+        // phase up-to-date.
+        this.state.verificationRequest?.on(VerificationRequestEvent.Change, this.onRequestChange);
+
         this.props.verificationRequestPromise?.then((r) => {
-            this.setState({ verificationRequest: r });
+            // The request promise completed, so we have a new request
+
+            // Stop listening to the old request (if we have one, which normally we won't)
+            this.state.verificationRequest?.off(VerificationRequestEvent.Change, this.onRequestChange);
+
+            // And start listening to the new one
+            r.on(VerificationRequestEvent.Change, this.onRequestChange);
+
+            this.setState({ verificationRequest: r, phase: r.phase });
         });
     }
 
+    public componentWillUnmount(): void {
+        // Stop listening for changes to the request when we close
+        this.state.verificationRequest?.off(VerificationRequestEvent.Change, this.onRequestChange);
+    }
+
+    /**
+     * The verificationRequest changed, so we need to make sure we update our
+     * state to have the correct phase.
+     *
+     * Note: this is called when verificationRequest changes in some way, not
+     * when we replace verificationRequest with some new request.
+     */
+    private readonly onRequestChange = (): void => {
+        this.setState((prevState) => ({
+            phase: prevState.verificationRequest?.phase,
+        }));
+    };
+
     public render(): React.ReactNode {
         const request = this.state.verificationRequest;
         const otherUserId = request?.otherUserId;
         const member = this.props.member || (otherUserId ? MatrixClientPeg.safeGet().getUser(otherUserId) : null);
-        const title = request?.isSelfVerification
+        const title = this.dialogTitle(request);
-            ? _t("encryption|verification|verification_dialog_title_device")
-            : _t("encryption|verification|verification_dialog_title_user");
 
         if (!member) return null;
 
@@ -69,4 +108,16 @@ export default class VerificationRequestDialog extends React.Component<IProps, I
             </BaseDialog>
         );
     }
+
+    private dialogTitle(request?: VerificationRequest): string {
+        if (request?.isSelfVerification) {
+            if (request.phase === VerificationPhase.Cancelled) {
+                return _t("encryption|verification|verification_dialog_title_failed");
+            } else {
+                return _t("encryption|verification|verification_dialog_title_device");
+            }
+        } else {
+            return _t("encryption|verification|verification_dialog_title_user");
+        }
+    }
 }

src/i18n/strings/en_EN.json

@@ -1068,6 +1068,7 @@
             "use_another_device": "Use another device",
             "use_recovery_key": "Use recovery key",
             "verification_dialog_title_device": "Verify other device",
+            "verification_dialog_title_failed": "Verification failed",
             "verification_dialog_title_user": "Verification Request",
             "verification_skip_warning": "Without verifying, you won't have access to all your messages and may appear as untrusted to others.",
             "verification_success_with_backup": "Your new device is now verified. It has access to your encrypted messages, and other users will see it as trusted.",

test/unit-tests/components/structures/MatrixChat-test.tsx

@@ -1106,7 +1106,7 @@ describe("<MatrixChat />", () => {
                 act(() => verify.click());
 
                 // And close the device verification dialog
-                const closeButton = await screen.findByRole("button", { name: "Close dialog" });
+                const closeButton = screen.getByRole("button", { name: "Close dialog" });
                 act(() => closeButton.click());
 
                 // Then we are not allowed in - we are still being asked to verify
@@ -1179,7 +1179,7 @@ describe("<MatrixChat />", () => {
                         .fn()
                         .mockResolvedValue({ signedByOwner: true } as DeviceVerificationStatus),
                     isCrossSigningReady: jest.fn().mockReturnValue(false),
-                    requestOwnUserVerification: jest.fn().mockResolvedValue({ cancel: jest.fn() }),
+                    requestOwnUserVerification: jest.fn().mockResolvedValue({ cancel: jest.fn(), on: jest.fn() }),
                 } as any;
             }
         });

test/unit-tests/components/views/dialogs/VerificationRequestDialog-test.tsx

@@ -7,18 +7,20 @@ Please see LICENSE files in the repository root for full details.
 
 import React from "react";
 import { act, render, screen } from "jest-matrix-react";
-import { User } from "matrix-js-sdk/src/matrix";
+import { TypedEventEmitter, User } from "matrix-js-sdk/src/matrix";
 import {
     type ShowSasCallbacks,
     VerificationPhase,
     type Verifier,
     type VerificationRequest,
     type ShowQrCodeCallbacks,
+    VerificationRequestEvent,
+    type VerificationRequestEventHandlerMap,
 } from "matrix-js-sdk/src/crypto-api";
 import { VerificationMethod } from "matrix-js-sdk/src/types";
 
-import VerificationRequestDialog from "../../../../../src/components/views/dialogs/VerificationRequestDialog";
 import { stubClient } from "../../../../test-utils";
+import VerificationRequestDialog from "../../../../../src/components/views/dialogs/VerificationRequestDialog";
 
 describe("VerificationRequestDialog", () => {
     function renderComponent(phase: VerificationPhase, method?: "emoji" | "qr"): ReturnType<typeof render> {
@@ -86,7 +88,7 @@ describe("VerificationRequestDialog", () => {
     it("Shows a failure message if verification was cancelled", async () => {
         const dialog = renderComponent(VerificationPhase.Cancelled);
 
-        expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
+        expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument();
         expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
 
         expect(
@@ -116,7 +118,7 @@ describe("VerificationRequestDialog", () => {
         await act(async () => await new Promise(process.nextTick));
 
         // Then it renders the resolved information
-        expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
+        expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument();
         expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
 
         expect(
@@ -146,7 +148,28 @@ describe("VerificationRequestDialog", () => {
         await act(async () => await new Promise(process.nextTick));
 
         // Then it renders the information from the request in the promise
-        expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
+        expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument();
+        expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
+
+        expect(
+            screen.getByText(
+                "You cancelled verification on your other device. Start verification again from the notification.",
+            ),
+        ).toBeInTheDocument();
+    });
+
+    it("Changes the dialog contents when the request changes phase", async () => {
+        // Given we rendered the component with a phase of Unsent
+        const member = User.createUser("@alice:example.org", stubClient());
+        const request = createRequest(VerificationPhase.Unsent);
+
+        render(<VerificationRequestDialog onFinished={jest.fn()} member={member} verificationRequest={request} />);
+
+        // When I cancel the request (which changes phase and emits a Changed event)
+        await act(async () => await request.cancel());
+
+        // Then the dialog is updated to reflect that
+        expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument();
         expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
 
         expect(
@@ -157,9 +180,9 @@ describe("VerificationRequestDialog", () => {
     });
 });
 
-function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): VerificationRequest {
+function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): MockVerificationRequest {
     let verifier = undefined;
-    let chosenMethod = undefined;
+    let chosenMethod = null;
 
     switch (method) {
         case "emoji":
@@ -172,24 +195,7 @@ function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): Verif
             break;
     }
 
-    return {
+    return new MockVerificationRequest(phase, verifier, chosenMethod);
-        phase: jest.fn().mockReturnValue(phase),
-
-        // VerificationRequest is an emitter - ignore any events that are emitted.
-        on: jest.fn(),
-        off: jest.fn(),
-
-        // These tests (so far) only check for when we are initiating a verificiation of our own device.
-        isSelfVerification: jest.fn().mockReturnValue(true),
-        initiatedByMe: jest.fn().mockReturnValue(true),
-
-        // Always returning true means we can support QR code and emoji verification.
-        otherPartySupportsMethod: jest.fn().mockReturnValue(true),
-
-        // If we asked for emoji, these are populated.
-        verifier,
-        chosenMethod,
-    } as unknown as VerificationRequest;
 }
 
 function createEmojiVerifier(): Verifier {
@@ -226,3 +232,109 @@ function createQrVerifier(): Verifier {
         verify: jest.fn(),
     } as unknown as Verifier;
 }
+
+class MockVerificationRequest
+    extends TypedEventEmitter<VerificationRequestEvent, VerificationRequestEventHandlerMap>
+    implements VerificationRequest
+{
+    phase_: VerificationPhase;
+    verifier_: Verifier | undefined;
+    chosenMethod_: string | null;
+
+    constructor(phase: VerificationPhase, verifier: Verifier | undefined, chosenMethod: string | null) {
+        super();
+        this.phase_ = phase;
+        this.verifier_ = verifier;
+        this.chosenMethod_ = chosenMethod;
+    }
+
+    get phase(): VerificationPhase {
+        return this.phase_;
+    }
+
+    get isSelfVerification(): boolean {
+        // So far we are only testing verification of our own devices
+        return true;
+    }
+
+    get initiatedByMe(): boolean {
+        // So far we are only testing verification started by this device
+        return true;
+    }
+
+    otherPartySupportsMethod(): boolean {
+        // This makes both emoji and QR verification options appear
+        return true;
+    }
+
+    get verifier(): Verifier | undefined {
+        return this.verifier_;
+    }
+
+    get chosenMethod(): string | null {
+        return this.chosenMethod_;
+    }
+
+    async cancel(): Promise<void> {
+        this.phase_ = VerificationPhase.Cancelled;
+        this.emit(VerificationRequestEvent.Change);
+    }
+
+    get transactionId(): string | undefined {
+        return undefined;
+    }
+
+    get roomId(): string | undefined {
+        return undefined;
+    }
+
+    get otherUserId(): string {
+        return "otheruser";
+    }
+
+    get otherDeviceId(): string | undefined {
+        return undefined;
+    }
+
+    get pending(): boolean {
+        return false;
+    }
+
+    get accepting(): boolean {
+        return false;
+    }
+
+    get declining(): boolean {
+        return false;
+    }
+
+    get timeout(): number | null {
+        return null;
+    }
+
+    get methods(): string[] {
+        return [];
+    }
+
+    async accept(): Promise<void> {}
+
+    startVerification(_method: string): Promise<Verifier> {
+        throw new Error("Method not implemented.");
+    }
+
+    scanQRCode(_qrCodeData: Uint8ClampedArray): Promise<Verifier> {
+        throw new Error("Method not implemented.");
+    }
+
+    async generateQRCode(): Promise<Uint8ClampedArray | undefined> {
+        return undefined;
+    }
+
+    get cancellationCode(): string | null {
+        return null;
+    }
+
+    get cancellingUserId(): string | undefined {
+        return "otheruser";
+    }
+}

test/unit-tests/components/views/dialogs/__snapshots__/VerificationRequestDialog-test.tsx.snap

@@ -236,7 +236,7 @@ exports[`VerificationRequestDialog Shows a failure message if verification was c
         class="mx_Heading_h3 mx_Dialog_title"
         id="mx_BaseDialog_title"
       >
-        Verify other device
+        Verification failed
       </h1>
     </div>
     <div