Skip to content

refactor: streamline policy creation and editing components, enhance …#246

Merged
carhartlewis merged 2 commits into
mainfrom
lewis/shared-tables
Apr 3, 2025
Merged

refactor: streamline policy creation and editing components, enhance …#246
carhartlewis merged 2 commits into
mainfrom
lewis/shared-tables

Conversation

@carhartlewis

@carhartlewis carhartlewis commented Apr 3, 2025

Copy link
Copy Markdown
Contributor

…validation logic, and improve UI responsiveness

Summary by CodeRabbit

  • New Features

    • Enhanced policy management with updated table columns showing "Policy Name" and a new "Last Updated" field.
    • Introduced a new toolbar button for creating policies and clickable rows that simplify navigation.
    • Improved filtering capabilities for policies by status.
    • Expanded rich-text editor functionality with additional formatting options.
  • Style

    • Refined input fields and pagination displays with clearer item counts and responsive icon visibility.
    • Updated form labels for clearer user actions.

…validation logic, and improve UI responsiveness
@vercel

vercel Bot commented Apr 3, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
app ❌ Failed (Inspect) Apr 3, 2025 4:18pm
comp-portal ❌ Failed (Inspect) Apr 3, 2025 4:18pm

@coderabbitai

coderabbitai Bot commented Apr 3, 2025

Copy link
Copy Markdown

Walkthrough

This pull request makes a series of modifications across policy-related files. The changes include cosmetic formatting updates (e.g., switching from two-space to tab indentation), refactoring and simplification of component props and hooks, and minor UI adjustments such as renaming columns and updating button labels. New helper functions (like cleanContent) and asynchronous routines for fetching policies were introduced. Additionally, query filtering has been enhanced and import paths adjusted for improved module resolution, with redundant code and unused variables removed.

Changes

File(s) Change Summary
apps/app/src/actions/.../create-new-policy.ts,
apps/app/src/app/[locale]/.../data/queries.ts,
apps/app/src/lib/validations.ts,
apps/app/src/app/[locale]/.../[policyId]/editor/actions/update-policy.ts
Adjusted policy creation formatting and validation; added a new cleanContent helper to sanitize update input; enhanced query filtering (status check) and removed unused schema fields.
apps/app/src/app/[locale]/.../components/policies-table-columns.tsx,
apps/app/src/app/[locale]/.../components/policies-table.tsx,
apps/app/src/app/[locale]/.../(overview)/page.tsx,
apps/app/src/app/[locale]/.../[policyId]/page.tsx
Modified policy table UI: renamed "Name" to "Policy Name", integrated a new "Last Updated" column, added CreatePolicySheet support, and simplified usage of Suspense and the getPolicy routine.
apps/app/src/app/[locale]/.../[policyId]/editor/components/PolicyDetails.tsx,
apps/app/src/app/[locale]/.../[policyId]/editor/page.tsx,
apps/app/src/app/[locale]/.../[policyId]/layout.tsx
Refactored PolicyDetails to accept a policyContent prop instead of fetching data via a hook; introduced an async getPolicy function for retrieving policy details; removed redundant session and policy checks from layout.
apps/app/src/app/[locale]/.../employees/[employeeId]/layout.tsx Removed the unused assignment of the user variable from the session.
apps/app/src/components/.../data-table-pagination.tsx,
.../data-table-sort-list.tsx,
.../data-table-toolbar.tsx,
.../data-table.tsx,
apps/app/src/components/editor/advanced-editor.tsx,
.../editor/extensions.ts,
.../editor/policy-editor.tsx,
apps/app/src/components/forms/policies/create-new-policy.tsx,
.../forms/policies/policy-overview.tsx,
apps/app/src/components/policies/policy-overview.tsx,
packages/ui/src/components/input.tsx
Updated various UI components and editors: added an item count in pagination, refined responsive icon display, introduced new toolbar props (sheet and action), enabled row-click navigation via a new getRowId property, reformatted editor code, updated form button labels from "save" to "create", removed a user selection component, and streamlined input styling.

Sequence Diagram(s)

Policy Update Flow

