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

@@ -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<string, IPowerLevels>,
+  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<RoomJoinRulesEventContent>();
-
-        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<RoomJoinRulesEventContent>();
+
+          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() {
                       );
                     })}
                   </div>
+                  {reordering && (
+                    <Box
+                      style={{
+                        position: 'absolute',
+                        bottom: config.space.S400,
+                        left: 0,
+                        right: 0,
+                        zIndex: 2,
+                        pointerEvents: 'none',
+                      }}
+                      justifyContent="Center"
+                    >
+                      <Chip
+                        variant="Secondary"
+                        outlined
+                        radii="Pill"
+                        before={<Spinner variant="Secondary" fill="Soft" size="100" />}
+                      >
+                        <Text size="L400">Reordering</Text>
+                      </Chip>
+                    </Box>
+                  )}
                 </PageContentCenter>
               </PageContent>
             </Scroll>

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<HTMLDivElement, SpaceHierarchyProps>(
       draggingItem,
       onDragging,
       canDrop,
+      disabledReorder,
       nextSpaceId,
       getRoom,
       pinned,
@@ -116,7 +118,9 @@ export const SpaceHierarchy = forwardRef<HTMLDivElement, SpaceHierarchyProps>(
           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<HTMLDivElement, SpaceHierarchyProps>(
                   dm={mDirects.has(roomItem.roomId)}
                   onOpen={onOpenRoom}
                   getRoom={getRoom}
-                  canReorder={canEditSpaceChild(spacePowerLevels)}
+                  canReorder={canEditSpaceChild(spacePowerLevels) && !disabledReorder}
                   options={
                     <HierarchyItemMenu
                       item={roomItem}

src/app/molecules/space-add-existing/SpaceAddExisting.jsx

@@ -28,9 +28,11 @@ import { allRoomsAtom } from '../../state/room-list/roomList';
 import { mDirectAtom } from '../../state/mDirectList';
 import { useMatrixClient } from '../../hooks/useMatrixClient';
 import { getViaServers } from '../../plugins/via-servers';
+import { rateLimitedActions } from '../../utils/matrix';
+import { useAlive } from '../../hooks/useAlive';
 
 function SpaceAddExistingContent({ roomId, spaces: onlySpaces }) {
-  const mountStore = useStore(roomId);
+  const alive = useAlive();
   const [debounce] = useState(new Debounce());
   const [process, setProcess] = useState(null);
   const [allRoomIds, setAllRoomIds] = useState([]);
@@ -68,14 +70,14 @@ function SpaceAddExistingContent({ roomId, spaces: onlySpaces }) {
   const handleAdd = async () => {
     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(

src/app/utils/matrix.ts

@@ -300,7 +300,7 @@ export const downloadEncryptedMedia = async (
 
 export const rateLimitedActions = async <T, R = void>(
   data: T[],
-  callback: (item: T) => Promise<R>,
+  callback: (item: T, index: number) => Promise<R>,
   maxRetryCount?: number
 ) => {
   let retryCount = 0;
@@ -312,8 +312,8 @@ export const rateLimitedActions = async <T, R = void>(
       setTimeout(resolve, ms);
     });
 
-  const performAction = async (dataItem: T) => {
-    const [err] = await to<R, MatrixError>(callback(dataItem));
+  const performAction = async (dataItem: T, index: number) => {
+    const [err] = await to<R, MatrixError>(callback(dataItem, index));
 
     if (err?.httpStatus === 429) {
       if (retryCount === maxRetryCount) {
@@ -321,11 +321,11 @@ export const rateLimitedActions = async <T, R = void>(
       }
 
       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 <T, R = void>(
     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);