From a23279e63369876c25409f9adbbd67f4c7a85c75 Mon Sep 17 00:00:00 2001 From: Ajay Bura <32841439+ajbura@users.noreply.github.com> Date: Mon, 26 May 2025 14:21:27 +0530 Subject: [PATCH] Fix rate limit when reordering in space lobby (#2254) * move can drop lobby item logic to hook * add comment * resolve rate limit when reordering space children --- src/app/features/lobby/Lobby.tsx | 425 +++++++++++------- src/app/features/lobby/SpaceHierarchy.tsx | 8 +- .../space-add-existing/SpaceAddExisting.jsx | 12 +- src/app/utils/matrix.ts | 12 +- 4 files changed, 270 insertions(+), 187 deletions(-) diff --git a/src/app/features/lobby/Lobby.tsx b/src/app/features/lobby/Lobby.tsx index 757db23..069e925 100644 --- a/src/app/features/lobby/Lobby.tsx +++ b/src/app/features/lobby/Lobby.tsx @@ -1,5 +1,5 @@ import React, { MouseEventHandler, useCallback, useMemo, useRef, useState } from 'react'; -import { Box, Icon, IconButton, Icons, Line, Scroll, config } from 'folds'; +import { Box, Chip, Icon, IconButton, Icons, Line, Scroll, Spinner, Text, config } from 'folds'; import { useVirtualizer } from '@tanstack/react-virtual'; import { useAtom, useAtomValue } from 'jotai'; import { useNavigate } from 'react-router-dom'; @@ -36,7 +36,7 @@ import { makeLobbyCategoryId } from '../../state/closedLobbyCategories'; import { useCategoryHandler } from '../../hooks/useCategoryHandler'; import { useMatrixClient } from '../../hooks/useMatrixClient'; import { allRoomsAtom } from '../../state/room-list/roomList'; -import { getCanonicalAliasOrRoomId } from '../../utils/matrix'; +import { getCanonicalAliasOrRoomId, rateLimitedActions } from '../../utils/matrix'; import { getSpaceRoomPath } from '../../pages/pathUtils'; import { StateEvent } from '../../../types/matrix/room'; import { CanDropCallback, useDnDMonitor } from './DnD'; @@ -53,6 +53,95 @@ import { roomToParentsAtom } from '../../state/room/roomToParents'; import { AccountDataEvent } from '../../../types/matrix/accountData'; import { useRoomMembers } from '../../hooks/useRoomMembers'; import { SpaceHierarchy } from './SpaceHierarchy'; +import { useGetRoom } from '../../hooks/useGetRoom'; +import { AsyncStatus, useAsyncCallback } from '../../hooks/useAsyncCallback'; + +const useCanDropLobbyItem = ( + space: Room, + roomsPowerLevels: Map, + getRoom: (roomId: string) => Room | undefined, + canEditSpaceChild: (powerLevels: IPowerLevels) => boolean +): CanDropCallback => { + const mx = useMatrixClient(); + + const canDropSpace: CanDropCallback = useCallback( + (item, container) => { + if (!('space' in container.item)) { + // can not drop around rooms. + // space can only be drop around other spaces + return false; + } + + const containerSpaceId = space.roomId; + + if ( + getRoom(containerSpaceId) === undefined || + !canEditSpaceChild(roomsPowerLevels.get(containerSpaceId) ?? {}) + ) { + return false; + } + + return true; + }, + [space, roomsPowerLevels, getRoom, canEditSpaceChild] + ); + + const canDropRoom: CanDropCallback = useCallback( + (item, container) => { + const containerSpaceId = + 'space' in container.item ? container.item.roomId : container.item.parentId; + + const draggingOutsideSpace = item.parentId !== containerSpaceId; + const restrictedItem = mx.getRoom(item.roomId)?.getJoinRule() === JoinRule.Restricted; + + // check and do not allow restricted room to be dragged outside + // current space if can't change `m.room.join_rules` `content.allow` + if (draggingOutsideSpace && restrictedItem) { + const itemPowerLevel = roomsPowerLevels.get(item.roomId) ?? {}; + const userPLInItem = powerLevelAPI.getPowerLevel( + itemPowerLevel, + mx.getUserId() ?? undefined + ); + const canChangeJoinRuleAllow = powerLevelAPI.canSendStateEvent( + itemPowerLevel, + StateEvent.RoomJoinRules, + userPLInItem + ); + if (!canChangeJoinRuleAllow) { + return false; + } + } + + if ( + getRoom(containerSpaceId) === undefined || + !canEditSpaceChild(roomsPowerLevels.get(containerSpaceId) ?? {}) + ) { + return false; + } + return true; + }, + [mx, getRoom, canEditSpaceChild, roomsPowerLevels] + ); + + const canDrop: CanDropCallback = useCallback( + (item, container): boolean => { + if (item.roomId === container.item.roomId || item.roomId === container.nextRoomId) { + // can not drop before or after itself + return false; + } + + // if we are dragging a space + if ('space' in item) { + return canDropSpace(item, container); + } + + return canDropRoom(item, container); + }, + [canDropSpace, canDropRoom] + ); + + return canDrop; +}; export function Lobby() { const navigate = useNavigate(); @@ -92,15 +181,7 @@ export function Lobby() { useCallback((w, height) => setHeroSectionHeight(height), []) ); - const getRoom = useCallback( - (rId: string) => { - if (allJoinedRooms.has(rId)) { - return mx.getRoom(rId) ?? undefined; - } - return undefined; - }, - [mx, allJoinedRooms] - ); + const getRoom = useGetRoom(allJoinedRooms); const canEditSpaceChild = useCallback( (powerLevels: IPowerLevels) => @@ -150,180 +231,155 @@ export function Lobby() { ) ); - const canDrop: CanDropCallback = useCallback( - (item, container): boolean => { - const restrictedItem = mx.getRoom(item.roomId)?.getJoinRule() === JoinRule.Restricted; - if (item.roomId === container.item.roomId || item.roomId === container.nextRoomId) { - // can not drop before or after itself - return false; - } - - if ('space' in item) { - if (!('space' in container.item)) return false; - const containerSpaceId = space.roomId; - - if ( - getRoom(containerSpaceId) === undefined || - !canEditSpaceChild(roomsPowerLevels.get(containerSpaceId) ?? {}) - ) { - return false; - } - - return true; - } - - const containerSpaceId = - 'space' in container.item ? container.item.roomId : container.item.parentId; - - const dropOutsideSpace = item.parentId !== containerSpaceId; - - if (dropOutsideSpace && restrictedItem) { - // do not allow restricted room to drop outside - // current space if can't change join rule allow - const itemPowerLevel = roomsPowerLevels.get(item.roomId) ?? {}; - const userPLInItem = powerLevelAPI.getPowerLevel( - itemPowerLevel, - mx.getUserId() ?? undefined - ); - const canChangeJoinRuleAllow = powerLevelAPI.canSendStateEvent( - itemPowerLevel, - StateEvent.RoomJoinRules, - userPLInItem - ); - if (!canChangeJoinRuleAllow) { - return false; - } - } - - if ( - getRoom(containerSpaceId) === undefined || - !canEditSpaceChild(roomsPowerLevels.get(containerSpaceId) ?? {}) - ) { - return false; - } - return true; - }, - [getRoom, space.roomId, roomsPowerLevels, canEditSpaceChild, mx] + const canDrop: CanDropCallback = useCanDropLobbyItem( + space, + roomsPowerLevels, + getRoom, + canEditSpaceChild ); - const reorderSpace = useCallback( - (item: HierarchyItemSpace, containerItem: HierarchyItem) => { - if (!item.parentId) return; + const [reorderSpaceState, reorderSpace] = useAsyncCallback( + useCallback( + async (item: HierarchyItemSpace, containerItem: HierarchyItem) => { + if (!item.parentId) return; - const itemSpaces: HierarchyItemSpace[] = hierarchy - .map((i) => i.space) - .filter((i) => i.roomId !== item.roomId); + const itemSpaces: HierarchyItemSpace[] = hierarchy + .map((i) => i.space) + .filter((i) => i.roomId !== item.roomId); - const beforeIndex = itemSpaces.findIndex((i) => i.roomId === containerItem.roomId); - const insertIndex = beforeIndex + 1; + const beforeIndex = itemSpaces.findIndex((i) => i.roomId === containerItem.roomId); + const insertIndex = beforeIndex + 1; - itemSpaces.splice(insertIndex, 0, { - ...item, - content: { ...item.content, order: undefined }, - }); + itemSpaces.splice(insertIndex, 0, { + ...item, + content: { ...item.content, order: undefined }, + }); - const currentOrders = itemSpaces.map((i) => { - if (typeof i.content.order === 'string' && lex.has(i.content.order)) { - return i.content.order; - } - return undefined; - }); + const currentOrders = itemSpaces.map((i) => { + if (typeof i.content.order === 'string' && lex.has(i.content.order)) { + return i.content.order; + } + return undefined; + }); - const newOrders = orderKeys(lex, currentOrders); + const newOrders = orderKeys(lex, currentOrders); - newOrders?.forEach((orderKey, index) => { - const itm = itemSpaces[index]; - if (!itm || !itm.parentId) return; - const parentPL = roomsPowerLevels.get(itm.parentId); - const canEdit = parentPL && canEditSpaceChild(parentPL); - if (canEdit && orderKey !== currentOrders[index]) { - mx.sendStateEvent( - itm.parentId, - StateEvent.SpaceChild as any, - { ...itm.content, order: orderKey }, - itm.roomId - ); - } - }); - }, - [mx, hierarchy, lex, roomsPowerLevels, canEditSpaceChild] - ); + const reorders = newOrders + ?.map((orderKey, index) => ({ + item: itemSpaces[index], + orderKey, + })) + .filter((reorder, index) => { + if (!reorder.item.parentId) return false; + const parentPL = roomsPowerLevels.get(reorder.item.parentId); + const canEdit = parentPL && canEditSpaceChild(parentPL); + return canEdit && reorder.orderKey !== currentOrders[index]; + }); - const reorderRoom = useCallback( - (item: HierarchyItem, containerItem: HierarchyItem): void => { - const itemRoom = mx.getRoom(item.roomId); - if (!item.parentId) { - return; - } - const containerParentId: string = - 'space' in containerItem ? containerItem.roomId : containerItem.parentId; - const itemContent = item.content; - - if (item.parentId !== containerParentId) { - mx.sendStateEvent(item.parentId, StateEvent.SpaceChild as any, {}, item.roomId); - } - - if ( - itemRoom && - itemRoom.getJoinRule() === JoinRule.Restricted && - item.parentId !== containerParentId - ) { - // change join rule allow parameter when dragging - // restricted room from one space to another - const joinRuleContent = getStateEvent( - itemRoom, - StateEvent.RoomJoinRules - )?.getContent(); - - if (joinRuleContent) { - const allow = - joinRuleContent.allow?.filter((allowRule) => allowRule.room_id !== item.parentId) ?? []; - allow.push({ type: RestrictedAllowType.RoomMembership, room_id: containerParentId }); - mx.sendStateEvent(itemRoom.roomId, StateEvent.RoomJoinRules as any, { - ...joinRuleContent, - allow, + if (reorders) { + await rateLimitedActions(reorders, async (reorder) => { + if (!reorder.item.parentId) return; + await mx.sendStateEvent( + reorder.item.parentId, + StateEvent.SpaceChild as any, + { ...reorder.item.content, order: reorder.orderKey }, + reorder.item.roomId + ); }); } - } - - const itemSpaces = Array.from( - hierarchy?.find((i) => i.space.roomId === containerParentId)?.rooms ?? [] - ); - - const beforeItem: HierarchyItem | undefined = - 'space' in containerItem ? undefined : containerItem; - const beforeIndex = itemSpaces.findIndex((i) => i.roomId === beforeItem?.roomId); - const insertIndex = beforeIndex + 1; - - itemSpaces.splice(insertIndex, 0, { - ...item, - parentId: containerParentId, - content: { ...itemContent, order: undefined }, - }); - - const currentOrders = itemSpaces.map((i) => { - if (typeof i.content.order === 'string' && lex.has(i.content.order)) { - return i.content.order; - } - return undefined; - }); - - const newOrders = orderKeys(lex, currentOrders); - - newOrders?.forEach((orderKey, index) => { - const itm = itemSpaces[index]; - if (itm && orderKey !== currentOrders[index]) { - mx.sendStateEvent( - containerParentId, - StateEvent.SpaceChild as any, - { ...itm.content, order: orderKey }, - itm.roomId - ); - } - }); - }, - [mx, hierarchy, lex] + }, + [mx, hierarchy, lex, roomsPowerLevels, canEditSpaceChild] + ) ); + const reorderingSpace = reorderSpaceState.status === AsyncStatus.Loading; + + const [reorderRoomState, reorderRoom] = useAsyncCallback( + useCallback( + async (item: HierarchyItem, containerItem: HierarchyItem) => { + const itemRoom = mx.getRoom(item.roomId); + if (!item.parentId) { + return; + } + const containerParentId: string = + 'space' in containerItem ? containerItem.roomId : containerItem.parentId; + const itemContent = item.content; + + // remove from current space + if (item.parentId !== containerParentId) { + mx.sendStateEvent(item.parentId, StateEvent.SpaceChild as any, {}, item.roomId); + } + + if ( + itemRoom && + itemRoom.getJoinRule() === JoinRule.Restricted && + item.parentId !== containerParentId + ) { + // change join rule allow parameter when dragging + // restricted room from one space to another + const joinRuleContent = getStateEvent( + itemRoom, + StateEvent.RoomJoinRules + )?.getContent(); + + if (joinRuleContent) { + const allow = + joinRuleContent.allow?.filter((allowRule) => allowRule.room_id !== item.parentId) ?? + []; + allow.push({ type: RestrictedAllowType.RoomMembership, room_id: containerParentId }); + mx.sendStateEvent(itemRoom.roomId, StateEvent.RoomJoinRules as any, { + ...joinRuleContent, + allow, + }); + } + } + + const itemSpaces = Array.from( + hierarchy?.find((i) => i.space.roomId === containerParentId)?.rooms ?? [] + ); + + const beforeItem: HierarchyItem | undefined = + 'space' in containerItem ? undefined : containerItem; + const beforeIndex = itemSpaces.findIndex((i) => i.roomId === beforeItem?.roomId); + const insertIndex = beforeIndex + 1; + + itemSpaces.splice(insertIndex, 0, { + ...item, + parentId: containerParentId, + content: { ...itemContent, order: undefined }, + }); + + const currentOrders = itemSpaces.map((i) => { + if (typeof i.content.order === 'string' && lex.has(i.content.order)) { + return i.content.order; + } + return undefined; + }); + + const newOrders = orderKeys(lex, currentOrders); + + const reorders = newOrders + ?.map((orderKey, index) => ({ + item: itemSpaces[index], + orderKey, + })) + .filter((reorder, index) => reorder.item && reorder.orderKey !== currentOrders[index]); + + if (reorders) { + await rateLimitedActions(reorders, async (reorder) => { + await mx.sendStateEvent( + containerParentId, + StateEvent.SpaceChild as any, + { ...reorder.item.content, order: reorder.orderKey }, + reorder.item.roomId + ); + }); + } + }, + [mx, hierarchy, lex] + ) + ); + const reorderingRoom = reorderRoomState.status === AsyncStatus.Loading; + const reordering = reorderingRoom || reorderingSpace; useDnDMonitor( scrollRef, @@ -449,6 +505,7 @@ export function Lobby() { draggingItem={draggingItem} onDragging={setDraggingItem} canDrop={canDrop} + disabledReorder={reordering} nextSpaceId={nextSpaceId} getRoom={getRoom} pinned={sidebarSpaces.has(item.space.roomId)} @@ -460,6 +517,28 @@ export function Lobby() { ); })} + {reordering && ( + + } + > + Reordering + + + )} diff --git a/src/app/features/lobby/SpaceHierarchy.tsx b/src/app/features/lobby/SpaceHierarchy.tsx index 2c43282..a152bc1 100644 --- a/src/app/features/lobby/SpaceHierarchy.tsx +++ b/src/app/features/lobby/SpaceHierarchy.tsx @@ -31,6 +31,7 @@ type SpaceHierarchyProps = { draggingItem?: HierarchyItem; onDragging: (item?: HierarchyItem) => void; canDrop: CanDropCallback; + disabledReorder?: boolean; nextSpaceId?: string; getRoom: (roomId: string) => Room | undefined; pinned: boolean; @@ -54,6 +55,7 @@ export const SpaceHierarchy = forwardRef( draggingItem, onDragging, canDrop, + disabledReorder, nextSpaceId, getRoom, pinned, @@ -116,7 +118,9 @@ export const SpaceHierarchy = forwardRef( handleClose={handleClose} getRoom={getRoom} canEditChild={canEditSpaceChild(spacePowerLevels)} - canReorder={parentPowerLevels ? canEditSpaceChild(parentPowerLevels) : false} + canReorder={ + parentPowerLevels && !disabledReorder ? canEditSpaceChild(parentPowerLevels) : false + } options={ parentId && parentPowerLevels && ( @@ -174,7 +178,7 @@ export const SpaceHierarchy = forwardRef( dm={mDirects.has(roomItem.roomId)} onOpen={onOpenRoom} getRoom={getRoom} - canReorder={canEditSpaceChild(spacePowerLevels)} + canReorder={canEditSpaceChild(spacePowerLevels) && !disabledReorder} options={ { setProcess(`Adding ${selected.length} items...`); - const promises = selected.map((rId) => { + await rateLimitedActions(selected, async (rId) => { const room = mx.getRoom(rId); const via = getViaServers(room); if (via.length === 0) { via.push(getIdServer(rId)); } - return mx.sendStateEvent( + await mx.sendStateEvent( roomId, 'm.space.child', { @@ -87,9 +89,7 @@ function SpaceAddExistingContent({ roomId, spaces: onlySpaces }) { ); }); - mountStore.setItem(true); - await Promise.allSettled(promises); - if (mountStore.getItem() !== true) return; + if (!alive()) return; const roomIds = onlySpaces ? [...spaces] : [...rooms, ...directs]; const allIds = roomIds.filter( diff --git a/src/app/utils/matrix.ts b/src/app/utils/matrix.ts index 810f720..a495e8d 100644 --- a/src/app/utils/matrix.ts +++ b/src/app/utils/matrix.ts @@ -300,7 +300,7 @@ export const downloadEncryptedMedia = async ( export const rateLimitedActions = async ( data: T[], - callback: (item: T) => Promise, + callback: (item: T, index: number) => Promise, maxRetryCount?: number ) => { let retryCount = 0; @@ -312,8 +312,8 @@ export const rateLimitedActions = async ( setTimeout(resolve, ms); }); - const performAction = async (dataItem: T) => { - const [err] = await to(callback(dataItem)); + const performAction = async (dataItem: T, index: number) => { + const [err] = await to(callback(dataItem, index)); if (err?.httpStatus === 429) { if (retryCount === maxRetryCount) { @@ -321,11 +321,11 @@ export const rateLimitedActions = async ( } const waitMS = err.getRetryAfterMs() ?? 3000; - actionInterval = waitMS + 500; + actionInterval = waitMS * 1.5; await sleepForMs(waitMS); retryCount += 1; - await performAction(dataItem); + await performAction(dataItem, index); } }; @@ -333,7 +333,7 @@ export const rateLimitedActions = async ( const dataItem = data[i]; retryCount = 0; // eslint-disable-next-line no-await-in-loop - await performAction(dataItem); + await performAction(dataItem, i); if (actionInterval > 0) { // eslint-disable-next-line no-await-in-loop await sleepForMs(actionInterval);