Yuki Takei 3 месяцев назад
Родитель
Сommit
26a29d3833

+ 2 - 1
.serena/memories/page-state-hooks-useLatestRevision-degradation.md

@@ -305,7 +305,8 @@ export const useFetchCurrentPage = () => {
       const { page: newData } = data;
 
       set(currentPageDataAtom, newData);
-      set(currentPageIdAtom, newData._id);
+      set(currentPageEntityIdAtom, newData._id);
+      set(currentPageEmptyIdAtom, undefined);
 
       // ✅ 追加: PageInfo を再フェッチ
       mutatePageInfo();  // 引数なし = revalidate (再フェッチ)

+ 1 - 1
.serena/memories/page-transition-and-rendering-flow.md

@@ -53,7 +53,7 @@
     - **3d. API通信**: `apiv3Get('/page', ...)` を実行してサーバーから新しいページデータを取得します。パラメータには、パス、ページID、リビジョンIDなどが含まれます。
 4.  **アトミックな状態更新**:
     - **API成功時**:
-        - 関連する **すべてのatomを一度に更新** します (`currentPageDataAtom`, `currentPageIdAtom`, `pageNotFoundAtom`, `pageLoadingAtom` など)。
+        - 関連する **すべてのatomを一度に更新** します (`currentPageDataAtom`, `currentPageEntityIdAtom`, `currentPageEmptyIdAtom`, `pageNotFoundAtom`, `pageLoadingAtom` など)。
         - これにより、中間的な状態(`pageId`が`undefined`になるなど)が発生することなく、データが完全に揃った状態で一度だけ状態が更新されます。
     - **APIエラー時 (例: 404 Not Found)**:
         - `pageErrorAtom` にエラーオブジェクトを設定します。

+ 153 - 24
apps/app/src/states/page/use-fetch-current-page.spec.tsx

@@ -7,7 +7,7 @@ import type {
   Lang,
   PageGrant,
   PageStatus,
-} from '@growi/core';
+} from '@growi/core/dist/interfaces';
 import { renderHook, waitFor } from '@testing-library/react';
 // biome-ignore lint/style/noRestrictedImports: import only types
 import type { AxiosResponse } from 'axios';
@@ -19,7 +19,8 @@ import * as apiv3Client from '~/client/util/apiv3-client';
 import { useFetchCurrentPage } from '~/states/page';
 import {
   currentPageDataAtom,
-  currentPageIdAtom,
+  currentPageEmptyIdAtom,
+  currentPageEntityIdAtom,
   isForbiddenAtom,
   pageErrorAtom,
   pageLoadingAtom,
@@ -167,7 +168,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/initial/path',
       'initial content',
     );
-    store.set(currentPageIdAtom, initialPageData._id);
+    store.set(currentPageEntityIdAtom, initialPageData._id);
     store.set(currentPageDataAtom, initialPageData);
 
     // Arrange: Navigate to a new page
@@ -191,7 +192,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       );
 
       // 2. Atoms were updated
-      expect(store.get(currentPageIdAtom)).toBe(newPageData._id);
+      expect(store.get(currentPageEntityIdAtom)).toBe(newPageData._id);
       expect(store.get(currentPageDataAtom)).toEqual(newPageData);
       expect(store.get(pageLoadingAtom)).toBe(false);
       expect(store.get(pageNotFoundAtom)).toBe(false);
@@ -206,7 +207,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/same/path',
       'current content',
     );
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Act
@@ -226,7 +227,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/some/path',
       'current content',
     );
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Act
@@ -245,7 +246,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/same/path',
       'current content',
     );
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Arrange: API returns different revision
@@ -289,7 +290,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       'current content',
     );
     const currentRevisionId = currentPageData.revision?._id;
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Act
@@ -311,7 +312,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/same/path',
       'old content',
     );
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Arrange: API returns old revision
@@ -354,7 +355,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/same/path',
       'old content',
     );
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Arrange: API returns updated data
@@ -392,7 +393,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/some/path',
       'old content',
     );
-    store.set(currentPageIdAtom, currentPageData._id);
+    store.set(currentPageEntityIdAtom, currentPageData._id);
     store.set(currentPageDataAtom, currentPageData);
 
     // Arrange: API returns updated data
@@ -431,7 +432,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/actual/path',
       'old content',
     );
-    store.set(currentPageIdAtom, permalinkId);
+    store.set(currentPageEntityIdAtom, permalinkId);
     store.set(currentPageDataAtom, currentPageData);
 
     // Arrange: API returns updated data
@@ -478,7 +479,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
     await result.current.fetchCurrentPage({ path: '/some/page' });
 
     await waitFor(() => {
-      expect(store.get(currentPageIdAtom)).toBe('regularPageId');
+      expect(store.get(currentPageEntityIdAtom)).toBe('regularPageId');
     });
 
     // Arrange: Navigate to the root page
