feat: add policy layout components and enhance policy details page - #273
Conversation
- Introduced new layout components for policy overview and individual policy pages, improving structure and navigation. - Updated PolicyDetails page to include a PolicyPageEditor for editing policy content. - Removed deprecated PolicyEditor component to streamline the codebase. - Implemented responsive design with Tailwind CSS for better user experience.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes update the policy editing functionality and layout structure across multiple files. A key function export has been renamed from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PolicyPageEditor
participant SaveHandler
User->>PolicyPageEditor: Edit and update policy content
PolicyPageEditor->>SaveHandler: Invoke handleSavePolicy()
SaveHandler-->>PolicyPageEditor: Return save status
PolicyPageEditor-->>User: Display update result
sequenceDiagram
participant Browser
participant Layout
participant i18nService
participant SecondaryMenu
Browser->>Layout: Request policy page
Layout->>i18nService: Fetch internationalization strings
i18nService-->>Layout: Return localized content
Layout->>SecondaryMenu: Render navigation items
Layout-->>Browser: Display layout with children content
Possibly related PRs
Poem
✨ Finishing Touches
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:
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: 2
🧹 Nitpick comments (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx (1)
11-11: Unused variable in the componentThe
policyIdis extracted from params but not used in the component rendering. Consider removing it if not needed.- const { orgId, policyId } = await params; + const { orgId } = await params;apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx (1)
24-29: Consider improving error handlingThe current error handling logs the error but then rethrows it, which could lead to unhandled promise rejections if the caller doesn't catch errors. Consider adding a more user-friendly error handling approach.
try { await updatePolicy({ policyId, content: policyContent }); } catch (error) { console.error("Error saving policy:", error); - throw error; + // Consider adding toast notification or similar user feedback + // toast.error("Failed to save policy. Please try again."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/page.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/layout.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/page.tsx
🧰 Additional context used
🧬 Code Definitions (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/actions/update-policy.ts (1)
updatePolicy(31-132)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/layout.tsx (1)
Layout(9-31)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/layout.tsx (1)
Layout(9-31)
🔇 Additional comments (7)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx (2)
1-31: Well-structured layout component with consistent designThis new layout component follows the established pattern seen in other policy layout files, providing a consistent navigation structure with internationalization support. The secondary menu implementation ensures users can easily navigate between policy views.
4-7:Details
❓ Verification inconclusive
Consider revising the params type definition
The
paramsproperty is defined as a Promise, but it's worth checking if this is necessary. In Next.js 13+, route params are typically directly available as objects rather than Promises.
🏁 Script executed:
#!/bin/bash # Check other layout components in the project to verify params pattern rg -A 1 -B 1 "interface.*Props" --glob "**/*layout.tsx" | grep -A 3 "params"Length of output: 76
Action Required: Revisit the 'params' Type Definition
The initial automated check did not reveal any similar usage of a Promise-wrapped params in other layout components. In Next.js 13, route parameters are usually directly available as objects rather than Promises. Please verify manually whether the Promise wrapper is intentional in this context or if the type should be updated to a plain object:
interface LayoutProps { children: React.ReactNode; params: { policyId: string; orgId: string }; }Ensure to check any related logic that might expect the async resolution. If no asynchronous behavior is needed, consider revising accordingly.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/layout.tsx (1)
9-31: Layout implementation looks goodThe layout provides consistent navigation across policy views and properly handles internationalization. The component correctly extracts only the
orgIdfrom params.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx (2)
9-12: Appropriate imports for the updated componentThe added imports for
JSONContentandPolicyPageEditorsupport the enhanced functionality of the policy page.
13-38: Good integration of policy view and edit componentsThe page now properly integrates both the policy overview and editor components, providing a complete policy management interface. The breadcrumb navigation is also well-implemented.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx (2)
12-12: Component renaming improves clarityRenaming from
PolicyDetailstoPolicyPageEditorbetter reflects the component's purpose and aligns with its usage in the page component.
33-33: Styling enhancement with border and paddingThe styling change adds visual structure around the editor, making it clearer where the editable area begins and ends.
| interface LayoutProps { | ||
| children: React.ReactNode; | ||
| params: Promise<{ policyId: string; orgId: string }>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Revise the params interface for the all policies layout
The LayoutProps interface includes policyId, but this isn't applicable for the "all policies" view which doesn't have a specific policy ID in its route parameters.
interface LayoutProps {
children: React.ReactNode;
- params: Promise<{ policyId: string; orgId: string }>;
+ params: Promise<{ orgId: string }>;
}📝 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.
| interface LayoutProps { | |
| children: React.ReactNode; | |
| params: Promise<{ policyId: string; orgId: string }>; | |
| } | |
| interface LayoutProps { | |
| children: React.ReactNode; | |
| params: Promise<{ orgId: string }>; | |
| } |
| <PolicyPageEditor | ||
| policyId={policyId} | ||
| policyContent={policy?.content as JSONContent[]} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for policy content
The code casts policy?.content directly to JSONContent[] without verification. This might cause runtime errors if the content is null or undefined.
<PolicyPageEditor
policyId={policyId}
- policyContent={policy?.content as JSONContent[]}
+ policyContent={policy?.content ? (policy.content as JSONContent[]) : []}
/>📝 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.
| <PolicyPageEditor | |
| policyId={policyId} | |
| policyContent={policy?.content as JSONContent[]} | |
| /> | |
| <PolicyPageEditor | |
| policyId={policyId} | |
| policyContent={policy?.content ? (policy.content as JSONContent[]) : []} | |
| /> |
This branch was successfully deployed
1 participant
Summary by CodeRabbit
New Features
Style
Removed Features