Kaynağa Gözat

omit remoteRevisionIdAtom and refactor useIsRevisionOutdated

Yuki Takei 5 ay önce
ebeveyn
işleme
e3b69ebf29

+ 2 - 3
apps/app/src/client/components/PageEditor/ConflictDiffModal/ConflictDiffModal.tsx

@@ -14,11 +14,11 @@ import {
   Modal, ModalHeader, ModalBody, ModalFooter,
 } from 'reactstrap';
 
+
 import { useCurrentUser } from '~/states/global';
 import {
   useCurrentPageData,
   useRemoteRevisionBody,
-  useRemoteRevisionId,
   useRemoteRevisionLastUpdatedAt,
   useRemoteRevisionLastUpdateUser,
 } from '~/states/page';
@@ -206,12 +206,11 @@ export const ConflictDiffModal = (): React.JSX.Element => {
   const conflictDiffModalStatus = useConflictDiffModalStatus();
 
   // state for latest page
-  const remoteRevisionId = useRemoteRevisionId();
   const remoteRevisionBody = useRemoteRevisionBody();
   const remoteRevisionLastUpdateUser = useRemoteRevisionLastUpdateUser();
   const remoteRevisionLastUpdatedAt = useRemoteRevisionLastUpdatedAt();
 
-  const isRemotePageDataInappropriate = remoteRevisionId == null || remoteRevisionBody == null || remoteRevisionLastUpdateUser == null;
+  const isRemotePageDataInappropriate = remoteRevisionBody == null || remoteRevisionLastUpdateUser == null;
 
   const [isModalExpanded, setIsModalExpanded] = useState<boolean>(false);
 

+ 3 - 6
apps/app/src/client/components/PageStatusAlert.tsx

@@ -3,9 +3,10 @@ import React, { useCallback, type JSX } from 'react';
 import { useTranslation } from 'next-i18next';
 
 import { useIsGuestUser, useIsReadOnlyUser } from '~/states/context';
-import { useCurrentPageData, useRemoteRevisionId, useRemoteRevisionLastUpdateUser } from '~/states/page';
+import { useRemoteRevisionLastUpdateUser } from '~/states/page';
 import { useEditorMode } from '~/states/ui/editor';
 import { usePageStatusAlertStatus } from '~/states/ui/modal/page-status-alert';
+import { useIsRevisionOutdated } from '~/stores/page';
 
 import { Username } from '../../components/User/Username';
 
@@ -18,9 +19,8 @@ export const PageStatusAlert = (): JSX.Element => {
   const isGuestUser = useIsGuestUser();
   const isReadOnlyUser = useIsReadOnlyUser();
   const pageStatusAlertData = usePageStatusAlertStatus();
-  const remoteRevisionId = useRemoteRevisionId();
+  const isRevisionOutdated = useIsRevisionOutdated();
   const remoteRevisionLastUpdateUser = useRemoteRevisionLastUpdateUser();
-  const pageData = useCurrentPageData();
 
   const onClickRefreshPage = useCallback(() => {
     pageStatusAlertData?.onRefleshPage?.();
@@ -33,9 +33,6 @@ export const PageStatusAlert = (): JSX.Element => {
   const hasResolveConflictHandler = pageStatusAlertData?.onResolveConflict != null;
   const hasRefreshPageHandler = pageStatusAlertData?.onRefleshPage != null;
 
-  const currentRevisionId = pageData?.revision?._id;
-  const isRevisionOutdated = (currentRevisionId != null || remoteRevisionId != null) && currentRevisionId !== remoteRevisionId;
-
   if (!pageStatusAlertData?.isOpen || !!isGuestUser || !!isReadOnlyUser || !isRevisionOutdated) {
     return <></>;
   }

+ 1 - 1
apps/app/src/client/components/ReactMarkdownComponents/DrawioViewerWithEditButton.tsx

@@ -11,8 +11,8 @@ import { useTranslation } from 'next-i18next';
 
 import { useCurrentPageYjsData } from '~/features/collaborative-editor/states';
 import { useIsGuestUser, useIsReadOnlyUser, useIsSharedUser } from '~/states/context';
-import { useIsRevisionOutdated } from '~/states/page';
 import { useShareLinkId } from '~/states/page/hooks';
+import { useIsRevisionOutdated } from '~/stores/page';
 
 import '@growi/remark-drawio/dist/style.css';
 import styles from './DrawioViewerWithEditButton.module.scss';

+ 1 - 1
apps/app/src/client/components/ReactMarkdownComponents/TableWithEditButton.tsx

@@ -6,8 +6,8 @@ import type { Element } from 'hast';
 
 import { useCurrentPageYjsData } from '~/features/collaborative-editor/states';
 import { useIsGuestUser, useIsReadOnlyUser, useIsSharedUser } from '~/states/context';
-import { useIsRevisionOutdated } from '~/states/page';
 import { useShareLinkId } from '~/states/page/hooks';
+import { useIsRevisionOutdated } from '~/stores/page';
 
 import styles from './TableWithEditButton.module.scss';
 

+ 21 - 0
apps/app/src/states/context.ts

@@ -1,3 +1,4 @@
+import { useRouter } from 'next/router';
 import { atom, useAtomValue, useSetAtom } from 'jotai';
 
 import { currentUserAtomGetter, growiCloudUriAtomGetter } from './global';
@@ -78,6 +79,26 @@ const growiDocumentationUrlAtom = atom((get) => {
 export const useGrowiDocumentationUrl = () =>
   useAtomValue(growiDocumentationUrlAtom);
 
+/**
+ * Hook to get revisionId from URL query parameters
+ * Returns undefined if revisionId is not present in the URL
+ */
+export const useRevisionIdFromUrl = (): string | undefined => {
+  const router = useRouter();
+  const revisionId = router.query.revisionId;
+  return typeof revisionId === 'string' ? revisionId : undefined;
+};
+
+/**
+ * Hook to check if user is intentionally viewing a specific (old) revision
+ * Returns true when URL has ?revisionId=xxx parameter
+ * This indicates the user explicitly wants to see that revision
+ */
+export const useIsViewingSpecificRevision = (): boolean => {
+  const revisionId = useRevisionIdFromUrl();
+  return revisionId != null;
+};
+
 /**
  * Internal atoms for derived atom usage (special naming convention)
  * These atoms are exposed only for creating derived atoms in other modules

+ 0 - 11
apps/app/src/states/page/hooks.ts

@@ -14,13 +14,11 @@ import {
   currentPagePathAtom,
   isForbiddenAtom,
   isIdenticalPathAtom,
-  isRevisionOutdatedAtom,
   isTrashPageAtom,
   isUntitledPageAtom,
   pageNotFoundAtom,
   redirectFromAtom,
   remoteRevisionBodyAtom,
-  remoteRevisionIdAtom,
   remoteRevisionLastUpdatedAtAtom,
   remoteRevisionLastUpdateUserAtom,
   shareLinkIdAtom,
@@ -51,8 +49,6 @@ export const useTemplateTags = () => useAtomValue(templateTagsAtom);
 export const useTemplateBody = () => useAtomValue(templateBodyAtom);
 
 // Remote revision hooks (replacements for stores/remote-latest-page.ts)
-export const useRemoteRevisionId = () => useAtomValue(remoteRevisionIdAtom);
-
 export const useRemoteRevisionBody = () => useAtomValue(remoteRevisionBodyAtom);
 
 export const useRemoteRevisionLastUpdateUser = () =>
@@ -88,13 +84,6 @@ export const useCurrentPagePath = (): string | undefined => {
  */
 export const useIsTrashPage = (): boolean => useAtomValue(isTrashPageAtom);
 
-/**
- * Check if current revision is outdated
- * Pure Jotai replacement for stores/page.tsx useIsRevisionOutdated
- */
-export const useIsRevisionOutdated = (): boolean =>
-  useAtomValue(isRevisionOutdatedAtom);
-
 /**
  * Computed hook for checking if current page is creatable
  */

+ 2 - 4
apps/app/src/states/page/hydrate.ts

@@ -14,7 +14,6 @@ import {
   pageNotFoundAtom,
   redirectFromAtom,
   remoteRevisionBodyAtom,
-  remoteRevisionIdAtom,
   shareLinkIdAtom,
   templateBodyAtom,
   templateTagsAtom,
@@ -29,7 +28,7 @@ import {
  *
  * Data sources:
  * - page._id, page.revision -> Auto-extracted from IPagePopulatedToShowRevision
- * - remoteRevisionId, remoteRevisionBody -> Auto-extracted from page.revision
+ * - remoteRevisionBody -> Auto-extracted from page.revision
  * - templateTags, templateBody -> Explicitly provided via options
  *
  * @example
@@ -68,8 +67,7 @@ export const useHydratePageAtoms = (
       isIPageNotFoundInfo(pageMeta) ? pageMeta.isForbidden : false,
     ],
 
-    // Remote revision data - auto-extracted from page.revision
-    [remoteRevisionIdAtom, page?.revision?._id],
+    // Remote revision data - used by ConflictDiffModal
     [remoteRevisionBodyAtom, page?.revision?.body],
   ]);
 

+ 2 - 18
apps/app/src/states/page/internal-atoms.ts

@@ -61,7 +61,6 @@ export const isUntitledPageAtom = atom(
 );
 
 // Remote revision data atoms
-export const remoteRevisionIdAtom = atom<string>();
 export const remoteRevisionBodyAtom = atom<string>();
 export const remoteRevisionLastUpdateUserAtom = atom<IUserHasId>();
 export const remoteRevisionLastUpdatedAtAtom = atom<Date>();
@@ -72,21 +71,10 @@ export const isTrashPageAtom = atom((get) => {
   return pagePath != null ? pagePathUtils.isTrashPage(pagePath) : false;
 });
 
-export const isRevisionOutdatedAtom = atom((get) => {
-  const currentRevisionId = get(currentRevisionIdAtom);
-  const remoteRevisionId = get(remoteRevisionIdAtom);
-
-  if (currentRevisionId == null || remoteRevisionId == null) {
-    return false;
-  }
-
-  return remoteRevisionId !== currentRevisionId;
-});
-
 // Update atoms for template and remote revision data
 export const setTemplateContentAtom = atom(
   null,
-  (get, set, data: { tags?: string[]; body?: string }) => {
+  (_get, set, data: { tags?: string[]; body?: string }) => {
     if (data.tags !== undefined) {
       set(templateTagsAtom, data.tags);
     }
@@ -99,18 +87,14 @@ export const setTemplateContentAtom = atom(
 export const setRemoteRevisionDataAtom = atom(
   null,
   (
-    get,
+    _get,
     set,
     data: {
-      id?: string;
       body?: string;
       lastUpdateUser?: IUserHasId;
       lastUpdatedAt?: Date;
     },
   ) => {
-    if (data.id !== undefined) {
-      set(remoteRevisionIdAtom, data.id);
-    }
     if (data.body !== undefined) {
       set(remoteRevisionBodyAtom, data.body);
     }

+ 0 - 3
apps/app/src/states/page/use-fetch-current-page.spec.tsx

@@ -25,7 +25,6 @@ import {
   pageLoadingAtom,
   pageNotFoundAtom,
   remoteRevisionBodyAtom,
-  remoteRevisionIdAtom,
 } from '~/states/page/internal-atoms';
 
 // Mock Next.js router
@@ -736,7 +735,6 @@ describe('useFetchCurrentPage - Integration Test', () => {
     );
     store.set(currentPageIdAtom, existingPage._id);
     store.set(currentPageDataAtom, existingPage);
-    store.set(remoteRevisionIdAtom, 'rev_xxx');
     store.set(remoteRevisionBodyAtom, 'remote body');
 
     // Mock API rejection with ErrorV3 like object
@@ -760,7 +758,6 @@ describe('useFetchCurrentPage - Integration Test', () => {
       });
       expect(store.get(currentPageDataAtom)).toBeUndefined();
       expect(store.get(currentPageIdAtom)).toBeUndefined();
-      expect(store.get(remoteRevisionIdAtom)).toBeUndefined();
       expect(store.get(remoteRevisionBodyAtom)).toBeUndefined();
     });
   });

+ 9 - 11
apps/app/src/states/page/use-fetch-current-page.ts

@@ -13,6 +13,7 @@ import { useAtomCallback } from 'jotai/utils';
 import { apiv3Get } from '~/client/util/apiv3-client';
 import loggerFactory from '~/utils/logger';
 
+import { useRevisionIdFromUrl } from '../context';
 import {
   currentPageDataAtom,
   currentPageIdAtom,
@@ -21,7 +22,6 @@ import {
   pageLoadingAtom,
   pageNotFoundAtom,
   remoteRevisionBodyAtom,
-  remoteRevisionIdAtom,
   shareLinkIdAtom,
 } from './internal-atoms';
 
@@ -105,6 +105,7 @@ type BuildApiParamsArgs = {
   decodedPathname: string | undefined;
   currentPageId: string | undefined;
   shareLinkId: string | undefined;
+  revisionIdFromUrl: string | undefined;
 };
 type ApiParams = { params: Record<string, string>; shouldSkip: boolean };
 
@@ -116,21 +117,17 @@ const buildApiParams = ({
   decodedPathname,
   currentPageId,
   shareLinkId,
+  revisionIdFromUrl,
 }: BuildApiParamsArgs): ApiParams => {
-  const revisionId =
-    fetchPageArgs?.revisionId ??
-    (isClient()
-      ? new URLSearchParams(window.location.search).get('revisionId')
-      : undefined);
+  // Priority: explicit arg > URL query parameter
+  const revisionId = fetchPageArgs?.revisionId ?? revisionIdFromUrl;
 
   const params: {
     path?: string;
     pageId?: string;
     revisionId?: string;
     shareLinkId?: string;
-  } = {
-    revisionId: fetchPageArgs?.revisionId,
-  };
+  } = {};
 
   if (shareLinkId != null) {
     params.shareLinkId = shareLinkId;
@@ -179,6 +176,7 @@ export const useFetchCurrentPage = (): {
   error: Error | null;
 } => {
   const shareLinkId = useAtomValue(shareLinkIdAtom);
+  const revisionIdFromUrl = useRevisionIdFromUrl();
 
   const isLoading = useAtomValue(pageLoadingAtom);
   const error = useAtomValue(pageErrorAtom);
@@ -217,6 +215,7 @@ export const useFetchCurrentPage = (): {
           decodedPathname,
           currentPageId,
           shareLinkId,
+          revisionIdFromUrl,
         });
 
         if (shouldSkip) {
@@ -252,7 +251,6 @@ export const useFetchCurrentPage = (): {
               set(isForbiddenAtom, error.args.isForbidden ?? false);
               set(currentPageDataAtom, undefined);
               set(currentPageIdAtom, undefined);
-              set(remoteRevisionIdAtom, undefined);
               set(remoteRevisionBodyAtom, undefined);
             }
           }
@@ -262,7 +260,7 @@ export const useFetchCurrentPage = (): {
 
         return null;
       },
-      [shareLinkId],
+      [shareLinkId, revisionIdFromUrl],
     ),
   );
 

+ 2 - 4
apps/app/src/states/page/use-set-remote-latest-page-data.ts

@@ -4,7 +4,6 @@ import { useSetAtom } from 'jotai/react';
 
 import {
   remoteRevisionBodyAtom,
-  remoteRevisionIdAtom,
   remoteRevisionLastUpdatedAtAtom,
   remoteRevisionLastUpdateUserAtom,
 } from './internal-atoms';
@@ -22,7 +21,6 @@ type SetRemoteLatestPageData = (pageData: RemoteRevisionData) => void;
  * Set remote data all at once
  */
 export const useSetRemoteLatestPageData = (): SetRemoteLatestPageData => {
-  const setRemoteRevisionId = useSetAtom(remoteRevisionIdAtom);
   const setRemoteRevisionBody = useSetAtom(remoteRevisionBodyAtom);
   const setRemoteRevisionLastUpdateUser = useSetAtom(
     remoteRevisionLastUpdateUserAtom,
@@ -33,7 +31,8 @@ export const useSetRemoteLatestPageData = (): SetRemoteLatestPageData => {
 
   return useCallback(
     (remoteRevisionData: RemoteRevisionData) => {
-      setRemoteRevisionId(remoteRevisionData.remoteRevisionId);
+      // Note: remoteRevisionId is part of the type for conflict resolution
+      // but not stored in atom (we use useSWRxPageInfo.data.latestRevisionId instead)
       setRemoteRevisionBody(remoteRevisionData.remoteRevisionBody);
       setRemoteRevisionLastUpdateUser(
         remoteRevisionData.remoteRevisionLastUpdateUser,
@@ -46,7 +45,6 @@ export const useSetRemoteLatestPageData = (): SetRemoteLatestPageData => {
       setRemoteRevisionLastUpdateUser,
       setRemoteRevisionLastUpdatedAt,
       setRemoteRevisionBody,
-      setRemoteRevisionId,
     ],
   );
 };

+ 29 - 1
apps/app/src/stores/page.tsx

@@ -26,7 +26,7 @@ import type {
   IRecordApplicableGrant,
   IResCurrentGrantData,
 } from '~/interfaces/page-grant';
-import { useIsGuestUser, useIsReadOnlyUser } from '~/states/context';
+import { useIsGuestUser, useIsReadOnlyUser, useIsViewingSpecificRevision } from '~/states/context';
 import { useCurrentPageData, usePageNotFound } from '~/states/page';
 import { useShareLinkId } from '~/states/page/hooks';
 
@@ -190,6 +190,34 @@ export const useIsLatestRevision = (): SWRResponse<boolean, Error> => {
   );
 };
 
+/**
+ * Check if current revision is outdated and user should be notified to refetch
+ *
+ * Returns true when:
+ * - User is NOT intentionally viewing a specific (old) revision (no ?revisionId in URL)
+ * - AND the current page data is not the latest revision
+ *
+ * This indicates "new data is available, please refetch" rather than
+ * "you are viewing an old revision" (which is handled by useIsLatestRevision)
+ */
+export const useIsRevisionOutdated = (): boolean => {
+  const { data: isLatestRevision } = useIsLatestRevision();
+  const isViewingSpecificRevision = useIsViewingSpecificRevision();
+
+  // If user intentionally views a specific revision, don't show "outdated" alert
+  if (isViewingSpecificRevision) {
+    return false;
+  }
+
+  // If we can't determine yet, assume not outdated
+  if (isLatestRevision == null) {
+    return false;
+  }
+
+  // User expects latest, but it's not latest = outdated
+  return !isLatestRevision;
+};
+
 export const useSWRxPageRevision = (
   pageId: string,
   revisionId: Ref<IRevision>,