feat: Add folder/grouping functionality for pages #7522
Conversation
WalkthroughThis change introduces comprehensive support for organizing pages into folders. It adds new folder-related types, store management, service logic, and UI components for creating, displaying, and modifying folders and their contents. The update also extends page actions to allow assigning pages to folders and adjusts the page list UI to reflect folder organization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (PagesListRoot)
participant FolderStore
participant PageFolderService
participant Modal (CreateFolderModal/AddToFolderModal)
User->>UI (PagesListRoot): Open page list
UI (PagesListRoot)->>FolderStore: fetchFoldersList()
FolderStore->>PageFolderService: fetchAll()
PageFolderService-->>FolderStore: Return folders
FolderStore-->>UI (PagesListRoot): Folders data
User->>UI (PagesListRoot): Click "Create Folder"
UI (PagesListRoot)->>Modal (CreateFolderModal): Open modal
User->>Modal (CreateFolderModal): Submit folder details
Modal (CreateFolderModal)->>FolderStore: createFolder()
FolderStore->>PageFolderService: create()
PageFolderService-->>FolderStore: Return new folder
FolderStore-->>Modal (CreateFolderModal): Folder created
Modal (CreateFolderModal)-->>UI (PagesListRoot): Close modal
User->>UI (PagesListRoot): Select "Add to Folder" on page
UI (PagesListRoot)->>Modal (AddToFolderModal): Open modal
User->>Modal (AddToFolderModal): Select folder and submit
Modal (AddToFolderModal)->>FolderStore: addPageToFolder()
FolderStore->>PageFolderService: addPageToFolder()
PageFolderService-->>FolderStore: Success
FolderStore-->>Modal (AddToFolderModal): Update state
Modal (AddToFolderModal)-->>UI (PagesListRoot): Close modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
web/core/components/pages/list/block-item-action.tsx (2)
46-48: Loss of tooltip functionality.The Info icon is no longer wrapped in a tooltip, reducing user experience by removing the creation date information display.
52-61: Simplified but functional favorite toggle.The native button implementation works but is less polished than the previous
FavoriteStarcomponent. Consider if this change aligns with the design system.web/core/components/pages/folder/folder-item.tsx (1)
79-84: Review accessibility and positioning of remove button.The remove button is positioned absolutely which may cause accessibility issues and could overlap with page content on narrow screens.
Consider alternative approaches for better UX:
- <button - onClick={() => page?.id && handleRemovePageFromFolder(page.id)} - className="absolute right-2 top-1/2 -translate-y-1/2 text-xs text-custom-text-400 hover:text-custom-text-100 transition-colors" - > - Remove - </button> + <div className="flex items-center justify-between"> + <PageListBlock pageId={page?.id || ""} storeType={storeType} /> + <button + onClick={() => page?.id && handleRemovePageFromFolder(page.id)} + className="ml-2 text-xs text-custom-text-400 hover:text-custom-text-100 transition-colors" + aria-label={`Remove ${page?.title || 'page'} from folder`} + > + Remove + </button> + </div>web/core/components/pages/dropdowns/actions.tsx (2)
54-54: Consider improving type safety.The
extraOptionsprop type was changed from a specific type toany, which reduces type safety.If possible, maintain stronger typing:
- extraOptions?: (any & { key: TPageActions })[]; + extraOptions?: Array<{ + key: TPageActions; + action: () => void; + title: string; + icon: React.ComponentType<any>; + shouldRender: boolean; + }>;
193-193: Complete the context menu implementation.The placeholder comment suggests incomplete implementation. Consider either implementing the context menu or removing the placeholder.
Would you like me to help implement the context menu functionality or should this placeholder be removed?
web/core/components/pages/folder/add-to-folder-modal.tsx (1)
51-52: Consider using semantic HTML for better accessibility.The modal implementation uses custom styling instead of semantic dialog elements, which may impact accessibility.
Consider using the HTML dialog element or ARIA attributes:
- <div className="fixed inset-0 z-50 flex items-center justify-center bg-black bg-opacity-50"> - <div className="bg-white rounded-lg shadow-xl w-full max-w-md mx-4"> + <div className="fixed inset-0 z-50 flex items-center justify-center bg-black bg-opacity-50" role="dialog" aria-modal="true" aria-labelledby="modal-title"> + <div className="bg-white rounded-lg shadow-xl w-full max-w-md mx-4">And add an id to the title:
- <h3 className="text-lg font-medium">Add to Folder</h3> + <h3 id="modal-title" className="text-lg font-medium">Add to Folder</h3>web/core/components/pages/folder/create-folder-modal.tsx (1)
111-120: Validate color input format.The color text input allows free-form text but doesn't validate if it's a valid color format, which could cause issues.
Add color format validation:
<input type="text" value={formData.color} onChange={(e: React.ChangeEvent<HTMLInputElement>) => setFormData({ ...formData, color: e.target.value }) } placeholder="#3B82F6" + pattern="^#[0-9A-Fa-f]{6}$" className="flex-1 px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500" />Or add client-side validation in the onChange handler.
web/core/store/pages/page-folder.store.ts (2)
38-38: Consider initializing loader as undefinedThe loader is initialized as
"init-loader"which might cause theisAnyFolderAvailablecomputed property to returntrueeven when no data has been fetched yet. Consider initializing it asundefinedto avoid false positives.- loader: TLoader = "init-loader"; + loader: TLoader = undefined;
105-108: Optimize folder data updatesConsider using a more efficient approach to update multiple folders at once instead of iterating with lodash set.
const folders = await this.service.fetchAll(workspaceSlug, projectId); runInAction(() => { - for (const folder of folders) if (folder?.id) set(this.data, [folder.id], folder); + const folderMap = folders.reduce((acc, folder) => { + if (folder?.id) acc[folder.id] = folder; + return acc; + }, {} as Record<string, TPageFolder>); + Object.assign(this.data, folderMap); this.loader = undefined; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/types/src/pages.d.ts(2 hunks)web/ce/hooks/store/use-page-store.ts(2 hunks)web/core/components/pages/dropdowns/actions.tsx(7 hunks)web/core/components/pages/folder/add-to-folder-modal.tsx(1 hunks)web/core/components/pages/folder/create-folder-modal.tsx(1 hunks)web/core/components/pages/folder/folder-item.tsx(1 hunks)web/core/components/pages/list/block-item-action.tsx(1 hunks)web/core/components/pages/list/root.tsx(2 hunks)web/core/services/page-folder.ts(1 hunks)web/core/store/pages/page-folder.store.ts(1 hunks)web/core/store/root.store.ts(4 hunks)
🔇 Additional comments (21)
web/ce/hooks/store/use-page-store.ts (2)
6-6: LGTM!The import follows the established pattern and integrates well with the existing store structure.
27-32: LGTM!The hook implementation follows the same pattern as
usePageStorewith proper error handling and type safety.web/core/store/root.store.ts (4)
27-27: LGTM!The import is properly placed and follows the existing import structure.
54-54: LGTM!The property declaration follows the established pattern with proper typing.
93-93: LGTM!The store initialization in the constructor is consistent with other stores and uses proper type casting.
126-126: LGTM!The store reset in
resetOnSignOutis properly implemented and consistent with other stores.packages/types/src/pages.d.ts (2)
4-16: Well-structured type definition for folders.The
TPageFoldertype includes all necessary properties for folder management with appropriate optional fields and standard metadata.
38-38: LGTM!The
folder_idproperty properly establishes the relationship between pages and folders.web/core/components/pages/list/block-item-action.tsx (1)
69-69: LGTM!The "add-to-folder" option properly integrates the new folder functionality into the page actions.
web/core/components/pages/list/root.tsx (6)
1-3: LGTM!The imports properly bring in the necessary dependencies for folder functionality.
22-22: LGTM!Proper state management for the create folder modal.
26-26: LGTM!The hook integration follows the established pattern.
33-38: Well-implemented folder-page separation logic.The logic correctly identifies which pages are in folders and which are not, enabling proper organization in the UI.
40-46: LGTM!Clean modal state management functions.
52-86: Well-structured folder organization UI.The component properly renders folders first, followed by a create folder button, and then remaining pages. The modal integration is correctly implemented.
web/core/components/pages/folder/folder-item.tsx (1)
39-39: filter(Boolean) correctly filters out missing pagesThe
getPageByIdfunction is defined as:getPageById = computedFn((pageId: string) => this.data?.[pageId] || undefined);—which means it only ever returns a
TProjectPageinstance orundefined. Usingfilter(Boolean)here cleanly removes anyundefinedentries and preserves all valid page objects. No change is needed.web/core/components/pages/dropdowns/actions.tsx (1)
106-112: LGTM! Well-structured folder action.The new "add-to-folder" action is properly integrated with appropriate conditions (only showing for non-archived pages) and follows the established pattern.
web/core/components/pages/folder/add-to-folder-modal.tsx (1)
80-86: Excellent empty state handling.The component gracefully handles the case when no folders exist with helpful messaging and visual feedback.
web/core/components/pages/folder/create-folder-modal.tsx (1)
34-38: LGTM! Proper input sanitization.The component correctly trims whitespace from inputs and handles empty description by setting it to undefined.
web/core/services/page-folder.ts (2)
94-108: Verify HTTP method for adding pages to folders.Using POST for adding a page to a folder is appropriate, but consider if this should be idempotent (using PUT instead) to handle duplicate additions gracefully.
The API design choice depends on the expected behavior. Should multiple calls to add the same page to the same folder be idempotent?
6-17: LGTM! Clean and consistent API structure.The service methods follow a consistent pattern with proper HTTP methods, error handling, and return types.
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| if (!selectedFolderId) return; | ||
|
|
||
| setIsSubmitting(true); | ||
| try { | ||
| await addPageToFolder(selectedFolderId, pageId); | ||
| setSelectedFolderId(""); | ||
| onClose(); | ||
| } catch (error) { | ||
| console.error("Failed to add page to folder", error); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and user feedback.
Similar to the folder item component, this only logs errors to console without providing user feedback.
Add user-visible error handling:
const handleSubmit = async (e: React.FormEvent) => {
e.preventDefault();
if (!selectedFolderId) return;
setIsSubmitting(true);
try {
await addPageToFolder(selectedFolderId, pageId);
setSelectedFolderId("");
onClose();
+ // Add success notification
} catch (error) {
console.error("Failed to add page to folder", error);
+ // Show error message to user
} finally {
setIsSubmitting(false);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSubmit = async (e: React.FormEvent) => { | |
| e.preventDefault(); | |
| if (!selectedFolderId) return; | |
| setIsSubmitting(true); | |
| try { | |
| await addPageToFolder(selectedFolderId, pageId); | |
| setSelectedFolderId(""); | |
| onClose(); | |
| } catch (error) { | |
| console.error("Failed to add page to folder", error); | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| }; | |
| const handleSubmit = async (e: React.FormEvent) => { | |
| e.preventDefault(); | |
| if (!selectedFolderId) return; | |
| setIsSubmitting(true); | |
| try { | |
| await addPageToFolder(selectedFolderId, pageId); | |
| setSelectedFolderId(""); | |
| onClose(); | |
| // Add success notification | |
| } catch (error) { | |
| console.error("Failed to add page to folder", error); | |
| // Show error message to user | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/core/components/pages/folder/add-to-folder-modal.tsx around lines 25 to
39, the error handling only logs errors to the console without informing the
user. Enhance this by adding user-visible error feedback, such as setting an
error state and displaying an error message in the UI when addPageToFolder
fails, so users are aware of the failure.
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| if (!formData.name.trim()) return; | ||
|
|
||
| setIsSubmitting(true); | ||
| try { | ||
| await createFolder({ | ||
| name: formData.name.trim(), | ||
| description: formData.description.trim() || undefined, | ||
| color: formData.color, | ||
| }); | ||
| setFormData({ name: "", description: "", color: "#3B82F6" }); | ||
| onClose(); | ||
| } catch (error) { | ||
| console.error("Failed to create folder", error); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistent error handling with other modals.
The error handling pattern is the same as in other modals - only console logging without user feedback.
Apply the same improvement pattern as suggested for other modals to provide user feedback on creation success/failure.
🤖 Prompt for AI Agents
In web/core/components/pages/folder/create-folder-modal.tsx around lines 28 to
46, the error handling only logs errors to the console without providing user
feedback. Update the catch block to show an error notification or message to the
user indicating the folder creation failed. Additionally, add a success
notification after the folder is created successfully before calling onClose.
Use the same notification or feedback mechanism as other modals in the project
for consistency.
| const handleRemovePageFromFolder = async (pageId: string) => { | ||
| try { | ||
| await removePageFromFolder(folder.id, pageId); | ||
| } catch (error) { | ||
| console.error("Failed to remove page from folder", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add user feedback for remove operation.
The error handling logs to console but provides no user feedback when removing a page from folder fails. Users won't know if the operation succeeded or failed.
Consider adding a toast notification or error state to inform users about operation status:
const handleRemovePageFromFolder = async (pageId: string) => {
try {
await removePageFromFolder(folder.id, pageId);
+ // Add success feedback here
} catch (error) {
console.error("Failed to remove page from folder", error);
+ // Add user-visible error feedback here
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleRemovePageFromFolder = async (pageId: string) => { | |
| try { | |
| await removePageFromFolder(folder.id, pageId); | |
| } catch (error) { | |
| console.error("Failed to remove page from folder", error); | |
| } | |
| }; | |
| const handleRemovePageFromFolder = async (pageId: string) => { | |
| try { | |
| await removePageFromFolder(folder.id, pageId); | |
| + // Add success feedback here | |
| } catch (error) { | |
| console.error("Failed to remove page from folder", error); | |
| + // Add user-visible error feedback here | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/core/components/pages/folder/folder-item.tsx around lines 31 to 37, the
error handling for removing a page from a folder only logs to the console
without notifying the user. Enhance the function by adding user feedback such as
a toast notification or an error message display to inform users whether the
removal succeeded or failed. Implement this by integrating a notification
mechanism in both the try and catch blocks to clearly communicate the operation
status.
| <div className="cursor-default">{/* Avatar would be implemented here */}</div> | ||
| <div className="cursor-default text-custom-text-300">{/* Access icon would be implemented here */}</div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incomplete implementation with placeholder divs.
The avatar and access icon have been replaced with placeholder divs, which removes functionality and user experience features. Consider implementing proper alternatives or restoring the original components.
🤖 Prompt for AI Agents
In web/core/components/pages/list/block-item-action.tsx at lines 40 to 41, the
placeholder divs for the avatar and access icon remove their intended
functionality and degrade user experience. Replace these placeholder divs with
the original avatar and access icon components or implement suitable
alternatives that restore their visual and interactive features.
| // derived values | ||
| const filteredPageIds = getCurrentProjectFilteredPageIds(pageType); | ||
| const folders = getFoldersByProject(""); // We'll need to get projectId from context |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: Hardcoded empty project ID will not work.
The empty string passed to getFoldersByProject("") will not return the correct folders for the current project. This needs to be resolved before the feature can function properly.
You need to get the project ID from context or props. Consider using a project store hook or passing the project ID as a prop to this component.
🏁 Script executed:
#!/bin/bash
# Description: Search for how project ID is typically accessed in similar components
# Expected: Find patterns for accessing current project ID in list components
rg -A 5 -B 5 "getCurrentProject|projectId|project\.id" --type tsxLength of output: 95
🏁 Script executed:
#!/bin/bash
# Search for getFoldersByProject usage and its definition across the codebase
rg -C3 "getFoldersByProject" -g '*.ts' -g '*.tsx'Length of output: 2668
🏁 Script executed:
#!/bin/bash
# Display the beginning of the component to locate props or hooks for projectId
sed -n '1,200p' web/core/components/pages/list/root.tsxLength of output: 2970
Critical: Replace hardcoded "" with actual projectId from route or context
The empty string passed to getFoldersByProject("") prevents retrieving any folders. You need to obtain the current projectId—for example via your routing params or a project store hook—and pass it in.
Locations to update:
web/core/components/pages/list/root.tsx, line where you callgetFoldersByProject("")
Suggested change (using React Router‘s useParams, adjust if you use Next.js or another router):
-import { FC, useState } from "react";
+import { FC, useState } from "react";
+import { useParams } from "react-router-dom"; // or your framework’s hook
// ...
export const PagesListRoot: FC<TPagesListRoot> = observer((props) => {
// extract projectId from URL params
- const { pageType, storeType } = props;
+ const { pageType, storeType } = props;
+ const { projectId } = useParams<{ projectId: string }>();
const [isCreateFolderModalOpen, setIsCreateFolderModalOpen] = useState(false);
// store hooks
const { getCurrentProjectFilteredPageIds } = usePageStore(storeType);
const { getFoldersByProject } = usePageFolderStore();
// derived values
const filteredPageIds = getCurrentProjectFilteredPageIds(pageType);
- const folders = getFoldersByProject(""); // We'll need to get projectId from context
+ const folders = getFoldersByProject(projectId!);If your routing or context differs (e.g. Next.js useRouter or a custom project store), replace useParams with the appropriate hook. Ensure projectId is non-empty before calling getFoldersByProject.
🤖 Prompt for AI Agents
In web/core/components/pages/list/root.tsx at line 30, replace the hardcoded
empty string argument in getFoldersByProject("") with the actual projectId
obtained from the route parameters or context. Use a hook like React Router's
useParams or your framework's equivalent to retrieve projectId, ensure it is
defined and non-empty, then pass it to getFoldersByProject to correctly fetch
folders for the current project.
| if (!response.ok) { | ||
| throw new Error("Failed to fetch page folder"); | ||
| } | ||
|
|
||
| return response.json(); | ||
| } | ||
|
|
||
| async create(workspaceSlug: string, projectId: string, folderData: Partial<TPageFolder>): Promise<TPageFolder> { | ||
| const response = await fetch(`${API_BASE_URL}/api/workspaces/${workspaceSlug}/projects/${projectId}/page-folders`, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(folderData), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to create page folder"); | ||
| } | ||
|
|
||
| return response.json(); | ||
| } | ||
|
|
||
| async update( | ||
| workspaceSlug: string, | ||
| projectId: string, | ||
| folderId: string, | ||
| folderData: Partial<TPageFolder> | ||
| ): Promise<TPageFolder> { | ||
| const response = await fetch( | ||
| `${API_BASE_URL}/api/workspaces/${workspaceSlug}/projects/${projectId}/page-folders/${folderId}`, | ||
| { | ||
| method: "PATCH", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(folderData), | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to update page folder"); | ||
| } | ||
|
|
||
| return response.json(); | ||
| } | ||
|
|
||
| async remove(workspaceSlug: string, projectId: string, folderId: string): Promise<void> { | ||
| const response = await fetch( | ||
| `${API_BASE_URL}/api/workspaces/${workspaceSlug}/projects/${projectId}/page-folders/${folderId}`, | ||
| { | ||
| method: "DELETE", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to delete page folder"); | ||
| } | ||
| } | ||
|
|
||
| async addPageToFolder(workspaceSlug: string, projectId: string, folderId: string, pageId: string): Promise<void> { | ||
| const response = await fetch( | ||
| `${API_BASE_URL}/api/workspaces/${workspaceSlug}/projects/${projectId}/page-folders/${folderId}/pages/${pageId}`, | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to add page to folder"); | ||
| } | ||
| } | ||
|
|
||
| async removePageFromFolder( | ||
| workspaceSlug: string, | ||
| projectId: string, | ||
| folderId: string, | ||
| pageId: string | ||
| ): Promise<void> { | ||
| const response = await fetch( | ||
| `${API_BASE_URL}/api/workspaces/${workspaceSlug}/projects/${projectId}/page-folders/${folderId}/pages/${pageId}`, | ||
| { | ||
| method: "DELETE", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to remove page from folder"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add authentication headers and improve error handling.
The service methods are missing authentication headers that are likely required for API calls, and error messages could be more descriptive.
Consider these improvements:
- Add authentication headers:
headers: {
"Content-Type": "application/json",
+ "Authorization": `Bearer ${token}`, // or however auth is handled
},- Improve error handling with more specific error messages:
if (!response.ok) {
- throw new Error("Failed to fetch page folders");
+ const errorData = await response.json().catch(() => ({}));
+ throw new Error(errorData.message || `Failed to fetch page folders (${response.status})`);
}Let me check how authentication is typically handled in this codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check how other services handle authentication headers
# Search for existing service files to see auth patterns
rg -A 5 -B 5 "Authorization|Bearer|headers.*:" web/core/services/
# Look for auth helper functions
rg "auth|token" web/helpers/Length of output: 12427
#!/bin/bash
Show common API_BASE_URL helper
if [ -f web/helpers/common.helper.ts ]; then
sed -n '1,200p' web/helpers/common.helper.ts
else
echo "common.helper.ts not found"
fi
echo -e "\n---\n"
Show authentication helper to see if there's a function to get auth headers
if [ -f web/helpers/authentication.helper.tsx ]; then
sed -n '1,200p' web/helpers/authentication.helper.tsx
else
echo "authentication.helper.tsx not found"
fi```
🏁 Script executed:
#!/bin/bash
# Inspect how workspace.service sets up HTTP client and auth
sed -n '1,50p' web/core/services/workspace.service.ts
# Locate BaseService definition if it exists
rg -l "class .*Service" -n web/core/services | grep base.service.ts || echo "No base.service.ts in services"
# If BaseService exists, show its content
if [ -f web/core/services/base.service.ts ]; then
sed -n '1,200p' web/core/services/base.service.ts
fiLength of output: 1607
🏁 Script executed:
#!/bin/bash
# Locate APIService implementation
file=$(rg -l "class APIService" -n . | head -n1)
echo "APIService found at: $file"
echo "-----"
sed -n '1,200p' "$file"Length of output: 4277
Unify PageFolderService with APIService for consistent authentication and error handling
PageFolderService currently uses the native fetch API, bypassing our centralized axios instance (APIService) that automatically includes credentials, intercepts 401s, and normalizes errors. To align with the rest of the codebase, refactor this class to extend APIService:
• Change the class signature and constructor:
-export class PageFolderService {
+export class PageFolderService extends APIService {
+ constructor() {
+ super(API_BASE_URL);
+ }• Replace each fetch call with the corresponding axios helper, e.g.:
- async fetchAll(ws: string, proj: string): Promise<TPageFolder[]> {
- const response = await fetch(`${API_BASE_URL}/api/workspaces/${ws}/projects/${proj}/page-folders`, {
- method: "GET",
- headers: { "Content-Type": "application/json" },
- });
- if (!response.ok) {
- const err = await response.json().catch(() => ({}));
- throw new Error(err.message || `Failed to fetch page folders (${response.status})`);
- }
- return response.json();
- }
+ async fetchAll(workspaceSlug: string, projectId: string): Promise<TPageFolder[]> {
+ return this.get(`/api/workspaces/${workspaceSlug}/projects/${projectId}/page-folders/`)
+ .then((res) => res.data)
+ .catch((error) => {
+ throw error?.response?.data;
+ });
+ }• Apply the same pattern for fetchById, create, update, remove, addPageToFolder, removePageFromFolder.
This ensures:
withCredentials: trueis applied automatically.- 401 responses redirect via the interceptor.
- Error payloads are thrown as
error.response.datafor consistency.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/core/services/page-folder.ts lines 1 to 130, refactor PageFolderService
to extend the centralized APIService class instead of using native fetch calls.
Replace all fetch calls with the corresponding axios methods provided by
APIService, ensuring requests automatically include credentials and benefit from
interceptors handling 401 responses and error normalization. Update the class
signature to extend APIService and remove manual headers and error handling,
relying on APIService's built-in mechanisms to throw error.response.data for
consistency across all methods including fetchAll, fetchById, create, update,
remove, addPageToFolder, and removePageFromFolder.
| get isAnyFolderAvailable() { | ||
| if (this.loader) return true; | ||
| return Object.keys(this.data).length > 0; | ||
| } |
There was a problem hiding this comment.
Misleading computed property logic
The isAnyFolderAvailable property returns true when the loader is active, which contradicts its name. This could cause UI components to incorrectly assume folders are available during loading states.
get isAnyFolderAvailable() {
- if (this.loader) return true;
- return Object.keys(this.data).length > 0;
+ return Object.keys(this.data).length > 0;
}If you need to check loading state separately, consider adding a dedicated computed property:
get isLoading() {
return this.loader !== undefined;
}🤖 Prompt for AI Agents
In web/core/store/pages/page-folder.store.ts around lines 73 to 76, the computed
property isAnyFolderAvailable incorrectly returns true when the loader is
active, which misleads consumers into thinking folders are available during
loading. Modify isAnyFolderAvailable to only check if this.data has keys and
does not consider the loader state. Add a separate computed property isLoading
that returns true when the loader is active to clearly distinguish loading state
from folder availability.
| removeFolder = async (folderId: string) => { | ||
| try { | ||
| const { workspaceSlug, projectId } = this.store.router; | ||
| if (!workspaceSlug || !projectId || !folderId) return undefined; | ||
|
|
||
| await this.service.remove(workspaceSlug, projectId, folderId); | ||
| runInAction(() => { | ||
| unset(this.data, [folderId]); | ||
| }); | ||
| } catch (error) { | ||
| runInAction(() => { | ||
| this.loader = undefined; | ||
| this.error = { | ||
| title: "Failed", | ||
| description: "Failed to delete a folder, Please try again later.", | ||
| }; | ||
| }); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add loader state management and fix return type
The removeFolder method lacks loader state management unlike other async methods, and has an inconsistent return type.
removeFolder = async (folderId: string) => {
try {
const { workspaceSlug, projectId } = this.store.router;
- if (!workspaceSlug || !projectId || !folderId) return undefined;
+ if (!workspaceSlug || !projectId || !folderId) return;
+
+ runInAction(() => {
+ this.loader = "mutation-loader";
+ this.error = undefined;
+ });
await this.service.remove(workspaceSlug, projectId, folderId);
runInAction(() => {
unset(this.data, [folderId]);
+ this.loader = undefined;
});
} catch (error) {
runInAction(() => {
this.loader = undefined;
this.error = {
title: "Failed",
description: "Failed to delete a folder, Please try again later.",
};
});
throw error;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| removeFolder = async (folderId: string) => { | |
| try { | |
| const { workspaceSlug, projectId } = this.store.router; | |
| if (!workspaceSlug || !projectId || !folderId) return undefined; | |
| await this.service.remove(workspaceSlug, projectId, folderId); | |
| runInAction(() => { | |
| unset(this.data, [folderId]); | |
| }); | |
| } catch (error) { | |
| runInAction(() => { | |
| this.loader = undefined; | |
| this.error = { | |
| title: "Failed", | |
| description: "Failed to delete a folder, Please try again later.", | |
| }; | |
| }); | |
| throw error; | |
| } | |
| }; | |
| removeFolder = async (folderId: string) => { | |
| try { | |
| const { workspaceSlug, projectId } = this.store.router; | |
| if (!workspaceSlug || !projectId || !folderId) return; | |
| runInAction(() => { | |
| this.loader = "mutation-loader"; | |
| this.error = undefined; | |
| }); | |
| await this.service.remove(workspaceSlug, projectId, folderId); | |
| runInAction(() => { | |
| unset(this.data, [folderId]); | |
| this.loader = undefined; | |
| }); | |
| } catch (error) { | |
| runInAction(() => { | |
| this.loader = undefined; | |
| this.error = { | |
| title: "Failed", | |
| description: "Failed to delete a folder, Please try again later.", | |
| }; | |
| }); | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/core/store/pages/page-folder.store.ts around lines 226 to 245, the
removeFolder method is missing loader state management and has an inconsistent
return type. Add setting this.loader to true at the start of the method and
reset it to undefined in both success and error paths to manage loading state
properly. Also, ensure the method consistently returns a value or undefined by
adding an explicit return statement after successful removal or when early
returning due to missing parameters.
| addPageToFolder = async (folderId: string, pageId: string) => { | ||
| try { | ||
| const { workspaceSlug, projectId } = this.store.router; | ||
| if (!workspaceSlug || !projectId || !folderId || !pageId) return undefined; | ||
|
|
||
| await this.service.addPageToFolder(workspaceSlug, projectId, folderId, pageId); | ||
| runInAction(() => { | ||
| const folder = this.data[folderId]; | ||
| if (folder && folder.page_ids) { | ||
| if (!folder.page_ids.includes(pageId)) { | ||
| folder.page_ids.push(pageId); | ||
| } | ||
| } else if (folder) { | ||
| folder.page_ids = [pageId]; | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| console.error("Failed to add page to folder", error); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add proper state management and error handling
The method lacks loader/error state management and uses console.error which may not be appropriate for production.
addPageToFolder = async (folderId: string, pageId: string) => {
try {
const { workspaceSlug, projectId } = this.store.router;
- if (!workspaceSlug || !projectId || !folderId || !pageId) return undefined;
+ if (!workspaceSlug || !projectId || !folderId || !pageId) return;
+
+ runInAction(() => {
+ this.loader = "mutation-loader";
+ this.error = undefined;
+ });
await this.service.addPageToFolder(workspaceSlug, projectId, folderId, pageId);
runInAction(() => {
const folder = this.data[folderId];
if (folder && folder.page_ids) {
if (!folder.page_ids.includes(pageId)) {
folder.page_ids.push(pageId);
}
} else if (folder) {
folder.page_ids = [pageId];
}
+ this.loader = undefined;
});
} catch (error) {
- console.error("Failed to add page to folder", error);
+ runInAction(() => {
+ this.loader = undefined;
+ this.error = {
+ title: "Failed",
+ description: "Failed to add page to folder, Please try again later.",
+ };
+ });
throw error;
}
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/core/store/pages/page-folder.store.ts around lines 252 to 272, the
addPageToFolder method lacks proper loader and error state management and uses
console.error for error logging, which is not suitable for production. Introduce
observable state variables to track loading and error states, set the loading
state to true at the start and false at the end of the operation, and update the
error state appropriately on failure. Replace console.error with a proper
logging mechanism or error handling approach consistent with the application's
standards.
| removePageFromFolder = async (folderId: string, pageId: string) => { | ||
| try { | ||
| const { workspaceSlug, projectId } = this.store.router; | ||
| if (!workspaceSlug || !projectId || !folderId || !pageId) return undefined; | ||
|
|
||
| await this.service.removePageFromFolder(workspaceSlug, projectId, folderId, pageId); | ||
| runInAction(() => { | ||
| const folder = this.data[folderId]; | ||
| if (folder && folder.page_ids) { | ||
| folder.page_ids = folder.page_ids.filter((id) => id !== pageId); | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| console.error("Failed to remove page from folder", error); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add proper state management and error handling
Similar to addPageToFolder, this method lacks loader/error state management and uses console.error.
removePageFromFolder = async (folderId: string, pageId: string) => {
try {
const { workspaceSlug, projectId } = this.store.router;
- if (!workspaceSlug || !projectId || !folderId || !pageId) return undefined;
+ if (!workspaceSlug || !projectId || !folderId || !pageId) return;
+
+ runInAction(() => {
+ this.loader = "mutation-loader";
+ this.error = undefined;
+ });
await this.service.removePageFromFolder(workspaceSlug, projectId, folderId, pageId);
runInAction(() => {
const folder = this.data[folderId];
if (folder && folder.page_ids) {
folder.page_ids = folder.page_ids.filter((id) => id !== pageId);
}
+ this.loader = undefined;
});
} catch (error) {
- console.error("Failed to remove page from folder", error);
+ runInAction(() => {
+ this.loader = undefined;
+ this.error = {
+ title: "Failed",
+ description: "Failed to remove page from folder, Please try again later.",
+ };
+ });
throw error;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| removePageFromFolder = async (folderId: string, pageId: string) => { | |
| try { | |
| const { workspaceSlug, projectId } = this.store.router; | |
| if (!workspaceSlug || !projectId || !folderId || !pageId) return undefined; | |
| await this.service.removePageFromFolder(workspaceSlug, projectId, folderId, pageId); | |
| runInAction(() => { | |
| const folder = this.data[folderId]; | |
| if (folder && folder.page_ids) { | |
| folder.page_ids = folder.page_ids.filter((id) => id !== pageId); | |
| } | |
| }); | |
| } catch (error) { | |
| console.error("Failed to remove page from folder", error); | |
| throw error; | |
| } | |
| }; | |
| removePageFromFolder = async (folderId: string, pageId: string) => { | |
| try { | |
| const { workspaceSlug, projectId } = this.store.router; | |
| if (!workspaceSlug || !projectId || !folderId || !pageId) return; | |
| runInAction(() => { | |
| this.loader = "mutation-loader"; | |
| this.error = undefined; | |
| }); | |
| await this.service.removePageFromFolder(workspaceSlug, projectId, folderId, pageId); | |
| runInAction(() => { | |
| const folder = this.data[folderId]; | |
| if (folder && folder.page_ids) { | |
| folder.page_ids = folder.page_ids.filter((id) => id !== pageId); | |
| } | |
| this.loader = undefined; | |
| }); | |
| } catch (error) { | |
| runInAction(() => { | |
| this.loader = undefined; | |
| this.error = { | |
| title: "Failed", | |
| description: "Failed to remove page from folder, Please try again later.", | |
| }; | |
| }); | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/core/store/pages/page-folder.store.ts around lines 279 to 295, the
removePageFromFolder method lacks proper loader and error state management and
uses console.error for error logging. Refactor the method to set a loading state
before the async call and clear it afterward, handle errors by updating an error
state instead of console.error, and ensure the UI can react to these state
changes consistently like in addPageToFolder.
Description
Implemented folder/grouping functionality for pages to allow users to organize pages into folders for better discoverability and organization. This feature addresses the need for organizing pages in projects with many pages and long-running projects.
#7511
Key Features:
Technical Implementation:
TPageFoldertype andfolder_idfield toTPagetypePageFolderStorefor folder state managementPageFolderServicefor API operationsFolderItem,CreateFolderModal,AddToFolderModalSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores