Преглед изворни кода

Refactor conflict resolution logic

Shun Miyazawa пре 2 година
родитељ
комит
ea50b22eef

+ 30 - 9
apps/app/src/client/services/side-effects/page-updated.ts

@@ -3,17 +3,24 @@ import { useCallback, useEffect } from 'react';
 import { useGlobalSocket } from '@growi/core/dist/swr';
 
 import { SocketEventName } from '~/interfaces/websocket';
-import { useCurrentPageId } from '~/stores/page';
+import { usePageStatusAlert } from '~/stores/alert';
+import { useSWRxCurrentPage, useSWRMUTxCurrentPage } from '~/stores/page';
 import { useSetRemoteLatestPageData, type RemoteRevisionData } from '~/stores/remote-latest-page';
+import { useEditorMode, EditorMode } from '~/stores/ui';
+
 
 export const usePageUpdatedEffect = (): void => {
 
   const { setRemoteLatestPageData } = useSetRemoteLatestPageData();
 
   const { data: socket } = useGlobalSocket();
-  const { data: currentPageId } = useCurrentPageId();
+  const { data: editorMode } = useEditorMode();
+  const { data: currentPage } = useSWRxCurrentPage();
+  const { trigger: mutateCurrentPage } = useSWRMUTxCurrentPage();
+  const { open: openPageStatusAlert, close: closePageStatusAlert } = usePageStatusAlert();
 
-  const setLatestRemotePageData = useCallback((data) => {
+  const remotePageDataUpdateHandler = useCallback((data) => {
+    // Set remote page data
     const { s2cMessagePageUpdated } = data;
 
     const remoteData: RemoteRevisionData = {
@@ -23,22 +30,36 @@ export const usePageUpdatedEffect = (): void => {
       remoteRevisionLastUpdatedAt: s2cMessagePageUpdated.revisionUpdateAt,
     };
 
-    if (currentPageId != null && currentPageId === s2cMessagePageUpdated.pageId) {
+    if (currentPage?._id != null && currentPage._id === s2cMessagePageUpdated.pageId) {
       setRemoteLatestPageData(remoteData);
-    }
 
-  }, [currentPageId, setRemoteLatestPageData]);
+      // Open page status alert if revision is outdated
+      const currentRevisionId = currentPage?.revision?._id;
+      const remoteRevisionId = s2cMessagePageUpdated.revisionId;
+      const isRevisionOutdated = (currentRevisionId != null || remoteRevisionId != null) && currentRevisionId !== remoteRevisionId;
+
+      // !!CAUTION!! Timing of calling openPageStatusAlert may clash with components/PageEditor.tsx
+      if (isRevisionOutdated && editorMode === EditorMode.View) {
+        openPageStatusAlert({ hideEditorMode: EditorMode.Editor, onRefleshPage: mutateCurrentPage });
+      }
+
+      // Clear cache
+      if (!isRevisionOutdated) {
+        closePageStatusAlert();
+      }
+    }
+  }, [currentPage?._id, currentPage?.revision?._id, editorMode, mutateCurrentPage, openPageStatusAlert, closePageStatusAlert, setRemoteLatestPageData]);
 
   // listen socket for someone updating this page
   useEffect(() => {
 
     if (socket == null) { return }
 
-    socket.on(SocketEventName.PageUpdated, setLatestRemotePageData);
+    socket.on(SocketEventName.PageUpdated, remotePageDataUpdateHandler);
 
     return () => {
-      socket.off(SocketEventName.PageUpdated, setLatestRemotePageData);
+      socket.off(SocketEventName.PageUpdated, remotePageDataUpdateHandler);
     };
 
-  }, [setLatestRemotePageData, socket]);
+  }, [remotePageDataUpdateHandler, socket]);
 };

+ 17 - 10
apps/app/src/components/PageEditor/PageEditor.tsx

@@ -124,7 +124,7 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
   const { data: defaultIndentSize } = useDefaultIndentSize();
   const { data: acceptedUploadFileType } = useAcceptedUploadFileType();
   const { open: openConflictDiffModal, close: closeConflictDiffModal } = useConflictDiffModal();
