Feat(settings): add management for NDA agreement documents #1651
Conversation
|
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new NDA agreements management feature is introduced, including a settings page for listing, creating, and deleting agreements. Supporting UI primitives are added, such as an alert dialog component. Various files are updated to support agreements, including navigation, data fetching, and type definitions. Minor refactors and key prop adjustments are also present. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AgreementsPage
participant useAgreementsHook
participant AgreementCard
participant AlertDialog
participant API
User->>AgreementsPage: Visits /settings/agreements
AgreementsPage->>useAgreementsHook: Fetch agreements
useAgreementsHook->>API: GET /api/teams/{teamId}/agreements
API-->>useAgreementsHook: Returns agreements list
useAgreementsHook-->>AgreementsPage: Agreements data
AgreementsPage->>AgreementCard: Render each agreement
User->>AgreementCard: Clicks delete menu
AgreementCard->>AlertDialog: Opens confirmation dialog
User->>AlertDialog: Confirms deletion
AgreementCard->>API: PUT /api/teams/{teamId}/agreements/{id}/delete
API-->>AgreementCard: Deletion response
AgreementCard->>AgreementsPage: Calls onDelete(id)
AgreementsPage->>useAgreementsHook: Updates local cache
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 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: 3
🧹 Nitpick comments (2)
pages/settings/nda-agreements.tsx (1)
118-123: Consider making upgrade plan more dynamic.The upgrade plan modal uses a hardcoded
PlanEnum.Business. Consider making this more flexible if different plans might be required for NDA agreements in the future.<UpgradePlanModal - clickedPlan={PlanEnum.Business} + clickedPlan={PlanEnum.Datarooms} // Or use a variable/constant that can be easily changed trigger="nda_agreements_page" open={showUpgradeModal} setOpen={setShowUpgradeModal} />components/agreements/agreement-card.tsx (1)
1-26: Consider importing Agreement interface from a shared location.The
Agreementinterface might be defined elsewhere in the codebase. Consider importing it from a shared location to maintain consistency.+import { Agreement } from "@/types"; // or appropriate path import { useTeam } from "@/context/team-context"; import { FileTextIcon, MoreVertical, TrashIcon } from "lucide-react"; import { toast } from "sonner"; import { Button } from "@/components/ui/button"; import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; -interface Agreement { - id: string; - name: string; - content: string; - createdAt: Date; - updatedAt: Date; -} interface AgreementCardProps { agreement: Agreement; onDelete: (id: string) => void; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/agreements/agreement-card.tsx(1 hunks)components/links/link-sheet/agreement-section.tsx(1 hunks)components/settings/settings-header.tsx(1 hunks)pages/settings/nda-agreements.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/agreements/agreement-card.tsx (2)
components/ui/dropdown-menu.tsx (4)
DropdownMenu(189-189)DropdownMenuTrigger(190-190)DropdownMenuContent(191-191)DropdownMenuItem(192-192)components/ui/button.tsx (1)
Button(71-71)
🔇 Additional comments (4)
components/settings/settings-header.tsx (1)
60-64: LGTM! Navigation item for NDA Agreements added correctly.The new navigation item for "NDA Agreements" is correctly implemented, following the consistent pattern of other navigation items in the settings menu.
components/links/link-sheet/agreement-section.tsx (1)
49-49: Good bug fix: Added agreementId to dependency array.Adding
agreementIdto the dependency array of theuseMemohook ensures the filtered agreements list correctly recalculates when the selected agreement changes. This fixes a potential reactivity issue.pages/settings/nda-agreements.tsx (2)
1-30: LGTM! Well-structured imports and state management.The imports are organized logically, and the hooks for data fetching and state management are set up correctly. The
activeAgreementsmemo properly filters out deleted agreements.
31-37: LGTM! Optimistic UI update for agreement deletion.The implementation uses SWR's
mutatefunction to optimistically update the UI when an agreement is deleted, providing a smooth user experience.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/agreements/agreement-card.tsx (1)
45-64: Use DELETE HTTP method instead of PUT for deletions.The API endpoint for deletion should use the DELETE HTTP method instead of PUT to follow REST conventions.
const handleDelete = async () => { toast.promise( fetch( `/api/teams/${teamInfo?.currentTeam?.id}/agreements/${agreement.id}`, { - method: "PUT", + method: "DELETE", }, ).then(async (response) => { if (!response.ok) { throw new Error("Failed to delete agreement"); } onDelete(agreement.id); }), { loading: "Deleting agreement...", success: "Agreement deleted successfully", error: "Failed to delete agreement", }, ); };
🧹 Nitpick comments (1)
components/agreements/agreement-card.tsx (1)
105-105: Fix unescaped quotes in the JSX.The quotes around the agreement name should be escaped to comply with React's JSX requirements.
- This will permanently delete the NDA agreement "{agreement.name}". + This will permanently delete the NDA agreement "{agreement.name}".🧰 Tools
🪛 ESLint
[error] 105-105:
"can be escaped with",“,",”.(react/no-unescaped-entities)
[error] 105-105:
"can be escaped with",“,",”.(react/no-unescaped-entities)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/agreements/agreement-card.tsx(1 hunks)components/ui/alert-dialog.tsx(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/ui/alert-dialog.tsx (2)
lib/utils.ts (1)
cn(18-20)components/ui/button.tsx (1)
buttonVariants(71-71)
🪛 ESLint
components/agreements/agreement-card.tsx
[error] 105-105: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 105-105: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
🔇 Additional comments (12)
components/agreements/agreement-card.tsx (5)
25-31: Well-defined Agreement interface.The interface clearly defines all required properties for an agreement with appropriate types.
33-36: Good component props design.The props interface is properly defined, making it clear what data the component needs to function.
67-98: Well-designed card UI with appropriate layout.The card layout is clean and organized, using appropriate semantic HTML and styling classes for accessibility and visual hierarchy.
100-126: Great implementation of the deletion confirmation dialog.The AlertDialog implementation provides an important safeguard against accidental deletions and clearly explains the consequences to users.
🧰 Tools
🪛 ESLint
[error] 105-105:
"can be escaped with",“,",”.(react/no-unescaped-entities)
[error] 105-105:
"can be escaped with",“,",”.(react/no-unescaped-entities)
109-113: Excellent user guidance about referenced agreements.The note about references in documents or dataroom links provides valuable information to users about the implications of deletion.
components/ui/alert-dialog.tsx (7)
1-14: Well-structured component library setup.The imports and component declarations follow best practices for creating reusable UI components based on Radix primitives.
15-28: Properly implemented overlay with animation.The AlertDialogOverlay component correctly uses forwardRef and applies appropriate styling for a clean modal backdrop with animation support.
30-46: Well-implemented modal content with responsive animations.The AlertDialogContent component has comprehensive styling for positioning, animations, and responsiveness. The use of data attributes for state-based animations is particularly good.
48-74: Good layout components for header and footer.The AlertDialogHeader and AlertDialogFooter components provide useful layout structure with responsive adjustments between mobile and desktop views.
76-99: Well-styled title and description components.The AlertDialogTitle and AlertDialogDescription components follow the pattern of forwarding refs and applying appropriate typography styles.
101-127: Consistent button styling using buttonVariants.The AlertDialogAction and AlertDialogCancel components properly integrate with your button styling system, ensuring UI consistency throughout the application.
129-141: Complete export of all dialog components.All components are properly exported for easy imports, following the component composition pattern.
mfts
left a comment
mfts
left a comment
There was a problem hiding this comment.
good first version! let's build on top of it
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/agreements/agreement-card.tsx (1)
47-66:⚠️ Potential issueUse DELETE HTTP method instead of PUT for deletions.
The API endpoint for deletion should use the DELETE HTTP method instead of PUT to follow REST conventions.
const handleDelete = async () => { toast.promise( fetch( `/api/teams/${teamInfo?.currentTeam?.id}/agreements/${agreement.id}`, { - method: "PUT", + method: "DELETE", }, ).then(async (response) => { if (!response.ok) { throw new Error("Failed to delete agreement"); } onDelete(agreement.id); }), { loading: "Deleting agreement...", success: "Agreement deleted successfully", error: "Failed to delete agreement", }, ); };
🧹 Nitpick comments (4)
components/agreements/agreement-card.tsx (4)
1-1: Remove unused import.The
useEffectimport is not being used in this component.-import { useEffect, useState } from "react"; +import { useState } from "react";
112-112: Escape quotes in JSX text content.React's ESLint rule flags unescaped quotation marks in JSX. Use HTML entities or template syntax to avoid this warning.
- This will permanently delete the NDA agreement "{agreement.name}". + This will permanently delete the NDA agreement "{agreement.name}".Alternatively, you can use template syntax:
This will permanently delete the NDA agreement "{agreement.name}".🧰 Tools
🪛 ESLint
[error] 112-112:
"can be escaped with",“,",”.(react/no-unescaped-entities)
[error] 112-112:
"can be escaped with",“,",”.(react/no-unescaped-entities)
27-33: Unused interface definition.The
Agreementinterface is defined but not used in this component. The component usesAgreementWithLinksCountinterface instead. Consider removing this unused interface to reduce code clutter.-interface Agreement { - id: string; - name: string; - content: string; - createdAt: Date; - updatedAt: Date; -} -
83-86: Add formatting for the links count.Consider adding proper pluralization for the links count text.
<div className="text-sm text-muted-foreground"> - {agreement._count?.links || 0} links + {agreement._count?.links || 0} {agreement._count?.links === 1 ? 'link' : 'links'} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
components/agreements/agreement-card.tsx(1 hunks)components/analytics/views-table.tsx(3 hunks)components/settings/settings-header.tsx(1 hunks)components/ui/alert-dialog.tsx(1 hunks)components/ui/dialog.tsx(2 hunks)components/ui/dropdown-menu.tsx(1 hunks)components/visitors/dataroom-visitors-table.tsx(1 hunks)components/visitors/visitors-table.tsx(1 hunks)lib/swr/use-agreements.ts(1 hunks)pages/settings/agreements.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- components/ui/dropdown-menu.tsx
- components/visitors/dataroom-visitors-table.tsx
- lib/swr/use-agreements.ts
- components/visitors/visitors-table.tsx
- components/analytics/views-table.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- components/settings/settings-header.tsx
- components/ui/alert-dialog.tsx
🧰 Additional context used
🪛 ESLint
components/agreements/agreement-card.tsx
[error] 112-112: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 112-112: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
🔇 Additional comments (3)
components/ui/dialog.tsx (2)
9-10: LGTM: Import order adjustment.The import order has been adjusted to maintain a logical grouping.
56-59: Added focus management and pointer events reset on dialog close.This change prevents the default focus behavior when the dialog closes and resets the pointer events style on the document body, which helps ensure proper UI state restoration.
pages/settings/agreements.tsx (1)
1-147: LGTM: Well-structured NDA agreements management page.The implementation is thorough with proper error handling, loading states, and empty states. The component follows React best practices with appropriate state management and conditional rendering based on the user's subscription plan.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores