Merge branch 'fix/11272-sync-selected-grant' into fix/11272-mobile-grant-selector-initialization

apps/app/src/client/components/PageEditor/EditorNavbarBottom/GrantSelector.spec.tsx

@@ -0,0 +1,54 @@
+import type { ReactNode } from 'react';
+import { PageGrant } from '@growi/core';
+import { act, render, renderHook } from '@testing-library/react';
+import { createStore, Provider } from 'jotai';
+
+import type { IPageSelectedGrant } from '~/interfaces/page';
+import { useSelectedGrant } from '~/states/ui/editor';
+
+import { GrantSelector } from './GrantSelector';
+
+vi.mock('next-i18next', () => ({
+  useTranslation: () => ({ t: (key: string) => key }),
+}));
+vi.mock('~/states/global', () => ({ useCurrentUser: vi.fn(() => undefined) }));
+vi.mock('~/states/page', () => ({ useCurrentPageId: vi.fn(() => 'page1') }));
+vi.mock('~/stores/page', () => ({
+  useSWRxCurrentGrantData: vi.fn(() => ({ data: undefined })),
+}));
+
+const renderGrantSelector = (
+  seed?: (set: (grant: IPageSelectedGrant | null) => void) => void,
+) => {
+  const store = createStore();
+  const wrapper = ({ children }: { children: ReactNode }) => (
+    <Provider store={store}>{children}</Provider>
+  );
+
+  if (seed != null) {
+    const { result } = renderHook(() => useSelectedGrant(), { wrapper });
+    act(() => seed(result.current[1]));
+  }
+
+  return render(<GrantSelector />, { wrapper });
+};
+
+describe('GrantSelector', () => {
+  // Before the current page's grant is loaded, selectedGrant is null. Showing the
+  // default "Public" option would mislead the user; show a loading state instead.
+  // See: https://github.com/growilabs/growi/issues/11272
+  it('shows a loading state while the grant is not yet resolved (null)', () => {
+    const { queryByTestId } = renderGrantSelector();
+
+    expect(queryByTestId('grw-grant-selector-loading')).not.toBeNull();
+  });
+
+  it('shows the grant selector once the grant is available', () => {
+    const { queryByTestId } = renderGrantSelector((set) =>
+      set({ grant: PageGrant.GRANT_OWNER }),
+    );
+
+    expect(queryByTestId('grw-grant-selector-loading')).toBeNull();
+    expect(queryByTestId('grw-grant-selector-dropdown-menu')).not.toBeNull();
+  });
+});

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);
@@ -159,6 +141,27 @@ export const GrantSelector = (props: Props): JSX.Element => {
    * Render grant selector DOM.
    */
   const renderGrantSelector = useCallback(() => {
+    // Until the current page grant is loaded, selectedGrant is null. Show a loading
+    // state instead of defaulting the toggle to "Public", which would mislead the
+    // user about the page's actual visibility. See issue #11272.
+    if (selectedGrant == null) {
+      return (
+        <div
+          className="grw-grant-selector mb-0"
+          data-testid="grw-grant-selector"
+        >
+          <button
+            type="button"
+            className="btn btn-outline-secondary btn-sm w-100 d-flex justify-content-center align-items-center"
+            disabled
+            data-testid="grw-grant-selector-loading"
+          >
+            <LoadingSpinner />
+          </button>
+        </div>
+      );
+    }
+
     let dropdownToggleBtnColor: string | undefined;
     let dropdownToggleLabelElm: ReactNode | undefined;
 

apps/app/src/client/components/PageEditor/EditorNavbarBottom/SavePageControls.tsx

@@ -34,6 +34,7 @@ import {
   useEditorMode,
   useIsSlackEnabled,
   useSelectedGrant,
+  useSyncSelectedGrantWithCurrentPage,
   useWaitingSaveProcessing,
 } from '~/states/ui/editor';
 import { useSWRxSlackChannels } from '~/stores/editor';
@@ -222,6 +223,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(() => {

apps/app/src/client/components/PageEditor/PageEditor.tsx

@@ -47,6 +47,7 @@ import {
 } from '~/states/server-configurations';
 import {
   EditorMode,
+  toPageUpdateGrantParams,
   useCurrentIndentSize,
   useCurrentIndentSizeActions,
   useEditingMarkdown,
@@ -223,11 +224,8 @@ export const PageEditorSubstance = (props: Props): JSX.Element => {
 
   const save: Save = useCallback(
     async (revisionId, markdown, opts, onConflict) => {
-      if (pageId == null || selectedGrant == null) {
-        logger.error(
-          { pageId, selectedGrant },
-          'Some materials to save are invalid',
-        );
+      if (pageId == null) {
+        logger.error({ pageId }, 'Some materials to save are invalid');
         throw new Error('Some materials to save are invalid');
       }
 
@@ -239,9 +237,10 @@ export const PageEditorSubstance = (props: Props): JSX.Element => {
           revisionId,
           wip: opts?.wip,
           body: markdown ?? '',
-          grant: selectedGrant?.grant,
           origin: Origin.Editor,
-          userRelatedGrantUserGroupIds: selectedGrant?.userRelatedGrantedGroups,
+          // Omits grant when none is selected (null) so the server preserves the
+          // page's existing grant instead of overwriting it — see issue https://github.com/growilabs/growi/issues/11272.
+          ...toPageUpdateGrantParams(selectedGrant),
           ...(opts ?? {}),
         });
 

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';

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

@@ -0,0 +1,82 @@
+import { GroupType, PageGrant } from '@growi/core';
+
+import type { IPageGrantData } from '~/interfaces/page';
+import { UserGroupPageGrantStatus } from '~/interfaces/page';
+
+import { toPageUpdateGrantParams, 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 },
+    ]);
+  });
+});
+
+describe('toPageUpdateGrantParams', () => {
+  // When the grant has not been chosen/loaded (null), the update must omit grant
+  // so the server preserves the page's existing grant — see issue #11272.
+  it('omits grant fields when no grant is selected (null)', () => {
+    expect(toPageUpdateGrantParams(null)).toEqual({
+      grant: undefined,
+      userRelatedGrantUserGroupIds: undefined,
+    });
+  });
+
+  it('passes through the selected grant and granted groups', () => {
+    const userRelatedGrantedGroups = [
+      { item: 'group-1', type: GroupType.userGroup },
+    ];
+
+    expect(
+      toPageUpdateGrantParams({
+        grant: PageGrant.GRANT_USER_GROUP,
+        userRelatedGrantedGroups,
+      }),
+    ).toEqual({
+      grant: PageGrant.GRANT_USER_GROUP,
+      userRelatedGrantUserGroupIds: userRelatedGrantedGroups,
+    });
+  });
+});

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

@@ -1,18 +1,62 @@
-import { PageGrant } from '@growi/core/dist/interfaces';
 import { atom, useAtom } from 'jotai';
 
-import type { IPageSelectedGrant } from '~/interfaces/page';
+import type {
+  IOptionsForUpdate,
+  IPageGrantData,
+  IPageSelectedGrant,
+} from '~/interfaces/page';
+import { UserGroupPageGrantStatus } from '~/interfaces/page';
 
 /**
  * Atom for selected grant in page editor
- * Stores temporary grant selection before it's applied to the page
+ * Stores temporary grant selection before it's applied to the page.
+ *
+ * Defaults to null ("not yet loaded") — NOT GRANT_PUBLIC. The real grant is the
+ * page's current grant (which, for a newly created page, is inherited from the
+ * closest ancestor), supplied asynchronously by useSyncSelectedGrantWithCurrentPage.
+ * A GRANT_PUBLIC default would let an early save publish a restricted page before
+ * that value arrives — see [[use-sync-selected-grant]] and issue #11272.
  */
-const selectedGrantAtom = atom<IPageSelectedGrant | null>({
-  grant: PageGrant.GRANT_PUBLIC,
-});
+const selectedGrantAtom = atom<IPageSelectedGrant | null>(null);
 
 /**
  * Hook for managing selected grant in page editor
  * 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,
+  };
+};
+
+/**
+ * Build the grant-related params for a page update from the selected grant.
+ *
+ * When nothing is selected (null) — e.g. the grant has not loaded yet, or
+ * GrantSelector never mounted on mobile — both fields are omitted (undefined),
+ * so the update endpoint preserves the page's existing grant rather than
+ * overwriting it with a stale default. See issue #11272.
+ */
+export const toPageUpdateGrantParams = (
+  selectedGrant: IPageSelectedGrant | null,
+): Pick<IOptionsForUpdate, 'grant' | 'userRelatedGrantUserGroupIds'> => ({
+  grant: selectedGrant?.grant,
+  userRelatedGrantUserGroupIds: selectedGrant?.userRelatedGrantedGroups,
+});

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 });
+  });
+});

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]);
+};