Browse Source

Merge branch 'dev/7.4.x' into fix/disable-logo-update-without-file

hikaruNAKANO 4 months ago
parent
commit
2959b160e0

+ 16 - 4
apps/app/src/client/components/Common/Dropdown/PageItemControl.tsx

@@ -58,6 +58,7 @@ type CommonProps = {
 type DropdownMenuProps = CommonProps & {
   pageId: string,
   isLoading?: boolean,
+  isDataUnavailable?: boolean,
   operationProcessData?: IPageOperationProcessData,
 }
 
@@ -65,7 +66,7 @@ const PageItemControlDropdownMenu = React.memo((props: DropdownMenuProps): JSX.E
   const { t } = useTranslation('');
 
   const {
-    pageId, isLoading, pageInfo, isEnableActions, isReadOnlyUser, forceHideMenuItems, operationProcessData,
+    pageId, isLoading, isDataUnavailable, pageInfo, isEnableActions, isReadOnlyUser, forceHideMenuItems, operationProcessData,
     onClickBookmarkMenuItem, onClickRenameMenuItem, onClickDuplicateMenuItem, onClickDeleteMenuItem,
     onClickRevertMenuItem, onClickPathRecoveryMenuItem,
     additionalMenuItemOnTopRenderer: AdditionalMenuItemsOnTop,
@@ -130,7 +131,15 @@ const PageItemControlDropdownMenu = React.memo((props: DropdownMenuProps): JSX.E
 
   let contents = <></>;
 
-  if (isLoading) {
+  if (isDataUnavailable) {
+    // Show message when data is not available (e.g., fetch error)
+    contents = (
+      <div className="text-warning text-center px-3">
+        <span className="material-symbols-outlined">error_outline</span> No data available
+      </div>
+    );
+  }
+  else if (isLoading) {
     contents = (
       <div className="text-muted text-center my-2">
         <LoadingSpinner />
@@ -284,7 +293,7 @@ export const PageItemControlSubstance = (props: PageItemControlSubstanceProps):
   const [isOpen, setIsOpen] = useState(false);
   const [shouldFetch, setShouldFetch] = useState(false);
 
-  const { data: fetchedPageInfo, mutate: mutatePageInfo } = useSWRxPageInfo(shouldFetch ? pageId : null);
+  const { data: fetchedPageInfo, error: fetchError, mutate: mutatePageInfo } = useSWRxPageInfo(shouldFetch ? pageId : null);
 
   // update shouldFetch (and will never be false)
   useEffect(() => {
@@ -307,7 +316,9 @@ export const PageItemControlSubstance = (props: PageItemControlSubstanceProps):
     }
   }, [mutatePageInfo, onClickBookmarkMenuItem, shouldFetch]);
 
-  const isLoading = shouldFetch && fetchedPageInfo == null;
+  // isLoading should be true only when fetching is in progress (data and error are both undefined)
+  const isLoading = shouldFetch && fetchedPageInfo == null && fetchError == null;
+  const isDataUnavailable = !isLoading && fetchedPageInfo == null && presetPageInfo == null;
 
   const renameMenuItemClickHandler = useCallback(async() => {
     if (onClickRenameMenuItem == null) {
@@ -350,6 +361,7 @@ export const PageItemControlSubstance = (props: PageItemControlSubstanceProps):
           <PageItemControlDropdownMenu
             {...props}
             isLoading={isLoading}
+            isDataUnavailable={isDataUnavailable}
             pageInfo={fetchedPageInfo ?? presetPageInfo}
             onClickBookmarkMenuItem={bookmarkMenuItemClickHandler}
             onClickRenameMenuItem={renameMenuItemClickHandler}

+ 4 - 3
apps/app/src/client/components/PageSideContents/PageSideContents.tsx

@@ -5,7 +5,8 @@ import React, {
   type JSX,
 } from 'react';
 
-import type { IPagePopulatedToShowRevision, IPageInfoForOperation } from '@growi/core';
+import type { IPagePopulatedToShowRevision } from '@growi/core';
+import { isIPageInfoForOperation } from '@growi/core/dist/interfaces';
 import { pagePathUtils } from '@growi/core/dist/utils';
 import { useAtomValue } from 'jotai';
 import { useTranslation } from 'next-i18next';
@@ -129,7 +130,7 @@ export const PageSideContents = (props: PageSideContentsProps): JSX.Element => {
               icon={<span className="material-symbols-outlined">subject</span>}
               label={t('page_list')}
               // Do not display CountBadge if '/trash/*': https://github.com/growilabs/growi/pull/7600
-              count={!isTrash && pageInfo != null ? (pageInfo as IPageInfoForOperation).descendantCount : undefined}
+              count={!isTrash && isIPageInfoForOperation(pageInfo) ? pageInfo.descendantCount : undefined}
               offset={1}
               onClick={() => openDescendantPageListModal(pagePath)}
             />
@@ -142,7 +143,7 @@ export const PageSideContents = (props: PageSideContentsProps): JSX.Element => {
             <PageAccessoriesControl
               icon={<span className="material-symbols-outlined">chat</span>}
               label={t('comments')}
-              count={pageInfo != null ? (pageInfo as IPageInfoForOperation).commentCount : undefined}
+              count={isIPageInfoForOperation(pageInfo) ? pageInfo.commentCount : undefined}
               onClick={() => scroller.scrollTo('comments-container', { smooth: false, offset: -120 })}
             />
           </div>

+ 4 - 2
apps/app/src/client/components/Sidebar/PageTree/PageTreeSubstance.tsx

@@ -4,6 +4,7 @@ import React, {
 
 import { useTranslation } from 'next-i18next';
 
+import { ItemsTree } from '~/features/page-tree/components';
 import { usePageTreeInformationUpdate } from '~/features/page-tree/states/page-tree-update';
 import { useIsGuestUser, useIsReadOnlyUser } from '~/states/context';
 import { useCurrentPageId, useCurrentPagePath } from '~/states/page';
@@ -13,7 +14,6 @@ import {
 } from '~/stores/page-listing';
 import loggerFactory from '~/utils/logger';
 
-import { ItemsTree } from '~/features/page-tree/components';
 import { PageTreeItem, pageTreeItemSize } from '../PageTreeItem';
 import { SidebarHeaderReloadButton } from '../SidebarHeaderReloadButton';
 
@@ -108,6 +108,8 @@ export const PageTreeContent = memo(({ isWipPageShown }: PageTreeContentProps) =
 
   const sidebarScrollerElem = useSidebarScrollerElem();
 
+  const estimateTreeItemSize = useCallback(() => pageTreeItemSize, []);
+
   if (!migrationStatus?.isV5Compatible) {
     return <PageTreeUnavailable />;
   }
@@ -130,7 +132,7 @@ export const PageTreeContent = memo(({ isWipPageShown }: PageTreeContentProps) =
         targetPath={path}
         targetPathOrId={targetPathOrId}
         CustomTreeItem={PageTreeItem}
-        estimateTreeItemSize={() => pageTreeItemSize}
+        estimateTreeItemSize={estimateTreeItemSize}
         scrollerElem={sidebarScrollerElem}
       />
 

+ 11 - 50
apps/app/src/components/PageView/PageView.tsx

@@ -1,5 +1,6 @@
 import { type JSX, memo, useCallback, useEffect, useMemo, useRef } from 'react';
 import dynamic from 'next/dynamic';
+import { isDeepEquals } from '@growi/core/dist/utils/is-deep-equals';
 import { isUsersHomepage } from '@growi/core/dist/utils/page-path-utils';
 import { useSlidesByFrontmatter } from '@growi/presentation/dist/services';
 
@@ -85,9 +86,13 @@ type Props = {
   className?: string;
 };
 
-export const PageView = memo((props: Props): JSX.Element => {
-  const renderStartTime = performance.now();
+// Custom comparison function for memo to prevent unnecessary re-renders
+const arePropsEqual = (prevProps: Props, nextProps: Props): boolean =>
+  prevProps.pagePath === nextProps.pagePath &&
+  prevProps.className === nextProps.className &&
+  isDeepEquals(prevProps.rendererConfig, nextProps.rendererConfig);
 
+const PageViewComponent = (props: Props): JSX.Element => {
   const commentsContainerRef = useRef<HTMLDivElement>(null);
 
   const { pagePath, rendererConfig, className } = props;
@@ -101,15 +106,6 @@ export const PageView = memo((props: Props): JSX.Element => {
   const page = useCurrentPageData();
   const { data: viewOptions } = useViewOptions();
 
-  // DEBUG: Log PageView render start
-  console.log('[PAGEVIEW-DEBUG] PageView render started:', {
-    pagePath,
-    currentPageId,
-    pageId: page?._id,
-    timestamp: new Date().toISOString(),
-    renderStartTime,
-  });
-
   const isNotFound = isNotFoundMeta || page == null;
   const isUsersHomepagePath = isUsersHomepage(pagePath);
 
@@ -123,23 +119,13 @@ export const PageView = memo((props: Props): JSX.Element => {
 
   // ***************************  Auto Scroll  ***************************
   useEffect(() => {
-    const scrollEffectStartTime = performance.now();
-    console.log('[PAGEVIEW-DEBUG] Auto scroll effect triggered:', {
-      currentPageId,
-      hash: window.location.hash,
-      timestamp: new Date().toISOString(),
-      effectStartTime: scrollEffectStartTime,
-    });
-
     if (currentPageId == null) {
-      console.log('[PAGEVIEW-DEBUG] Auto scroll skipped - no currentPageId');
       return;
     }
 
     // do nothing if hash is empty
     const { hash } = window.location;
     if (hash.length === 0) {
-      console.log('[PAGEVIEW-DEBUG] Auto scroll skipped - no hash');
       return;
     }
 
@@ -204,18 +190,7 @@ export const PageView = memo((props: Props): JSX.Element => {
     ) : null;
 
   const Contents = useCallback(() => {
-    const contentsRenderStartTime = performance.now();
-    console.log('[PAGEVIEW-DEBUG] Contents component render started:', {
-      isNotFound,
-      hasPage: page != null,
-      hasRevision: page?.revision != null,
-      pageId: page?._id,
-      timestamp: new Date().toISOString(),
-      contentsRenderStartTime,
-    });
-
     if (isNotFound || page?.revision == null) {
-      console.log('[PAGEVIEW-DEBUG] Rendering NotFoundPage');
       return <NotFoundPage path={pagePath} />;
     }
 
@@ -223,13 +198,6 @@ export const PageView = memo((props: Props): JSX.Element => {
     const rendererOptions =
       viewOptions ?? generateSSRViewOptions(rendererConfig, pagePath);
 
-    console.log('[PAGEVIEW-DEBUG] Rendering page content:', {
-      markdownLength: markdown?.length,
-      hasViewOptions: viewOptions != null,
-      isSlide: isSlide != null,
-      renderDuration: performance.now() - contentsRenderStartTime,
-    });
-
     return (
       <>
         <PageContentsUtilities />
@@ -268,16 +236,6 @@ export const PageView = memo((props: Props): JSX.Element => {
     page,
   ]);
 
-  // DEBUG: Log final render completion time
-  const renderEndTime = performance.now();
-  console.log('[PAGEVIEW-DEBUG] PageView render completed:', {
-    pagePath,
-    currentPageId,
-    pageId: page?._id,
-    totalRenderDuration: renderEndTime - renderStartTime,
-    timestamp: new Date().toISOString(),
-  });
-
   return (
     <PageViewLayout
       className={className}
@@ -301,4 +259,7 @@ export const PageView = memo((props: Props): JSX.Element => {
       )}
     </PageViewLayout>
   );
-});
+};
+
+export const PageView = memo(PageViewComponent, arePropsEqual);
+PageView.displayName = 'PageView';

+ 32 - 4
apps/app/src/features/page-tree/components/ItemsTree.tsx

@@ -168,18 +168,40 @@ export const ItemsTree: FC<Props> = (props: Props) => {
     onExpanded: triggerTreeRebuild,
   });
 
+  const getScrollElement = useCallback(
+    () => scrollerElem ?? null,
+    [scrollerElem],
+  );
+
+  const stableEstimateSize = useCallback(() => {
+    return estimateTreeItemSize();
+  }, [estimateTreeItemSize]);
+
+  const measureElement = useCallback(
+    (element: Element | null) => {
+      // Return consistent height measurement
+      return element?.getBoundingClientRect().height ?? stableEstimateSize();
+    },
+    [stableEstimateSize],
+  );
+
   const virtualizer = useVirtualizer({
     count: items.length,
-    getScrollElement: () => scrollerElem ?? null,
-    estimateSize: estimateTreeItemSize,
+    getScrollElement,
+    estimateSize: stableEstimateSize,
     overscan: 5,
+    measureElement,
   });
 
   // Scroll to selected item on mount or when targetPathOrId changes
   useScrollToSelectedItem({ targetPathOrId, items, virtualizer });
 
   return (
-    <div {...tree.getContainerProps()} className="list-group">
+    <div
+      {...tree.getContainerProps()}
+      className="list-group position-relative"
+      style={{ height: `${virtualizer.getTotalSize()}px` }}
+    >
       {virtualizer.getVirtualItems().map((virtualItem) => {
         const item = items[virtualItem.index];
         const itemData = item.getItemData();
@@ -202,8 +224,14 @@ export const ItemsTree: FC<Props> = (props: Props) => {
           <div
             key={virtualItem.key}
             data-index={virtualItem.index}
+            style={{
+              position: 'absolute',
+              top: 0,
+              left: 0,
+              width: '100%',
+              transform: `translateY(${virtualItem.start}px)`,
+            }}
             ref={(node) => {
-              virtualizer.measureElement(node);
               if (node && itemRef) {
                 (itemRef as (node: HTMLElement) => void)(node);
               }

+ 6 - 5
apps/app/src/pages/[[...path]]/index.page.tsx

@@ -1,12 +1,9 @@
-import type React from 'react';
 import type { JSX, ReactNode } from 'react';
-import { useEffect } from 'react';
+import { useEffect, useMemo } from 'react';
 import type { GetServerSideProps, GetServerSidePropsContext } from 'next';
 import dynamic from 'next/dynamic';
 import Head from 'next/head';
-import EventEmitter from 'node:events';
 import { isIPageInfo } from '@growi/core';
-import { isClient } from '@growi/core/dist/utils';
 
 // biome-ignore-start lint/style/noRestrictedImports: no-problem lazy loaded components
 import { DescendantsPageListModalLazyLoaded } from '~/client/components/DescendantsPageListModal';
@@ -153,7 +150,11 @@ const Page: NextPageWithLayout<Props> = (props: Props) => {
 
   // If the data on the page changes without router.push, pageWithMeta remains old because getServerSideProps() is not executed
   // So preferentially take page data from useSWRxCurrentPage
-  const pagePath = currentPagePath ?? props.currentPathname;
+  // Note: Memoize to prevent unnecessary re-renders of PageView
+  const pagePath = useMemo(
+    () => currentPagePath ?? props.currentPathname,
+    [currentPagePath, props.currentPathname],
+  );
 
   const title = useCustomTitleForPage(pagePath);
 

+ 1 - 1
apps/app/src/server/models/openapi/page.ts

@@ -96,7 +96,7 @@
  *              revisionShortBody:
  *                type: string
  *                description: Short body of the revision
- *      PageInfoAll:
+ *      PageInfoExt:
  *        description: Page information (union of all page info types)
  *        oneOf:
  *          - $ref: '#/components/schemas/PageInfo'

+ 152 - 0
apps/app/src/server/routes/apiv3/page-listing.ts

@@ -174,6 +174,158 @@ const routerFactory = (crowi: Crowi): Router => {
     },
   );
 
+  /**
+   * @swagger
+   *
+   * /page-listing/info:
+   *   get:
+   *     tags: [PageListing]
+   *     security:
+   *       - bearer: []
+   *       - accessTokenInQuery: []
+   *     summary: /page-listing/info
+   *     description: Get summary information of pages
+   *     parameters:
+   *       - name: pageIds
+   *         in: query
+   *         description: Array of page IDs to retrieve information for (One of pageIds or path is required)
+   *         schema:
+   *           type: array
+   *           items:
+   *             type: string
+   *       - name: path
+   *         in: query
+   *         description: Path of the page to retrieve information for (One of pageIds or path is required)
+   *         schema:
+   *           type: string
+   *       - name: attachBookmarkCount
+   *         in: query
+   *         schema:
+   *           type: boolean
+   *       - name: attachShortBody
+   *         in: query
+   *         schema:
+   *           type: boolean
+   *     responses:
+   *       200:
+   *         description: Get the information of a page
+   *         content:
+   *           application/json:
+   *             schema:
+   *               type: object
+   *               additionalProperties:
+   *                 $ref: '#/components/schemas/PageInfoExt'
+   */
+  router.get(
+    '/info',
+    accessTokenParser([SCOPE.READ.FEATURES.PAGE], { acceptLegacy: true }),
+    validator.pageIdsOrPathRequired,
+    validator.infoParams,
+    apiV3FormValidator,
+    async (req: AuthorizedRequest, res: ApiV3Response) => {
+      const {
+        pageIds,
+        path,
+        attachBookmarkCount: attachBookmarkCountParam,
+        attachShortBody: attachShortBodyParam,
+      } = req.query;
+
+      const attachBookmarkCount: boolean = attachBookmarkCountParam === 'true';
+      const attachShortBody: boolean = attachShortBodyParam === 'true';
+
+      const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>(
+        'Page',
+      );
+      // biome-ignore lint/suspicious/noExplicitAny: ignore
+      const Bookmark = mongoose.model<any, any>('Bookmark');
+      const pageService = crowi.pageService;
+      const pageGrantService: IPageGrantService = crowi.pageGrantService;
+
+      try {
+        const pages =
+          pageIds != null
+            ? await Page.findByIdsAndViewer(
+                pageIds as string[],
+                req.user,
+                null,
+                true,
+              )
+            : await Page.findByPathAndViewer(
+                path as string,
+                req.user,
+                null,
+                false,
+                true,
+              );
+
+        const foundIds = pages.map((page) => page._id);
+
+        let shortBodiesMap: Record<string, string | null> | undefined;
+        if (attachShortBody) {
+          shortBodiesMap = await pageService.shortBodiesMapByPageIds(
+            foundIds,
+            req.user,
+          );
+        }
+
+        let bookmarkCountMap: Record<string, number> | undefined;
+        if (attachBookmarkCount) {
+          bookmarkCountMap = (await Bookmark.getPageIdToCountMap(
+            foundIds,
+          )) as Record<string, number>;
+        }
+
+        const idToPageInfoMap: Record<string, IPageInfo | IPageInfoForListing> =
+          {};
+
+        const isGuestUser = req.user == null;
+
+        const userRelatedGroups = await pageGrantService.getUserRelatedGroups(
+          req.user,
+        );
+
+        for (const page of pages) {
+          const basicPageInfo = {
+            ...pageService.constructBasicPageInfo(page, isGuestUser),
+            bookmarkCount:
+              bookmarkCountMap != null
+                ? (bookmarkCountMap[page._id.toString()] ?? 0)
+                : 0,
+          };
+
+          // TODO: use pageService.getCreatorIdForCanDelete to get creatorId (https://redmine.weseek.co.jp/issues/140574)
+          const canDeleteCompletely = pageService.canDeleteCompletely(
+            page,
+            page.creator == null ? null : getIdForRef(page.creator),
+            req.user,
+            false,
+            userRelatedGroups,
+          ); // use normal delete config
+
+          const pageInfo = !isIPageInfoForEntity(basicPageInfo)
+            ? basicPageInfo
+            : ({
+                ...basicPageInfo,
+                isAbleToDeleteCompletely: canDeleteCompletely,
+                revisionShortBody:
+                  shortBodiesMap != null
+                    ? (shortBodiesMap[page._id.toString()] ?? undefined)
+                    : undefined,
+              } satisfies IPageInfoForListing);
+
+          idToPageInfoMap[page._id.toString()] = pageInfo;
+        }
+
+        return res.apiv3(idToPageInfoMap);
+      } catch (err) {
+        logger.error('Error occurred while fetching page informations.', err);
+        return res.apiv3Err(
+          new ErrorV3('Error occurred while fetching page informations.'),
+        );
+      }
+    },
+  );
+
   /**
    * @swagger
    *

+ 15 - 2
apps/app/src/server/routes/apiv3/page/index.ts

@@ -567,7 +567,7 @@ module.exports = (crowi: Crowi) => {
    *            content:
    *              application/json:
    *                schema:
-   *                  $ref: '#/components/schemas/PageInfoAll'
+   *                  $ref: '#/components/schemas/PageInfoExt'
    *          500:
    *            description: Internal server error.
    */
@@ -591,7 +591,20 @@ module.exports = (crowi: Crowi) => {
         );
 
         if (isIPageNotFoundInfo(meta)) {
-          return res.apiv3Err(`Page '${pageId}' is not found or forbidden`);
+          // Return error only when the page is forbidden
+          // Empty pages (isEmpty: true) should return page info for UI operations
+          if (meta.isForbidden) {
+            return res.apiv3Err(
+              new ErrorV3(
+                'Page is forbidden',
+                'page-is-forbidden',
+                undefined,
+                meta,
+              ),
+              403,
+            );
+          }
+          // For not found but not forbidden pages (isEmpty: true), return the meta info
         }
 
         return res.apiv3(meta);

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

@@ -37,25 +37,24 @@ type FetchPageArgs = {
 
 /**
  * Process path to handle URL decoding and hash fragment removal
+ *
+ * Note: new URL().pathname re-encodes the path, so we decode it at the end
+ * to ensure the final result is always decoded (e.g., "/path/にほんご")
  */
 const decodeAndRemoveFragment = (pathname?: string): string | undefined => {
   if (pathname == null) return;
 
-  // Decode path
-  let decodedPathname: string;
   try {
-    decodedPathname = decodeURIComponent(pathname);
-  } catch {
-    decodedPathname = pathname;
-  }
-
-  // Strip hash fragment from path to properly detect permalinks
-  try {
-    const url = new URL(decodedPathname, 'http://example.com');
-    return url.pathname;
+    const url = new URL(pathname, 'http://example.com');
+    return decodeURIComponent(url.pathname);
   } catch {
     // Fallback to simple split if URL parsing fails
-    return decodedPathname.split('#')[0];
+    const pathOnly = pathname.split('#')[0];
+    try {
+      return decodeURIComponent(pathOnly);
+    } catch {
+      return pathOnly;
+    }
   }
 };
 
@@ -215,6 +214,7 @@ export const useFetchCurrentPage = (): {
 
         set(pageLoadingAtom, true);
         set(pageErrorAtom, null);
+        set(pageNotFoundAtom, false);
 
         // Build API parameters
         const { params, shouldSkip } = buildApiParams({