feat: implement comments feature#307
Conversation
- Added `addCommentAction` for server-side comment creation with validation using Zod. - Introduced `Comments` component for displaying and adding comments in various pages. - Updated relevant pages (Policy, Risk, Vendor) to include comments functionality. - Created `CreateCommentForm` for user input and submission of comments. - Added database schema for `Comment` and updated related models in Prisma. - Ensured comments are fetched and displayed correctly in the UI.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request introduces a comment management feature across the application. It adds a new action ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant F as CreateCommentForm
participant A as addCommentAction
participant DB as Database
participant R as Revalidation
U->>F: Enter comment and submit
F->>A: Send comment data + entityId
A->>A: Validate session and organization
A->>DB: Insert new comment record
DB-->>A: Return confirmation or error
A->>R: Trigger cache revalidation
A-->>F: Return created comment data or error
F-->>U: Display success/error message
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (15)
packages/db/prisma/schema/common.prisma (1)
1-12: Comment model design looks solidThe Comment model is well-structured with appropriate fields and relationships. The index on
entityIdis a good performance optimization for querying comments by entity.Consider adding an updatedAt field for future extensibility
While the current implementation may not require comment editing, adding an
updatedAtfield would follow the pattern of other models and provide flexibility for future feature enhancements.model Comment { id String @id @default(dbgenerated("generate_prefixed_cuid('cmt'::text)")) content String entityId String authorId String author Member @relation(fields: [authorId], references: [id]) organizationId String organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt @@index([entityId]) }Consider adding a type field for comment categorization
If you anticipate different types of comments in the future (e.g., questions, feedback, notes), a
typefield could be valuable.model Comment { id String @id @default(dbgenerated("generate_prefixed_cuid('cmt'::text)")) content String entityId String + type String? // Optional field for comment categorization authorId String author Member @relation(fields: [authorId], references: [id]) organizationId String organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) createdAt DateTime @default(now()) @@index([entityId]) }apps/app/src/actions/schema.ts (1)
286-296: Consider more robust HTML sanitization.
Currently, this.replace(/<[^>]*>/g, "")transform strips out basic HTML tags. If malicious scripting or other HTML attacks are a concern, you may want to use specialized libraries (e.g., DOMPurify) or perform additional sanitization measures server-side.apps/app/src/actions/add-comment.ts (1)
24-75: Ensure the entity existence is verified if needed.
While organization membership validation is performed, consider whether you also need to check that the referenced entity (policy, risk, etc.) exists to prevent ghost comments pointing to invalid entities. The rest of the logic, including revalidatePath usage, looks solid.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx (1)
106-136: Consider pagination for large comment sets.
If a policy might have a large number of comments, returning all comments in a single query could degrade performance. Consider lazy-loading or paginating them for an enhanced user experience.apps/app/src/components/comments.tsx (2)
29-52: Consider improving state management for comment creation.The toggle mechanism for showing/hiding the comment form works, but consider enhancing the UX by automatically hiding the form after successful submission.
- const [isAddingComment, setIsAddingComment] = useState(false); + const [isAddingComment, setIsAddingComment] = useState(false); + + // Add this to CreateCommentForm props + const onCommentAdded = useCallback(() => { + setIsAddingComment(false); + }, []);And pass the callback to CreateCommentForm:
- <CreateCommentForm entityId={entityId} /> + <CreateCommentForm entityId={entityId} onSuccess={onCommentAdded} />
29-103: Consider adding pagination for large comment volumes.The current implementation displays all comments at once, which could lead to performance issues if there are many comments. Consider implementing pagination or infinite scrolling for better performance.
For large volumes of comments, consider implementing pagination:
interface CommentsProps { entityId: string; comments: Comment[]; + totalComments?: number; + currentPage?: number; + onPageChange?: (page: number) => void; } export function Comments({ entityId, comments, + totalComments = comments.length, + currentPage = 1, + onPageChange, }: CommentsProps) { // ...existing code return ( <Card> {/* ...existing code */} <CardContent className="pt-6"> {/* ...existing code */} <div className="space-y-6"> {/* ...existing comment rendering */} + {totalComments > 10 && ( + <div className="flex justify-center mt-4"> + <Pagination + currentPage={currentPage} + totalPages={Math.ceil(totalComments / 10)} + onPageChange={onPageChange} + /> + </div> + )} </div> </CardContent> </Card> ); }packages/db/prisma/migrations/20250406004306_added_common_comment_table/migration.sql (1)
5-6: Consider adding a type field for comment categorization.The current schema doesn't include a way to categorize comments (e.g., feedback, question, bug report). Adding a type field could enhance filtering and display capabilities.
CREATE TABLE "Comment" ( "id" TEXT NOT NULL DEFAULT generate_prefixed_cuid('cmt'::text), "content" TEXT NOT NULL, "entityId" TEXT NOT NULL, + "type" TEXT DEFAULT 'general', "authorId" TEXT NOT NULL, "organizationId" TEXT NOT NULL, "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, CONSTRAINT "Comment_pkey" PRIMARY KEY ("id") );apps/app/src/components/forms/create-comment-form.tsx (4)
69-69: Internationalize the accordion trigger text.The "Comment" text in the AccordionTrigger is hardcoded, while other text in the form uses the internationalization system.
- <AccordionTrigger>Comment</AccordionTrigger> + <AccordionTrigger>{t("common.comments.form.title")}</AccordionTrigger>
72-84: Consider adding character count feedback.Since the schema limits comments to 1000 characters, it would be helpful to provide visual feedback about the remaining character count.
<FormItem> <FormControl> <Textarea {...field} /> </FormControl> + <div className="text-xs text-muted-foreground text-right mt-1"> + {field.value.length}/1000 + </div> <FormMessage /> </FormItem>
86-96: Consider adding a cancel button.Adding a cancel button would improve the user experience by allowing users to easily dismiss the form without having to toggle the add comment button again.
<div className="flex justify-end mt-4"> + <Button + type="button" + variant="outline" + className="mr-2" + onClick={() => form.reset()} + > + {t("common.actions.cancel")} + </Button> <Button type="submit" variant="action" disabled={addComment.status === "executing"} > <div className="flex items-center justify-center"> {t("common.actions.create")} <ArrowRightIcon className="ml-2 h-4 w-4" /> </div> </Button> </div>
62-104: Add loading indication during form submission.Currently, the button is disabled during submission, but there's no visual indicator that something is happening. Consider adding a loading spinner to improve user feedback.
import { Button } from "@comp/ui/button"; import { useI18n } from "@/locales/client"; -import { ArrowRightIcon } from "lucide-react"; +import { ArrowRightIcon, Loader2 } from "lucide-react"; // ... <Button type="submit" variant="action" disabled={addComment.status === "executing"} > <div className="flex items-center justify-center"> - {t("common.actions.create")} - <ArrowRightIcon className="ml-2 h-4 w-4" /> + {addComment.status === "executing" ? ( + <> + {t("common.states.submitting")} + <Loader2 className="ml-2 h-4 w-4 animate-spin" /> + </> + ) : ( + <> + {t("common.actions.create")} + <ArrowRightIcon className="ml-2 h-4 w-4" /> + </> + )} </div> </Button>apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx (4)
23-30: Validate redirect behavior.
Currently, when no vendor is found, the code redirects to the homepage ("/"). This may not be ideal if you want to inform users that the vendor doesn't exist or if they lack permissions. Consider redirecting to a dedicated 404 or 403 page for better user feedback.
53-77: Fetching vendor data with caching is fine.
The logic is straightforward, and the caching mechanism should help performance by preventing repeated DB calls. Optionally, you could consolidate fetching the vendor and comments if you prefer a single DB call. However, keeping them separate is equally valid for maintainability.
79-110: Consider pagination for large comment volumes.
While this implementation works well, if vendors can accumulate many comments, returning them all at once may impact performance. Adding pagination or limiting results can keep the UI responsive.
137-147: Metadata generation is correct; consider adding the vendor name.
You’re setting a generic title for the page. If you’d like a more descriptive title, prepend vendor-specific details (e.g.,Vendor - {vendor.name}). Otherwise, this is fine as is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/app/src/actions/add-comment.ts(1 hunks)apps/app/src/actions/schema.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/[riskId]/page.tsx(6 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/components/create-vendor-form.tsx(1 hunks)apps/app/src/components/comments.tsx(1 hunks)apps/app/src/components/forms/create-comment-form.tsx(1 hunks)packages/db/prisma/migrations/20250406004306_added_common_comment_table/migration.sql(1 hunks)packages/db/prisma/schema/auth.prisma(1 hunks)packages/db/prisma/schema/common.prisma(1 hunks)packages/db/prisma/schema/organization.prisma(1 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx (1)
apps/app/src/components/comments.tsx (1)
Comments(29-103)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/components/create-vendor-form.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/actions/create-vendor-action.ts (1)
createVendorAction(21-54)
apps/app/src/components/forms/create-comment-form.tsx (2)
apps/app/src/actions/add-comment.ts (1)
addCommentAction(10-75)apps/app/src/actions/schema.ts (1)
addCommentSchema(286-296)
apps/app/src/components/comments.tsx (1)
apps/app/src/components/forms/create-comment-form.tsx (1)
CreateCommentForm(29-105)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/components/title-and-description/title-and-description.tsx (1)
TitleAndDescription(12-44)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/components/secondary-fields/secondary-fields.tsx (1)
SecondaryFields(8-33)
🔇 Additional comments (30)
packages/db/prisma/schema/auth.prisma (1)
91-91: Relationship effectively established for comments and membersThe addition of the
comments Comment[]field to theMembermodel correctly establishes a one-to-many relationship, enabling members to have multiple associated comments.packages/db/prisma/schema/organization.prisma (1)
23-23: Relationship effectively established for comments and organizationsThe addition of the
comments Comment[]field to theOrganizationmodel correctly establishes a one-to-many relationship, enabling organizations to have multiple associated comments.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/components/create-vendor-form.tsx (1)
52-52: Code simplification looks goodThe changes simplify the form's behavior by removing unnecessary session checks and complex navigation, focusing solely on closing the vendor creation sheet upon success.
Also applies to: 57-57
apps/app/src/actions/schema.ts (1)
2-285: No functional changes aside from indentation.apps/app/src/actions/add-comment.ts (1)
1-9: Initial setup looks good.
No issues found in import statements or server directive.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx (4)
12-39: Integration of Comments component is well-structured.
Fetching comments alongside policy data and displaying them in theCommentscomponent appears seamless. Great job unifying the policy overview, editor, and comments into a single page.
43-56: No material changes aside from formatting in generateMetadata.
58-78: Policy retrieval logic remains consistent.
No concerns found.
80-104: No notable changes aside from indentation in getAssignees.apps/app/src/components/comments.tsx (5)
1-13: Comprehensive imports and setup.All the necessary components and utilities are correctly imported for handling comments rendering and functionality.
14-27: Clean interface definitions for comment data structures.The Comment and CommentsProps interfaces are well-structured, providing type safety for the component. The interfaces accurately capture the relationship between comments, authors, and users.
53-91: Good implementation of comment list with fallbacks.The comment rendering implementation properly handles user information with fallbacks for missing data. The avatar component gracefully handles missing images, and the UI design for each comment is clean with appropriate spacing and borders.
92-98: Good empty state handling.The component properly handles the case when there are no comments by displaying a helpful message, which maintains a good user experience.
65-71:Details
✅ Verification successful
Consider additional security for user content.
The content of comments is rendered directly without sanitization, which could potentially lead to XSS attacks if the backend validation is bypassed.
🏁 Script executed:
#!/bin/bash # Check if there's any client-side sanitization in the app for user content # Look for sanitization libraries in package.json cat package.json | grep -E 'sanitize|xss|dompurify' # Check if the addCommentSchema in schema.ts includes sanitization rg -A 10 "addCommentSchema" --type tsLength of output: 3102
Security Verification Summary:
Our investigation confirms that user comment input is sanitized server-side in theaddCommentSchema. The schema applies a transformation that strips out HTML tags using a regex, which mitigates the XSS concern for comment content. Additionally, the displayed user data (avatar image and name) is rendered as plain text, with React handling proper escaping, so no additional client-side sanitization is necessary for these elements.
- File:
apps/app/src/components/comments.tsx- Verification Details:
- The
addCommentSchemainapps/app/src/actions/schema.tstransforms thecontentfield withval.replace(/<[^>]*>/g, "")to remove HTML tags.- React's default behavior escapes interpolated strings, ensuring safe rendering of user names and other text data.
While the current sanitization strategy effectively addresses XSS risks, consider evaluating dedicated sanitization libraries (e.g., DOMPurify) in the future for more comprehensive security, especially if comment rendering requirements become more complex.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/[riskId]/page.tsx (5)
16-16: Appropriately imported Comments component.The Comments component is correctly imported to support the new comment feature on the risk page.
32-32: Proper data fetching for comments.The page correctly fetches comments related to the risk by awaiting the getComments function with the appropriate riskId.
53-53: Good integration of the Comments component.The Comments component is well-integrated into the page layout, passing the required props (entityId and comments) correctly.
59-83: Effective use of caching for the getRisk function.Using the cache function will optimize performance by preventing redundant database queries, which is particularly valuable for frequently accessed risk data.
118-140: Effective use of caching for the getAssignees function.The cache implementation for getAssignees will help improve performance by preventing unnecessary database queries.
packages/db/prisma/migrations/20250406004306_added_common_comment_table/migration.sql (4)
1-11: Well-structured Comment table creation.The Comment table is well-designed with appropriate fields for content, entity references, author information, and timestamps. The use of generate_prefixed_cuid for ID generation aligns with best practices for creating human-readable unique identifiers.
13-14: Good use of indexing for query optimization.Creating an index on the entityId field is a good practice that will improve query performance when filtering comments by entity.
19-20: Consistent foreign key constraint for Organization.The CASCADE constraint for organizationId is appropriate, ensuring that when an organization is deleted, all associated comments are also deleted, which helps maintain data integrity.
16-17:Details
❓ Verification inconclusive
Consider the implications of ON DELETE RESTRICT.
The RESTRICT constraint on authorId means that a Member cannot be deleted if they have created comments. Consider whether this is the intended behavior or if a CASCADE or SET NULL approach might be more appropriate for your application's requirements.
🏁 Script executed:
#!/bin/bash # Check if there are other places in the codebase where similar foreign key constraints are defined # Look for similar constraint patterns in other migration files rg -A 1 "ON DELETE RESTRICT" --type sql --glob "*/migrations/*" # Look for member deletion logic in the codebase rg -A 5 "member.*delete" --type tsLength of output: 611
Reassess the deletion behavior for Member records
The migration currently uses
ON DELETE RESTRICTon the foreign key fromComment.authorIdtoMember.id, which prevents deleting a Member if they have any associated comments. We found that the application deletion endpoint inapps/app/src/app/api/v1/employees/[id]/route.tscallsdb.member.delete, which could lead to errors if the Member being deleted has comments. Please review whether this restriction is intentional. If members should be removable regardless of their comments, consider usingCASCADE(to delete related comments) orSET NULL(to disassociate them) instead, or adjust the deletion flow to handle the presence of comments appropriately.apps/app/src/components/forms/create-comment-form.tsx (3)
1-28: Comprehensive import list for form functionality.All necessary components and utilities for form handling, validation, and UI are properly imported.
29-42: Good error and success handling.The form provides appropriate feedback to the user through toast notifications and resets the form upon successful submission, which contributes to a good user experience.
44-58: Form submission and validation setup is well-implemented.The form correctly uses Zod for validation, sets appropriate default values, and handles the submission process effectively.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx (4)
16-17: Confirm compatibility of thecacheAPI usage.
cacheis a relatively new server component API introduced in Next.js 13. Please verify that your project environment and React/Next.js versions support this usage.Would you like me to run a script to confirm that your Next.js version supports
cache?
20-20: Ensure thatparamsas a Promise is intentional.
Typically, route parameters are passed synchronously in Next.js. Make sure that passing aPromisetype is indeed required in your use case and is functioning as expected.
33-50: Layout integration for comments looks good.
The inclusion of the<Comments>component under the vendor details effectively groups relevant data. Overall, this improves user experience by making comments easily discoverable.
112-134: Assignees retrieval logic appears correct.
ThegetAssigneesfunction is clean, concise, and follows the same session validation pattern.
- Wrapped comments fetching logic in try-catch blocks across Policy, Risk, and Vendor pages to improve error handling. - Ensured that errors are logged and an empty array is returned in case of failure, maintaining UI stability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx (1)
79-115: Error handling in getComments function could be improved.While error handling has been implemented correctly, the error logging could be more descriptive to provide better context for debugging.
} catch (error) { - console.error(error); + console.error("Error fetching comments for vendor:", error); return []; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/[riskId]/page.tsx(6 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx
🧰 Additional context used
🧬 Code Definitions (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/[riskId]/page.tsx (2)
apps/app/src/components/comments.tsx (1)
Comments(29-103)apps/app/src/utils/auth.ts (1)
auth(11-86)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/components/title-and-description/title-and-description.tsx (1)
TitleAndDescription(12-44)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/components/secondary-fields/secondary-fields.tsx (1)
SecondaryFields(8-33)
🔇 Additional comments (9)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/[riskId]/page.tsx (4)
1-1: LGTM! New comment feature properly integrated.The imports for React's cache utility and the Comments component are correctly added. The page now fetches comments using the new getComments function and correctly renders the Comments component with the required props.
Also applies to: 16-16, 32-32, 53-53
59-83: Good optimization with cache implementation.Converting getRisk to use React's cache function is a good optimization that will prevent redundant database calls when the same risk is requested multiple times.
85-121: Error handling in getComments function could be improved.While error handling has been added as suggested in the previous review, the error logging could be more descriptive to provide better context for debugging.
} catch (error) { - console.error(error); + console.error("Error fetching comments for risk:", error); return []; }
123-145: LGTM! Cache implementation for getAssignees.Adding cache to the getAssignees function is a good optimization that will prevent redundant database calls.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/[vendorId]/page.tsx (5)
16-20: LGTM! Proper imports and interface simplification.The imports for React's cache and Comments component are correctly added. The PageProps interface has been appropriately simplified to remove unused searchParams.
23-51: LGTM! Clean implementation of VendorPage with comments.The VendorPage component has been streamlined to focus on displaying vendor details and comments, which aligns well with the PR objectives. The Comments component is properly integrated with the required props.
53-77: Good optimization with cache implementation for getVendor.Converting getVendor to use React's cache function is a good optimization that will prevent redundant database calls.
117-139: LGTM! Cache implementation for getAssignees.Adding cache to the getAssignees function is a good optimization that will prevent redundant database calls.
141-153: LGTM! No functional changes to the metadata generation.The formatting changes to generateMetadata are minor and don't affect functionality.
|
Elite PR 🙂 |
addCommentActionfor server-side comment creation with validation using Zod.Commentscomponent for displaying and adding comments in various pages.CreateCommentFormfor user input and submission of comments.Commentand updated related models in Prisma.Summary by CodeRabbit
New Features
Bug Fixes
Chores