From 5d9ff5f7c5c2464dd721ab24c5e46e06489e7c12 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 13 Feb 2023 15:36:39 +0100 Subject: [PATCH 1/3] Fix email lookup in invite dialog --- src/components/views/dialogs/InviteDialog.tsx | 5 +- .../views/dialogs/InviteDialog-test.tsx | 106 ++++++++++++------ 2 files changed, 75 insertions(+), 36 deletions(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index fc20150a630..91c4e01e254 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -684,7 +684,8 @@ export default class InviteDialog extends React.PureComponent t.userId === r.userId)} diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx index 3c95aa95f19..9c0dcda498b 100644 --- a/test/components/views/dialogs/InviteDialog-test.tsx +++ b/test/components/views/dialogs/InviteDialog-test.tsx @@ -17,11 +17,11 @@ limitations under the License. import React from "react"; import { render, screen } from "@testing-library/react"; import { RoomType } from "matrix-js-sdk/src/@types/event"; +import { Room } from "matrix-js-sdk/src/matrix"; import InviteDialog from "../../../../src/components/views/dialogs/InviteDialog"; -import { KIND_INVITE } from "../../../../src/components/views/dialogs/InviteDialogTypes"; -import { getMockClientWithEventEmitter, mkStubRoom } from "../../../test-utils"; -import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; +import { KIND_DM, KIND_INVITE } from "../../../../src/components/views/dialogs/InviteDialogTypes"; +import { getMockClientWithEventEmitter, mkMembership, mkMessage, mkRoomCreateEvent } from "../../../test-utils"; import DMRoomMap from "../../../../src/utils/DMRoomMap"; import SdkConfig from "../../../../src/SdkConfig"; import { ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig"; @@ -37,8 +37,11 @@ jest.mock("../../../../src/IdentityAuthClient", () => describe("InviteDialog", () => { const roomId = "!111111111111111111:example.org"; const aliceId = "@alice:example.org"; + const aliceEmail = "foobar@email.com"; + const bobId = "@bob:example.org"; const mockClient = getMockClientWithEventEmitter({ - getUserId: jest.fn().mockReturnValue(aliceId), + getUserId: jest.fn().mockReturnValue(bobId), + getSafeUserId: jest.fn().mockReturnValue(bobId), isGuest: jest.fn().mockReturnValue(false), getVisibleRooms: jest.fn().mockReturnValue([]), getRoom: jest.fn(), @@ -58,27 +61,53 @@ describe("InviteDialog", () => { getOpenIdToken: jest.fn().mockResolvedValue({}), getIdentityAccount: jest.fn().mockResolvedValue({}), getTerms: jest.fn().mockResolvedValue({ policies: [] }), + supportsThreads: jest.fn().mockReturnValue(false), + isInitialSyncComplete: jest.fn().mockReturnValue(true), }); + let room: Room; beforeEach(() => { SdkConfig.put({ validated_server_config: {} as ValidatedServerConfig } as IConfigOptions); DMRoomMap.makeShared(); jest.clearAllMocks(); - mockClient.getUserId.mockReturnValue("@bob:example.org"); - - const room = mkStubRoom(roomId, "Room", mockClient); + mockClient.getUserId.mockReturnValue(bobId); + + room = new Room(roomId, mockClient, mockClient.getSafeUserId()); + room.addLiveEvents([ + mkMessage({ + msg: "Hello", + relatesTo: undefined, + event: true, + room: roomId, + user: mockClient.getSafeUserId(), + ts: Date.now(), + }), + ]); + room.currentState.setStateEvents([ + mkRoomCreateEvent(bobId, roomId), + mkMembership({ + event: true, + room: roomId, + mship: "join", + user: aliceId, + skey: aliceId, + }), + ]); + jest.spyOn(DMRoomMap.shared(), "getUniqueRoomsWithIndividuals").mockReturnValue({ + [aliceId]: room, + }); mockClient.getRooms.mockReturnValue([room]); mockClient.getRoom.mockReturnValue(room); }); afterAll(() => { - jest.spyOn(MatrixClientPeg, "get").mockRestore(); + jest.restoreAllMocks(); }); it("should label with space name", () => { - mockClient.getRoom(roomId)!.isSpaceRoom = jest.fn().mockReturnValue(true); - mockClient.getRoom(roomId)!.getType = jest.fn().mockReturnValue(RoomType.Space); - mockClient.getRoom(roomId)!.name = "Space"; + room.isSpaceRoom = jest.fn().mockReturnValue(true); + room.getType = jest.fn().mockReturnValue(RoomType.Space); + room.name = "Space"; render(); expect(screen.queryByText("Invite to Space")).toBeTruthy(); @@ -86,8 +115,7 @@ describe("InviteDialog", () => { it("should label with room name", () => { render(); - - expect(screen.queryByText("Invite to Room")).toBeTruthy(); + expect(screen.getByText(`Invite to ${roomId}`)).toBeInTheDocument(); }); it("should suggest valid MXIDs even if unknown", async () => { @@ -116,27 +144,37 @@ describe("InviteDialog", () => { expect(screen.queryByText("@localpart:server:tld")).toBeFalsy(); }); - it("should lookup inputs which look like email addresses", async () => { - mockClient.getIdentityServerUrl.mockReturnValue("https://identity-server"); - mockClient.lookupThreePid.mockResolvedValue({ - address: "foobar@email.com", - medium: "email", - mxid: "@foobar:server", - }); - mockClient.getProfileInfo.mockResolvedValue({ - displayname: "Mr. Foo", - avatar_url: "mxc://foo/bar", - }); - - render( - , - ); - - await screen.findByText("Mr. Foo"); - await screen.findByText("@foobar:server"); - expect(mockClient.lookupThreePid).toHaveBeenCalledWith("email", "foobar@email.com", expect.anything()); - expect(mockClient.getProfileInfo).toHaveBeenCalledWith("@foobar:server"); - }); + it.each([[KIND_DM], [KIND_INVITE]] as [typeof KIND_DM | typeof KIND_INVITE][])( + "should lookup inputs which look like email addresses (%s)", + async (kind: typeof KIND_DM | typeof KIND_INVITE) => { + mockClient.getIdentityServerUrl.mockReturnValue("https://identity-server"); + mockClient.lookupThreePid.mockResolvedValue({ + address: aliceEmail, + medium: "email", + mxid: aliceId, + }); + mockClient.getProfileInfo.mockResolvedValue({ + displayname: "Mrs Alice", + avatar_url: "mxc://foo/bar", + }); + + render( + , + ); + + await screen.findByText("Mrs Alice"); + // expect the email and MXID to be visible + await screen.findByText(aliceId); + await screen.findByText(aliceEmail); + expect(mockClient.lookupThreePid).toHaveBeenCalledWith("email", aliceEmail, expect.anything()); + expect(mockClient.getProfileInfo).toHaveBeenCalledWith(aliceId); + }, + ); it("should suggest e-mail even if lookup fails", async () => { mockClient.getIdentityServerUrl.mockReturnValue("https://identity-server"); From a7f319b006fdce0270812ec3b39e04526bdfe9fc Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 14 Feb 2023 10:07:50 +0100 Subject: [PATCH 2/3] Refactor InviteDialog logic --- src/components/views/dialogs/InviteDialog.tsx | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index 91c4e01e254..9866cd799dc 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -684,8 +684,7 @@ export default class InviteDialog extends React.PureComponent m.user.name.toLowerCase().includes(filterBy) || m.userId.toLowerCase().includes(filterBy), + ); + } + // Mix in the server results if we have any, but only if we're searching. We track the additional // members separately because we want to filter sourceMembers but trust the mixin arrays to have // the right members in them. @@ -888,17 +895,9 @@ export default class InviteDialog extends React.PureComponent 0 || otherAdditionalMembers.length > 0; - // Hide the section if there's nothing to filter by - if (sourceMembers.length === 0 && !hasAdditionalMembers) return null; - - // Do some simple filtering on the input before going much further. If we get no results, say so. - if (this.state.filterText) { - const filterBy = this.state.filterText.toLowerCase(); - sourceMembers = sourceMembers.filter( - (m) => m.user.name.toLowerCase().includes(filterBy) || m.userId.toLowerCase().includes(filterBy), - ); - - if (sourceMembers.length === 0 && !hasAdditionalMembers) { + if (sourceMembers.length === 0 && !hasAdditionalMembers) { + if (this.state.filterText) { + // There was a search without results. Tell about it. return (

{sectionName}

@@ -906,6 +905,9 @@ export default class InviteDialog extends React.PureComponent ); } + + // Hide section if there was no search and there are no results. + return null; } // Now we mix in the additional members. Again, we presume these have already been filtered. We @@ -935,7 +937,7 @@ export default class InviteDialog extends React.PureComponent t.userId === r.userId)} From aff24ed78e4f148b334d4912a400af7e5a5dacc2 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 14 Feb 2023 11:44:56 +0100 Subject: [PATCH 3/3] Revert "Refactor InviteDialog logic" This reverts commit a7f319b006fdce0270812ec3b39e04526bdfe9fc. --- src/components/views/dialogs/InviteDialog.tsx | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index 9866cd799dc..91c4e01e254 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -684,7 +684,8 @@ export default class InviteDialog extends React.PureComponent m.user.name.toLowerCase().includes(filterBy) || m.userId.toLowerCase().includes(filterBy), - ); - } - // Mix in the server results if we have any, but only if we're searching. We track the additional // members separately because we want to filter sourceMembers but trust the mixin arrays to have // the right members in them. @@ -895,9 +888,17 @@ export default class InviteDialog extends React.PureComponent 0 || otherAdditionalMembers.length > 0; - if (sourceMembers.length === 0 && !hasAdditionalMembers) { - if (this.state.filterText) { - // There was a search without results. Tell about it. + // Hide the section if there's nothing to filter by + if (sourceMembers.length === 0 && !hasAdditionalMembers) return null; + + // Do some simple filtering on the input before going much further. If we get no results, say so. + if (this.state.filterText) { + const filterBy = this.state.filterText.toLowerCase(); + sourceMembers = sourceMembers.filter( + (m) => m.user.name.toLowerCase().includes(filterBy) || m.userId.toLowerCase().includes(filterBy), + ); + + if (sourceMembers.length === 0 && !hasAdditionalMembers) { return (

{sectionName}

@@ -905,9 +906,6 @@ export default class InviteDialog extends React.PureComponent ); } - - // Hide section if there was no search and there are no results. - return null; } // Now we mix in the additional members. Again, we presume these have already been filtered. We @@ -937,7 +935,7 @@ export default class InviteDialog extends React.PureComponent t.userId === r.userId)}