From 5bfbca9eb075c76ef559cabf37310b75bbcf8799 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 5 Sep 2024 16:37:24 +0200 Subject: [PATCH] Migrate all pinning checks and actions into `PinningUtils` (#12964) --- .../context_menus/MessageContextMenu.tsx | 2 +- .../views/dialogs/UnpinAllDialog.tsx | 5 +- .../views/messages/MessageActionBar.tsx | 5 +- .../views/right_panel/PinnedMessagesCard.tsx | 8 +- .../views/rooms/PinnedEventTile.tsx | 22 +--- src/utils/PinningUtils.ts | 49 +++++++- .../views/rooms/PinnedEventTile-test.tsx | 4 + test/utils/PinningUtils-test.ts | 118 ++++++++++++------ 8 files changed, 146 insertions(+), 67 deletions(-) diff --git a/src/components/views/context_menus/MessageContextMenu.tsx b/src/components/views/context_menus/MessageContextMenu.tsx index 2d5a81a89b..9da62c7d23 100644 --- a/src/components/views/context_menus/MessageContextMenu.tsx +++ b/src/components/views/context_menus/MessageContextMenu.tsx @@ -177,7 +177,7 @@ export default class MessageContextMenu extends React.Component this.props.mxEvent.getType() !== EventType.RoomServerAcl && this.props.mxEvent.getType() !== EventType.RoomEncryption; - const canPin = PinningUtils.canPinOrUnpin(cli, this.props.mxEvent); + const canPin = PinningUtils.canPin(cli, this.props.mxEvent) || PinningUtils.canUnpin(cli, this.props.mxEvent); this.setState({ canRedact, canPin }); }; diff --git a/src/components/views/dialogs/UnpinAllDialog.tsx b/src/components/views/dialogs/UnpinAllDialog.tsx index ef7439d858..4b752e820f 100644 --- a/src/components/views/dialogs/UnpinAllDialog.tsx +++ b/src/components/views/dialogs/UnpinAllDialog.tsx @@ -16,11 +16,12 @@ import React, { JSX } from "react"; import { Button, Text } from "@vector-im/compound-web"; -import { EventType, MatrixClient } from "matrix-js-sdk/src/matrix"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import BaseDialog from "../dialogs/BaseDialog"; import { _t } from "../../../languageHandler"; +import PinningUtils from "../../../utils/PinningUtils.ts"; /** * Properties for {@link UnpinAllDialog}. @@ -59,7 +60,7 @@ export function UnpinAllDialog({ matrixClient, roomId, onFinished }: UnpinAllDia destructive={true} onClick={async () => { try { - await matrixClient.sendStateEvent(roomId, EventType.RoomPinnedEvents, { pinned: [] }, ""); + await PinningUtils.unpinAllEvents(matrixClient, roomId); } catch (e) { logger.error("Failed to unpin all events:", e); } diff --git a/src/components/views/messages/MessageActionBar.tsx b/src/components/views/messages/MessageActionBar.tsx index 9130183e84..bb870e3dbe 100644 --- a/src/components/views/messages/MessageActionBar.tsx +++ b/src/components/views/messages/MessageActionBar.tsx @@ -432,7 +432,10 @@ export default class MessageActionBar extends React.PureComponent - state.mayClientSendStateEvent(EventType.RoomPinnedEvents, matrixClient), - ); + const canUnpin = useRoomState(room, () => PinningUtils.userHasPinOrUnpinPermission(matrixClient, room)); /** * Opens the unpin all dialog. diff --git a/src/components/views/rooms/PinnedEventTile.tsx b/src/components/views/rooms/PinnedEventTile.tsx index c77cffe776..af4ff71916 100644 --- a/src/components/views/rooms/PinnedEventTile.tsx +++ b/src/components/views/rooms/PinnedEventTile.tsx @@ -41,6 +41,7 @@ import { getForwardableEvent } from "../../../events"; import { OpenForwardDialogPayload } from "../../../dispatcher/payloads/OpenForwardDialogPayload"; import { createRedactEventDialog } from "../dialogs/ConfirmRedactDialog"; import { ShowThreadPayload } from "../../../dispatcher/payloads/ShowThreadPayload"; +import PinningUtils from "../../../utils/PinningUtils.ts"; const AVATAR_SIZE = "32px"; @@ -162,30 +163,17 @@ function PinMenu({ event, room, permalinkCreator }: PinMenuProps): JSX.Element { /** * Whether the client can unpin the event. - * Pin and unpin are using the same permission. + * If the room state change, we want to check again the permission */ - const canUnpin = useRoomState(room, (state) => - state.mayClientSendStateEvent(EventType.RoomPinnedEvents, matrixClient), - ); + const canUnpin = useRoomState(room, () => PinningUtils.canUnpin(matrixClient, event)); /** * Unpin the event. * @param event */ const onUnpin = useCallback(async (): Promise => { - const pinnedEvents = room - .getLiveTimeline() - .getState(EventTimeline.FORWARDS) - ?.getStateEvents(EventType.RoomPinnedEvents, ""); - if (pinnedEvents?.getContent()?.pinned) { - const pinned = pinnedEvents.getContent().pinned; - const index = pinned.indexOf(event.getId()); - if (index !== -1) { - pinned.splice(index, 1); - await matrixClient.sendStateEvent(room.roomId, EventType.RoomPinnedEvents, { pinned }, ""); - } - } - }, [event, room, matrixClient]); + await PinningUtils.pinOrUnpinEvent(matrixClient, event); + }, [event, matrixClient]); const contentActionable = isContentActionable(event); // Get the forwardable event for the given event diff --git a/src/utils/PinningUtils.ts b/src/utils/PinningUtils.ts index 9a20a721b9..66fee10efb 100644 --- a/src/utils/PinningUtils.ts +++ b/src/utils/PinningUtils.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MatrixEvent, EventType, M_POLL_START, MatrixClient, EventTimeline } from "matrix-js-sdk/src/matrix"; +import { MatrixEvent, EventType, M_POLL_START, MatrixClient, EventTimeline, Room } from "matrix-js-sdk/src/matrix"; import { isContentActionable } from "./EventUtils"; import SettingsStore from "../settings/SettingsStore"; @@ -71,23 +71,53 @@ export default class PinningUtils { } /** - * Determines if the given event may be pinned or unpinned by the current user. - * This checks if the user has the necessary permissions to pin or unpin the event, and if the event is pinnable. + * Determines if the given event may be pinned or unpinned by the current user + * It doesn't check if the event is pinnable or unpinnable. * @param matrixClient * @param mxEvent + * @private */ - public static canPinOrUnpin(matrixClient: MatrixClient, mxEvent: MatrixEvent): boolean { + private static canPinOrUnpin(matrixClient: MatrixClient, mxEvent: MatrixEvent): boolean { if (!SettingsStore.getValue("feature_pinning")) return false; if (!isContentActionable(mxEvent)) return false; const room = matrixClient.getRoom(mxEvent.getRoomId()); if (!room) return false; + return PinningUtils.userHasPinOrUnpinPermission(matrixClient, room); + } + + /** + * Determines if the given event may be pinned by the current user. + * This checks if the user has the necessary permissions to pin or unpin the event, and if the event is pinnable. + * @param matrixClient + * @param mxEvent + */ + public static canPin(matrixClient: MatrixClient, mxEvent: MatrixEvent): boolean { + return PinningUtils.canPinOrUnpin(matrixClient, mxEvent) && PinningUtils.isPinnable(mxEvent); + } + + /** + * Determines if the given event may be unpinned by the current user. + * This checks if the user has the necessary permissions to pin or unpin the event, and if the event is unpinnable. + * @param matrixClient + * @param mxEvent + */ + public static canUnpin(matrixClient: MatrixClient, mxEvent: MatrixEvent): boolean { + return PinningUtils.canPinOrUnpin(matrixClient, mxEvent) && PinningUtils.isUnpinnable(mxEvent); + } + + /** + * Determines if the current user has permission to pin or unpin events in the given room. + * @param matrixClient + * @param room + */ + public static userHasPinOrUnpinPermission(matrixClient: MatrixClient, room: Room): boolean { return Boolean( room .getLiveTimeline() .getState(EventTimeline.FORWARDS) - ?.mayClientSendStateEvent(EventType.RoomPinnedEvents, matrixClient) && PinningUtils.isPinnable(mxEvent), + ?.mayClientSendStateEvent(EventType.RoomPinnedEvents, matrixClient), ); } @@ -128,4 +158,13 @@ export default class PinningUtils { roomAccountDataPromise, ]); } + + /** + * Unpin all events in the given room. + * @param matrixClient + * @param roomId + */ + public static async unpinAllEvents(matrixClient: MatrixClient, roomId: string): Promise { + await matrixClient.sendStateEvent(roomId, EventType.RoomPinnedEvents, { pinned: [] }, ""); + } } diff --git a/test/components/views/rooms/PinnedEventTile-test.tsx b/test/components/views/rooms/PinnedEventTile-test.tsx index 124138d834..6d3723019d 100644 --- a/test/components/views/rooms/PinnedEventTile-test.tsx +++ b/test/components/views/rooms/PinnedEventTile-test.tsx @@ -27,6 +27,7 @@ import dis from "../../../../src/dispatcher/dispatcher"; import { Action } from "../../../../src/dispatcher/actions"; import { getForwardableEvent } from "../../../../src/events"; import { createRedactEventDialog } from "../../../../src/components/views/dialogs/ConfirmRedactDialog"; +import SettingsStore from "../../../../src/settings/SettingsStore.ts"; jest.mock("../../../../src/components/views/dialogs/ConfirmRedactDialog", () => ({ createRedactEventDialog: jest.fn(), @@ -43,7 +44,10 @@ describe("", () => { mockClient = stubClient(); room = new Room(roomId, mockClient, userId); permalinkCreator = new RoomPermalinkCreator(room); + mockClient.getRoom = jest.fn().mockReturnValue(room); jest.spyOn(dis, "dispatch").mockReturnValue(undefined); + // Enable feature_pinning + jest.spyOn(SettingsStore, "getValue").mockReturnValue(true); }); /** diff --git a/test/utils/PinningUtils-test.ts b/test/utils/PinningUtils-test.ts index adfd268bf1..4f37443582 100644 --- a/test/utils/PinningUtils-test.ts +++ b/test/utils/PinningUtils-test.ts @@ -147,49 +147,65 @@ describe("PinningUtils", () => { }); }); - describe("canPinOrUnpin", () => { - test("should return false if pinning is disabled", () => { - // Disable feature pinning - jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); - const event = makePinEvent(); + describe("canPin & canUnpin", () => { + describe("canPin", () => { + test("should return false if pinning is disabled", () => { + // Disable feature pinning + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + const event = makePinEvent(); - expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(false); + expect(PinningUtils.canPin(matrixClient, event)).toBe(false); + }); + + test("should return false if event is not actionable", () => { + mockedIsContentActionable.mockImplementation(() => false); + const event = makePinEvent(); + + expect(PinningUtils.canPin(matrixClient, event)).toBe(false); + }); + + test("should return false if no room", () => { + matrixClient.getRoom = jest.fn().mockReturnValue(undefined); + const event = makePinEvent(); + + expect(PinningUtils.canPin(matrixClient, event)).toBe(false); + }); + + test("should return false if client cannot send state event", () => { + jest.spyOn( + matrixClient.getRoom(roomId)!.getLiveTimeline().getState(EventTimeline.FORWARDS)!, + "mayClientSendStateEvent", + ).mockReturnValue(false); + const event = makePinEvent(); + + expect(PinningUtils.canPin(matrixClient, event)).toBe(false); + }); + + test("should return false if event is not pinnable", () => { + const event = makePinEvent({ type: EventType.RoomCreate }); + + expect(PinningUtils.canPin(matrixClient, event)).toBe(false); + }); + + test("should return true if all conditions are met", () => { + const event = makePinEvent(); + + expect(PinningUtils.canPin(matrixClient, event)).toBe(true); + }); }); - test("should return false if event is not actionable", () => { - mockedIsContentActionable.mockImplementation(() => false); - const event = makePinEvent(); + describe("canUnpin", () => { + test("should return false if event is not unpinnable", () => { + const event = makePinEvent({ type: EventType.RoomCreate }); - expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(false); - }); + expect(PinningUtils.canUnpin(matrixClient, event)).toBe(false); + }); - test("should return false if no room", () => { - matrixClient.getRoom = jest.fn().mockReturnValue(undefined); - const event = makePinEvent(); + test("should return true if all conditions are met", () => { + const event = makePinEvent(); - expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(false); - }); - - test("should return false if client cannot send state event", () => { - jest.spyOn( - matrixClient.getRoom(roomId)!.getLiveTimeline().getState(EventTimeline.FORWARDS)!, - "mayClientSendStateEvent", - ).mockReturnValue(false); - const event = makePinEvent(); - - expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(false); - }); - - test("should return false if event is not pinnable", () => { - const event = makePinEvent({ type: EventType.RoomCreate }); - - expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(false); - }); - - test("should return true if all conditions are met", () => { - const event = makePinEvent(); - - expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(true); + expect(PinningUtils.canUnpin(matrixClient, event)).toBe(true); + }); }); }); @@ -258,4 +274,32 @@ describe("PinningUtils", () => { ); }); }); + + describe("userHasPinOrUnpinPermission", () => { + test("should return true if user can pin or unpin", () => { + expect(PinningUtils.userHasPinOrUnpinPermission(matrixClient, room)).toBe(true); + }); + + test("should return false if client cannot send state event", () => { + jest.spyOn( + matrixClient.getRoom(roomId)!.getLiveTimeline().getState(EventTimeline.FORWARDS)!, + "mayClientSendStateEvent", + ).mockReturnValue(false); + + expect(PinningUtils.userHasPinOrUnpinPermission(matrixClient, room)).toBe(false); + }); + }); + + describe("unpinAllEvents", () => { + it("should unpin all events in the given room", async () => { + await PinningUtils.unpinAllEvents(matrixClient, roomId); + + expect(matrixClient.sendStateEvent).toHaveBeenCalledWith( + roomId, + EventType.RoomPinnedEvents, + { pinned: [] }, + "", + ); + }); + }); });