Merge pull request #2708 from daostack/CW-system-crash-react

System crash with #310 error in useDiscussionById.ts:376
daostack · Jul 2, 2024 · b1dde57 · b1dde57
2 parents 7fc4213 + 95d8d63
commit b1dde57
Show file tree

Hide file tree

Showing 3 changed files with 202 additions and 185 deletions.
diff --git a/src/shared/components/Chat/ChatMessage/ChatMessage.tsx b/src/shared/components/Chat/ChatMessage/ChatMessage.tsx
@@ -405,12 +405,8 @@ const ChatMessage = ({
     [isMessageEditLoading, handleCheckboxChange, isMessageEditAllowed],
   );
 
-  if (isSystemMessage && discussionMessage.parsedText.length === 0) {
-    return null;
-  }
-
-  const emojiButton = useCallback(
-    () => (
+  const EmojiButton = useCallback(() => {
+    return (
       <ReactWithEmoji
         emojiButtonClassName={styles.emojiButton}
         discussionId={discussionMessage.discussionId}
@@ -422,13 +418,16 @@ const ChatMessage = ({
         }
         isNotCurrentUserMessage={isNotCurrentUserMessage}
       />
-    ),
-    [
-      discussionMessage.discussionId,
-      discussionMessage.id,
-      isNotCurrentUserMessage,
-    ],
-  );
+    );
+  }, [
+    discussionMessage.discussionId,
+    discussionMessage.id,
+    isNotCurrentUserMessage,
+  ]);
+
+  if (isSystemMessage && discussionMessage.parsedText.length === 0) {
+    return null;
+  }
 
   return (
     <ChatMessageContext.Provider value={chatMessageContextValue}>
@@ -442,7 +441,7 @@ const ChatMessage = ({
             [styles.systemMessageContainer]: isSystemMessage,
           })}
         >
-          {!isSystemMessage && !isNotCurrentUserMessage && emojiButton}
+          {!isSystemMessage && !isNotCurrentUserMessage && <EmojiButton />}
           {isNotCurrentUserMessage &&
             (isUserDiscussionMessage || isBotMessage) && (
               <div className={styles.iconWrapper} onClick={handleUserClick}>
@@ -584,7 +583,7 @@ const ChatMessage = ({
               </div>
             </>
           )}
-          {!isSystemMessage && isNotCurrentUserMessage && emojiButton}
+          {!isSystemMessage && isNotCurrentUserMessage && <EmojiButton />}
         </div>
       </li>
     </ChatMessageContext.Provider>

diff --git a/src/shared/hooks/useCases/useDiscussionMessagesById.ts b/src/shared/hooks/useCases/useDiscussionMessagesById.ts
@@ -238,75 +238,79 @@ export const useDiscussionMessagesById = ({
       setIsBatchLoading(true);
     }
 
-    DiscussionMessageService.subscribeToDiscussionMessagesByDiscussionId(
-      discussionId,
-      lastVisible && lastVisible[discussionId],
-      async (
-        addedDiscussionMessages,
-        modifiedDiscussionMessages,
-        removedDiscussionMessages,
-        lastVisibleDocument,
-      ) => {
-        const updatedDiscussionMessages = [
-          ...addedDiscussionMessages,
-          ...modifiedDiscussionMessages,
-        ];
-        setLastVisible((prevVisible) => ({
-          ...prevVisible,
-          [discussionId]: lastVisibleDocument,
-        }));
-
-        const hasLastVisibleDocument = !!lastVisibleDocument?.data();
-
-        const discussionsWithText = await Promise.all(
-          updatedDiscussionMessages.map(async (discussionMessage) => {
-            const isUserDiscussionMessage =
-              checkIsUserDiscussionMessage(discussionMessage);
-            const isSystemMessage =
-              checkIsSystemDiscussionMessage(discussionMessage);
-
-            const parsedText = await getTextFromTextEditorString({
-              userId,
-              ownerId: isUserDiscussionMessage
-                ? discussionMessage.ownerId
-                : null,
-              textEditorString: discussionMessage.text,
-              users,
-              commonId: discussionMessage.commonId,
-              systemMessage: isSystemMessage ? discussionMessage : undefined,
-              getCommonPagePath,
-              getCommonPageAboutTabPath,
-              directParent,
-              onUserClick,
-              onFeedItemClick,
-              onInternalLinkClick,
-            });
-
-            return {
-              ...discussionMessage,
-              parsedText,
-            };
-          }),
-        );
-        if (
-          discussionsWithText.length < MESSAGES_NUMBER_IN_BATCH &&
-          !hasLastVisibleDocument
-        ) {
-          setIsEndOfList((prevIsEndOfList) => ({
-            ...prevIsEndOfList,
-            [discussionId]: true,
+    try {
+      DiscussionMessageService.subscribeToDiscussionMessagesByDiscussionId(
+        discussionId,
+        lastVisible && lastVisible[discussionId],
+        async (
+          addedDiscussionMessages,
+          modifiedDiscussionMessages,
+          removedDiscussionMessages,
+          lastVisibleDocument,
+        ) => {
+          const updatedDiscussionMessages = [
+            ...addedDiscussionMessages,
+            ...modifiedDiscussionMessages,
+          ];
+          setLastVisible((prevVisible) => ({
+            ...prevVisible,
+            [discussionId]: lastVisibleDocument,
           }));
-        }
-        dispatch(
-          cacheActions.updateDiscussionMessagesStateByDiscussionId({
-            discussionId,
-            removedDiscussionMessages,
-            updatedDiscussionMessages: discussionsWithText,
-          }),
-        );
-        setIsBatchLoading(false);
-      },
-    );
+
+          const hasLastVisibleDocument = !!lastVisibleDocument?.data();
+
+          const discussionsWithText = await Promise.all(
+            updatedDiscussionMessages.map(async (discussionMessage) => {
+              const isUserDiscussionMessage =
+                checkIsUserDiscussionMessage(discussionMessage);
+              const isSystemMessage =
+                checkIsSystemDiscussionMessage(discussionMessage);
+
+              const parsedText = await getTextFromTextEditorString({
+                userId,
+                ownerId: isUserDiscussionMessage
+                  ? discussionMessage.ownerId
+                  : null,
+                textEditorString: discussionMessage.text,
+                users,
+                commonId: discussionMessage.commonId,
+                systemMessage: isSystemMessage ? discussionMessage : undefined,
+                getCommonPagePath,
+                getCommonPageAboutTabPath,
+                directParent,
+                onUserClick,
+                onFeedItemClick,
+                onInternalLinkClick,
+              });
+
+              return {
+                ...discussionMessage,
+                parsedText,
+              };
+            }),
+          );
+          if (
+            discussionsWithText.length < MESSAGES_NUMBER_IN_BATCH &&
+            !hasLastVisibleDocument
+          ) {
+            setIsEndOfList((prevIsEndOfList) => ({
+              ...prevIsEndOfList,
+              [discussionId]: true,
+            }));
+          }
+          dispatch(
+            cacheActions.updateDiscussionMessagesStateByDiscussionId({
+              discussionId,
+              removedDiscussionMessages,
+              updatedDiscussionMessages: discussionsWithText,
+            }),
+          );
+          setIsBatchLoading(false);
+        },
+      );
+    } catch(err) {
+      setIsBatchLoading(false);
+    }
   };
 
   useDeepCompareEffect(() => {
@@ -324,70 +328,74 @@ export const useDiscussionMessagesById = ({
         return;
       }
 
-      const discussionMessages = [...state.data];
-
-      // if (discussionMessages.length > 0 && users.length === 0) {
-      // return;
-      // }
+      try {
+        const discussionMessages = [...state.data];
 
-      const filteredMessages = discussionMessages.filter(
-        ({ moderation }) =>
-          moderation?.flag !== ModerationFlags.Hidden || hasPermissionToHide,
-      );
-      const loadedDiscussionMessages = await Promise.all(
-        filteredMessages.map(async (d) => {
-          const newDiscussionMessage = { ...d };
-          const parentMessage = filteredMessages.find(
-            ({ id }) => id === d.parentId,
-          );
-          if (
-            checkIsUserDiscussionMessage(d) &&
-            checkIsUserDiscussionMessage(newDiscussionMessage)
-          ) {
-            const commonMemberMessageOwner = [
-              AI_USER,
-              AI_PRO_USER,
-              ...users,
-              ...externalCommonUsers,
-            ].find((o) => o.uid === d.ownerId);
-            const messageOwner =
-              commonMemberMessageOwner ||
-              (await UserService.getUserById(d.ownerId));
-            newDiscussionMessage.owner = messageOwner;
-            if (!commonMemberMessageOwner && messageOwner) {
-              dispatch(
-                cacheActions.addUserToExternalCommonUsers({
-                  user: messageOwner,
-                }),
-              );
-            }
-          }
-          newDiscussionMessage.parentMessage = parentMessage
-            ? {
-                id: parentMessage.id,
-                text: parentMessage.text,
-                ownerName: parentMessage?.ownerName,
-                ...(checkIsUserDiscussionMessage(parentMessage) && {
-                  ownerId: parentMessage.ownerId,
-                }),
-                moderation: parentMessage?.moderation,
-                images: parentMessage?.images,
-                files: parentMessage?.files,
-                createdAt: parentMessage.createdAt,
+        const filteredMessages = discussionMessages.filter(
+          ({ moderation }) =>
+            moderation?.flag !== ModerationFlags.Hidden || hasPermissionToHide,
+        );
+        const loadedDiscussionMessages = await Promise.all(
+          filteredMessages.map(async (d) => {
+            const newDiscussionMessage = { ...d };
+            const parentMessage = filteredMessages.find(
+              ({ id }) => id === d.parentId,
+            );
+            if (
+              checkIsUserDiscussionMessage(d) &&
+              checkIsUserDiscussionMessage(newDiscussionMessage)
+            ) {
+              const commonMemberMessageOwner = [
+                AI_USER,
+                AI_PRO_USER,
+                ...users,
+                ...externalCommonUsers,
+              ].find((o) => o.uid === d.ownerId);
+              const messageOwner =
+                commonMemberMessageOwner ||
+                (await UserService.getUserById(d.ownerId));
+              newDiscussionMessage.owner = messageOwner;
+              if (!commonMemberMessageOwner && messageOwner) {
+                dispatch(
+                  cacheActions.addUserToExternalCommonUsers({
+                    user: messageOwner,
+                  }),
+                );
               }
-            : null;
-
-          return newDiscussionMessage;
-        }),
-      );
-
-      setDiscussionMessagesWithOwners(loadedDiscussionMessages);
-      setIsFirstBatchLoaded((prev) => ({
-        ...prev,
-        [discussionId]: true,
-      }));
+            }
+            newDiscussionMessage.parentMessage = parentMessage
+              ? {
+                  id: parentMessage.id,
+                  text: parentMessage.text,
+                  ownerName: parentMessage?.ownerName,
+                  ...(checkIsUserDiscussionMessage(parentMessage) && {
+                    ownerId: parentMessage.ownerId,
+                  }),
+                  moderation: parentMessage?.moderation,
+                  images: parentMessage?.images,
+                  files: parentMessage?.files,
+                  createdAt: parentMessage.createdAt,
+                }
+              : null;
+
+            return newDiscussionMessage;
+          }),
+        );
+
+        setDiscussionMessagesWithOwners(loadedDiscussionMessages);
+        setIsFirstBatchLoaded((prev) => ({
+          ...prev,
+          [discussionId]: true,
+        }));
+      } catch(err) {
+        setDiscussionMessagesWithOwners([]);
+        setIsFirstBatchLoaded((prev) => ({
+          ...prev,
+          [discussionId]: true,
+        }));
+      }
     })();
-  }, [state.data, hasPermissionToHide, users, externalCommonUsers]);
+  }, [state.data, hasPermissionToHide, users, externalCommonUsers, discussionId]);
 
   return {
     ...state,