Yuki Takei 6 месяцев назад
Родитель
Сommit
362bd1c65a

+ 192 - 0
apps/app/src/states/page/use-fetch-current-page.spec.tsx

@@ -305,4 +305,196 @@ describe('useFetchCurrentPage - Integration Test', () => {
       expect(store.get(pageErrorAtom)).toEqual(apiError);
     });
   });
+
+  it('should handle path with permalink and convert to pageId for API call', async () => {
+    // Arrange: A path that looks like a permalink
+    const permalinkPath = '/58a4569921a8424d00a1aa0e';
+    const expectedPageId = '58a4569921a8424d00a1aa0e';
+    const pageData = createPageDataMock(
+      expectedPageId,
+      '/actual/page/path',
+      'permalink content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: permalinkPath });
+
+    // Assert
+    await waitFor(() => {
+      // 1. API should be called with pageId instead of path
+      expect(mockedApiv3Get).toHaveBeenCalledWith(
+        '/page',
+        expect.objectContaining({ pageId: expectedPageId }),
+      );
+      // 2. API should NOT be called with 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(currentPageDataAtom)).toEqual(pageData);
+      expect(store.get(pageLoadingAtom)).toBe(false);
+      expect(store.get(pageNotFoundAtom)).toBe(false);
+      expect(store.get(pageErrorAtom)).toBeNull();
+    });
+  });
+
+  it('should prioritize explicit pageId over permalink path', async () => {
+    // Arrange: Both pageId and permalink path are provided, pageId should take priority
+    const explicitPageId = 'explicit123456789012345678901234';
+    const permalinkPath = '/58a4569921a8424d00a1aa0e';
+    const pageData = createPageDataMock(
+      explicitPageId,
+      '/prioritized/page',
+      'explicit pageId content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({
+      path: permalinkPath,
+      pageId: explicitPageId
+    });
+
+    // Assert
+    await waitFor(() => {
+      // 1. API should be called with explicit pageId, not the permalink from path
+      expect(mockedApiv3Get).toHaveBeenCalledWith(
+        '/page',
+        expect.objectContaining({ pageId: explicitPageId }),
+      );
+      // 2. Should NOT use the permalink ID from path
+      expect(mockedApiv3Get).not.toHaveBeenCalledWith(
+        '/page',
+        expect.objectContaining({ pageId: '58a4569921a8424d00a1aa0e' }),
+      );
+
+      // 3. State should be updated with explicit pageId
+      expect(store.get(currentPageIdAtom)).toBe(explicitPageId);
+      expect(store.get(currentPageDataAtom)).toEqual(pageData);
+    });
+  });
+
+  it('should handle regular path (non-permalink) correctly', async () => {
+    // Arrange: Regular path that is NOT a permalink
+    const regularPath = '/regular/page/path';
+    const pageData = createPageDataMock(
+      'regularPageId123',
+      regularPath,
+      'regular page content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: regularPath });
+
+    // Assert
+    await waitFor(() => {
+      // 1. API should be called with path, not pageId
+      expect(mockedApiv3Get).toHaveBeenCalledWith(
+        '/page',
+        expect.objectContaining({ path: regularPath }),
+      );
+      // 2. API should NOT be called with pageId
+      expect(mockedApiv3Get).toHaveBeenCalledWith(
+        '/page',
+        expect.not.objectContaining({ pageId: expect.anything() }),
+      );
+
+      // 3. State should be updated correctly
+      expect(store.get(currentPageIdAtom)).toBe('regularPageId123');
+      expect(store.get(currentPageDataAtom)).toEqual(pageData);
+    });
+  });
+
+  it('should handle permalink path with hash fragment and strip hash for API call', async () => {
+    // Arrange: Permalink path with hash fragment like #edit
+    const permalinkWithHash = '/58a4569921a8424d00a1aa0e#edit';
+    const expectedPageId = '58a4569921a8424d00a1aa0e';
+    const pageData = createPageDataMock(
+      expectedPageId,
+      '/actual/page/path',
+      'permalink with hash content',
+    );
+    mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+    // Act
+    const { result } = renderHookWithProvider();
+    await result.current.fetchCurrentPage({ path: permalinkWithHash });
+
+    // Assert
+    await waitFor(() => {
+      // 1. API should be called with pageId (hash stripped), not with path
+      expect(mockedApiv3Get).toHaveBeenCalledWith(
+        '/page',
+        expect.objectContaining({ pageId: expectedPageId }),
+      );
+      // 2. API should NOT be called with path containing hash
+      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(currentPageDataAtom)).toEqual(pageData);
+      expect(store.get(pageLoadingAtom)).toBe(false);
+      expect(store.get(pageNotFoundAtom)).toBe(false);
+      expect(store.get(pageErrorAtom)).toBeNull();
+    });
+  });
+
+  it('should handle various hash fragments with permalink', async () => {
+    // Test various hash patterns that commonly occur
+    const testCases = [
+      {
+        input: '/58a4569921a8424d00a1aa0e#edit',
+        expectedPageId: '58a4569921a8424d00a1aa0e',
+        description: 'edit hash'
+      },
+      {
+        input: '/58a4569921a8424d00a1aa0e#section-header',
+        expectedPageId: '58a4569921a8424d00a1aa0e',
+        description: 'section hash'
+      },
+      {
+        input: '/58a4569921a8424d00a1aa0e#',
+        expectedPageId: '58a4569921a8424d00a1aa0e',
+        description: 'empty hash'
+      }
+    ];
+
+    for (const testCase of testCases) {
+      // Clean up for each iteration - ensure fresh state
+      vi.clearAllMocks();
+      store = createStore();
+
+      // Arrange
+      const pageData = createPageDataMock(
+        testCase.expectedPageId,
+        '/any/path',
+        `content for ${testCase.description}`,
+      );
+      mockedApiv3Get.mockResolvedValue(mockApiResponse(pageData));
+
+      // Act
+      const { result } = renderHookWithProvider();
+      await result.current.fetchCurrentPage({ path: testCase.input });
+
+      // Assert
+      await waitFor(() => {
+        expect(mockedApiv3Get).toHaveBeenCalledWith(
+          '/page',
+          expect.objectContaining({ pageId: testCase.expectedPageId }),
+        );
+        expect(store.get(currentPageIdAtom)).toBe(testCase.expectedPageId);
+      });
+    }
+  });
 });

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

@@ -1,16 +1,17 @@
-import { isIPageNotFoundInfo, type IPagePopulatedToShowRevision } from '@growi/core';
-import { isClient } from '@growi/core/dist/utils';
-import { isErrorV3 } from '@growi/core/dist/models';
 import {
-  isPermalink,
-} from '@growi/core/dist/utils/page-path-utils';
+  type IPagePopulatedToShowRevision,
+  isIPageNotFoundInfo,
+} from '@growi/core';
+import { isErrorV3 } from '@growi/core/dist/models';
+import { isClient } from '@growi/core/dist/utils';
+import { isPermalink } from '@growi/core/dist/utils/page-path-utils';
 import { removeHeadingSlash } from '@growi/core/dist/utils/path-utils';
 import { useAtomValue } from 'jotai';
 import { useAtomCallback } from 'jotai/utils';
 import { useCallback } from 'react';
 
 import { apiv3Get } from '~/client/util/apiv3-client';
-
+import loggerFactory from '~/utils/logger';
 import {
   currentPageDataAtom,
   currentPageIdAtom,
@@ -20,12 +21,9 @@ import {
   pageNotFoundAtom,
   shareLinkIdAtom,
 } from './internal-atoms';
-import loggerFactory from '~/utils/logger';
-
 
 const logger = loggerFactory('growi:states:page:useFetchCurrentPage');
 
-
 type FetchPageArgs = {
   path?: string;
   pageId?: string;
@@ -58,7 +56,7 @@ export const useFetchCurrentPage = (): {
         const currentPageId = get(currentPageIdAtom);
         const currentPageData = get(currentPageDataAtom);
 
-        // Process path first to handle permalinks
+        // Process path first to handle permalinks and strip hash fragments
         let decodedPath: string | undefined;
         if (args?.path != null) {
           try {
@@ -68,18 +66,30 @@ export const useFetchCurrentPage = (): {
           }
         }
 
+        // Strip hash fragment from path to properly detect permalinks
+        let pathWithoutHash: string | undefined;
+        if (decodedPath != null) {
+          try {
+            const url = new URL(decodedPath, 'http://example.com');
+            pathWithoutHash = url.pathname;
+          } catch {
+            // Fallback to simple split if URL parsing fails
+            pathWithoutHash = decodedPath.split('#')[0];
+          }
+        }
+
         // Guard clause to prevent unnecessary fetching
         if (args?.pageId != null && args.pageId === currentPageId) {
           return currentPageData ?? null;
         }
-        if (decodedPath != null) {
+        if (pathWithoutHash != null) {
           if (
-            isPermalink(decodedPath) &&
-            removeHeadingSlash(decodedPath) === currentPageId
+            isPermalink(pathWithoutHash) &&
+            removeHeadingSlash(pathWithoutHash) === currentPageId
           ) {
             return currentPageData ?? null;
           }
-          if (decodedPath === currentPageData?.path) {
+          if (pathWithoutHash === currentPageData?.path) {
             return currentPageData ?? null;
           }
         }
@@ -112,8 +122,8 @@ export const useFetchCurrentPage = (): {
         // priority: pageId > permalink > path
         if (pageId != null) {
           params.pageId = pageId;
-        } else if (decodedPath != null && isPermalink(decodedPath)) {
-          params.pageId = removeHeadingSlash(decodedPath);
+        } else if (pathWithoutHash != null && isPermalink(pathWithoutHash)) {
+          params.pageId = removeHeadingSlash(pathWithoutHash);
         } else if (decodedPath != null) {
           params.path = decodedPath;
         }
@@ -154,8 +164,7 @@ export const useFetchCurrentPage = (): {
               set(isForbiddenAtom, error.args.isForbidden ?? false);
               set(currentPageDataAtom, undefined);
             }
-          }
-          else {
+          } else {
             logger.error('Unhandled error when fetching current page:', err);
           }
         } finally {