Ver Fonte

fix(editor): sync selectedGrant from current page in always-mounted control

On mobile, GrantSelector is rendered inside a closed Modal and never mounts,
so its effect that synced the page's grant into selectedGrantAtom never ran —
leaving the atom at its GRANT_PUBLIC default and silently re-publishing
owner/group-restricted pages on save.

Extract the mapping into a pure toSelectedGrant() and a shared
useSyncSelectedGrantWithCurrentPage() hook, call it once from the
always-mounted SavePageControls, and drop GrantSelector's duplicate init
effect (its reselect path now reuses toSelectedGrant). This keeps a single
sync point and removes drift between the two copies.

Refs #11272

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Yuki Takei há 3 dias atrás
pai
commit
00400513cf

+ 9 - 27
apps/app/src/client/components/PageEditor/EditorNavbarBottom/GrantSelector.tsx

@@ -1,10 +1,4 @@
-import React, {
-  type JSX,
-  type ReactNode,
-  useCallback,
-  useEffect,
-  useState,
-} from 'react';
+import React, { type JSX, type ReactNode, useCallback, useState } from 'react';
 import { GroupType, getIdForRef, PageGrant } from '@growi/core';
 import { LoadingSpinner } from '@growi/ui/dist/components';
 import { useTranslation } from 'next-i18next';
@@ -22,7 +16,7 @@ import type { UserRelatedGroupsData } from '~/interfaces/page';
 import { UserGroupPageGrantStatus } from '~/interfaces/page';
 import { useCurrentUser } from '~/states/global';
 import { useCurrentPageId } from '~/states/page';