sequenceDiagram
    participant User
    participant PolicyEditor
    participant getPolicy
    participant DB
    participant UpdatePolicy
    participant cleanContent

    User->>PolicyEditor: Open Policy Editor Page
    PolicyEditor->>getPolicy: Request policy detail (by ID)
    getPolicy->>DB: Query policy (with session/org check)
    DB-->>getPolicy: Return policy data
    getPolicy-->>PolicyEditor: Send policyContent
    PolicyEditor->>User: Render editor with policyContent
    User->>PolicyEditor: Modify content and click Save
    PolicyEditor->>UpdatePolicy: Submit updated content
    UpdatePolicy->>cleanContent: Clean the content payload
    cleanContent-->>UpdatePolicy: Return sanitized content
    UpdatePolicy->>DB: Update policy record
    DB-->>UpdatePolicy: Confirm update
    UpdatePolicy-->>PolicyEditor: Return update success
Loading

DataTable Row Navigation

sequenceDiagram
    participant User
    participant DataTable
    participant getRowId

    User->>DataTable: Click on a table row
    DataTable->>getRowId: Retrieve row's unique ID
    getRowId-->>DataTable: Return row ID
    DataTable->>User: Navigate to policy details page using row ID
Loading

Possibly related PRs

Suggested labels

enhancement

Poem

Oh, I’m a rabbit with hops so fleet,
Dancing through code with a rhythmic beat.
Formatting changes and features so bright,
Policies updated from morning till night.
In each little line, my joy is replete—
Happy code and carrots, all perfectly neat! 🐰🌟

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
apps/app/src/components/editor/advanced-editor.tsx (2)

28-36: Well-Defined Props Interface
The AdvancedEditorProps interface now lists the component’s configurable properties in a clear, consistent fashion. One suggestion is to explicitly type the charsCount state (declared later) for added type safety.


47-56: State Initialization and Unused State Warning

  • The states for saveStatus, initialLoadComplete, and UI toggles (e.g., openNode, openLink, openAI) are well declared.
  • Unused State: The state variable openColor is declared (line 54) but is not used anywhere in the component. If it is not planned for future use, consider removing it to reduce clutter.
  • Initialization Note: The charsCount state is declared without an initial value. If this state is meant to represent a numeric word count, initializing it as 0 (and typing it accordingly) could improve clarity and prevent potential issues when evaluating its truthiness.

A suggested diff for charsCount is as follows:

-  const [charsCount, setCharsCount] = useState();
+  const [charsCount, setCharsCount] = useState<number>(0);
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/page.tsx (2)

20-22: Type assertion for policy content should be validated

The type assertion policy?.content as JSONContent[] bypasses TypeScript's type checking. Consider adding runtime validation before the assertion to ensure type safety.

- return <PolicyDetails policyId={policyId} policyContent={policy?.content as JSONContent[]} />;
+ // Validate content structure before passing it
+ const policyContent = policy?.content && Array.isArray(policy.content) 
+   ? policy.content as JSONContent[]
+   : undefined;
+ return <PolicyDetails policyId={policyId} policyContent={policyContent} />;

40-60: Well-implemented policy retrieval with caching

The getPolicy function is well structured with:

  • Proper use of React's cache for performance optimization
  • Authentication and authorization checks
  • Clear error handling with redirect for missing policies
  • Clean separation of concerns

The redirect to root ("/") when a policy is not found is appropriate but consider providing a more descriptive error or notification to explain why the user was redirected.

if (!policy) {
-  return redirect("/");
+  // Consider redirecting to a more specific path with error information
+  return redirect("/?error=policy-not-found");
}
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/page.tsx (1)

42-62: Policy retrieval behavior differs from editor page

This implementation of getPolicy returns null when a policy is not found, while the editor page implementation redirects to "/". Consider standardizing this behavior across both files for consistency, unless the different behavior is intentional.

Could you confirm if the different handling of missing policies (redirect vs. returning null) between the editor and overview pages is intentional? If not, consider unifying the approach:

