-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(ee): dataroom invitations #1933
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a dataroom invitation system: UI (modal, badges), backend APIs for group/link invites and uninvited queries, email templates/sender updates, DB schema for ViewerInvitation, feature-flag gating, and related client hooks/utilities. Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 3
🧹 Nitpick comments (15)
lib/utils/link-url.ts (1)
1-12: Guard against missing or oddly formattedNEXT_PUBLIC_MARKETING_URLThe logic is clear, but if
NEXT_PUBLIC_MARKETING_URLis unset or includes a trailing slash, the fallback can produce"undefined/view/..."or double slashes. Consider normalizing the base URL (and possibly throwing early if it’s missing) to avoid hard‑to‑debug runtime URLs.ee/features/dataroom-invitations/emails/components/dataroom-viewer-invitation.tsx (1)
16-87: Avoid placeholder default forrecipientEmailThe footer’s “This email was intended for …” line is great, but defaulting
recipientEmailto"[email protected]"means a missed prop at a call site would silently produce misleading text. Consider removing the default (and treatingrecipientEmailas required for real sends), keeping any sample/preview values in stories/tests instead.prisma/schema/schema.prisma (1)
176-202: ViewerInvitation schema is consistent; consider minor extensionsThe ViewerInvitation model and Viewer.invitations relation line up with how the API writes invitations (viewerId/linkId/groupId, status
"SENT" | "FAILED", optional customMessage). Two optional improvements you may want to consider later:
- If you expect status transitions (e.g., updating to
BOUNCEDfrom webhooks), adding anupdatedAtfield would make those changes auditable.invitedByis a bareString; if it’s always aUserID in practice, a proper relation toUsercould improve referential integrity and make Prisma queries easier.Not blockers, the current schema will work fine with the existing API usage.
Also applies to: 455-479
components/datarooms/groups/group-member-table.tsx (2)
66-68: Simplify the groupKey fallback pattern.The
groupKeyis computed on lines 66-68, but then every mutation handler (lines 71-73, 101-103, 128-130) uses a fallback pattern that reconstructs the identical URL. Since the fallback always produces the same value asgroupKey, this is redundant.Apply this diff to simplify the handlers:
const groupKey = teamId ? `/api/teams/${teamId}/datarooms/${dataroomId}/groups/${groupId}` : null; const handleToggleAllowAll = async () => { - const key = - groupKey ?? - `/api/teams/${teamId}/datarooms/${dataroomId}/groups/${groupId}`; + if (!groupKey) return; const response = await fetch( `/api/teams/${teamId}/datarooms/${dataroomId}/groups/${groupId}`, { method: "PATCH", body: JSON.stringify({ allowAll: !viewerGroupAllowAll, }), headers: { "Content-Type": "application/json", }, }, ); if (!response.ok) { - mutate(key); + mutate(groupKey); toast.error("Failed to update group settings"); return; } - mutate(key); + mutate(groupKey); toast.success( viewerGroupAllowAll ? "Group access restricted to specific members and domains" : "Group now allows access from any email", ); };Apply similar changes to
handleRemoveDomainandhandleRemoveMember.Also applies to: 71-73, 101-103, 128-130
187-205: Consider disabling the invite button when "Allow all emails" is enabled.The "Add members" button is disabled when
viewerGroupAllowAllis true (line 182), but the "Share invite" button remains enabled. This creates an inconsistency: if the group already allows all email addresses, sending targeted invitations may be confusing or redundant.Apply this diff to align the behavior:
{isFeatureEnabled("dataroomInvitations") && ( <Button size="sm" variant="secondary" onClick={() => setInviteOpen(true)} className="relative h-8 gap-1" + disabled={viewerGroupAllowAll} > <SendIcon className="h-4 w-4" /> Share invite {uninvitedCount > 0 ? ( <Badge variant="secondary" className="ml-2 h-5 rounded-full px-2 text-xs font-medium" > {uninvitedCount} </Badge> ) : null} </Button> )}ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts (1)
3-3: Consider exporting MAX_CUSTOM_MESSAGE_LENGTH for frontend use.The
MAX_CUSTOM_MESSAGE_LENGTHconstant is defined but not exported. The frontend likely needs this value to show character counts and validate input before submission (e.g., in the InviteViewersModal component which shows "0/500 characters").Apply this diff:
-const MAX_CUSTOM_MESSAGE_LENGTH = 500; +export const MAX_CUSTOM_MESSAGE_LENGTH = 500;prisma/migrations/20251111000000_add_viewer_invitations/migration.sql (2)
5-14: Consider the redundancy betweensentAtandcreatedAt.Both
sentAt(line 12) andcreatedAt(line 14) are defined asNOT NULLwithDEFAULT CURRENT_TIMESTAMP. This means they will always have the same value when a record is created.Consider one of these alternatives:
Option 1: Make
sentAtnullable and only set it on successful send"sentAt" TIMESTAMP(3),Then update it in the API when the email is actually sent.
Option 2: Remove
sentAtentirely
UsecreatedAtas the timestamp and letstatusindicate whether it was sent.Option 3: Rename
sentAttoattemptedAt
If you want to track when the attempt was made regardless of success/failure.The current schema works but creates unnecessary duplication.
19-26: LGTM! Indexes support common query patterns.The indexes on
viewerId,linkId, andgroupIdare appropriate for the expected queries (finding invitations by viewer, link, or group).If you find queries filtering by both
groupIdandviewerIdare common (e.g., in the uninvited endpoint), consider adding a composite index:CREATE INDEX "ViewerInvitation_groupId_viewerId_idx" ON "ViewerInvitation"("groupId" ASC, "viewerId" ASC);components/datarooms/groups/group-permissions.tsx (7)
58-73: Root item rendering withHomeIconis clear and consistentUsing
item.id === "__dataroom_root__"to detect the virtual root and render aHomeIconplus the samegetHierarchicalDisplayNamepipeline keeps naming consistent while giving a clear visual distinction for the root row. One minor consideration: the__dataroom_root__sentinel is now repeated in several places; if this ID is going to be reused more widely, extracting it into a shared constant (e.g.,const DATAROOM_ROOT_ID = "__dataroom_root__") would reduce the chance of typos.
101-131: Root-aware name column behavior looks correct; small cleanup opportunityThe updated
"name"column:
- Correctly suppresses the expand/collapse button for the virtual root (placeholder
div).- Uses
row.getCanExpand()only for non-root items, which is appropriate.- Delegates name/icon rendering to
PermissionItemName, keeping the cell logic focused on expansion controls.One minor nit:
disabled={isRoot}on the<Button>is redundant because that branch never renders for the root (isRootis checked beforerow.getCanExpand()), so the value is alwaysfalse. You can drop that prop for clarity.
297-346: Virtual root construction and aggregated root permissions look correct
buildTreeWithRoot:
- Wraps the existing tree in a synthetic
__dataroom_root__node withsubItemsset to all real items.- Aggregates root permissions by flattening the entire tree and computing
view,download, and partial flags based on any/all items being allowed.Even though the aggregation counts both folders and leaf documents, the boolean partial flags remain correct (partial if “some but not all accessible”), so this is logically sound. If you find yourself needing similar flattening elsewhere, you could consider extracting a shared helper, but that’s optional at this point.
376-497: Root-level permission updates correctly cascade and avoid persisting the virtual rootThe new root-specific branch in
updatePermissions:
- Detects
isRootearly and usesupdateAllItemsto set consistentview/downloadon the root and all descendants, clearing partial flags.- Rebuilds the entire
datatree via functionalsetData, so UI stays in sync.- Uses
collectAllChangesto build a flatItemPermissionmap for all real items, explicitly skipping__dataroom_root__before merging intopendingChanges.This ensures a root toggle produces a full, deterministic update across the tree without ever attempting to persist the synthetic root row. Note that
collectAllChangesreads fromdataRef.current(which still reflects the pre-root-change tree when invoked), but since it only usesitem.idanditem.itemTypeand writes uniformupdatedPermissionsfor each item, this does not introduce a correctness issue.
509-563: Non-root permission updates and parent/root recomputation are handled comprehensivelyFor non-root updates:
- The target item’s
partialView/partialDownloadare reset tofalse, and folder toggles correctly cascade down viaupdateSubItems.updateParentPermissionsrecalculates parent folders’view/downloadand partial flags based on their updatedsubItems.- When the parent is the virtual root, a dedicated
calculatePermissionsflattens descendants (excluding the root itself) to recompute root-level aggregate permissions.This keeps folder and root state derived from children, which is the right model. The logic is a bit dense; if it grows further, extracting
calculatePermissionsinto a shared helper used both here and inbuildTreeWithRootcould simplify reasoning, but functionally this looks sound.Also applies to: 564-638
643-715: Change collection correctly skips the virtual root and keeps parent folders consistentThe
collectChangeslogic:
- Never creates DB records for
__dataroom_root__(checks for the root on item, subitems, and parents before adding entries).- Propagates changes to all descendants of a toggled item, ensuring the backend sees the full cascade when a folder is toggled.
- Ensures parent folders become viewable when a child gains access, and re-computes parent view/download flags when access is turned off, based on siblings only.
This aligns well with the UI semantics and avoids polluting persistence with synthetic nodes. The only subtlety is that for the “make parents viewable” path you preserve
parent.permissions.downloadwhenupdatedPermissions.downloadis false, which matches the current behavior but might not be obvious at a glance—an inline comment could clarify that intent if you think future readers might be confused.
787-798: Verified: Row ID"0"assumption is guaranteed in your TanStack Table v8 configurationYour assumption is correct. With TanStack Table v8.21.3 and no custom
getRowIdimplementation, the first top-level row is guaranteed to have id"0"by default. The expanding feature enabled in your configuration doesn't override this for top-level rows.However, the secondary point remains valid: returning
falsefromgetRowCanExpandfor the root row would be semantically clearer and better match the comment "cannot be collapsed," whileinitialState.expandedandsubRowsstill ensure proper visibility.Optional refactor suggestion:
getRowCanExpand: (row) => { // Root folder cannot be expanded/collapsed (controlled via initialState) if (row.original.id === "__dataroom_root__") { return false; } return (row.subRows?.length ?? 0) > 0; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
components/datarooms/groups/group-member-table.tsx(7 hunks)components/datarooms/groups/group-permissions.tsx(17 hunks)components/datarooms/invite-viewers-modal.tsx(1 hunks)components/links/links-table.tsx(8 hunks)ee/features/dataroom-invitations/api/group-invite.ts(1 hunks)ee/features/dataroom-invitations/api/link-invite.ts(1 hunks)ee/features/dataroom-invitations/api/uninvited.ts(1 hunks)ee/features/dataroom-invitations/emails/components/dataroom-viewer-invitation.tsx(3 hunks)ee/features/dataroom-invitations/emails/lib/send-dataroom-viewer-invite.ts(2 hunks)ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts(1 hunks)ee/features/dataroom-invitations/lib/swr/use-dataroom-invitations.ts(1 hunks)lib/featureFlags/index.ts(2 hunks)lib/utils/link-url.ts(1 hunks)pages/api/jobs/send-dataroom-view-invitation.ts(0 hunks)pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/invite.ts(1 hunks)pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/uninvited.ts(1 hunks)pages/api/teams/[teamId]/datarooms/[id]/links/[linkId]/invite.ts(1 hunks)pages/datarooms/[id]/groups/[groupId]/links.tsx(2 hunks)pages/datarooms/[id]/index.tsx(2 hunks)pages/datarooms/[id]/permissions/index.tsx(2 hunks)prisma/migrations/20251111000000_add_viewer_invitations/migration.sql(1 hunks)prisma/schema/dataroom.prisma(1 hunks)prisma/schema/link.prisma(1 hunks)prisma/schema/schema.prisma(2 hunks)
💤 Files with no reviewable changes (1)
- pages/api/jobs/send-dataroom-view-invitation.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-19T07:46:44.421Z
Learnt from: CR
Repo: mfts/papermark PR: 0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing schema-validated tasks, use `schemaTask` from `trigger.dev/sdk/v3` and provide a schema using Zod or another supported library.
Applied to files:
ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts
🧬 Code graph analysis (14)
ee/features/dataroom-invitations/lib/swr/use-dataroom-invitations.ts (2)
context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)
ee/features/dataroom-invitations/api/link-invite.ts (8)
ee/features/dataroom-invitations/api/group-invite.ts (1)
handle(15-242)ee/features/dataroom-invitations/api/uninvited.ts (1)
handle(9-112)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts (2)
SendLinkInvitationSchema(34-34)invitationEmailSchema(5-5)lib/featureFlags/index.ts (1)
getFeatureFlags(20-63)lib/utils/link-url.ts (1)
constructLinkUrl(1-12)ee/features/dataroom-invitations/emails/lib/send-dataroom-viewer-invite.ts (1)
sendDataroomViewerInvite(5-35)
ee/features/dataroom-invitations/api/group-invite.ts (8)
ee/features/dataroom-invitations/api/link-invite.ts (1)
handle(18-222)ee/features/dataroom-invitations/api/uninvited.ts (1)
handle(9-112)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts (1)
SendGroupInvitationSchema(33-33)lib/featureFlags/index.ts (1)
getFeatureFlags(20-63)lib/utils/link-url.ts (1)
constructLinkUrl(1-12)ee/features/dataroom-invitations/emails/lib/send-dataroom-viewer-invite.ts (1)
sendDataroomViewerInvite(5-35)
pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/uninvited.ts (2)
pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/invite.ts (1)
handler(5-10)pages/api/teams/[teamId]/datarooms/[id]/links/[linkId]/invite.ts (1)
handler(5-10)
ee/features/dataroom-invitations/api/uninvited.ts (2)
pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)
components/datarooms/groups/group-permissions.tsx (1)
lib/utils.ts (1)
cn(18-20)
pages/datarooms/[id]/index.tsx (1)
components/links/links-table.tsx (1)
LinksTable(95-1091)
components/datarooms/invite-viewers-modal.tsx (4)
context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)ee/features/dataroom-invitations/lib/swr/use-dataroom-invitations.ts (1)
useUninvitedMembers(12-41)ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts (1)
invitationEmailSchema(5-5)
pages/datarooms/[id]/permissions/index.tsx (1)
components/links/links-table.tsx (1)
LinksTable(95-1091)
components/links/links-table.tsx (5)
lib/types.ts (1)
LinkWithViews(61-70)components/view/viewer/dataroom-viewer.tsx (1)
DocumentVersion(87-94)lib/hooks/use-feature-flags.ts (1)
useFeatureFlags(10-31)ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts (1)
invitationEmailSchema(5-5)components/datarooms/invite-viewers-modal.tsx (1)
InviteViewersModal(58-428)
components/datarooms/groups/group-member-table.tsx (5)
lib/swr/use-dataroom.ts (1)
useDataroom(21-55)lib/swr/use-dataroom-groups.ts (1)
useDataroomGroup(87-118)ee/features/dataroom-invitations/lib/swr/use-dataroom-invitations.ts (1)
useUninvitedMembers(12-41)lib/hooks/use-feature-flags.ts (1)
useFeatureFlags(10-31)components/datarooms/invite-viewers-modal.tsx (1)
InviteViewersModal(58-428)
pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/invite.ts (2)
pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/uninvited.ts (1)
handler(5-10)pages/api/teams/[teamId]/datarooms/[id]/links/[linkId]/invite.ts (1)
handler(5-10)
pages/api/teams/[teamId]/datarooms/[id]/links/[linkId]/invite.ts (2)
pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/invite.ts (1)
handler(5-10)pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/uninvited.ts (1)
handler(5-10)
pages/datarooms/[id]/groups/[groupId]/links.tsx (1)
components/links/links-table.tsx (1)
LinksTable(95-1091)
🪛 GitHub Check: CodeQL
components/datarooms/invite-viewers-modal.tsx
[failure] 221-231: Server-side request forgery
The URL of this request depends on a user-provided value.
🔇 Additional comments (32)
lib/featureFlags/index.ts (1)
15-15: LGTM! Feature flag properly added.The
dataroomInvitationsfeature flag is correctly added to both theBetaFeaturestype union and the defaultteamFeaturesrecord, following the established pattern for beta features.Also applies to: 33-33
pages/datarooms/[id]/index.tsx (2)
3-4: LGTM! Import consolidation.The import statement correctly consolidates the hooks from a single source.
46-50: LGTM! Dataroom name context added.Passing
dataroomName={dataroom.name}toLinksTableenables contextual display of the dataroom name in the invitation flow and other UI elements.pages/datarooms/[id]/permissions/index.tsx (1)
7-8: LGTM! Consistent with other dataroom pages.The import consolidation and addition of the
dataroomNameprop follow the same pattern as other dataroom pages, ensuring consistent contextual display across the application.Also applies to: 71-75
prisma/schema/dataroom.prisma (1)
127-127: LGTM! Verification confirms the ViewerInvitation model is properly configured.The
ViewerInvitationmodel is defined inprisma/schema/schema.prismawith the required back-relation toViewerGroup:
groupIdfield exists as optional (String?) on line 461- Back-relation defined with proper cascade behavior (line 462):
group ViewerGroup? @relation(fields: [groupId], references: [id], onDelete: SetNull)- Index configured for performance (line 472)
The one-to-many relationship from
ViewerGrouptoViewerInvitationis correctly established and the schema changes are valid.pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/invite.ts (1)
1-10: LGTM! Verification complete and successful.The shared handler implementation at
ee/features/dataroom-invitations/api/group-invite.tsconfirms:
- Authentication: Session validation with 401 response (line 26)
- Authorization: Team access check (line 51-55) + feature flag validation (line 58-62) with appropriate 401/403 responses
- Input Validation: Schema validation with
SendGroupInvitationSchema.safeParse()returning detailed error responses (lines 43-48)- Error Handling: Comprehensive error handling with proper HTTP status codes (400, 401, 403, 404, 500), detailed error responses, per-email error tracking in batch operations (lines 155-172), and global error logging/handling (lines 191-195)
The thin-wrapper pattern is properly implemented and the shared handler is production-ready.
pages/api/teams/[teamId]/datarooms/[id]/groups/[groupId]/uninvited.ts (1)
1-10: Verification complete—handler is properly implemented.The shared handler in
ee/features/dataroom-invitations/api/uninvited.tsincludes all required security and error handling:
- Authentication: Session validation via
getServerSessionwith 401 fallback- Authorization: Team membership check (
userTeam.findUnique) with 401 response- HTTP validation: Method check with 405 for non-GET requests
- Error handling: Comprehensive try-catch with 500 response, plus specific checks for missing groups (404)
- Input validation: Safely typed query parameters
- Response structure: Consistent JSON responses with proper status codes
pages/api/teams/[teamId]/datarooms/[id]/links/[linkId]/invite.ts (1)
1-10: LGTM! All verification checks passed.The shared handler in
ee/features/dataroom-invitations/api/link-invite.tsis well-implemented with comprehensive security controls:
- Authentication: Session validation via getServerSession (line 27-29)
- Authorization: Multi-layered checks including team membership (line 54-58), feature flag validation (line 61-65), and resource ownership (line 67-80)
- Input validation: Method validation (line 22-25), request body schema validation (line 39-45), email format validation (line 101, 105-109), and email list validation (line 113-117)
- Error handling: Appropriate HTTP status codes (400, 401, 403, 404, 405, 500) with descriptive messages; per-email error tracking; and console logging for debugging
prisma/schema/link.prisma (1)
90-90: LGTM! ViewerInvitation model is properly configured.Verification confirms the
viewerInvitationsrelation field correctly establishes a one-to-many relationship from Link to ViewerInvitation. The ViewerInvitation model exists in prisma/schema/schema.prisma (line 455) with the proper back-relation vialinkId(String field, line 459) and @relation configuration with onDelete: Cascade (line 460). The relation includes a performance index on linkId (line 471).pages/datarooms/[id]/groups/[groupId]/links.tsx (1)
1-5: LinksTable wiring withdataroomNamelooks correctUsing the group hooks and passing
dataroomName={dataroom.name}keeps this page aligned with the updated LinksTable API, and the early guard avoids rendering before data is ready. No issues from this change.Also applies to: 46-50
ee/features/dataroom-invitations/api/uninvited.ts (1)
1-112: Uninvited-members endpoint is well-scoped and auth-checkedThe handler cleanly restricts to GET, validates session and team access, scopes the group by
teamIdanddataroomId, and only returns emails for members without existing invitations. Early returns for no members, missing group, and error cases make the behavior predictable. Looks solid.ee/features/dataroom-invitations/lib/swr/use-dataroom-invitations.ts (1)
1-41: Hook nicely encapsulates uninvited-members APIConditionally constructing the key from
teamId,dataroomId, andgroupIdplus defaultingcount/emailskeeps consumers simple and safe. The returnedloading/error/mutateshape is consistent with other data hooks. Looks good.ee/features/dataroom-invitations/emails/lib/send-dataroom-viewer-invite.ts (1)
3-28: Email helper correctly extended for customMessage and recipientEmailThe updated
sendDataroomViewerInvitesignature and payload cleanly addcustomMessageandrecipientEmailwithout changing existing behaviour. The helper remains a thin, focused wrapper aroundsendEmail.ee/features/dataroom-invitations/api/link-invite.ts (1)
18-221: Link invitation handler is well‑structured and aligned with group invitesThe handler has appropriate method checks, auth/team checks, feature‑flag gating, and validates emails both via the schema and
invitationEmailSchema. Viewer creation,ViewerInvitationpersistence, and error reporting are consistent with the Prisma model and the group invite flow.No blocking issues spotted here.
components/links/links-table.tsx (1)
16-17: Dataroom invitation entry point is cleanly integrated and feature‑gatedThe LinksTable changes plug the invitation flow in coherently:
handleSendInvitationsscopes to DATAROOM links and pre-sanitizesallowListemails.- The “Send Invitations” menu item and the modal rendering are both guarded by
isFeatureEnabled("dataroomInvitations").- Passing
dataroomId,dataroomName,groupId,linkId, anddefaultEmailsintoInviteViewersModal, withonSuccessmutating the appropriatelinksApiRoute, keeps the UI and data in sync.Looks good once the modal’s default recipient handling is adjusted as noted in the modal file.
Also applies to: 30-32, 95-107, 189-199, 337-353, 942-950, 1045-1065
components/datarooms/groups/group-member-table.tsx (4)
7-7: LGTM! Imports properly support the invitation feature.The new imports for invitation functionality (SendIcon, Badge, InviteViewersModal) and data hooks (useDataroom, useUninvitedMembers, useFeatureFlags) are appropriately integrated.
Also applies to: 14-17, 19-19, 41-41
70-98: LGTM! Toggle logic is correct.The
handleToggleAllowAllfunction properly updates the group settings and provides appropriate user feedback. The cache key usage can be simplified as noted in the previous comment.
209-351: LGTM! Table rendering handles all states appropriately.The table properly handles the three primary states (allow-all, empty, populated) and includes skeleton loading states for a better user experience.
359-379: Verify the double mutation pattern is intentional.The
InviteViewersModaltriggersmutateUninvited()in two places:
- In the
setOpencallback when closing (lines 364-366)- In the
onSuccesscallback (line 376)Since
onSuccessis called before the modal closes (based on the InviteViewersModal implementation callinghandleClose()afteronSuccess?.()), this results inmutateUninvited()being called twice in quick succession on successful invitation.This may be intentional to ensure data freshness, but if not needed, you could optimize by removing the mutation from the
setOpencallback:<InviteViewersModal open={inviteOpen} setOpen={(next) => { setInviteOpen(next); - if (!next) { - mutateUninvited(); - } }} dataroomId={dataroomId} dataroomName={dataroom?.name ?? "this dataroom"} groupId={groupId} defaultEmails={uninvitedEmails} onSuccess={() => { if (groupKey) { mutate(groupKey); } mutateUninvited(); }} />Alternatively, keep both if you want to ensure data refresh even when the modal is closed without sending invitations.
ee/features/dataroom-invitations/api/group-invite.ts (5)
1-22: LGTM! Imports and method validation are appropriate.The imports are well-organized and the HTTP method check follows standard Next.js API route patterns.
24-70: LGTM! Authentication and authorization checks are comprehensive.The handler properly validates:
- User session
- Request body against the Zod schema
- Team membership
- Feature flag enablement
This provides a solid security foundation.
127-152: LGTM! Email filtering logic is secure.The target email logic correctly:
- Extracts available emails from group members (lines 136-138)
- Filters the provided emails to only those present in the group (lines 140-145)
- Validates that at least one valid email remains (lines 148-152)
This prevents sending invitations to arbitrary email addresses not in the group.
154-177: LGTM! Viewer lookup and URL construction are correct.The code properly queries existing viewers and constructs a lookup map by email for efficient access during the invitation loop.
232-241: LGTM! Response structure and error handling are appropriate.The handler returns a clear response with both successes and failures, and includes proper error logging for debugging.
ee/features/dataroom-invitations/lib/schema/dataroom-invitations.ts (2)
22-37: LGTM! Invitation schemas are well-designed.The schemas correctly model the requirements:
sendGroupInvitationSchemarequires alinkId(since group invitations must specify which link to use)sendLinkInvitationSchemadoesn't requirelinkId(it's derived from the route)- Both support optional custom messages and recipient emails
The type exports and aliases follow good practices.
12-20: The optional custom message schema works correctly.The transform logic properly normalizes all empty/falsy inputs to
undefined, which simplifies downstream handling. The union withz.literal("")ensures empty strings are accepted before transformation.prisma/migrations/20251111000000_add_viewer_invitations/migration.sql (1)
28-36: LGTM! Foreign key constraints are well-designed.The cascade behaviors are appropriate:
viewerIdandlinkIdwithCASCADEdelete (lines 29, 32): When a viewer or link is deleted, their invitations are removedgroupIdwithSET NULL(line 35): When a group is deleted, invitations are retained for historical/audit purposes with the groupId clearedThis provides good data integrity while preserving audit trails.
components/datarooms/groups/group-permissions.tsx (5)
3-38: Imports and new icons look consistent; watch the unusedSwitchimportThe extended React imports (
useCallback,useEffect,useMemo,useRef,useState) and newHomeIconare consistent with the new behavior. The newly addedSwitchimport is currently unused in this file; if it’s not needed for a follow-up change, consider removing it to avoid lint warnings, otherwise keeping it is fine if you plan to use it shortly.
[ suggest_optional_refactor ]
367-372: UsingdataRefto avoid stale closures inupdatePermissionsis a good patternThe
dataRefplususeEffectpattern letsupdatePermissionshave[]as its dependency array while still operating on the latest tree when callingfindItemAndParents. This avoids constant re-creations of the callback and is safe here since you only read fromdataRef.currentand use functionalsetData/setPendingChanges. This is a solid approach for this use case.Also applies to: 398-399
728-733: Tree initialization with virtual root is straightforward and guarded by loading stateBuilding the data via
buildTreeWithRoot(folders, permissions, "Dataroom Home")oncefoldersare loaded andloadingis false is clean and ensures the state always contains exactly one root at index0. This fits neatly with the laterinitialState.expandedconfiguration that assumes the root row is first.
827-855: Row rendering with root highlighting and depth-based indentation looks goodRendering rows via
table.getRowModel().rows.mapand:
- Applying a subtle background to the root row (
bg-blue-50/50/dark:bg-blue-950/50).- Indenting the first column based on
row.depth * 1.25rem.- Keeping the last cell flex-aligned to the right.
produces a clear tree-like structure and helps visually distinguish the root from its descendants without affecting the underlying data semantics. This is a nice UX improvement.
196-214: Update the comment to accurately reflect the permission defaulting logic and clarify the semantic intentThe code and comment have a meaningful discrepancy:
- Comment (line 205) claims: "default to true for consistency"
- Actual logic (line 209) implements:
view: permission ? permission.canView : hasPermissionData ? false : trueThis means once a group has any permission entry, missing items default to
view: false, nottrue. For new items added to a dataroom after permissions are configured, they will appear inaccessible to that group until explicitly granted access.The original review question is valid: confirm that this "deny-by-default-once-configured" behavior matches your UX/backend expectations. If the intent is indeed to deny new items by default after initial configuration, the comment should be rewritten to say so. If new items should inherit an open-by-default state, the
getPermissionslogic needs adjustment.Folder aggregation logic (lines 245-272) correctly derives folder permissions from sub-items' actual permissions, so it functions as designed.
| const defaultRecipients = | ||
| defaultEmails.length > 0 ? defaultEmails : groupId ? uninvitedEmails : []; | ||
|
|
||
| const fallbackSubject = `You are invited to view ${dataroomName}`; | ||
|
|
||
| const handleClose = () => { | ||
| setOpen(false); | ||
| setLoading(false); | ||
| }; | ||
|
|
||
| const handleSend = async () => { | ||
| if (!teamId) { | ||
| toast.error("No active team selected"); | ||
| return; | ||
| } | ||
|
|
||
| if (groupId && !selectedLinkId) { | ||
| toast.error("Select a link to include in the invitation"); | ||
| return; | ||
| } | ||
|
|
||
| const parsedEmails = hasEditedRecipients | ||
| ? parseRecipientInput(recipientInput) | ||
| : []; | ||
|
|
||
| if (parsedEmails.length > 0) { | ||
| const invalidEmails = parsedEmails.filter( | ||
| (email) => !invitationEmailSchema.safeParse(email).success, | ||
| ); | ||
|
|
||
| if (invalidEmails.length > 0) { | ||
| toast.error( | ||
| `Found invalid emails: ${invalidEmails | ||
| .slice(0, 3) | ||
| .join(", ")}${invalidEmails.length > 3 ? "..." : ""}`, | ||
| ); | ||
| return; | ||
| } | ||
| } |
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.
Default recipient handling conflicts with “uninvited members” semantics
Today, parsedEmails is only populated when the user edits the field:
const parsedEmails = hasEditedRecipients
? parseRecipientInput(recipientInput)
: [];and the request body only includes emails when parsedEmails.length > 0. For group invites this means:
- The UI pre-fills the textarea with
uninvitedEmailsand copy says “Leave blank to invite N uninvited members”. - But if the user truly “leaves it blank” (doesn’t type),
emailsis omitted and the backend falls back to all group member emails, re‑inviting already invited members.
A tighter, more intuitive behaviour is to treat the derived default recipients as the implicit selection when the user hasn’t edited anything:
- const parsedEmails = hasEditedRecipients
- ? parseRecipientInput(recipientInput)
- : [];
+ const parsedEmails = hasEditedRecipients
+ ? parseRecipientInput(recipientInput)
+ : defaultRecipients;
@@
- emails: parsedEmails.length > 0 ? parsedEmails : undefined,
+ emails: parsedEmails.length > 0 ? parsedEmails : undefined,This makes group invites respect the “uninvited members” semantics and keeps link invites explicit (using the same sanitized/default list the UI shows) while still allowing users to override recipients by editing the field.
Additionally, to keep the character limit aligned with the schema, consider importing and using the shared MAX_CUSTOM_MESSAGE_LENGTH instead of the hard-coded 500 in the Textarea and counter.
Also applies to: 319-347
🤖 Prompt for AI Agents
In components/datarooms/invite-viewers-modal.tsx around lines 167-205, the code
only populates parsedEmails when the user edits the recipients field, which
causes leaving the prefilled "uninvitedEmails" textarea unchanged to omit emails
from the request and re-invite already-invited group members; update the logic
so that when hasEditedRecipients is false you use the derived defaultRecipients
(defaultEmails or uninvitedEmails) as the effective recipient list
(sanitized/validated the same way as parsed input) and include that list in the
request body, while preserving the ability for users to override by editing;
also replace the hard-coded 500 character limit in the Textarea and counter with
the shared MAX_CUSTOM_MESSAGE_LENGTH constant imported from the shared schema so
limits stay in sync.
| for (const email of targetEmails) { | ||
| const viewer = viewerByEmail[email]; | ||
| if (!viewer) { | ||
| failures.push({ | ||
| email, | ||
| error: "Viewer not found", | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| await sendDataroomViewerInvite({ | ||
| dataroomName: group.dataroom.name, | ||
| senderEmail: teamMember.email, | ||
| to: email, | ||
| url: linkUrl, | ||
| customMessage, | ||
| }); | ||
|
|
||
| await prisma.viewerInvitation.create({ | ||
| data: { | ||
| viewerId: viewer.id, | ||
| linkId: link.id, | ||
| groupId, | ||
| invitedBy: user.id, | ||
| customMessage, | ||
| status: "SENT", | ||
| }, | ||
| }); | ||
|
|
||
| successes.push(email); | ||
| } catch (error: any) { | ||
| failures.push({ | ||
| email, | ||
| error: error?.message ?? "Unknown error", | ||
| }); | ||
|
|
||
| await prisma.viewerInvitation.create({ | ||
| data: { | ||
| viewerId: viewer.id, | ||
| linkId: link.id, | ||
| groupId, | ||
| invitedBy: user.id, | ||
| customMessage, | ||
| status: "FAILED", | ||
| }, | ||
| }); | ||
| } | ||
| } |
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.
Consider wrapping email sending and database writes in a transaction.
The current implementation sends emails and creates database records sequentially without transactional guarantees:
- Line 193-199:
sendDataroomViewerInviteis called - Line 201-210:
prisma.viewerInvitation.createis called
If the email send succeeds but the database write fails, the email has been sent but there's no record of it. On retry, the same email could be sent again. Similarly, if the database write fails, the error handler at lines 213-229 will create a FAILED record even though the email was successfully sent.
Consider one of these approaches:
Option 1: Create invitation record before sending (recommended)
for (const email of targetEmails) {
const viewer = viewerByEmail[email];
if (!viewer) {
failures.push({
email,
error: "Viewer not found",
});
continue;
}
+ const invitation = await prisma.viewerInvitation.create({
+ data: {
+ viewerId: viewer.id,
+ linkId: link.id,
+ groupId,
+ invitedBy: user.id,
+ customMessage,
+ status: "SENT",
+ },
+ });
try {
await sendDataroomViewerInvite({
dataroomName: group.dataroom.name,
senderEmail: teamMember.email,
to: email,
url: linkUrl,
customMessage,
});
- await prisma.viewerInvitation.create({
- data: {
- viewerId: viewer.id,
- linkId: link.id,
- groupId,
- invitedBy: user.id,
- customMessage,
- status: "SENT",
- },
- });
successes.push(email);
} catch (error: any) {
+ await prisma.viewerInvitation.update({
+ where: { id: invitation.id },
+ data: { status: "FAILED" },
+ });
+
failures.push({
email,
error: error?.message ?? "Unknown error",
});
-
- await prisma.viewerInvitation.create({
- data: {
- viewerId: viewer.id,
- linkId: link.id,
- groupId,
- invitedBy: user.id,
- customMessage,
- status: "FAILED",
- },
- });
}
}Option 2: Add idempotency checks
Query for existing invitations before sending to avoid duplicates on retry.
📝 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.
| for (const email of targetEmails) { | |
| const viewer = viewerByEmail[email]; | |
| if (!viewer) { | |
| failures.push({ | |
| email, | |
| error: "Viewer not found", | |
| }); | |
| continue; | |
| } | |
| try { | |
| await sendDataroomViewerInvite({ | |
| dataroomName: group.dataroom.name, | |
| senderEmail: teamMember.email, | |
| to: email, | |
| url: linkUrl, | |
| customMessage, | |
| }); | |
| await prisma.viewerInvitation.create({ | |
| data: { | |
| viewerId: viewer.id, | |
| linkId: link.id, | |
| groupId, | |
| invitedBy: user.id, | |
| customMessage, | |
| status: "SENT", | |
| }, | |
| }); | |
| successes.push(email); | |
| } catch (error: any) { | |
| failures.push({ | |
| email, | |
| error: error?.message ?? "Unknown error", | |
| }); | |
| await prisma.viewerInvitation.create({ | |
| data: { | |
| viewerId: viewer.id, | |
| linkId: link.id, | |
| groupId, | |
| invitedBy: user.id, | |
| customMessage, | |
| status: "FAILED", | |
| }, | |
| }); | |
| } | |
| } | |
| for (const email of targetEmails) { | |
| const viewer = viewerByEmail[email]; | |
| if (!viewer) { | |
| failures.push({ | |
| email, | |
| error: "Viewer not found", | |
| }); | |
| continue; | |
| } | |
| const invitation = await prisma.viewerInvitation.create({ | |
| data: { | |
| viewerId: viewer.id, | |
| linkId: link.id, | |
| groupId, | |
| invitedBy: user.id, | |
| customMessage, | |
| status: "SENT", | |
| }, | |
| }); | |
| try { | |
| await sendDataroomViewerInvite({ | |
| dataroomName: group.dataroom.name, | |
| senderEmail: teamMember.email, | |
| to: email, | |
| url: linkUrl, | |
| customMessage, | |
| }); | |
| successes.push(email); | |
| } catch (error: any) { | |
| await prisma.viewerInvitation.update({ | |
| where: { id: invitation.id }, | |
| data: { status: "FAILED" }, | |
| }); | |
| failures.push({ | |
| email, | |
| error: error?.message ?? "Unknown error", | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In ee/features/dataroom-invitations/api/group-invite.ts around lines 182–230,
the invite send and DB writes are done sequentially causing possible
inconsistencies; change to create or check an invitation record before sending,
then update its status after send so operations are idempotent. Implement: 1)
query for an existing viewerInvitation for (viewerId, linkId, groupId) and skip
or return if one already exists; 2) if none, create a viewerInvitation with
status "PENDING" (or upsert to PENDING) before calling sendDataroomViewerInvite;
3) call sendDataroomViewerInvite; 4) on success update that record to status
"SENT", on error update to "FAILED" and capture error.message; and 5) wrap the
DB create/update calls in Prisma transactions where possible to keep DB state
consistent while keeping the external email send outside the DB transaction.
Ensure you still push to successes/failures according to final status.
Summary by CodeRabbit