@@ -499,7 +500,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
         '/page',
         expect.objectContaining({ path: '/' }),
       );
-      expect(store.get(currentPageIdAtom)).toBe('rootPageId');
+      expect(store.get(currentPageEntityIdAtom)).toBe('rootPageId');
     });
   });
 
@@ -524,7 +525,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
         '/page',
         expect.objectContaining({ path: decodedPath }),
       );
-      expect(store.get(currentPageIdAtom)).toBe('encodedPageId');
+      expect(store.get(currentPageEntityIdAtom)).toBe('encodedPageId');
     });
   });
 
@@ -548,7 +549,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
         '/page',
         expect.objectContaining({ pageId: '65d4e0a0f7b7b2e5a8652e86' }),
       );
-      expect(store.get(currentPageIdAtom)).toBe('65d4e0a0f7b7b2e5a8652e86');
+      expect(store.get(currentPageEntityIdAtom)).toBe('65d4e0a0f7b7b2e5a8652e86');
     });
   });
 
@@ -574,14 +575,14 @@ describe('useFetchCurrentPage - Integration Test', () => {
         '/page',
         expect.objectContaining({ pageId: expectedPageId }),
       );
-      // 2. API should NOT be called with path
+      // 2. API should NOT use the permalink from path
       expect(mockedApiv3Get).toHaveBeenCalledWith(
         '/page',
         expect.not.objectContaining({ path: expect.anything() }),
       );
 
       // 3. State should be updated correctly
-      expect(store.get(currentPageIdAtom)).toBe(expectedPageId);
+      expect(store.get(currentPageEntityIdAtom)).toBe(expectedPageId);
       expect(store.get(currentPageDataAtom)).toEqual(pageData);
       expect(store.get(pageLoadingAtom)).toBe(false);
       expect(store.get(pageNotFoundAtom)).toBe(false);
@@ -621,7 +622,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       );
 
       // 3. State should be updated with explicit pageId
-      expect(store.get(currentPageIdAtom)).toBe(explicitPageId);
+      expect(store.get(currentPageEntityIdAtom)).toBe(explicitPageId);
       expect(store.get(currentPageDataAtom)).toEqual(pageData);
     });
   });
@@ -654,7 +655,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       );
 
       // 3. State should be updated correctly
-      expect(store.get(currentPageIdAtom)).toBe('regularPageId123');
+      expect(store.get(currentPageEntityIdAtom)).toBe('regularPageId123');
       expect(store.get(currentPageDataAtom)).toEqual(pageData);
     });
   });
@@ -688,7 +689,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       );
 
       // 3. State should be updated correctly
-      expect(store.get(currentPageIdAtom)).toBe(expectedPageId);
+      expect(store.get(currentPageEntityIdAtom)).toBe(expectedPageId);
       expect(store.get(currentPageDataAtom)).toEqual(pageData);
       expect(store.get(pageLoadingAtom)).toBe(false);
       expect(store.get(pageNotFoundAtom)).toBe(false);
@@ -739,7 +740,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
           '/page',
           expect.objectContaining({ pageId: testCase.expectedPageId }),
         );
-        expect(store.get(currentPageIdAtom)).toBe(testCase.expectedPageId);
+        expect(store.get(currentPageEntityIdAtom)).toBe(testCase.expectedPageId);
       });
     }
   });
@@ -751,7 +752,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
       '/some/existing',
       'existing body',
     );
-    store.set(currentPageIdAtom, existingPage._id);
+    store.set(currentPageEntityIdAtom, existingPage._id);
     store.set(currentPageDataAtom, existingPage);
     store.set(remoteRevisionBodyAtom, 'remote body');
 
@@ -776,7 +777,7 @@ describe('useFetchCurrentPage - Integration Test', () => {
         message: 'Page not found',
       });
       expect(store.get(currentPageDataAtom)).toBeUndefined();
-      expect(store.get(currentPageIdAtom)).toBeUndefined();
+      expect(store.get(currentPageEntityIdAtom)).toBeUndefined();
       expect(store.get(remoteRevisionBodyAtom)).toBeUndefined();
     });
   });
@@ -843,4 +844,132 @@ describe('useFetchCurrentPage - Integration Test', () => {
       expect(store.get(isForbiddenAtom)).toBe(false);
     });
   });
+
+  it('should set emptyPageId when page not found with IPageInfoForEmpty', async () => {
+    // Arrange: Mock API rejection with ErrorV3 and IPageInfoForEmpty args
+    const emptyPageId = 'empty123';
+    const notFoundErrorWithEmptyPage = {
+      code: 'not_found',
+      message: 'Page not found',
+      args: {
+        isNotFound: true,
+        isForbidden: false,
+        isEmpty: true, // Required for isIPageInfoForEmpty check
+        emptyPageId,
+      },
+    } as const;
+    mockedApiv3Get.mockRejectedValueOnce([notFoundErrorWithEmptyPage]);
+
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: '/empty/page' });
+
+    // Assert: emptyPageId should be set
+    await waitFor(() => {
+      expect(store.get(pageLoadingAtom)).toBe(false);
+      expect(store.get(pageNotFoundAtom)).toBe(true);
+      expect(store.get(isForbiddenAtom)).toBe(false);
+      expect(store.get(currentPageEmptyIdAtom)).toBe(emptyPageId);
+      expect(store.get(currentPageDataAtom)).toBeUndefined();
+      expect(store.get(currentPageEntityIdAtom)).toBeUndefined();
+    });
+  });
+
+  it('should not set emptyPageId when page not found without IPageInfoForEmpty', async () => {
+    // Arrange: Mock API rejection with ErrorV3 but without emptyPageId
+    const notFoundErrorWithoutEmptyPage = {
+      code: 'not_found',
+      message: 'Page not found',
+      args: {
+        isNotFound: true,
+        isForbidden: false,
+        // No emptyPageId property
+      },
+    } as const;
+    mockedApiv3Get.mockRejectedValueOnce([notFoundErrorWithoutEmptyPage]);
+
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: '/regular/not/found' });
+
+    // Assert: emptyPageId should be undefined
+    await waitFor(() => {
+      expect(store.get(pageLoadingAtom)).toBe(false);
+      expect(store.get(pageNotFoundAtom)).toBe(true);
+      expect(store.get(isForbiddenAtom)).toBe(false);
+      expect(store.get(currentPageEmptyIdAtom)).toBeUndefined();
+      expect(store.get(currentPageDataAtom)).toBeUndefined();
+      expect(store.get(currentPageEntityIdAtom)).toBeUndefined();
+    });
+  });
+
+  it('should reset emptyPageId to undefined on successful fetch', async () => {
+    // Arrange: Set emptyPageId from a previous failed fetch
+    store.set(currentPageEmptyIdAtom, 'previousEmptyPageId');
+    store.set(pageNotFoundAtom, true);
+
+    // Arrange: API returns successful page data
+    const successPageData = createPageDataMock(
+      'newPageId',
+      '/success/path',
+      'success content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(successPageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: '/success/path' });
+
+    // Assert: emptyPageId should be reset to undefined
+    await waitFor(() => {
+      expect(store.get(pageLoadingAtom)).toBe(false);
+      expect(store.get(pageNotFoundAtom)).toBe(false);
+      expect(store.get(currentPageEmptyIdAtom)).toBeUndefined();
+      expect(store.get(currentPageDataAtom)).toEqual(successPageData);
+      expect(store.get(currentPageEntityIdAtom)).toBe('newPageId');
+    });
+  });
+
+  it('should handle path with encoded Japanese characters', async () => {
+    // Arrange: Path with Japanese characters
+    const japanesePath = '/日本語/ページ';
+    const encodedPath = encodeURIComponent(japanesePath);
+    const pageData = createPageDataMock(
+      'japanesePageId',
+      japanesePath,
+      'Japanese content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: japanesePath });
+
+    // Assert: Path should be properly decoded and sent to API
+    await waitFor(() => {
+      expect(mockedApiv3Get).toHaveBeenCalledWith(
+        '/page',
+        expect.objectContaining({ path: japanesePath }),
+      );
+      expect(store.get(currentPageEntityIdAtom)).toBe('japanesePageId');
+    });
+  });
+
+  it('should call mutatePageInfo after successful fetch', async () => {
+    // Arrange
+    const pageData = createPageDataMock(
+      'pageId123',
+      '/test/path',
+      'test content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: '/test/path' });
+
+    // Assert: mutatePageInfo should be called to refetch metadata
+    await waitFor(() => {
+      expect(mockMutatePageInfo).toHaveBeenCalled();
+      expect(store.get(currentPageEntityIdAtom)).toBe('pageId123');
+    });
+  });
 });

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

@@ -3,7 +3,7 @@ import {
   type IPagePopulatedToShowRevision,
   isIPageInfoForEmpty,
   isIPageNotFoundInfo,
-} from '@growi/core';
+} from '@growi/core/dist/interfaces';
 import { isErrorV3 } from '@growi/core/dist/models';
 import { isClient } from '@growi/core/dist/utils';
 import { isPermalink } from '@growi/core/dist/utils/page-path-utils';