if (!policy) {
-  return null;
+  // Option 1: Match editor page behavior with redirect
+  return redirect("/?error=policy-not-found");
  
  // OR
  
  // Option 2: Update editor page to return null instead of redirecting
  // and handle the null case in the component
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96045b2 and 879b3ae.

📒 Files selected for processing (24)
  • apps/app/src/actions/policies/create-new-policy.ts (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/[employeeId]/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/components/policies-table-columns.tsx (3 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/components/policies-table.tsx (2 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/data/queries.ts (2 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/data/validations.ts (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/page.tsx (3 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/actions/update-policy.ts (3 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/components/PolicyDetails.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/page.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/page.tsx (1 hunks)
  • apps/app/src/components/data-table/data-table-pagination.tsx (1 hunks)
  • apps/app/src/components/data-table/data-table-sort-list.tsx (1 hunks)
  • apps/app/src/components/data-table/data-table-toolbar.tsx (5 hunks)
  • apps/app/src/components/data-table/data-table.tsx (4 hunks)
  • apps/app/src/components/editor/advanced-editor.tsx (2 hunks)
  • apps/app/src/components/editor/extensions.ts (1 hunks)
  • apps/app/src/components/editor/policy-editor.tsx (1 hunks)
  • apps/app/src/components/forms/policies/create-new-policy.tsx (1 hunks)
  • apps/app/src/components/forms/policies/policy-overview.tsx (0 hunks)
  • apps/app/src/components/policies/policy-overview.tsx (1 hunks)
  • apps/app/src/lib/validations.ts (1 hunks)
  • packages/ui/src/components/input.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/app/src/components/forms/policies/policy-overview.tsx
🧰 Additional context used
🧬 Code Definitions (7)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/components/policies-table.tsx (3)
apps/app/src/components/data-table/data-table-toolbar.tsx (1)
  • DataTableToolbar (22-84)
apps/app/src/components/data-table/data-table-sort-list.tsx (1)
  • DataTableSortList (52-262)
apps/app/src/components/data-table/data-table.tsx (1)
  • DataTable (23-126)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/data/validations.ts (1)
apps/app/src/lib/validations.ts (2)
  • searchParamsCache (12-25)
  • GetPolicySchema (32-34)
apps/app/src/components/editor/advanced-editor.tsx (2)
apps/app/src/components/editor/slash-command.tsx (1)
  • suggestionItems (16-113)
apps/app/src/components/editor/selectors/node-selector.tsx (1)
  • NodeSelector (93-133)
apps/app/src/actions/policies/create-new-policy.ts (1)
apps/app/src/lib/validations.ts (1)
  • createPolicySchema (27-30)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/components/PolicyDetails.tsx (3)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/actions/update-policy.ts (1)
  • updatePolicy (31-132)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/page.tsx (1)
  • PolicyEditor (12-23)
apps/app/src/components/editor/policy-editor.tsx (1)
  • PolicyEditor (13-51)
apps/app/src/components/policies/policy-overview.tsx (2)
apps/app/src/components/forms/policies/policy-overview.tsx (1)
  • UpdatePolicyOverview (48-287)
apps/app/src/components/policies/sheets/policy-overview-sheet.tsx (1)
  • PolicyOverviewSheet (20-76)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/page.tsx (1)
apps/app/src/components/policies/policy-overview.tsx (1)
  • PolicyOverview (17-119)
🔇 Additional comments (57)
apps/app/src/components/editor/advanced-editor.tsx (10)

1-13: Consistent Reformatting of Imports
The import block has been reformatted neatly, ensuring that imported components from "novel" (and others) are clearly aligned and easy to read. There’s no change in functionality, and the adjustments improve consistency.


38-46: Clear Props Destructuring in Component Declaration
The destructuring of props in the AdvancedEditor component is formatted cleanly, with sensible defaults (e.g., readOnly = false, placeholder = "Start writing...", saveDebounceMs = 500). This improves readability while preserving functionality.


58-60: Effect Hook Usage is Correct
The useEffect hook that sets initialLoadComplete to true on component mount is concise and works as expected.


62-74: Debounced Save Handler Implementation
The implementation of the debouncedSave function is solid. It correctly updates the save status, performs an asynchronous save with proper error handling, and leverages the useDebouncedCallback hook.


76-76: Guard Clause for Initial Content
The early return with if (!initialContent) return null; is a clear defensive check ensuring that the editor isn’t rendered without content. Confirm that this behavior aligns with your design requirements.


78-93: Editor Status Display is Clear
The JSX for displaying the save status and word count is structured well. The conditional rendering based on charsCount is easy to follow, providing immediate visual feedback to the user.


94-121: Rich Editor Configuration and Update Logic
The EditorContent is configured with multiple properties including initialContent, extensions, and a comprehensive onUpdate callback that:

  • Retrieves the latest JSON content from the editor,
  • Updates the character/word count using editor.storage.characterCount.words(),
  • Calls the external onUpdate prop if provided, and
  • Manages debounced saving based on the initialLoadComplete flag.

Note: Although the same name onUpdate is used both for the prop and the callback parameter, the closure ensures correctness. If there’s any risk of confusion in the future, consider renaming the internal callback variable.


123-147: Command Palette Implementation is Well-Formatted
The command palette section—rendering components like EditorCommand, EditorCommandEmpty, and EditorCommandList alongside mapping over the suggestionItems—is clearly structured. This refactored style supports maintainability and ensures the UI remains consistent.


148-160: Structured Menu and Selector Layout
The arrangement of the generative menu and various selectors (NodeSelector, LinkSelector, MathSelector, TextButtons) is clear and provides a logical grouping of related UI actions. The consistent use of Separator components further enhances readability.


167-168: Clean Component Export
The export of the AdvancedEditor component with a default export is straightforward and adheres to common conventions.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/[employeeId]/layout.tsx (1)

11-16: Code cleanup: Removed unused variable extraction.

The removal of the unused user variable extraction from the session object streamlines the component and removes unnecessary code, which aligns with the overall refactoring goal.

apps/app/src/components/data-table/data-table-sort-list.tsx (1)

178-178: Improved responsive design for sort button icon.

Adding the md:block hidden classes to the ArrowDownUp icon ensures better space utilization on mobile devices by hiding the icon on smaller screens and only displaying it on medium-sized screens and larger.

apps/app/src/components/forms/policies/create-new-policy.tsx (1)

111-122: Enhanced user interface clarity with more specific action label.

Changing the button label from "save" to "create" better reflects the actual action being performed, improving the user experience by setting clear expectations for the button's function.

apps/app/src/components/editor/policy-editor.tsx (1)

22-24: Improved code readability with better formatting.

The conditional expression for setting the content has been reformatted to span multiple lines, making it more readable while maintaining the same logic. This is a good practice for complex conditional expressions.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/components/policies-table.tsx (4)

11-11: New component integration adds policy creation capability.

Adding the CreatePolicySheet import enhances the policies table with creation functionality, which aligns with the PR's objective to streamline policy management components.


41-41: Good addition of row identification function.

Adding the getRowId prop ensures proper row identification for navigation, improving the user experience when interacting with policy items.


43-43: Sheet integration enhances toolbar functionality.

The addition of sheet and action props to the DataTableToolbar enables the "Create Policy" action button, providing a more intuitive user flow for policy creation.


47-47: Component inclusion completes the integration.

Adding the CreatePolicySheet component to the render output completes the integration of the policy creation functionality within the policies table view.

packages/ui/src/components/input.tsx (1)

11-18: UI enhancement with simplified styling.

The styling changes to the Input component improve its visual consistency by:

  1. Simplifying the border styling
  2. Reducing the text size to text-sm for better readability
  3. Updating focus states to use focus-visible:ring-muted-foreground for a more subtle effect

These changes align with modern UI practices and improve the overall user experience.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/page.tsx (1)

5-5: Code optimization with direct Suspense import.

Replacing the full React import with a specific Suspense import enhances code efficiency. Directly using Suspense instead of React.Suspense makes the code more concise while maintaining the same functionality.

Also applies to: 36-36, 55-55

apps/app/src/lib/validations.ts (2)

10-10: Import optimization by removing unused types.

Removing the unused Departments and Frequency types from imports keeps the code clean and reduces potential confusion about which types are actually used in the module.


27-30:

Details

❓ Verification inconclusive

Schema simplification improves policy creation flow.

The simplification of the createPolicySchema to include only the essential name and status fields streamlines the policy creation process, which aligns with the PR objective of enhancing the policy management components.

Can you verify if any downstream components relied on the removed fields (frequency and department) that might need updates?


🏁 Script executed:

#!/bin/bash
# Search for components that might have referenced the removed schema fields
echo "Searching for references to removed policy schema fields..."
rg -A 3 -B 3 "frequency|department" --type typescript --glob "**/policies/**/*.tsx"

Length of output: 249


Below is a revised review comment with updated instructions for verifying downstream dependencies. Note that the initial search failed due to an unrecognized file type; please run the new command manually to confirm that no components reference the removed fields.

Policy Schema Simplification: Downstream Dependencies Check

The simplified createPolicySchema (retaining only name and status) clearly streamlines policy creation. However, to ensure no downstream component still depends on the removed frequency and department fields, please verify manually by running the following search command:

#!/bin/bash
echo "Searching for references to removed policy schema fields in TypeScript files..."
rg -g "**/policies/**/*.ts" -g "**/policies/**/*.tsx" -A 3 -B 3 "frequency|department"

If the search returns no references, the change is safe to merge. Otherwise, please update any affected components accordingly.

apps/app/src/components/editor/extensions.ts (1)

189-213: Enhanced editor capabilities with additional extensions

The changes appropriately uncomment several previously defined extensions to enhance the editor's functionality. These added extensions (starterKit, placeholder, horizontalRule, aiHighlight, twitter, mathematics, HighlightExtension, TextStyle, and CustomKeymap) provide valuable editing capabilities that align with improving the policy editing experience.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/data/validations.ts (1)

1-29: Well-structured search parameters validation

This new file adds a clean implementation for policy search parameters validation using nuqs. The approach provides strong typing with zod and sensible defaults for pagination, sorting, filtering, and advanced filtering options. The exported GetPolicySchema type ensures type safety when consuming these parameters elsewhere.

apps/app/src/components/data-table/data-table.tsx (4)

17-21: LGTM: Row navigation capability added to DataTable

Adding the optional getRowId property to the interface enables row-level navigation, which improves the user experience for policy management.


31-39: Clean implementation of row click navigation

The implementation correctly uses Next.js router hooks to enable navigation to detail views when rows are clicked, improving user experience.


75-75: Good UX improvement with cursor pointer

Adding the cursor pointer style to table rows provides a clear visual indication that rows are clickable, enhancing user experience.


52-59: Responsive column display implementation

The conditional class application based on column index effectively handles responsive display, ensuring the first column is always visible while others show only on larger screens.

Also applies to: 86-92

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/components/policies-table-columns.tsx (3)

15-15: Improved naming consistency

Changing from "Name" to "Policy Name" provides better context and clarity in the UI. The corresponding metadata updates maintain consistency.

Also applies to: 27-28


6-7: Enhanced status display with dedicated component

Replacing the simple text display with the StatusPolicies component improves visual representation of policy statuses, enhancing the UI.

Also applies to: 40-40


48-66: Useful addition of Last Updated column

Adding the "Last Updated" column with formatted dates provides valuable information for users managing policies. The implementation follows the established pattern for column definitions.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/layout.tsx (1)

9-33:

Details

✅ Verification successful

Verify policy validation has been properly relocated

The Layout component has been simplified by removing session management, policy retrieval, and validation logic. While this creates a cleaner separation of concerns, please verify that these important validations (checking for valid organization ID and policy existence) have been properly implemented in the components that consume this layout.

According to the PR summary, this logic was moved to the editor page component, but it's essential to ensure proper validation occurs across all routes using this layout to maintain security and prevent unexpected behaviors.


🏁 Script executed:

#!/bin/bash
# Check if validation logic exists in the editor page or other components
echo "Searching for policy validation in editor page:"
cat apps/app/src/app/\[locale\]/\(app\)/\(dashboard\)/\[orgId\]/policies/all/\[policyId\]/editor/page.tsx 2>/dev/null | grep -A 10 "getPolicy\|redirect\|!organizationId" || echo "Not found in editor page"

echo -e "\nSearching for policy validation in other related files:"
fd --type f --extension tsx --extension ts . apps/app/src/app/\[locale\]/\(app\)/\(dashboard\)/\[orgId\]/policies/all/\[policyId\] --exec grep -l "getPolicy\|redirect\|!organizationId" {} \;

Length of output: 1988


Policy Validation Relocation Confirmed

The Layout component now strictly manages layout concerns while the validation logic has been successfully shifted to the Policy Editor and associated action handlers. For example, the PolicyEditor component and related action files now use the getPolicy function to verify the session, confirm the existence of an organization, and redirect when a valid policy isn’t found. This separation of concerns appears intentional and appropriate.

Please ensure that all routes relying on this layout consistently apply the same validation logic to maintain security across the application.

apps/app/src/components/data-table/data-table-pagination.tsx (2)

38-42: Great addition of item count display!

Adding the total item count provides valuable context to users about the data they're viewing.


43-43: Good UX improvement with select-none

Adding the select-none class prevents accidental text selection when users interact with the pagination controls, improving the overall user experience.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/data/queries.ts (2)

5-5: Good module organization improvement

Changing from an absolute to a relative import path improves module organization by keeping related files' imports localized, which can help with code maintainability.


32-36:

Details

❓ Verification inconclusive

Nice enhancement to filtering capabilities

Adding the ability to filter policies by status is a valuable feature that improves the flexibility of the query system.

Please verify that the input.status property is properly initialized as an array in the GetPolicySchema validation to prevent potential runtime errors. If input.status can be undefined or null, the length check could throw an error.


🏁 Script executed:

#!/bin/bash
# Check the definition of GetPolicySchema to ensure status is properly initialized
echo "Checking GetPolicySchema definition:"
cat apps/app/src/app/\[locale\]/\(app\)/\(dashboard\)/\[orgId\]/policies/all/\(overview\)/data/validations.ts | grep -A 15 "GetPolicySchema"

Length of output: 301


Action Required: Verify input.status Array Initialization
The enhancement for filtering policies by status is well executed. However, please double-check that the input.status property in the GetPolicySchema is reliably initialized as an array. The current schema definition in validations (i.e., the use of ReturnType<typeof searchParamsCache.parse>) does not explicitly guarantee a default empty array for status, which might lead to runtime errors when invoking .length.

  • File to Review:
    apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/(overview)/data/validations.ts
    Ensure that the parsing or default values set in searchParamsCache.parse correctly initialize status as an array (or provide an array fallback if undefined).

  • Diff Context:

    ...(input.status.length > 0 && {
        status: {
            in: input.status,
        },
    }),
apps/app/src/actions/policies/create-new-policy.ts (1)

82-82: Good addition of revalidation path

Adding revalidation for the policies/all path ensures that the UI is properly updated after a new policy is created.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/actions/update-policy.ts (4)

10-29: Well-structured helper function for content sanitization

The cleanContent function effectively prevents serialization issues by recursively removing function references from the content before processing. This is a good approach for handling complex JSON content with potential function properties.


75-76: Good implementation of content cleaning

Using the helper function here ensures that function references won't cause serialization errors during the database operation.


77-92: Properly updated conditional logic

The conditional logic has been correctly updated to reference the cleaned content throughout the function, ensuring consistent handling of the sanitized data.


109-109: Security enhancement by explicitly including organizationId

Including the organizationId in the where clause improves security by ensuring users can only update policies within their organization.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/components/PolicyDetails.tsx (4)

6-10: Clean interface update with proper type definitions

The interface has been properly updated to include the new policyContent prop with appropriate typing. This clearly communicates the component's requirements to other developers.


12-19: Well-implemented content formatting logic

The formatting logic handles different content formats gracefully, providing good flexibility for various input types. The component now correctly accepts content directly instead of fetching it internally.


20-29: Simplified and more direct save functionality

The handleSavePolicy function has been streamlined to directly use the provided policyId and content, improving code clarity and maintainability. The error handling is appropriately implemented with logging and re-throwing.


31-35: Cleaner component rendering

The rendering has been simplified to a more focused component structure that passes the relevant props to the PolicyEditor. This makes the component more maintainable and easier to understand.

apps/app/src/components/policies/policy-overview.tsx (5)

18-21: Improved type definition to handle nullable policy

The update to accept a nullable policy prop increases the component's flexibility and robustness, allowing it to handle cases where policy data might not be available yet.


29-56: Robust null handling with optional chaining

The implementation of optional chaining (?.) throughout the component ensures it gracefully handles cases where policy might be null, preventing runtime errors.


39-44: Good fallback date handling

The addition of a fallback date (new Date()) ensures the component won't break if updatedAt is unexpectedly null or undefined.


70-80: Well-implemented conditional UI elements

The conditional rendering of UI elements based on the policy's status is properly implemented with the optional chaining operator, making the code more robust.


107-116: Appropriate conditional rendering

The component correctly conditionally renders the UpdatePolicyOverview and related components only when policy data is available, preventing errors when policy is null.

apps/app/src/components/data-table/data-table-toolbar.tsx (4)

18-20: Well-structured component props extension

The DataTableToolbarProps interface has been properly extended with optional sheet and action properties, maintaining backward compatibility while adding new functionality.

Also applies to: 24-26


31-32: Clean state management implementation

The use of useQueryState for managing the sheet's open state allows for URL-based state persistence, which is beneficial for preserving UI state across page reloads and sharing links.


75-80: Good conditional button rendering

The implementation of the conditional action button is clean and only appears when the sheet prop is provided, making the component more flexible and reusable across different contexts.


65-68: Improved button content structure

The button content has been restructured to include an icon that's properly responsive (hidden on mobile), improving the user experience across different device sizes.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/editor/page.tsx (1)

5-10: Well-structured imports for enhanced functionality

The addition of these imports supports the new authentication, database access, and navigation features. Good practice to group related imports together.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/[policyId]/page.tsx (2)

16-23: Clean implementation of policy details rendering

The code correctly:

  • Extracts parameters from the request
  • Sets locale appropriately
  • Fetches policy data
  • Handles the null case with the nullish coalescing operator

The PolicyOverview component is properly configured to handle null policies as seen in the relevant code snippet.


47-51: Good authorization check

The function properly checks for an active organization ID from the session before attempting to query the database, enhancing security and preventing unnecessary database operations.

Comment on lines +81 to +83
className={cn(
getRowId
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incomplete className definition

The cn() utility is being used incorrectly here. It's receiving only a function reference without any class strings.

className={cn(
-  getRowId
+  // Add appropriate conditional classes here if needed
)}

Committable suggestion skipped: line range outside the PR's diff.

},
})
.action(async ({ parsedInput, ctx }) => {
const { title, description, frameworkIds, controlIds } = parsedInput;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify schema and usage alignment

There appears to be a potential mismatch between the fields being destructured (title, description, frameworkIds, controlIds) and what's defined in the createPolicySchema (which according to the provided snippet only includes name and status).

Please verify that the schema correctly validates all fields being used in this function to prevent potential runtime errors.


🏁 Script executed:

#!/bin/bash
# Check the complete definition of createPolicySchema
echo "Checking createPolicySchema definition:"
cat apps/app/src/lib/schema.ts | grep -A 20 "export const createPolicySchema"

# Look for any field mapping or transformation between schema and action
echo -e "\nChecking for any mapping between schema fields and action parameters:"
grep -r "createPolicySchema" --include="*.ts" --include="*.tsx" apps/app/src/ | grep -v "import"

Length of output: 1169


🏁 Script executed:

#!/bin/bash
# Attempt to output the definition of createPolicySchema from the validations file.
echo "Checking createPolicySchema definition in apps/app/src/lib/validations.ts:"
grep -A 30 "export const createPolicySchema" apps/app/src/lib/validations.ts

Length of output: 498


Action Required: Align Schema Definition with Input Fields

It has been verified that the createPolicySchema in apps/app/src/lib/validations.ts only defines the fields name and status, while the action in apps/app/src/actions/policies/create-new-policy.ts is destructuring title, description, frameworkIds, and controlIds from the parsed input. This discrepancy likely indicates either an omitted transformation step or a misalignment between the expected schema and the actual input used in the action. Please review and update the schema or the input handling accordingly.

  • File: apps/app/src/actions/policies/create-new-policy.ts
    Issue: Destructured fields (title, description, frameworkIds, controlIds) do not match the schema fields (name, status).

@carhartlewis carhartlewis merged commit 329278b into main Apr 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant