-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Duplicate permission group for duplicated links #1886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Duplicate permission group for duplicated links #1886
Conversation
… links Co-authored-by: marcftone <[email protected]>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughUpdates the link-duplication API to derive userId from NextAuth session, authorize via user-team membership, and, when present, duplicate a link’s permissionGroup and its accessControls within the same flow. The new link references the newly created permissionGroupId and preserves prior tag duplication, webhook trigger, and response structure. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant API as /api/links/[id]/duplicate
participant Auth as NextAuth (getServerSession)
participant DB as Database
C->>API: POST duplicate(id)
API->>Auth: getServerSession()
Auth-->>API: session(userId) or null
alt No session
API-->>C: 401 Unauthorized
else Session
API->>DB: Fetch link by id incl. permissionGroup{ accessControls }, tags
DB-->>API: link { teamId, permissionGroup? }
API->>DB: Verify userTeam with userId + teamId
DB-->>API: allowed? (bool)
alt Not allowed
API-->>C: 403 Forbidden
else Allowed
opt permissionGroup exists
API->>DB: Create new permissionGroup (name, description, dataroomId, teamId)
DB-->>API: newPermissionGroupId
API->>DB: Clone accessControls to newPermissionGroupId
end
API->>DB: Create duplicated link (incl. newPermissionGroupId)
API->>DB: Recreate tags for new link
API-->>C: 200 { newLink with permissionGroupId }
note over API: Webhook trigger preserved
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pages/api/links/[id]/duplicate.ts (2)
149-158: Fix webhook teamId source to avoid mismatched eventsUse the created link’s teamId rather than the request body to prevent misattributed webhooks.
waitUntil( sendLinkCreatedWebhook({ - teamId, + teamId: newLink.teamId, data: { link_id: newLink.id, document_id: newLink.documentId, dataroom_id: newLink.dataroomId, }, }), );
47-49: Use findFirst when filtering by both id and teamId
findUnique accepts only a defined unique or composite-unique field—your Link model only definesidas @id, not a composite of(id, teamId). Replace with:- const link = await prisma.link.findUnique({ - where: { id, teamId }, + const link = await prisma.link.findFirst({ + where: { id, teamId }, include: {
🧹 Nitpick comments (3)
pages/api/links/[id]/duplicate.ts (3)
71-71: Avoid spreading entire Link into create payloadSpreading ...rest risks copying fields that shouldn’t be cloned (e.g., archived flags, counters, ownership). Prefer an explicit data shape or a Prisma select to retrieve only fields you intend to copy.
43-45: Use 403 for authorized session without required team accessOptional: return 403 Forbidden when the session exists but the user lacks team access; reserve 401 for unauthenticated requests.
23-23: Align method documentation/comments with implementationComment says PUT; route accepts POST. Update comments and Allow header documentation for clarity.
- // PUT /api/links/:id/duplicate + // POST /api/links/:id/duplicate @@ - // We only allow PUT requests + // We only allow POST requestsAlso applies to: 166-168
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/api/links/[id]/duplicate.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pages/api/links/[id]/duplicate.ts (1)
lib/types.ts (1)
CustomUser(17-17)
🔇 Additional comments (2)
pages/api/links/[id]/duplicate.ts (2)
92-104: Confirm parity of access control fieldsEnsure all relevant access control fields are cloned (timestamps/createdBy may rely on defaults; but policy/constraints like expiresAt, domain/email rules, etc., if present, should be included).
31-31: Confirm session user type has idEnsure session.user conforms to CustomUser and includes id in this API context to avoid runtime undefined.
Duplicate a link's permission group and its access controls when duplicating the link to ensure independent permission management.
Previously, duplicating a link that had an associated
permissionGroupIdwould only copy the ID, causing both the original and the new link to share the same permission group. This PR ensures that a new, independent permission group with identical access controls is created for the duplicated link, preventing unintended permission changes across links.Slack Thread
Summary by CodeRabbit
New Features
Bug Fixes