From 0d186dd3acd82f40e0d737b921b04a023c3ada66 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 22 Oct 2024 14:39:56 -0400 Subject: [PATCH] refactor to use functional component --- .../views/rooms/MessageComposer.tsx | 4 +- .../views/rooms/UserIdentityWarning.tsx | 487 ++++++++---------- .../views/rooms/UserIdentityWarning-test.tsx | 86 ++-- 3 files changed, 270 insertions(+), 307 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.tsx b/src/components/views/rooms/MessageComposer.tsx index 3e68b963a4..73a048d3dd 100644 --- a/src/components/views/rooms/MessageComposer.tsx +++ b/src/components/views/rooms/MessageComposer.tsx @@ -30,7 +30,7 @@ import E2EIcon from "./E2EIcon"; import SettingsStore from "../../../settings/SettingsStore"; import { aboveLeftOf, MenuProps } from "../../structures/ContextMenu"; import ReplyPreview from "./ReplyPreview"; -import UserIdentityWarning from "./UserIdentityWarning"; +import { UserIdentityWarning } from "./UserIdentityWarning"; import { UPDATE_EVENT } from "../../../stores/AsyncStore"; import VoiceRecordComposerTile from "./VoiceRecordComposerTile"; import { VoiceRecordingStore } from "../../../stores/VoiceRecordingStore"; @@ -672,7 +672,7 @@ export class MessageComposer extends React.Component {
{recordingTooltip}
- + { - /** - * Which room members need their identity approved. - */ - private membersNeedingApproval: Map; - /** - * Whether we got a verification status update while we were fetching a - * user's verification status. - * - * We set the entry for a user to `false` when we fetch a user's - * verification status, and remove the user's entry when we are done - * fetching. When we receive a verification status update, if the entry for - * the user is `false`, we set it to `true`. After we have finished - * fetching the user's verification status, if the entry for the user is - * `true`, rather than `false`, we know that we got an update, and so we - * discard the value that we fetched. We always use the value from the - * update and consider it as the most up-to-date version. If the fetched - * value is more up-to-date, then we should be getting a new update soon - * with the newer value, so it will fix itself in the end. - */ - private gotVerificationStatusUpdate: Map; - private mounted: boolean; - private initialised: boolean; - public constructor(props: UserIdentityWarningProps, context: React.ContextType) { - super(props, context); - this.state = { - currentPrompt: undefined, - }; - this.membersNeedingApproval = new Map(); - this.gotVerificationStatusUpdate = new Map(); - this.mounted = true; - this.initialised = false; - this.addListeners(); - } +export const UserIdentityWarning: React.FC = ({ room }) => { + const cli = MatrixClientPeg.safeGet(); + const crypto = cli.getCrypto(); - public componentDidMount(): void { - if (!MatrixClientPeg.safeGet().getCrypto()) return; - if (this.props.room.hasEncryptionStateEvent()) { - this.initialise().catch((e) => { - logger.error("Error initialising UserIdentityWarning:", e); - }); - } - } + // The current room member that we are prompting the user to approve. + const [currentPrompt, setCurrentPrompt] = useState(undefined); - public componentWillUnmount(): void { - this.mounted = false; - this.removeListeners(); - } + // Whether or not we've already initialised the component by loading the + // room membership. + const initialisedRef = useRef(false); + // Which room members need their identity approved. + const membersNeedingApprovalRef = useRef>(new Map()); + // Whether we got a verification status update while we were fetching a + // user's verification status. + // + // We set the entry for a user to `false` when we fetch a user's + // verification status, and remove the user's entry when we are done + // fetching. When we receive a verification status update, if the entry for + // the user is `false`, we set it to `true`. After we have finished + // fetching the user's verification status, if the entry for the user is + // `true`, rather than `false`, we know that we got an update, and so we + // discard the value that we fetched. We always use the value from the + // update and consider it as the most up-to-date version. If the fetched + // value is more up-to-date, then we should be getting a new update soon + // with the newer value, so it will fix itself in the end. + const gotVerificationStatusUpdateRef = useRef>(new Map()); - /** - * Select a new user to display a warning for. This is called after the - * current prompted user no longer needs their identity approved. - */ - private selectCurrentPrompt(): void { - if (this.membersNeedingApproval.size === 0) { - this.setState({ - currentPrompt: undefined, - }); - return; - } - // We return the user with the smallest user ID. - const keys = Array.from(this.membersNeedingApproval.keys()).sort((a, b) => a.localeCompare(b)); - this.setState({ - currentPrompt: this.membersNeedingApproval.get(keys[0]!), - }); - } + useEffect(() => { + if (!crypto) return; - /** - * Initialise the component. Get the room members, check which ones need - * their identity approved, and pick one to display. - */ - public async initialise(): Promise { - if (!this.mounted || this.initialised) { - return; - } - this.initialised = true; + const membersNeedingApproval = membersNeedingApprovalRef.current; + const gotVerificationStatusUpdate = gotVerificationStatusUpdateRef.current; - const crypto = MatrixClientPeg.safeGet().getCrypto()!; - const members = await this.props.room.getEncryptionTargetMembers(); - if (!this.mounted) { - return; - } - - for (const member of members) { - const userId = member.userId; - if (this.gotVerificationStatusUpdate.has(userId)) { - // We're already checking their verification status, so we don't - // need to do anything here. - continue; - } - this.gotVerificationStatusUpdate.set(userId, false); - if (await userNeedsApproval(crypto, userId)) { - if ( - !this.membersNeedingApproval.has(userId) && - this.gotVerificationStatusUpdate.get(userId) === false - ) { - this.membersNeedingApproval.set(userId, member); - } - } - this.gotVerificationStatusUpdate.delete(userId); - } - if (!this.mounted) { - return; - } - - this.selectCurrentPrompt(); - } - - private addMemberNeedingApproval(userId: string): void { - if (userId === MatrixClientPeg.safeGet().getUserId()) { - // We always skip our own user, because we can't pin our own identity. - return; - } - const member = this.props.room.getMember(userId); - if (member) { - this.membersNeedingApproval.set(userId, member); - if (!this.state.currentPrompt) { - // If we're not currently displaying a prompt, then we should - // display a prompt for this user. - this.selectCurrentPrompt(); - } - } - } - - private removeMemberNeedingApproval(userId: string): void { - this.membersNeedingApproval.delete(userId); - - // If we removed the currently displayed user, we need to pick a new one - // to display. - if (this.state.currentPrompt?.userId === userId) { - this.selectCurrentPrompt(); - } - } - - private addListeners(): void { - const cli = MatrixClientPeg.safeGet(); - cli.on(CryptoEvent.UserTrustStatusChanged, this.onUserTrustStatusChanged); - cli.on(RoomStateEvent.Events, this.onRoomStateEvent); - } - - private removeListeners(): void { - const cli = MatrixClientPeg.get(); - if (cli) { - cli.removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserTrustStatusChanged); - cli.removeListener(RoomStateEvent.Events, this.onRoomStateEvent); - } - } - - /** - * Handle a change in user trust. If the user's identity now needs - * approval, make sure that a warning is shown. If the user's identity - * doesn't need approval, remove the warning (if any). - */ - private onUserTrustStatusChanged = (userId: string, verificationStatus: UserVerificationStatus): void => { - if (!this.initialised) { - return; - } - - if (this.gotVerificationStatusUpdate.has(userId)) { - this.gotVerificationStatusUpdate.set(userId, true); - } - - if (verificationStatus.needsUserApproval) { - this.addMemberNeedingApproval(userId); - } else { - this.removeMemberNeedingApproval(userId); - } - }; - - private onRoomStateEvent = async (event: MatrixEvent): Promise => { - if (event.getRoomId() !== this.props.room.roomId) { - return; - } - - const eventType = event.getType(); - if (eventType === EventType.RoomEncryption && event.getStateKey() === "") { - // Room is now encrypted, so we can initialise the component. - return this.initialise().catch((e) => { - logger.error("Error initialising UserIdentityWarning:", e); - }); - } else if (eventType !== EventType.RoomMember) { - return; - } - - if (!this.initialised) { - return; - } - - const userId = event.getStateKey(); - - if (!userId) return; - - if ( - event.getContent().membership === KnownMembership.Join || - (event.getContent().membership === KnownMembership.Join && this.props.room.shouldEncryptForInvitedMembers()) - ) { - // Someone's membership changed and we will now encrypt to them. If - // their identity needs approval, show a warning. - if (this.gotVerificationStatusUpdate.has(userId)) { - // We're already checking their verification status, so we don't - // need to do anything here. + /** + * Select a new user to display a warning for. This is called after the + * current prompted user no longer needs their identity approved. + */ + function selectCurrentPrompt(): void { + if (membersNeedingApproval.size === 0) { + setCurrentPrompt(undefined); return; } - this.gotVerificationStatusUpdate.set(userId, false); - const crypto = MatrixClientPeg.safeGet().getCrypto()!; - if (await userNeedsApproval(crypto, userId)) { - if ( - !this.membersNeedingApproval.has(userId) && - this.gotVerificationStatusUpdate.get(userId) === false - ) { - this.addMemberNeedingApproval(userId); + + // We return the user with the smallest user ID. + const keys = Array.from(membersNeedingApproval.keys()).sort((a, b) => a.localeCompare(b)); + setCurrentPrompt(membersNeedingApproval.get(keys[0]!)); + } + + function addMemberNeedingApproval(userId: string): void { + if (userId === cli.getUserId()) { + // We always skip our own user, because we can't pin our own identity. + return; + } + const member = room.getMember(userId); + if (member) { + membersNeedingApproval.set(userId, member); + if (!currentPrompt) { + // If we're not currently displaying a prompt, then we should + // display a prompt for this user. + selectCurrentPrompt(); } } - this.gotVerificationStatusUpdate.delete(userId); - } else { - // Someone's membership changed and we no longer encrypt to them. - // If we're showing a warning about them, we don't need to any more. - this.removeMemberNeedingApproval(userId); } - }; - /** - * Callback for when the user hits the "OK" button. - */ - public confirmIdentity = async (): Promise => { - if (this.state.currentPrompt) { - await MatrixClientPeg.safeGet().getCrypto()!.pinCurrentUserIdentity(this.state.currentPrompt.userId); + function removeMemberNeedingApproval(userId: string): void { + membersNeedingApproval.delete(userId); + + // If we removed the currently displayed user, we need to pick a new one + // to display. + if (currentPrompt?.userId === userId) { + selectCurrentPrompt(); + } } - }; - public render(): React.ReactNode { - const { currentPrompt } = this.state; - if (currentPrompt) { - const substituteATag = (sub: string): React.ReactNode => ( - - {sub} - - ); - const substituteBTag = (sub: string): React.ReactNode => {sub}; - return ( -
- -
- - - {currentPrompt.rawDisplayName === currentPrompt.userId - ? _t( - "encryption|pinned_identity_changed_no_displayname", - { userId: currentPrompt.userId }, - { - a: substituteATag, - b: substituteBTag, - }, - ) - : _t( - "encryption|pinned_identity_changed", - { displayName: currentPrompt.rawDisplayName, userId: currentPrompt.userId }, - { - a: substituteATag, - b: substituteBTag, - }, - )} - - -
+ /** + * Initialise the component. Get the room members, check which ones need + * their identity approved, and pick one to display. + */ + async function initialise(): Promise { + if (initialisedRef.current) { + return; + } + initialisedRef.current = true; + + const members = await room.getEncryptionTargetMembers(); + + for (const member of members) { + const userId = member.userId; + if (gotVerificationStatusUpdate.has(userId)) { + // We're already checking their verification status, so we don't + // need to do anything here. + continue; + } + gotVerificationStatusUpdate.set(userId, false); + if (await userNeedsApproval(crypto!, userId)) { + if (!membersNeedingApproval.has(userId) && gotVerificationStatusUpdate.get(userId) === false) { + membersNeedingApproval.set(userId, member); + } + } + gotVerificationStatusUpdate.delete(userId); + } + + selectCurrentPrompt(); + } + + const onUserTrustStatusChanged = (userId: string, verificationStatus: UserVerificationStatus): void => { + if (!initialisedRef.current) { + return; + } + + if (gotVerificationStatusUpdate.has(userId)) { + gotVerificationStatusUpdate.set(userId, true); + } + + if (verificationStatus.needsUserApproval) { + addMemberNeedingApproval(userId); + } else { + removeMemberNeedingApproval(userId); + } + }; + + const onRoomStateEvent = async (event: MatrixEvent): Promise => { + if (event.getRoomId() !== room.roomId) { + return; + } + + const eventType = event.getType(); + if (eventType === EventType.RoomEncryption && event.getStateKey() === "") { + // Room is now encrypted, so we can initialise the component. + return initialise().catch((e) => { + logger.error("Error initialising UserIdentityWarning:", e); + }); + } else if (eventType !== EventType.RoomMember) { + return; + } + + if (!initialisedRef.current) { + return; + } + + const userId = event.getStateKey(); + + if (!userId) return; + + if ( + event.getContent().membership === KnownMembership.Join || + (event.getContent().membership === KnownMembership.Join && room.shouldEncryptForInvitedMembers()) + ) { + // Someone's membership changed and we will now encrypt to them. If + // their identity needs approval, show a warning. + if (gotVerificationStatusUpdate.has(userId)) { + // We're already checking their verification status, so we don't + // need to do anything here. + return; + } + gotVerificationStatusUpdate.set(userId, false); + const crypto = MatrixClientPeg.safeGet().getCrypto()!; + if (await userNeedsApproval(crypto!, userId)) { + if (!membersNeedingApproval.has(userId) && gotVerificationStatusUpdate.get(userId) === false) { + addMemberNeedingApproval(userId); + } + } + gotVerificationStatusUpdate.delete(userId); + } else { + // Someone's membership changed and we no longer encrypt to them. + // If we're showing a warning about them, we don't need to any more. + removeMemberNeedingApproval(userId); + } + }; + + cli.on(CryptoEvent.UserTrustStatusChanged, onUserTrustStatusChanged); + cli.on(RoomStateEvent.Events, onRoomStateEvent); + + if (room.hasEncryptionStateEvent()) { + initialise().catch((e) => { + logger.error("Error initialising UserIdentityWarning:", e); + }); + } + + return () => { + cli.removeListener(CryptoEvent.UserTrustStatusChanged, onUserTrustStatusChanged); + cli.removeListener(RoomStateEvent.Events, onRoomStateEvent); + }; + }, [currentPrompt, room, cli, crypto]); + + if (!crypto) return null; + + if (currentPrompt) { + const confirmIdentity = async (): Promise => { + await crypto.pinCurrentUserIdentity(currentPrompt.userId); + }; + + const substituteATag = (sub: string): React.ReactNode => ( + + {sub} + + ); + const substituteBTag = (sub: string): React.ReactNode => {sub}; + return ( +
+ +
+ + + {currentPrompt.rawDisplayName === currentPrompt.userId + ? _t( + "encryption|pinned_identity_changed_no_displayname", + { userId: currentPrompt.userId }, + { + a: substituteATag, + b: substituteBTag, + }, + ) + : _t( + "encryption|pinned_identity_changed", + { displayName: currentPrompt.rawDisplayName, userId: currentPrompt.userId }, + { + a: substituteATag, + b: substituteBTag, + }, + )} + +
- ); - } else { - return null; - } +
+ ); + } else { + return null; } -} +}; diff --git a/test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx b/test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx index b3b8cb5e71..94bbf982b1 100644 --- a/test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx +++ b/test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx @@ -18,10 +18,10 @@ import { RoomMember, } from "matrix-js-sdk/src/matrix"; import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; -import { fireEvent, render, screen, waitFor } from "jest-matrix-react"; +import { act, fireEvent, render, screen, waitFor } from "jest-matrix-react"; import { stubClient } from "../../../../test-utils"; -import UserIdentityWarning from "../../../../../src/components/views/rooms/UserIdentityWarning"; +import { UserIdentityWarning } from "../../../../../src/components/views/rooms/UserIdentityWarning"; import MatrixClientContext from "../../../../../src/contexts/MatrixClientContext"; const ROOM_ID = "!room:id"; @@ -88,7 +88,7 @@ describe("UserIdentityWarning", () => { return Promise.resolve(new UserVerificationStatus(false, false, false, true)); }); crypto.pinCurrentUserIdentity = jest.fn(); - const { container } = render(, { + const { container } = render(, { wrapper: ({ ...rest }) => , }); @@ -115,7 +115,7 @@ describe("UserIdentityWarning", () => { return Promise.resolve(new UserVerificationStatus(false, false, false, true)); }); - render(, { + render(, { wrapper: ({ ...rest }) => , }); await sleep(10); // give it some time to finish initialising @@ -156,18 +156,20 @@ describe("UserIdentityWarning", () => { crypto["getUserVerificationStatus"] = jest.fn(async () => { return Promise.resolve(new UserVerificationStatus(false, false, false, false)); }); - render(, { + render(, { wrapper: ({ ...rest }) => , }); await sleep(10); // give it some time to finish initialising expect(() => getWarningByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(); // The user changes their identity, so we should show the warning. - client.emit( - CryptoEvent.UserTrustStatusChanged, - "@alice:example.org", - new UserVerificationStatus(false, false, false, true), - ); + act(() => { + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, true), + ); + }); await waitFor(() => expect( getWarningByText("Alice's (@alice:example.org) identity appears to have changed."), @@ -176,11 +178,13 @@ describe("UserIdentityWarning", () => { // Simulate the user's new identity having been approved, so we no // longer show the warning. - client.emit( - CryptoEvent.UserTrustStatusChanged, - "@alice:example.org", - new UserVerificationStatus(false, false, false, false), - ); + act(() => { + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, false), + ); + }); await waitFor(() => expect(() => getWarningByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(), ); @@ -198,7 +202,7 @@ describe("UserIdentityWarning", () => { crypto["getUserVerificationStatus"] = jest.fn(async () => { return Promise.resolve(new UserVerificationStatus(false, false, false, true)); }); - render(, { + render(, { wrapper: ({ ...rest }) => , }); await sleep(10); // give it some time to finish initialising @@ -226,21 +230,23 @@ describe("UserIdentityWarning", () => { ); // Alice leaves, so we no longer show her warning. - client.emit( - RoomStateEvent.Events, - new MatrixEvent({ - event_id: "$event_id", - type: EventType.RoomMember, - state_key: "@alice:example.org", - content: { - membership: "leave", - }, - room_id: ROOM_ID, - sender: "@alice:example.org", - }), - dummyRoomState(), - null, - ); + act(() => { + client.emit( + RoomStateEvent.Events, + new MatrixEvent({ + event_id: "$event_id", + type: EventType.RoomMember, + state_key: "@alice:example.org", + content: { + membership: "leave", + }, + room_id: ROOM_ID, + sender: "@alice:example.org", + }), + dummyRoomState(), + null, + ); + }); await waitFor(() => expect(() => getWarningByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(), ); @@ -258,7 +264,7 @@ describe("UserIdentityWarning", () => { return Promise.resolve(new UserVerificationStatus(false, false, false, true)); }); - render(, { + render(, { wrapper: ({ ...rest }) => , }); // We should warn about Alice's identity first. @@ -270,11 +276,13 @@ describe("UserIdentityWarning", () => { // Simulate Alice's new identity having been approved, so now we warn // about Bob's identity. - client.emit( - CryptoEvent.UserTrustStatusChanged, - "@alice:example.org", - new UserVerificationStatus(false, false, false, false), - ); + act(() => { + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, false), + ); + }); await waitFor(() => expect(getWarningByText("@bob:example.org's identity appears to have changed.")).toBeInTheDocument(), ); @@ -300,7 +308,7 @@ describe("UserIdentityWarning", () => { ); return Promise.resolve(new UserVerificationStatus(false, false, false, false)); }); - render(, { + render(, { wrapper: ({ ...rest }) => , }); await sleep(10); // give it some time to finish initialising @@ -328,7 +336,7 @@ describe("UserIdentityWarning", () => { ); return Promise.resolve(new UserVerificationStatus(false, false, false, true)); }); - render(, { + render(, { wrapper: ({ ...rest }) => , }); await sleep(10); // give it some time to finish initialising