Explorar el Código

test(editor): make grant pre-load e2e assert behavior; address review feedback

Rework the pre-load race e2e (#11272) from asserting the request shape
("PUT omits grant") to asserting the user-facing contract: create a GRANT_OWNER
page, block grant-data so the editor opens with selectedGrant unresolved, save
immediately, then read the grant back and assert it is still owner-only.
Verified locally against the dev server — green with the fix, and fails
(Expected 4, Received 1) when the atom default is reverted to GRANT_PUBLIC.

Also from the essential-test-design review:
- GrantSelector.spec: assert the selector dropdown is absent in the loading state.
- use-sync-selected-grant: fix the stale "defaults to GRANT_PUBLIC" doc (now null).

Refs #11272

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Yuki Takei hace 3 días
padre
commit
5741f56c58

+ 47 - 44
apps/app/playwright/23-editor/grant-preload-race.spec.ts

@@ -7,68 +7,71 @@ import { appendTextToEditorUntilContains } from '../utils/AppendTextToEditorUnti
  *
  * When the editor opens, the current page's grant is fetched asynchronously
  * (GET /_api/v3/page/grant-data) and synced into selectedGrantAtom. Until that
- * resolves, selectedGrant is null. Saving in that window must NOT send a grant,
- * so the update endpoint preserves the page's existing grant instead of
- * overwriting it with a default — otherwise a restricted (or inherited-restricted)
- * page would be silently published.
+ * resolves, selectedGrant is null. Saving in that window must NOT change the
+ * page's grant — otherwise a restricted page is silently published.
  *
- * Reproducing the "save immediately" case deterministically: hold the grant-data
- * response so the null window stays open, edit + save right away, and assert the
- * PUT /_api/v3/page body omits `grant`. If the fix regressed (a default grant is
- * sent again), grant-data would resolve to the page's grant and the PUT would
- * include it, failing this test.
+ * This drives the real cross-stack behavior:
+ *   1. create a GRANT_OWNER ("only me") page,
+ *   2. hold the grant-data response so the editor opens with selectedGrant still null,
+ *   3. edit and save immediately (a real save to the DB),
+ *   4. read the page's grant back and assert it is still GRANT_OWNER.
  *
- * The loading indicator shown during this window is covered by the unit test in
- * GrantSelector.spec.tsx; here we assert the save-payload contract.
+ * page.request (APIRequestContext) is not subject to page.route, so the setup and
+ * verification calls bypass the hold that only affects the browser's fetch.
  */
 
 const GRANT_DATA_ROUTE = '**/_api/v3/page/grant-data**';
-const PAGE_UPDATE_ROUTE = '**/_api/v3/page';
+const GRANT_OWNER = 4; // PageGrant.GRANT_OWNER
 
-test('omits grant on save while the page grant is still loading (#11272)', async ({
+const readGrant = async (
+  request: import('@playwright/test').APIRequestContext,
+  pageId: string,
+): Promise<number> => {
+  const res = await request.get('/_api/v3/page/grant-data', {
+    params: { pageId },
+  });
+  expect(res.ok()).toBeTruthy();
+  return (await res.json()).grantData.currentPageGrant.grant;
+};
+
+test('keeps an owner-only grant when saving before the grant loads (#11272)', async ({
   page,
 }) => {
-  // Hold grant-data for the whole test so selectedGrant stays null (unresolved).
-  await page.route(GRANT_DATA_ROUTE, async (route) => {
-    if (route.request().method() !== 'GET') {
-      await route.continue();
-      return;
-    }
-    await new Promise((resolve) => setTimeout(resolve, 30_000));
-    await route.continue();
+  const pagePath = `/grant-preload-race-${Date.now()}`;
+
+  // 1. Create an "only me" (GRANT_OWNER) page.
+  const createRes = await page.request.post('/_api/v3/page', {
+    data: { path: pagePath, body: 'initial body', grant: GRANT_OWNER },
   });
+  expect(createRes.ok()).toBeTruthy();
+  const createdPageId: string = (await createRes.json()).page._id;
+  expect(await readGrant(page.request, createdPageId)).toBe(GRANT_OWNER);
 
-  // Capture the update payload and swallow the request so the DB is not mutated.
-  let updateBody: Record<string, unknown> | undefined;
-  await page.route(PAGE_UPDATE_ROUTE, async (route) => {
-    if (route.request().method() !== 'PUT') {
-      await route.continue();
+  // 2. Block the browser's grant-data fetch so the editor opens with
+  //    selectedGrant still unresolved (null) — the pre-load window. Aborting is a
+  //    deterministic stand-in for "not loaded yet". page.request (used below for
+  //    verification) is an APIRequestContext and is NOT affected by page.route.
+  await page.route(GRANT_DATA_ROUTE, async (route) => {
+    if (route.request().method() === 'GET') {
+      await route.abort();
       return;
     }
-    const data = route.request().postData();
-    updateBody = data != null ? JSON.parse(data) : {};
-    await route.fulfill({
-      status: 200,
-      contentType: 'application/json',
-      body: '{}',
-    });
+    await route.continue();
   });
 
-  await page.goto('/Sandbox');
-
+  await page.goto(pagePath);
   await page.getByTestId('editor-button').click();
   await expect(page.getByTestId('grw-editor-navbar-bottom')).toBeVisible();
 
-  // Edit and save immediately, before grant-data resolves.
-  await appendTextToEditorUntilContains(
-    page,
-    'pre-load race regression #11272',
+  // 3. Edit and save immediately, while selectedGrant is still null.
+  await appendTextToEditorUntilContains(page, 'edited before grant loaded');
+  const updateResponse = page.waitForResponse(
+    (res) =>
+      res.url().includes('/_api/v3/page') && res.request().method() === 'PUT',
   );
   await page.getByTestId('save-page-btn').click();
+  expect((await updateResponse).ok()).toBeTruthy();
 
-  // The update must be sent without a grant, so the server preserves the
-  // page's existing grant rather than overwriting it.
-  await expect.poll(() => updateBody).toBeDefined();
-  expect(updateBody).not.toHaveProperty('grant');
-  expect(updateBody).not.toHaveProperty('userRelatedGrantUserGroupIds');
+  // 4. The stored grant must still be owner-only (not published).
+  expect(await readGrant(page.request, createdPageId)).toBe(GRANT_OWNER);
 });

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

@@ -41,6 +41,8 @@ describe('GrantSelector', () => {
     const { queryByTestId } = renderGrantSelector();
 
     expect(queryByTestId('grw-grant-selector-loading')).not.toBeNull();
+    // ...and the selector dropdown is not shown yet (no misleading "Public").
+    expect(queryByTestId('grw-grant-selector-dropdown-menu')).toBeNull();
   });
 
   it('shows the grant selector once the grant is available', () => {

+ 6 - 5
apps/app/src/states/ui/editor/use-sync-selected-grant.ts

@@ -8,11 +8,12 @@ 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.
+ * The atom starts as null (unresolved); this fills it with the page's actual
+ * grant so the editor reflects the real visibility. It must run from an
+ * always-mounted component: on mobile, GrantSelector is rendered only inside a
+ * closed Modal and therefore never mounts, so it cannot own this sync. (Saving
+ * while the atom is still null omits the grant, so the server preserves it — the
+ * pre-load race is handled separately in PageEditor's save path.)
  *
  * Call this once from an always-mounted editor component (e.g. SavePageControls).
  *