-import { useSelectedGrant } from '~/states/ui/editor';
+import { toSelectedGrant, useSelectedGrant } from '~/states/ui/editor';
 import { useSWRxCurrentGrantData } from '~/stores/page';
 
 const AVAILABLE_GRANTS = [
@@ -79,26 +73,14 @@ export const GrantSelector = (props: Props): JSX.Element => {
   const currentPageGrantData = grantData?.grantData.currentPageGrant;
   const groupGrantData = currentPageGrantData?.groupGrantData;
 
+  // Re-apply the current page grant when the user (re)opens the group selection,
+  // so the modal reflects the groups currently granted to the page.
+  // Initial sync of selectedGrantAtom is owned by useSyncSelectedGrantWithCurrentPage
+  // (called from the always-mounted SavePageControls) — see issue #11272.
   const applyCurrentPageGrantToSelectedGrant = useCallback(() => {
-    const currentPageGrant = grantData?.grantData.currentPageGrant;
-    if (currentPageGrant == null) return;
-
-    const userRelatedGrantedGroups =
-      currentPageGrant.groupGrantData?.userRelatedGroups
-        .filter((group) => group.status === UserGroupPageGrantStatus.isGranted)
-        ?.map((group) => {
-          return { item: group.id, type: group.type };
-        }) ?? [];
-    setSelectedGrant({
-      grant: currentPageGrant.grant,
-      userRelatedGrantedGroups,
-    });
-  }, [grantData?.grantData.currentPageGrant, setSelectedGrant]);
-
-  // sync grant data
-  useEffect(() => {
-    applyCurrentPageGrantToSelectedGrant();
-  }, [applyCurrentPageGrantToSelectedGrant]);
+    if (currentPageGrantData == null) return;
+    setSelectedGrant(toSelectedGrant(currentPageGrantData));
+  }, [currentPageGrantData, setSelectedGrant]);
 
   const showSelectGroupModal = useCallback(() => {
     setIsSelectGroupModalShown(true);

+ 7 - 0
apps/app/src/client/components/PageEditor/EditorNavbarBottom/SavePageControls.tsx

@@ -32,6 +32,7 @@ import {
   useEditorMode,
   useIsSlackEnabled,
   useSelectedGrant,
+  useSyncSelectedGrantWithCurrentPage,
   useWaitingSaveProcessing,
 } from '~/states/ui/editor';
 import { useSWRxSlackChannels } from '~/stores/editor';
@@ -219,6 +220,12 @@ export const SavePageControls = (): JSX.Element | null => {
   const [isSavePageControlsModalShown, setIsSavePageControlsModalShown] =
     useState<boolean>(false);
 
+  // Initialize selectedGrantAtom from the current page's grant here, because
+  // SavePageControls is always mounted while editing. GrantSelector — which used
+  // to own this — is rendered inside a closed Modal on mobile and never mounts.
+  // See: https://github.com/growilabs/growi/issues/11272
+  useSyncSelectedGrantWithCurrentPage();
+
   // DO NOT dependent on slackChannelsData directly: https://github.com/growilabs/growi/pull/7332
   const slackChannelsDataString = slackChannelsData?.toString();
   useEffect(() => {

+ 1 - 0
apps/app/src/states/ui/editor/index.ts

@@ -8,6 +8,7 @@ export * from './reserved-next-caret-line';
 export * from './selected-grant';
 export type { EditorMode as EditorModeType } from './types';
 export { EditorMode } from './types';
+export { useSyncSelectedGrantWithCurrentPage } from './use-sync-selected-grant';
 // Export utility functions that might be needed elsewhere
 export { determineEditorModeByHash } from './utils';
 export * from './waiting-save-processing';

+ 55 - 0
apps/app/src/states/ui/editor/selected-grant.spec.ts

@@ -0,0 +1,55 @@
+import { GroupType, PageGrant } from '@growi/core';
+
+import type { IPageGrantData } from '~/interfaces/page';
+import { UserGroupPageGrantStatus } from '~/interfaces/page';
+
+import { toSelectedGrant } from './selected-grant';
+
+describe('toSelectedGrant', () => {
+  it('maps the grant of the current page', () => {
+    const currentPageGrant: IPageGrantData = { grant: PageGrant.GRANT_OWNER };
+
+    expect(toSelectedGrant(currentPageGrant).grant).toBe(PageGrant.GRANT_OWNER);
+  });
+
+  it('returns an empty userRelatedGrantedGroups when groupGrantData is absent', () => {
+    const currentPageGrant: IPageGrantData = { grant: PageGrant.GRANT_PUBLIC };
+
+    expect(toSelectedGrant(currentPageGrant).userRelatedGrantedGroups).toEqual(
+      [],
+    );
+  });
+
+  it('includes only groups whose status is isGranted, mapped to { item, type }', () => {
+    const currentPageGrant: IPageGrantData = {
+      grant: PageGrant.GRANT_USER_GROUP,
+      groupGrantData: {
+        userRelatedGroups: [
+          {
+            id: 'granted-group',
+            name: 'granted',
+            type: GroupType.userGroup,
+            status: UserGroupPageGrantStatus.isGranted,
+          },
+          {
+            id: 'not-granted-group',
+            name: 'not granted',
+            type: GroupType.userGroup,
+            status: UserGroupPageGrantStatus.notGranted,
+          },
+          {
+            id: 'cannot-grant-group',
+            name: 'cannot grant',
+            type: GroupType.externalUserGroup,
+            status: UserGroupPageGrantStatus.cannotGrant,
+          },
+        ],
+        nonUserRelatedGrantedGroups: [],
+      },
+    };
+
+    expect(toSelectedGrant(currentPageGrant).userRelatedGrantedGroups).toEqual([
+      { item: 'granted-group', type: GroupType.userGroup },
+    ]);
+  });
+});

+ 23 - 1
apps/app/src/states/ui/editor/selected-grant.ts

@@ -1,7 +1,8 @@
 import { PageGrant } from '@growi/core/dist/interfaces';
 import { atom, useAtom } from 'jotai';
 
-import type { IPageSelectedGrant } from '~/interfaces/page';
+import type { IPageGrantData, IPageSelectedGrant } from '~/interfaces/page';
+import { UserGroupPageGrantStatus } from '~/interfaces/page';
 
 /**
  * Atom for selected grant in page editor
@@ -16,3 +17,24 @@ const selectedGrantAtom = atom<IPageSelectedGrant | null>({
  * Used for temporary grant selection before applying to the page
  */
 export const useSelectedGrant = () => useAtom(selectedGrantAtom);
+
+/**
+ * Convert the page's current grant data (server-side shape) into the
+ * IPageSelectedGrant shape held by the editor's selected-grant state.
+ *
+ * Pure function so it can be reused from both the sync hook
+ * ([[use-sync-selected-grant]]) and GrantSelector's change handler.
+ */
+export const toSelectedGrant = (
+  currentPageGrant: IPageGrantData,
+): IPageSelectedGrant => {
+  const userRelatedGrantedGroups =
+    currentPageGrant.groupGrantData?.userRelatedGroups
+      .filter((group) => group.status === UserGroupPageGrantStatus.isGranted)
+      .map((group) => ({ item: group.id, type: group.type })) ?? [];
+
+  return {
+    grant: currentPageGrant.grant,
+    userRelatedGrantedGroups,
+  };
+};

+ 90 - 0
apps/app/src/states/ui/editor/use-sync-selected-grant.spec.tsx

@@ -0,0 +1,90 @@
+import { PageGrant } from '@growi/core';
+import { act, renderHook } from '@testing-library/react';
+import { createStore, Provider } from 'jotai';
+
+import { useCurrentPageId } from '~/states/page';
+import { useSWRxCurrentGrantData } from '~/stores/page';
+
+import { useSelectedGrant } from './selected-grant';
+import { useSyncSelectedGrantWithCurrentPage } from './use-sync-selected-grant';
+
+vi.mock('~/states/page', () => ({ useCurrentPageId: vi.fn() }));
+vi.mock('~/stores/page', () => ({ useSWRxCurrentGrantData: vi.fn() }));
+
+const mockedUseCurrentPageId = vi.mocked(useCurrentPageId);
+const mockedUseSWRxCurrentGrantData = vi.mocked(useSWRxCurrentGrantData);
+
+// Build a plain SWR response. vitest-mock-extended's mock<SWRResponse>() cannot be
+// used here: its deep proxy auto-stubs `.then`, so React treats `data` as a thenable
+// and breaks rendering. A plain object with a single localized cast is the repo norm
+// (see states/page/use-fetch-current-page.spec.tsx).
+const grantDataResponse = (currentPageGrant?: {
+  grant: PageGrant;
+}): ReturnType<typeof useSWRxCurrentGrantData> =>
+  ({
+    data:
+      currentPageGrant == null
+        ? undefined
+        : {
+            isGrantNormalized: true,
+            grantData: { isForbidden: false, currentPageGrant },
+          },
+    error: undefined,
+    isLoading: false,
+    isValidating: false,
+    mutate: vi.fn(),
+  }) as ReturnType<typeof useSWRxCurrentGrantData>;
+
+describe('useSyncSelectedGrantWithCurrentPage', () => {
+  let store: ReturnType<typeof createStore>;
+
+  // Render the consumer's view (useSelectedGrant) alongside the sync hook so we
+  // assert on the observable atom value, not on the setter being called.
+  const renderSyncHook = () =>
+    renderHook(
+      () => {
+        const selected = useSelectedGrant();
+        useSyncSelectedGrantWithCurrentPage();
+        return selected;
+      },
+      {
+        wrapper: ({ children }) => (
+          <Provider store={store}>{children}</Provider>
+        ),
+      },
+    );
+
+  beforeEach(() => {
+    store = createStore();
+    mockedUseCurrentPageId.mockReturnValue('page1');
+  });
+
+  it("initializes selectedGrant from the current page's grant", () => {
+    mockedUseSWRxCurrentGrantData.mockReturnValue(
+      grantDataResponse({ grant: PageGrant.GRANT_OWNER }),
+    );
+
+    // renderHook flushes mount effects in its internal act(), so the sync has
+    // already applied by the time it returns.
+    const { result } = renderSyncHook();
+
+    expect(result.current[0]).toEqual({
+      grant: PageGrant.GRANT_OWNER,
+      userRelatedGrantedGroups: [],
+    });
+  });
+
+  it('does not overwrite an existing selection while grant data is unavailable', () => {
+    mockedUseSWRxCurrentGrantData.mockReturnValue(grantDataResponse());
+
+    const { result } = renderSyncHook();
+
+    act(() => {
+      result.current[1]({ grant: PageGrant.GRANT_RESTRICTED });
+    });
+
+    // The sync effect re-runs on the update but must leave the selection intact
+    // because there is no grant data to apply yet.
+    expect(result.current[0]).toEqual({ grant: PageGrant.GRANT_RESTRICTED });
+  });
+});

+ 32 - 0
apps/app/src/states/ui/editor/use-sync-selected-grant.ts

@@ -0,0 +1,32 @@
+import { useEffect } from 'react';
+
+import { useCurrentPageId } from '~/states/page';
+import { useSWRxCurrentGrantData } from '~/stores/page';
+
+import { toSelectedGrant, useSelectedGrant } from './selected-grant';
+
+/**
+ * Sync selectedGrantAtom with the current page's grant.
+ *
+ * The atom defaults to GRANT_PUBLIC and only becomes meaningful once it has been
+ * initialized from the current page's actual grant. This initialization must run
+ * from an always-mounted component: on mobile, GrantSelector is rendered only
+ * inside a closed Modal and therefore never mounts, which previously left the atom
+ * at GRANT_PUBLIC and silently re-published owner/group-restricted pages on save.
+ *
+ * Call this once from an always-mounted editor component (e.g. SavePageControls).
+ *
+ * @see https://github.com/growilabs/growi/issues/11272
+ */
+export const useSyncSelectedGrantWithCurrentPage = (): void => {
+  const currentPageId = useCurrentPageId();
+  const { data } = useSWRxCurrentGrantData(currentPageId);
+  const [, setSelectedGrant] = useSelectedGrant();
+
+  const currentPageGrant = data?.grantData.currentPageGrant;
+
+  useEffect(() => {
+    if (currentPageGrant == null) return;
+    setSelectedGrant(toSelectedGrant(currentPageGrant));
+  }, [currentPageGrant, setSelectedGrant]);
+};