Explorar o código

fix(editor): preserve existing grant when grant is unresolved on save

The selected-grant atom now defaults to null instead of GRANT_PUBLIC, so an
early save — before the page's grant has loaded, or on mobile where
GrantSelector never mounts — no longer overwrites a restricted (or
inherited-restricted) page with GRANT_PUBLIC.

save() omits the grant when none is selected (toPageUpdateGrantParams), letting
the update endpoint preserve the page's existing grant. GrantSelector shows a
loading state while the grant is unresolved instead of defaulting its toggle to
"Public".

Refs #11272

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Yuki Takei hai 3 días
pai
achega
2831ed69d0

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

+ 21 - 0
apps/app/src/client/components/PageEditor/EditorNavbarBottom/GrantSelector.tsx

@@ -141,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;
 

+ 6 - 7
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 ?? {}),
         });
 

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

@@ -3,7 +3,7 @@ import { GroupType, PageGrant } from '@growi/core';
 import type { IPageGrantData } from '~/interfaces/page';
 import { UserGroupPageGrantStatus } from '~/interfaces/page';
 
-import { toSelectedGrant } from './selected-grant';
+import { toPageUpdateGrantParams, toSelectedGrant } from './selected-grant';
 
 describe('toSelectedGrant', () => {
   it('maps the grant of the current page', () => {
@@ -53,3 +53,30 @@ describe('toSelectedGrant', () => {
     ]);
   });
 });
+
+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,
+    });
+  });
+});

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

@@ -1,16 +1,23 @@
-import { PageGrant } from '@growi/core/dist/interfaces';
 import { atom, useAtom } from 'jotai';
 
-import type { IPageGrantData, 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
@@ -38,3 +45,18 @@ export const toSelectedGrant = (
     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,
+});