-  const { storeMethods: storeMethodsForPageStatusAlert, clearMethods: clearMethodsForPageStatusAlert } = usePageStatusAlert();
+  const { open: openPageStatusAlert, close: closePageStatusAlert } = usePageStatusAlert();
   const { data: editorSettings } = useEditorSettings();
   const { setRemoteLatestPageData } = useSetRemoteLatestPageData();
   const { data: user } = useCurrentUser();
@@ -183,13 +183,13 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
   const onConflictHandlerEffect = useCallback(() => {
     const resolveConflictHandler = (newMarkdown: string) => {
       codeMirrorEditor?.initDoc(newMarkdown);
-      clearMethodsForPageStatusAlert();
       closeConflictDiffModal();
+      closePageStatusAlert();
     };
 
     const markdown = codeMirrorEditor?.getDoc();
     openConflictDiffModal(markdown ?? '', resolveConflictHandler);
-  }, [clearMethodsForPageStatusAlert, closeConflictDiffModal, codeMirrorEditor, openConflictDiffModal]);
+  }, [closePageStatusAlert, closeConflictDiffModal, codeMirrorEditor, openConflictDiffModal]);
 
   useEffect(() => {
     const updateRemotePageDataHandler = (data) => {
@@ -198,8 +198,14 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
       const remoteRevisionOrigin = s2cMessagePageUpdated.revisionOrigin;
       const isRevisionOutdated = currentRevisionId !== remoteRevisionId;
 
-      if (isRevisionOutdated && (remoteRevisionOrigin === Origin.View || remoteRevisionOrigin === undefined)) {
-        storeMethodsForPageStatusAlert({ onResolveConflict: onConflictHandlerEffect });
+      // !!CAUTION!! Timing of calling openPageStatusAlert may clash with client/services/side-effects/page-updated.ts
+      if (isRevisionOutdated && editorMode === EditorMode.Editor && (remoteRevisionOrigin === Origin.View || remoteRevisionOrigin === undefined)) {
+        openPageStatusAlert({ onResolveConflict: onConflictHandlerEffect });
+      }
+
+      // Clear cache
+      if (!isRevisionOutdated) {
+        closePageStatusAlert();
       }
     };
 
@@ -211,7 +217,7 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
       socket.off(SocketEventName.PageUpdated, updateRemotePageDataHandler);
     };
 
-  }, [currentRevisionId, onConflictHandlerEffect, socket, storeMethodsForPageStatusAlert]);
+  }, [closePageStatusAlert, currentRevisionId, editorMode, onConflictHandlerEffect, openPageStatusAlert, socket]);
 
 
   const save: Save = useCallback(async(revisionId, markdown, opts, onConflict) => {
@@ -270,12 +276,13 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
       // Reflect conflict resolution results in CodeMirrorEditor
       codeMirrorEditor?.initDoc(newMarkdown);
 
-      clearMethodsForPageStatusAlert();
+      closePageStatusAlert();
       closeConflictDiffModal();
+
       toastSuccess(t('toaster.save_succeeded'));
       updateStateAfterSave?.();
     };
-  }, [save, codeMirrorEditor, clearMethodsForPageStatusAlert, closeConflictDiffModal, t, updateStateAfterSave]);
+  }, [save, codeMirrorEditor, closePageStatusAlert, closeConflictDiffModal, t, updateStateAfterSave]);
 
   const onConflictHandler: ConflictHandler = useCallback((remoteRevidsionData, newMarkdown, saveOptions) => {
     setRemoteLatestPageData(remoteRevidsionData);
@@ -286,8 +293,8 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
       openConflictDiffModal(newMarkdown, resolveConflictHandler);
     };
 
-    storeMethodsForPageStatusAlert({ onResolveConflict: conflictHandler });
-  }, [setRemoteLatestPageData, generateResolveConflictHandler, storeMethodsForPageStatusAlert, openConflictDiffModal]);
+    openPageStatusAlert({ onResolveConflict: conflictHandler });
+  }, [setRemoteLatestPageData, generateResolveConflictHandler, openPageStatusAlert, openConflictDiffModal]);
 
   const saveAndReturnToViewHandler = useCallback(async(opts: SaveOptions) => {
     const markdown = codeMirrorEditor?.getDoc();

+ 30 - 56
apps/app/src/components/PageStatusAlert.tsx

@@ -1,13 +1,12 @@
-import React, { useCallback, useMemo } from 'react';
+import React, { useCallback } from 'react';
 
 import { useTranslation } from 'next-i18next';
 
 import { usePageStatusAlert } from '~/stores/alert';
 import { useIsGuestUser, useIsReadOnlyUser } from '~/stores/context';
-import { useEditingMarkdown } from '~/stores/editor';
-import { useSWRMUTxCurrentPage, useSWRxCurrentPage } from '~/stores/page';
+import { useSWRxCurrentPage } from '~/stores/page';
 import { useRemoteRevisionId, useRemoteRevisionLastUpdateUser } from '~/stores/remote-latest-page';
-import { EditorMode, useEditorMode } from '~/stores/ui';
+import { useEditorMode } from '~/stores/ui';
 
 import { Username } from './User/Username';
 
@@ -15,83 +14,58 @@ import styles from './PageStatusAlert.module.scss';
 
 export const PageStatusAlert = (): JSX.Element => {
   const { t } = useTranslation();
-  const { mutate: mutateEditingMarkdown } = useEditingMarkdown();
+
+  const { data: editorMode } = useEditorMode();
   const { data: isGuestUser } = useIsGuestUser();
   const { data: isReadOnlyUser } = useIsReadOnlyUser();
   const { data: pageStatusAlertData } = usePageStatusAlert();
-  const { data: editorMode } = useEditorMode();
   const { data: remoteRevisionId } = useRemoteRevisionId();
   const { data: remoteRevisionLastUpdateUser } = useRemoteRevisionLastUpdateUser();
   const { data: pageData } = useSWRxCurrentPage();
-  const { trigger: mutatePageData } = useSWRMUTxCurrentPage();
 
   const onClickRefreshPage = useCallback(async() => {
-    const updatedPageData = await mutatePageData();
-    mutateEditingMarkdown(updatedPageData?.revision?.body);
-  }, [mutateEditingMarkdown, mutatePageData]);
+    pageStatusAlertData?.onRefleshPage?.();
+  }, [pageStatusAlertData]);
 
   const onClickResolveConflict = useCallback(() => {
     pageStatusAlertData?.onResolveConflict?.();
   }, [pageStatusAlertData]);
 
-  const alertContentsForView = useMemo(() => {
-    return (
-      <>
-        <p className="card-text grw-card-label-container">
-          <Username user={remoteRevisionLastUpdateUser} /> {t('edited this page')}
-        </p>
-        <p className="card-text grw-card-btn-container">
-          <button
-            type="button"
-            onClick={() => onClickRefreshPage()}
-            className="btn btn-outline-white me-4"
-          >
-            <span className="material-symbols-outlined">refresh</span>
-            {t('Load latest')}
-          </button>
-        </p>
-      </>
-    );
-  }, [onClickRefreshPage, remoteRevisionLastUpdateUser, t]);
-
-  const alertContentsForEditor = useMemo(() => {
-    return (
-      <>
-        <p className="card-text grw-card-label-container">
-          {t('modal_resolve_conflict.file_conflicting_with_newer_remote')}
-        </p>
-        <p className="card-text grw-card-btn-container">
-          <button
-            type="button"
-            onClick={onClickResolveConflict}
-            className="btn btn-outline-white"
-          >
-            <span className="material-symbols-outlined">description</span>
-            {t('modal_resolve_conflict.resolve_conflict')}
-          </button>
-        </p>
-      </>
-    );
-  }, [onClickResolveConflict, t]);
+  const hasResolveConflictHandler = pageStatusAlertData?.onResolveConflict != null;
+  const hasRefreshPageHandler = pageStatusAlertData?.onRefleshPage != null;
 
   const currentRevisionId = pageData?.revision?._id;
   const isRevisionOutdated = (currentRevisionId != null || remoteRevisionId != null) && currentRevisionId !== remoteRevisionId;
-  if (!!isGuestUser || !!isReadOnlyUser || !isRevisionOutdated) {
+
+  if (!pageStatusAlertData?.isOpen || !!isGuestUser || !!isReadOnlyUser || !isRevisionOutdated) {
     return <></>;
   }
 
-  const hasConflictHandler = pageStatusAlertData?.onResolveConflict != null;
-  if (!hasConflictHandler && editorMode === EditorMode.Editor) {
+  if (editorMode === pageStatusAlertData?.hideEditorMode) {
     return <></>;
   }
 
   return (
     <div className={`${styles['grw-page-status-alert']} card text-white fixed-bottom animated fadeInUp faster bg-warning text-dark`}>
       <div className="card-body">
-        { editorMode === EditorMode.View
-          ? alertContentsForView
-          : alertContentsForEditor
-        }
+        <p className="card-text grw-card-label-container">
+          { hasResolveConflictHandler
+            ? <> {t('modal_resolve_conflict.file_conflicting_with_newer_remote')}</>
+            : <><Username user={remoteRevisionLastUpdateUser} /> {t('edited this page')}</>
+          }
+        </p>
+        <p className="card-text grw-card-btn-container">
+          {hasRefreshPageHandler && (
+            <button type="button" onClick={onClickRefreshPage} className="btn btn-outline-white">
+              <span className="material-symbols-outlined">refresh</span>{t('Load latest')}
+            </button>
+          )}
+          {hasResolveConflictHandler && (
+            <button type="button" onClick={onClickResolveConflict} className="btn btn-outline-white">
+              <span className="material-symbols-outlined">description</span>{t('modal_resolve_conflict.resolve_conflict')}
+            </button>
+          )}
+        </p>
       </div>
     </div>
   );

+ 22 - 10
apps/app/src/stores/alert.tsx

@@ -1,27 +1,39 @@
 import { useSWRStatic } from '@growi/core/dist/swr';
 import type { SWRResponse } from 'swr';
 
+import type { EditorMode } from './ui';
+
 /*
 * PageStatusAlert
 */
-type PageStatusAlertMethods = {
-  onResolveConflict?: () => void,
+type OpenPageStatusAlertOptions = {
+  hideEditorMode?: EditorMode
+  onRefleshPage?: () => void
+  onResolveConflict?: () => void
+}
+
+type PageStatusAlertStatus = {
+  isOpen: boolean
+  hideEditorMode?: EditorMode,
+  onRefleshPage?: () => void
+  onResolveConflict?: () => void
 }
 
 type PageStatusAlertUtils = {
-  storeMethods: (methods: PageStatusAlertMethods) => void,
-  clearMethods: () => void,
+  open: (openPageStatusAlert: OpenPageStatusAlertOptions) => void,
+  close: () => void,
 }
-export const usePageStatusAlert = (): SWRResponse<PageStatusAlertMethods, Error> & PageStatusAlertUtils => {
-  const swrResponse = useSWRStatic<PageStatusAlertMethods, Error>('pageStatusAlert', undefined);
+export const usePageStatusAlert = (): SWRResponse<PageStatusAlertStatus, Error> & PageStatusAlertUtils => {
+  const initialData: PageStatusAlertStatus = { isOpen: false };
+  const swrResponse = useSWRStatic<PageStatusAlertStatus, Error>('pageStatusAlert', undefined, { fallbackData: initialData });
 
   return {
     ...swrResponse,
-    storeMethods(methods) {
-      swrResponse.mutate(methods);
+    open({ ...options }) {
+      swrResponse.mutate({ isOpen: true, ...options });
     },
-    clearMethods() {
-      swrResponse.mutate({});
+    close() {
+      swrResponse.mutate({ isOpen: false });
     },
   };
 };