-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Feat: Support team-level allow list groups for links and presets #1766
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
base: main
Are you sure you want to change the base?
Conversation
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces the concept of "allow list groups" to the application, enabling grouping of email/domain allow-lists for reuse across multiple links and presets. It adds backend models, API endpoints, and database schema for allow list groups, integrates group selection and management into the UI, and updates access control logic to check both individual and group-based allow-lists. Existing link and preset creation, duplication, and editing workflows are extended to support the new group association. Changes
Sequence Diagram(s)Allow List Group Access Check (New/Updated Flow)sequenceDiagram
participant User
participant Frontend
participant Backend (API)
participant DB
User->>Frontend: Request access (with email)
Frontend->>Backend (API): GET /views (with email)
Backend (API)->>DB: Fetch link (include allowList, allowListGroup)
DB-->>Backend (API): Return link, allowList, allowListGroup.allowList
Backend (API)->>Backend (API): isEmailAllowedByAllowList(email, allowList, allowListGroup)
alt Email allowed
Backend (API)-->>Frontend: Grant access
else Not allowed
Backend (API)-->>Frontend: 403 Unauthorized
end
Frontend-->>User: Show result
Allow List Group CRUD OperationssequenceDiagram
participant User
participant Frontend
participant Backend (API)
participant DB
User->>Frontend: Create/Edit/Delete Allow List Group
Frontend->>Backend (API): POST/PUT/DELETE /teams/{teamId}/allow-list-groups
Backend (API)->>DB: Insert/Update/Delete AllowListGroup
DB-->>Backend (API): Result
Backend (API)-->>Frontend: Success/Error
Frontend-->>User: Update UI/Show notification
Assigning Allow List Group to LinksequenceDiagram
participant User
participant Frontend
participant Backend (API)
participant DB
User->>Frontend: Select Allow List Group in Link Editor
Frontend->>Backend (API): PUT /links/{id} (allowListGroupId)
Backend (API)->>DB: Update Link.allowListGroupId
DB-->>Backend (API): Success
Backend (API)-->>Frontend: Confirm update
Frontend-->>User: Show updated link info
Viewing Allow List Group DetailssequenceDiagram
participant User
participant Frontend
participant Backend (API)
participant DB
User->>Frontend: Navigate to /visitors/groups/{groupId}
Frontend->>Backend (API): GET /teams/{teamId}/allow-list-groups/{groupId}
Backend (API)->>DB: Fetch AllowListGroup, related links
DB-->>Backend (API): Return group details
Backend (API)-->>Frontend: Group data
Frontend-->>User: Show group info, links, edit/delete options
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 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 (
|
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: 4
🧹 Nitpick comments (7)
lib/utils/allow-list-access.ts (1)
8-10
: Consider enhancing email validation.The current email validation only checks for the presence of "@" which is quite basic. Consider using a more robust email validation or leveraging the existing
validateEmail
utility function used elsewhere in the codebase.- if (!email || typeof email !== "string" || !email.includes("@")) { - return false; - } + if (!email || typeof email !== "string" || !validateEmail(email)) { + return false; + }You would need to import
validateEmail
from@/lib/utils/validate-email
.pages/api/teams/[teamId]/allow-list-groups/index.ts (1)
40-40
: Consider improving type safety for session user.The repeated use of
(session.user as any).id
bypasses TypeScript's type checking. Consider extending the session type or creating a typed helper.Create a typed helper or extend the session type:
+import { Session } from "next-auth"; + +interface SessionWithUser extends Session { + user: { + id: string; + email?: string | null; + name?: string | null; + }; +} + +// Then cast once at the top +const typedSession = session as SessionWithUser;Also applies to: 105-105, 142-142
pages/visitors/groups/[groupId].tsx (2)
252-309
: Consider pagination for large link lists.Groups with many links could create performance issues. Consider implementing pagination or virtualization.
For groups with many links, the current implementation might become unwieldy. Would you like me to create an issue to track adding pagination or virtualization for the links list?
244-248
: Consider UI improvements for long allow lists.The
<pre>
tag withwhitespace-pre-wrap
might not provide the best UX for very long lists. Consider a scrollable container with max height.Add a scrollable container with max height:
-<div className="rounded-lg bg-gray-50 p-4 dark:bg-gray-800/50"> - <pre className="whitespace-pre-wrap break-all font-mono text-sm text-foreground"> +<div className="rounded-lg bg-gray-50 p-4 dark:bg-gray-800/50 max-h-96 overflow-y-auto"> + <pre className="whitespace-pre-wrap break-all font-mono text-sm text-foreground min-h-[3rem]"> {group.allowList.join("\n")} </pre> </div>components/visitors/allow-list-group-modal.tsx (1)
105-107
: Consider using a consistent error extraction patternThe error handling could be more consistent with the API response pattern used in create/update functions.
- toast.error( - error instanceof Error ? error.message : "Something went wrong", - ); + const errorMessage = error instanceof Error + ? error.message + : (error?.error || "Something went wrong"); + toast.error(errorMessage);components/visitors/allow-list-groups-section.tsx (1)
5-8
: Consolidate icon importsConsider consolidating the lucide-react imports for cleaner code.
-import { LinkIcon, PlusIcon, UsersIcon } from "lucide-react"; -import { ShieldIcon } from "lucide-react"; -import { SearchIcon } from "lucide-react"; +import { LinkIcon, PlusIcon, UsersIcon, ShieldIcon, SearchIcon } from "lucide-react";pages/visitors/index.tsx (1)
119-123
: Consider removing or documenting commented codeThe commented "Access Requests" tab code should either be removed or documented with a TODO/FIXME comment explaining the future implementation plan.
- {/* <TabsTrigger value="access-requests" className="flex items-center gap-2"> - <UserPlus className="h-5 w-5" /> - Access Requests - </TabsTrigger> */} + {/* TODO: Implement Access Requests tab in future iteration + <TabsTrigger value="access-requests" className="flex items-center gap-2"> + <UserPlus className="h-5 w-5" /> + Access Requests + </TabsTrigger> */}Also applies to: 201-210
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
app/api/views-dataroom/route.ts
(3 hunks)app/api/views/route.ts
(3 hunks)components/layouts/breadcrumb.tsx
(5 hunks)components/links/link-sheet/allow-list-section.tsx
(4 hunks)components/links/link-sheet/index.tsx
(3 hunks)components/links/links-table.tsx
(2 hunks)components/visitors/allow-list-group-modal.tsx
(1 hunks)components/visitors/allow-list-groups-section.tsx
(1 hunks)components/visitors/create-group-button.tsx
(1 hunks)lib/swr/use-allow-list-groups.ts
(1 hunks)lib/utils/allow-list-access.ts
(1 hunks)lib/zod/schemas/presets.ts
(1 hunks)pages/api/links/[id]/duplicate.ts
(1 hunks)pages/api/links/[id]/index.ts
(3 hunks)pages/api/links/index.ts
(1 hunks)pages/api/teams/[teamId]/allow-list-groups/[groupId].ts
(1 hunks)pages/api/teams/[teamId]/allow-list-groups/index.ts
(1 hunks)pages/api/webhooks/services/[...path]/index.ts
(4 hunks)pages/settings/presets/[id].tsx
(3 hunks)pages/settings/presets/new.tsx
(1 hunks)pages/visitors/groups/[groupId].tsx
(1 hunks)pages/visitors/index.tsx
(4 hunks)prisma/migrations/20250716200424_added_allow_list_groups/migration.sql
(1 hunks)prisma/schema/link.prisma
(2 hunks)prisma/schema/schema.prisma
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
app/api/views/route.ts (1)
lib/utils/allow-list-access.ts (1)
isEmailAllowedByAllowList
(3-25)
components/layouts/breadcrumb.tsx (1)
lib/swr/use-allow-list-groups.ts (1)
useAllowListGroup
(85-103)
pages/api/teams/[teamId]/allow-list-groups/index.ts (2)
pages/api/auth/[...nextauth].ts (1)
authOptions
(32-215)lib/utils.ts (1)
log
(52-112)
pages/api/teams/[teamId]/allow-list-groups/[groupId].ts (2)
pages/api/auth/[...nextauth].ts (1)
authOptions
(32-215)lib/utils.ts (1)
log
(52-112)
lib/swr/use-allow-list-groups.ts (2)
context/team-context.tsx (1)
useTeam
(85-85)lib/utils.ts (1)
fetcher
(36-50)
app/api/views-dataroom/route.ts (1)
lib/utils/allow-list-access.ts (1)
isEmailAllowedByAllowList
(3-25)
components/visitors/allow-list-groups-section.tsx (4)
lib/swr/use-allow-list-groups.ts (2)
AllowListGroup
(6-16)useAllowListGroups
(34-63)components/ui/card.tsx (2)
Card
(80-80)CardContent
(85-85)components/ui/button.tsx (1)
Button
(71-71)components/visitors/allow-list-group-modal.tsx (1)
AllowListGroupModal
(33-190)
lib/utils/allow-list-access.ts (1)
lib/utils/email-domain.ts (1)
isEmailMatched
(33-53)
pages/visitors/index.tsx (6)
lib/swr/use-allow-list-groups.ts (1)
useAllowListGroups
(34-63)components/search-box.tsx (1)
SearchBoxPersisted
(116-167)components/ui/card.tsx (3)
Card
(80-80)CardHeader
(81-81)CardContent
(85-85)components/ui/tooltip.tsx (1)
BadgeTooltip
(53-107)components/visitors/create-group-button.tsx (1)
CreateGroupButton
(9-25)components/visitors/allow-list-groups-section.tsx (1)
AllowListGroupsSection
(19-172)
🔇 Additional comments (48)
pages/api/links/[id]/duplicate.ts (1)
84-84
: LGTM! Correct implementation of allow list group duplication.The
allowListGroupId
field is properly copied during link duplication, ensuring the duplicated link maintains the same allow list group association as the original.components/links/links-table.tsx (1)
223-223
: LGTM! Proper handling of allow list group ID in edit link flow.The
allowListGroupId
field is correctly set from the link data with appropriate null fallback, ensuring the edit link form receives the correct allow list group association.lib/zod/schemas/presets.ts (1)
45-45
: LGTM! Well-structured schema addition for allow list group ID.The
allowListGroupId
field is properly defined as an optional, nullable string, which is appropriate for an ID field that may not always be present. The positioning among access control properties is logical.pages/settings/presets/new.tsx (1)
138-138
: LGTM! Correct inclusion of allow list group ID in preset creation.The
allowListGroupId
field is properly included in the API request payload for preset creation, ensuring that new presets can be associated with allow list groups.pages/api/links/index.ts (1)
157-157
: LGTM! Proper handling of allow list group ID in link creation.The
allowListGroupId
field is correctly set from the request data with appropriate null fallback, ensuring links can be properly associated with allow list groups during creation.components/visitors/create-group-button.tsx (1)
1-26
: LGTM! Well-structured component with clean implementation.The component follows React best practices with proper state management, clean imports, and good component composition. The modal integration is implemented correctly with appropriate prop passing.
pages/settings/presets/[id].tsx (2)
126-126
: LGTM! Proper state initialization for allowListGroupId.The field is correctly added to the state initialization, following the same pattern as other preset fields.
169-169
: LGTM! Proper API payload integration for allowListGroupId.The field is correctly included in the PUT request payload, ensuring the allowListGroupId is persisted when updating the preset.
pages/api/links/[id]/index.ts (3)
67-67
: LGTM! Proper database field selection for allowListGroupId.The field is correctly added to the Prisma select query, ensuring it's fetched from the database along with other link properties.
154-154
: LGTM! Proper API response integration for allowListGroupId.The field is correctly included in the returned link object, making it available to the frontend.
295-295
: LGTM! Proper database update integration for allowListGroupId.The field is correctly included in the Prisma update data with appropriate null fallback handling, ensuring it can be properly persisted.
components/links/link-sheet/index.tsx (3)
67-67
: LGTM! Proper default property initialization for allowListGroupId.The field is correctly added to the default link properties with a null initial value, consistent with other optional fields.
110-110
: LGTM! Proper TypeScript type definition for allowListGroupId.The field is correctly added to the type definition with appropriate
string | null
typing, maintaining type safety.
249-249
: LGTM! Proper preset application logic for allowListGroupId.The field is correctly included in the preset application logic with appropriate fallback to the previous state value, maintaining consistency with other preset fields.
pages/api/webhooks/services/[...path]/index.ts (4)
46-46
: LGTM! Proper schema validation for allowListGroupId.The field is correctly added to the Zod schema with optional string validation, maintaining consistency with other optional fields.
452-452
: LGTM! Proper webhook link update integration for allowListGroupId.The field is correctly included in the link update data with appropriate null fallback, ensuring it's properly handled in document creation webhooks.
659-659
: LGTM! Proper webhook link creation integration for allowListGroupId.The field is correctly included in the link creation data with appropriate null fallback, ensuring it's properly handled in link creation webhooks.
831-831
: LGTM! Proper webhook dataroom link creation integration for allowListGroupId.The field is correctly included in the dataroom link creation data with appropriate null fallback, ensuring it's properly handled in dataroom creation webhooks.
lib/utils/allow-list-access.ts (1)
3-25
: Well-structured utility function with good logic flow.The function correctly combines allow lists from two sources using a Set to avoid duplicates and handles the "no restrictions" case appropriately. The implementation follows good practices by reusing the existing
isEmailMatched
function.prisma/schema/link.prisma (2)
83-84
: Appropriate schema design for allow list group association.The optional foreign key relationship with
SetNull
onDelete behavior is well-designed. When an allow list group is deleted, the associated links will simply lose their group reference without breaking functionality.
110-111
: Consistent implementation across Link and LinkPreset models.The same allow list group relationship is correctly implemented in LinkPreset, ensuring consistency in how presets and links handle allow list groups.
app/api/views-dataroom/route.ts (3)
30-30
: Good addition of the new utility import.The import for
isEmailAllowedByAllowList
is correctly added to support the enhanced allow list validation logic.
114-119
: Proper database query extension for allow list groups.The Prisma query is correctly updated to include the new
allowListGroupId
and nestedallowListGroup.allowList
fields, ensuring all necessary data is fetched for the enhanced access validation.
299-318
: Enhanced access validation with improved logic.The refactored access validation logic is more robust and maintainable. Using the centralized
isEmailAllowedByAllowList
utility function ensures consistency across the application and properly handles both individual and group-based allow lists.components/layouts/breadcrumb.tsx (6)
6-6
: Appropriate import for the new functionality.The
useAllowListGroup
hook import is correctly added to support the new visitor group breadcrumb functionality.
369-369
: Simplified breadcrumb implementation.The change from
BreadcrumbLink
toBreadcrumbPage
is appropriate since the visitors page is now the terminal page in this breadcrumb path.
398-418
: Well-implemented visitor group breadcrumb component.The new
SingleVisitorGroupBreadcrumb
component follows established patterns and properly handles loading states. The navigation back to the visitors page with the allow-lists tab is intuitive for users.
459-462
: Proper router query parameter handling.The extraction of
groupId
from router query is correctly implemented and follows the existing pattern used for theid
parameter.
541-544
: Appropriate route handling for visitor groups.The new route handler for
/visitors/groups/[groupId]
is correctly implemented and follows the established pattern used for other dynamic routes.
547-547
: Correct memoization dependency update.The memoization dependencies are properly updated to include
groupId
, ensuring the breadcrumb re-renders when the group changes.app/api/views/route.ts (3)
26-27
: Appropriate import updates for enhanced functionality.The import changes correctly remove the unused
extractEmailDomain
and add the newisEmailAllowedByAllowList
utility function, keeping imports clean and focused.
97-102
: Consistent database query extension.The Prisma query is properly updated to include the new allow list group fields, mirroring the implementation in the dataroom views route for consistency.
234-253
: Consistent and improved access validation logic.The access validation logic is correctly updated to use the centralized utility function, ensuring consistency with the dataroom views route and providing enhanced allow list group support.
prisma/schema/schema.prisma (1)
114-114
: Schema changes look good!The
AllowListGroup
model is well-structured with appropriate relations, constraints, and indexes. The cascade behavior for team deletion and SetNull for links/presets are sensible choices.Also applies to: 117-132
pages/api/teams/[teamId]/allow-list-groups/[groupId].ts (1)
164-177
: Good role-based access control!The DELETE handler properly restricts access to ADMIN and MANAGER roles only, following the principle of least privilege.
prisma/migrations/20250716200424_added_allow_list_groups/migration.sql (1)
1-33
: Migration looks correct!The SQL migration properly implements the schema changes with appropriate constraints, indexes, and cascade behaviors.
components/visitors/allow-list-group-modal.tsx (2)
39-56
: Clean state management and form initializationThe component properly manages form state and correctly initializes fields when switching between create and edit modes.
121-190
: Well-structured modal UI with good UXThe modal implementation provides clear user guidance with appropriate placeholders, loading states, and dynamic button text.
components/links/link-sheet/allow-list-section.tsx (4)
45-59
: Proper integration of allow list groupsThe component correctly initializes state considering both individual allow lists and group selection, maintaining backward compatibility.
94-111
: Verify preset loading doesn't override user selectionsThe preset loading effect should only run on initial mount to avoid overriding user selections during component lifecycle.
Consider adding a flag to prevent re-initialization:
+ const [presetsLoaded, setPresetsLoaded] = useState(false); + useEffect(() => { - if (isAllowed && presets) { + if (isAllowed && presets && !presetsLoaded) { // Load preset data if available if (presets.allowList && presets.allowList.length > 0) { setEnabled(true); setAllowListInput(presets.allowList.join("\n") || ""); } if (presets.allowListGroupId) { setEnabled(true); setData((prevData) => ({ ...prevData, allowListGroupId: presets.allowListGroupId, })); } + setPresetsLoaded(true); } - }, [presets, isAllowed, setData]); + }, [presets, isAllowed, setData, presetsLoaded]);
152-199
: Excellent UI state handlingThe component provides clear loading indicators and helpful empty states with actionable links for users to create their first allow list group.
200-283
: Well-designed group selection UIThe group selection interface provides excellent visibility with email count badges, expandable previews, and clear guidance about combining group and individual emails.
components/visitors/allow-list-groups-section.tsx (2)
40-63
: Good loading state implementationThe skeleton loading state properly matches the structure of the actual content, providing a smooth loading experience.
65-156
: Clean component structure with helpful empty statesThe component provides clear visual hierarchy with icons, counts, and appropriate empty states that guide users to take action.
lib/swr/use-allow-list-groups.ts (2)
65-103
: Well-structured SWR hooksThe hooks follow consistent patterns with proper conditional fetching and reasonable deduping intervals.
105-157
: Robust API functions with consistent error handlingThe CRUD functions follow consistent patterns with proper error extraction and meaningful error messages.
pages/visitors/index.tsx (2)
31-54
: Good URL state management for tabsUsing
nuqs
for tab state management enables shareable URLs and better navigation experience.
88-216
: Well-structured visitor management pageThe refactored page provides a clean tabbed interface with proper state management, pagination, and search functionality for both visitors and allow list groups.
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: 1
♻️ Duplicate comments (1)
pages/visitors/groups/[groupId].tsx (1)
270-306
: Type safety issue for link objects (previously identified).The code accesses properties on link objects without proper TypeScript typing, which can lead to runtime errors and poor developer experience.
🧹 Nitpick comments (2)
pages/visitors/groups/[groupId].tsx (2)
163-163
: Remove unused variable.The
totalUsage
variable is calculated but never used in the component.- const totalUsage = group._count.links;
311-318
: Consider conditional rendering optimization.The modal component is always rendered when
isEditModalOpen
is true. Consider using theisOpen
prop to control rendering instead of conditional component mounting for better performance.- {isEditModalOpen && ( - <AllowListGroupModal - isOpen={isEditModalOpen} - setIsOpen={setIsEditModalOpen} - group={group} - onSuccess={handleEditSuccess} - /> - )} + <AllowListGroupModal + isOpen={isEditModalOpen} + setIsOpen={setIsEditModalOpen} + group={group} + onSuccess={handleEditSuccess} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/swr/use-allow-list-groups.ts
(1 hunks)pages/api/teams/[teamId]/allow-list-groups/[groupId].ts
(1 hunks)pages/visitors/groups/[groupId].tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pages/api/teams/[teamId]/allow-list-groups/[groupId].ts
- lib/swr/use-allow-list-groups.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
pages/visitors/groups/[groupId].tsx (9)
components/ui/card.tsx (4)
Card
(80-80)CardContent
(85-85)CardHeader
(81-81)CardTitle
(83-83)context/team-context.tsx (1)
useTeam
(85-85)lib/swr/use-allow-list-groups.ts (2)
useAllowListGroup
(97-115)deleteAllowListGroup
(158-169)components/layouts/app.tsx (1)
AppLayout
(16-38)components/ui/button.tsx (1)
Button
(71-71)components/ui/alert-dialog.tsx (9)
AlertDialog
(138-138)AlertDialogTrigger
(141-141)AlertDialogContent
(142-142)AlertDialogHeader
(143-143)AlertDialogTitle
(145-145)AlertDialogDescription
(146-146)AlertDialogFooter
(144-144)AlertDialogCancel
(148-148)AlertDialogAction
(147-147)components/ui/badge.tsx (1)
Badge
(48-48)lib/utils.ts (1)
nFormatter
(185-206)components/visitors/allow-list-group-modal.tsx (1)
AllowListGroupModal
(33-190)
🔇 Additional comments (6)
pages/visitors/groups/[groupId].tsx (6)
1-40
: Imports are well-organized and appropriate.The import statements are properly structured and all dependencies are necessary for the component functionality.
41-95
: Skeleton component provides good loading UX.The skeleton structure appropriately matches the main component layout, providing users with clear visual feedback during loading states.
107-122
: Delete handler is properly implemented.The function includes appropriate guards, error handling, loading states, and user feedback. The navigation after successful deletion is correct.
124-127
: Edit success handler is correctly implemented.The function properly handles modal closure and data refresh after successful edits.
129-137
: Loading state handling is appropriate.The component properly shows a skeleton during loading while maintaining consistent layout structure.
139-161
: Not found state handling is well-implemented.The component provides clear error messaging and appropriate navigation options when a group is not found.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Backend/API