Migrate all pinning checks and actions into PinningUtils (#12964)

src/components/views/context_menus/MessageContextMenu.tsx

@@ -177,7 +177,7 @@ export default class MessageContextMenu extends React.Component<IProps, IState>
             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 });
     };

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);
                         }

src/components/views/messages/MessageActionBar.tsx

@@ -432,7 +432,10 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
             );
         }
 
-        if (PinningUtils.canPinOrUnpin(MatrixClientPeg.safeGet(), this.props.mxEvent)) {
+        if (
+            PinningUtils.canPin(MatrixClientPeg.safeGet(), this.props.mxEvent) ||
+            PinningUtils.canUnpin(MatrixClientPeg.safeGet(), this.props.mxEvent)
+        ) {
             const isPinned = PinningUtils.isPinned(MatrixClientPeg.safeGet(), this.props.mxEvent);
             toolbarOpts.push(
                 <RovingAccessibleButton

src/components/views/right_panel/PinnedMessagesCard.tsx

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import React, { useCallback, useEffect, JSX } from "react";
-import { Room, MatrixEvent, EventType } from "matrix-js-sdk/src/matrix";
+import { Room, MatrixEvent } from "matrix-js-sdk/src/matrix";
 import { Button, Separator } from "@vector-im/compound-web";
 import classNames from "classnames";
 import PinIcon from "@vector-im/compound-design-tokens/assets/web/icons/pin";
@@ -35,6 +35,7 @@ import Modal from "../../../Modal";
 import { UnpinAllDialog } from "../dialogs/UnpinAllDialog";
 import EmptyState from "./EmptyState";
 import { usePinnedEvents, useReadPinnedEvents, useSortedFetchedPinnedEvents } from "../../../hooks/usePinnedEvents";
+import PinningUtils from "../../../utils/PinningUtils.ts";
 
 /**
  * List the pinned messages in a room inside a Card.
@@ -141,10 +142,9 @@ function PinnedMessages({ events, room, permalinkCreator }: PinnedMessagesProps)
 
     /**
      * Whether the client can unpin events from the room.
+     * Listen to room state to update this value.
      */
-    const canUnpin = useRoomState(room, (state) =>
+    const canUnpin = useRoomState(room, () => PinningUtils.userHasPinOrUnpinPermission(matrixClient, room));
-        state.mayClientSendStateEvent(EventType.RoomPinnedEvents, matrixClient),
-    );
 
     /**
      * Opens the unpin all dialog.

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) =>
+    const canUnpin = useRoomState(room, () => PinningUtils.canUnpin(matrixClient, event));
-        state.mayClientSendStateEvent(EventType.RoomPinnedEvents, matrixClient),
-    );
 
     /**
      * Unpin the event.
      * @param event
      */
     const onUnpin = useCallback(async (): Promise<void> => {
-        const pinnedEvents = room
+        await PinningUtils.pinOrUnpinEvent(matrixClient, event);
-            .getLiveTimeline()
+    }, [event, matrixClient]);
-            .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]);
 
     const contentActionable = isContentActionable(event);
     // Get the forwardable event for the given event

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.
+     * 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.
+     * 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<void> {
+        await matrixClient.sendStateEvent(roomId, EventType.RoomPinnedEvents, { pinned: [] }, "");
+    }
 }

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("<PinnedEventTile />", () => {
         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);
     });
 
     /**

test/utils/PinningUtils-test.ts

@@ -147,49 +147,65 @@ describe("PinningUtils", () => {
         });
     });
 
-    describe("canPinOrUnpin", () => {
+    describe("canPin & canUnpin", () => {
-        test("should return false if pinning is disabled", () => {
+        describe("canPin", () => {
-            // Disable feature pinning
+            test("should return false if pinning is disabled", () => {
-            jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
+                // Disable feature pinning
-            const event = makePinEvent();
+                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", () => {
+        describe("canUnpin", () => {
-            mockedIsContentActionable.mockImplementation(() => false);
+            test("should return false if event is not unpinnable", () => {
-            const event = makePinEvent();
+                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", () => {
+            test("should return true if all conditions are met", () => {
-            matrixClient.getRoom = jest.fn().mockReturnValue(undefined);
+                const event = makePinEvent();
-            const event = makePinEvent();
 
-            expect(PinningUtils.canPinOrUnpin(matrixClient, event)).toBe(false);
+                expect(PinningUtils.canUnpin(matrixClient, event)).toBe(true);
-        });
+            });
-
-        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);
         });
     });
 
@@ -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: [] },
+                "",
+            );
+        });
+    });
 });