-
Notifications
You must be signed in to change notification settings - Fork 544
feat: create/delete teams #7293
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe changes introduce full support for team creation and deletion in the dashboard application. New asynchronous actions for creating and deleting teams are added, and corresponding UI components are updated to enable these features. Disabled or placeholder "Create Team" buttons are now active, and the delete functionality now performs real permission checks and API calls. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant Actions_Module
participant API_Server
participant Router
participant Toast
User->>UI_Component: Click "Create Team"
UI_Component->>Actions_Module: createTeam({})
Actions_Module->>API_Server: POST /v1/teams (with auth)
API_Server-->>Actions_Module: Team data / Error
Actions_Module-->>UI_Component: { ok: true, data } or { ok: false, errorMessage }
alt Success
UI_Component->>Router: Navigate to /team/[slug]
else Failure
UI_Component->>Toast: Show "Failed to create team"
end
sequenceDiagram
participant User
participant UI_Component
participant Actions_Module
participant API_Server
participant Router
participant Toast
User->>UI_Component: Click "Delete Team"
UI_Component->>Actions_Module: deleteTeam({ teamId })
Actions_Module->>API_Server: DELETE /v1/teams/{teamId} (with auth)
API_Server-->>Actions_Module: 200 OK / Error
alt Success
UI_Component->>Router: Navigate to /team
UI_Component->>Toast: Show "Team deleted"
else Failure
UI_Component->>Toast: Show error message
end
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (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 (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7293 +/- ##
=======================================
Coverage 55.58% 55.58%
=======================================
Files 909 909
Lines 58670 58670
Branches 4158 4158
=======================================
Hits 32609 32609
Misses 25954 25954
Partials 107 107
🚀 New features to boost your workflow:
|
size-limit report 📦
|
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 (4)
apps/dashboard/src/@/api/team.ts (1)
70-70
: Enhance the deprecation annotation with more context.The deprecation annotation should include the reason for deprecation and what developers should use instead to provide better guidance.
-/** @deprecated */ +/** @deprecated Use the new createTeam and deleteTeam actions instead. This function will be removed in a future version. */apps/dashboard/src/@/actions/createTeam.ts (3)
39-39
: Consider shorter slug generation for better UX.The current implementation generates a 40-character slug using
randomBytes(20).toString("hex")
, which may be too long for user-friendly URLs. Consider using a shorter identifier or a more URL-friendly generation method.- slug: options.slug ?? randomBytes(20).toString("hex"), + slug: options.slug ?? randomBytes(8).toString("hex"),Or consider using a more readable format:
- slug: options.slug ?? randomBytes(20).toString("hex"), + slug: options.slug ?? `team-${randomBytes(4).toString("hex")}`,
9-21
: Consider adding input validation for robustness.The function accepts optional parameters but doesn't validate them. Consider adding basic validation to ensure name and slug meet expected criteria (length limits, character restrictions, etc.).
+const createTeamSchema = z.object({ + name: z.string().max(50).optional(), + slug: z.string().max(30).regex(/^[a-z0-9-]+$/).optional(), +}); export async function createTeam(options: { name?: string; slug?: string; }): Promise< | { ok: true; data: Team; } | { ok: false; errorMessage: string; } > { + const validatedOptions = createTeamSchema.parse(options);
45-50
: Consider more specific error handling.The current error handling returns the raw response text, which might not always be user-friendly. Consider parsing the error response or providing more context about the failure.
if (!res.ok) { + let errorMessage: string; + try { + const errorJson = await res.json(); + errorMessage = errorJson.message || errorJson.error || "Failed to create team"; + } catch { + errorMessage = `Failed to create team (${res.status})`; + } return { ok: false, - errorMessage: await res.text(), + errorMessage, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/dashboard/src/@/actions/createTeam.ts
(1 hunks)apps/dashboard/src/@/actions/deleteTeam.ts
(1 hunks)apps/dashboard/src/@/api/team.ts
(1 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx
(2 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx
(1 hunks)apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx
(3 hunks)apps/dashboard/src/app/(app)/account/overview/AccountTeamsUI.tsx
(5 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/GeneralSettingsPage.stories.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/TeamGeneralSettingsPageUI.tsx
(3 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamAndProjectSelectorPopoverButton.tsx
(2 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx
(4 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamSelectionUI.tsx
(2 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamSelectorMobileMenuButton.tsx
(2 hunks)apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/GeneralSettingsPage.stories.tsx (1)
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/TeamGeneralSettingsPageUI.tsx (1)
DeleteTeamCard
(296-350)
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/TeamGeneralSettingsPageUI.tsx (1)
apps/dashboard/src/@/actions/deleteTeam.ts (1)
deleteTeam
(5-25)
apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx (1)
apps/dashboard/src/@/actions/createTeam.ts (1)
createTeam
(9-58)
apps/dashboard/src/@/actions/deleteTeam.ts (2)
apps/dashboard/src/app/(app)/api/lib/getAuthToken.ts (1)
getAuthToken
(6-14)apps/dashboard/src/@/constants/public-envs.ts (1)
NEXT_PUBLIC_THIRDWEB_API_HOST
(21-22)
apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx (1)
apps/dashboard/src/@/actions/createTeam.ts (1)
createTeam
(9-58)
apps/dashboard/src/@/actions/createTeam.ts (3)
apps/dashboard/src/@/api/team.ts (1)
Team
(9-9)apps/dashboard/src/app/(app)/api/lib/getAuthToken.ts (1)
getAuthToken
(6-14)apps/dashboard/src/@/constants/public-envs.ts (1)
NEXT_PUBLIC_THIRDWEB_API_HOST
(21-22)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (24)
apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.stories.tsx (1)
62-62
: LGTM! Consistent storybook prop addition.The
createTeam
prop addition follows the same pattern as the existingcreateProject
prop and uses an appropriate empty function for storybook testing.apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.stories.tsx (1)
158-158
: LGTM! Consistent storybook prop addition.The
createTeam
prop addition is consistent with the pattern used in other storybook files and uses an appropriate empty function for testing purposes.apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamSelectorMobileMenuButton.tsx (2)
20-20
: LGTM! Proper prop type definition.The
createTeam
callback prop type is correctly defined and follows TypeScript best practices with a clear function signature.
55-55
: LGTM! Correct prop passing.The
createTeam
prop is properly passed down to theTeamSelectionUI
component, enabling the team creation functionality in the mobile menu.apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/GeneralSettingsPage.stories.tsx (1)
65-66
: LGTM! Storybook stories correctly updated for new DeleteTeamCard interface.The prop changes from
enabled
tocanDelete
and the addition ofteamId
are properly reflected in the story variants, providing good test coverage for both permission states.apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamAndProjectSelectorPopoverButton.tsx (2)
25-25
: LGTM! Clean addition of createTeam callback to component interface.
104-107
: LGTM! Proper UX implementation for team creation.The implementation correctly closes the popover before triggering team creation, preventing potential UI state issues.
apps/dashboard/src/app/(app)/account/components/AccountHeaderUI.tsx (2)
24-24
: LGTM! Consistent addition of createTeam callback to component props.
68-68
: LGTM! Proper callback propagation in both desktop and mobile UI.The createTeam callback is consistently passed to both UI variants, maintaining feature parity across desktop and mobile experiences.
Also applies to: 122-122
apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx (2)
32-32
: LGTM! Consistent addition of createTeam callback to component interface.
79-79
: LGTM! Proper callback propagation to selector components.The createTeam callback is correctly passed to both team-selection and project-selection focused popover buttons.
Also applies to: 107-107
apps/dashboard/src/app/(app)/account/components/AccountHeader.tsx (3)
3-3
: LGTM: Clean import additionThe import statement correctly imports the createTeam action from the expected location.
11-11
: LGTM: Toast import added for error notificationsAdding the toast import from sonner aligns with the error handling pattern used in the createTeam function.
62-69
: LGTM: Well-implemented team creation with proper error handlingThe createTeam implementation follows good practices:
- Proper async/await usage
- Clear error handling with user-friendly toast messages
- Navigation to the new team page on success
- Consistent with the createTeam action's return type
The use of empty options
{}
is appropriate as the action provides sensible defaults for name and slug.apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamSelectionUI.tsx (2)
27-27
: LGTM: Clean prop addition for team creationThe createTeam callback prop is properly typed and follows the established pattern from other components in the codebase.
130-136
: LGTM: Create Team button properly activatedThe button implementation is correct:
- Removed the disabled state that was previously preventing interaction
- Added proper onClick handler that calls the createTeam prop
- Maintains consistent styling and icon usage
- The button text "Create Team" clearly indicates its purpose
This completes the integration that enables actual team creation functionality.
apps/dashboard/src/@/actions/deleteTeam.ts (1)
1-4
: LGTM: Proper server action setupThe file correctly uses the "use server" directive and imports the necessary dependencies for authentication and API configuration.
apps/dashboard/src/app/(app)/account/overview/AccountTeamsUI.tsx (3)
3-3
: LGTM: Proper imports for team creation functionalityThe imports correctly bring in the necessary dependencies:
createTeam
action for API interactionuseDashboardRouter
for navigationtoast
from sonner for error notificationsThis aligns with the pattern established in other components.
Also applies to: 14-14, 18-18
31-31
: LGTM: Well-implemented team creation with navigationThe implementation is excellent:
- Proper use of the dashboard router hook
- Clean async function that wraps the createTeam action
- Consistent error handling with user-friendly toast messages
- Navigation to the new team page on success
- Follows the same pattern established in
AccountHeader.tsx
The function name
createTeamAndRedirect
clearly describes its behavior.Also applies to: 41-48
60-63
: LGTM: Create Team button properly activatedThe button transformation is well-executed:
- Removed the disabled state that was blocking user interaction
- Added proper onClick handler that calls the new
createTeamAndRedirect
function- Maintains consistent styling with the PlusIcon and responsive classes
- Clear and descriptive button text
This completes the user flow from a disabled placeholder to a fully functional team creation feature.
apps/dashboard/src/app/(app)/team/components/TeamHeader/team-header-logged-in.client.tsx (1)
68-75
: LGTM! Clean implementation with proper error handling.The
createTeam
function correctly handles both success and error cases. The error toast provides good user feedback, and the navigation logic properly redirects to the newly created team's page using the slug from the API response.apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/TeamGeneralSettingsPageUI.tsx (3)
63-66
: Good improvement from hardcoded to dynamic permissions.The changes properly replace the hardcoded permission check with the actual
isOwnerAccount
prop and add the requiredteamId
for the delete operation. This enables real permission-based access control.
306-313
: Well-implemented delete mutation with proper error handling.The mutation correctly integrates with the
deleteTeam
action and handles the success case by redirecting to the teams overview page. The async/await pattern is appropriate for the server action call.
323-337
:❓ Verification inconclusive
Verify deleteTeam action handles all error scenarios properly.
The permission-based rendering works well, but ensure that the
deleteTeam
action properly handles edge cases like network failures, authorization errors, or cases where the team is referenced by other entities.
🏁 Script executed:
#!/bin/bash # Verify error handling in deleteTeam action and related components rg -A 10 -B 5 "deleteTeam.*throw\|deleteTeam.*error" --type=tsLength of output: 64
🏁 Script executed:
#!/bin/bash # Locate the definition of deleteTeamAndRedirect to inspect how errors are handled rg -nF "deleteTeamAndRedirect" -g '*.ts' -g '*.tsx'Length of output: 524
🏁 Script executed:
#!/bin/bash # Show context around deleteTeamAndRedirect definition to inspect error handling rg -n "const deleteTeamAndRedirect = useMutation" -g '*.tsx' -B5 -A15Length of output: 2946
🏁 Script executed:
#!/bin/bash # Show import statement for deleteTeam in the UI component rg -n "import.*deleteTeam" -g 'apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/general/TeamGeneralSettingsPageUI.tsx'Length of output: 284
🏁 Script executed:
#!/bin/bash # Search across the repo for the import of deleteTeam rg -n "import .* deleteTeam" -g '*.ts' -g '*.tsx'Length of output: 206
🏁 Script executed:
#!/bin/bash # Locate and display the deleteTeam action implementation to inspect error handling rg -n "export (const|function) deleteTeam" -g '*.ts' -g '*.tsx' -B0 -A50Length of output: 74
Verify deleteTeam action handles all error scenarios properly.
I wasn’t able to locate the implementation of
deleteTeam
in@/actions/deleteTeam
, so please double-check that this function:
- Catches network failures and request timeouts
- Handles 401/403 authorization errors
- Deals with referential-integrity issues (e.g. team still referenced elsewhere)
- Propagates clear error messages back to
toast.promise
createTeam={() => { | ||
alert("createTeam"); | ||
}} |
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.
🛠️ Refactor suggestion
Replace placeholder alert with proper createTeam implementation.
The current alert implementation appears to be placeholder code. This should use the actual createTeam prop passed from the parent component for consistency with other UI components.
Apply this diff to use the proper callback:
- createTeam={() => {
- alert("createTeam");
- }}
+ createTeam={props.createTeam}
📝 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.
createTeam={() => { | |
alert("createTeam"); | |
}} | |
createTeam={props.createTeam} |
🤖 Prompt for AI Agents
In apps/dashboard/src/app/(app)/team/components/TeamHeader/TeamHeaderUI.tsx
around lines 176 to 178, replace the placeholder alert in the createTeam prop
with the actual createTeam function passed down as a prop from the parent
component. This ensures the component uses the proper callback for creating a
team instead of just showing an alert.
export async function deleteTeam(options: { | ||
teamId: string; | ||
}): Promise<void> { | ||
const token = await getAuthToken(); | ||
if (!token) { | ||
throw new Error("You are not authorized to perform this action"); | ||
} |
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.
🛠️ Refactor suggestion
Consider input validation and improve error handling consistency
The function signature and authorization check are good, but there are a few areas for improvement:
- Input validation: The
teamId
parameter is not validated. Consider adding validation to ensure it's a non-empty string. - Error handling inconsistency: This action throws errors, while
createTeam
returns result objects withok
boolean. This inconsistency could confuse consumers.
Consider aligning the error handling pattern with createTeam
:
export async function deleteTeam(options: {
teamId: string;
}): Promise<
| { ok: true }
| { ok: false; errorMessage: string }
> {
const token = await getAuthToken();
if (!token) {
- throw new Error("You are not authorized to perform this action");
+ return {
+ ok: false,
+ errorMessage: "You are not authorized to perform this action"
+ };
}
+
+ if (!options.teamId?.trim()) {
+ return {
+ ok: false,
+ errorMessage: "Team ID is required"
+ };
+ }
🤖 Prompt for AI Agents
In apps/dashboard/src/@/actions/deleteTeam.ts around lines 5 to 11, add
validation to check that the teamId parameter is a non-empty string before
proceeding. Also, refactor the function to return a result object with an ok
boolean and error message instead of throwing errors, to align with the error
handling pattern used in createTeam. This will improve input validation and
maintain consistency in error handling across similar functions.
const res = await fetch( | ||
`${NEXT_PUBLIC_THIRDWEB_API_HOST}/v1/teams/${options.teamId}`, | ||
{ | ||
method: "DELETE", | ||
headers: { | ||
Authorization: `Bearer ${token}`, | ||
}, | ||
}, | ||
); | ||
if (!res.ok) { | ||
throw new Error(await res.text()); | ||
} | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Consider enhanced error handling and security measures
The API call implementation is functional but could be improved:
- Error handling: Currently throws generic errors. Consider providing more specific error messages based on response status.
- Permission verification: The action only checks authentication but doesn't verify if the user has permission to delete the specific team.
Consider these improvements:
const res = await fetch(
`${NEXT_PUBLIC_THIRDWEB_API_HOST}/v1/teams/${options.teamId}`,
{
method: "DELETE",
headers: {
Authorization: `Bearer ${token}`,
},
},
);
if (!res.ok) {
- throw new Error(await res.text());
+ const errorText = await res.text();
+ return {
+ ok: false,
+ errorMessage: res.status === 403
+ ? "You don't have permission to delete this team"
+ : res.status === 404
+ ? "Team not found"
+ : errorText || "Failed to delete team"
+ };
}
+ return { ok: true };
Let me verify if there are any existing permission checks for team deletion in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for team permission/authorization patterns
rg -A 5 -B 5 "delete.*team|team.*delete" --type ts
Length of output: 9979
🏁 Script executed:
#!/bin/bash
# Display contents of deleteTeam action file for context
cat apps/dashboard/src/@/actions/deleteTeam.ts
Length of output: 683
Enhance error reporting in deleteTeam action
The server enforces authorization and team-deletion permissions; we only need to map common failure statuses to more descriptive errors rather than return a new result type. In apps/dashboard/src/@/actions/deleteTeam.ts, update the error block as follows:
const res = await fetch(
`${NEXT_PUBLIC_THIRDWEB_API_HOST}/v1/teams/${options.teamId}`,
{
method: "DELETE",
headers: {
Authorization: `Bearer ${token}`,
},
},
);
- if (!res.ok) {
- throw new Error(await res.text());
- }
+ if (!res.ok) {
+ const errorText = await res.text();
+ if (res.status === 403) {
+ throw new Error("You don’t have permission to delete this team");
+ }
+ if (res.status === 404) {
+ throw new Error("Team not found");
+ }
+ throw new Error(errorText || "Failed to delete team");
+ }
• Preserves the existing Promise<void>
signature
• Leverages server-side permission checks and surfaces clear client-side messages
• UI’s toast.promise will display the thrown message automatically
📝 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.
const res = await fetch( | |
`${NEXT_PUBLIC_THIRDWEB_API_HOST}/v1/teams/${options.teamId}`, | |
{ | |
method: "DELETE", | |
headers: { | |
Authorization: `Bearer ${token}`, | |
}, | |
}, | |
); | |
if (!res.ok) { | |
throw new Error(await res.text()); | |
} | |
} | |
const res = await fetch( | |
`${NEXT_PUBLIC_THIRDWEB_API_HOST}/v1/teams/${options.teamId}`, | |
{ | |
method: "DELETE", | |
headers: { | |
Authorization: `Bearer ${token}`, | |
}, | |
}, | |
); | |
if (!res.ok) { | |
const errorText = await res.text(); | |
if (res.status === 403) { | |
throw new Error("You don’t have permission to delete this team"); | |
} | |
if (res.status === 404) { | |
throw new Error("Team not found"); | |
} | |
throw new Error(errorText || "Failed to delete team"); | |
} | |
} |
🤖 Prompt for AI Agents
In apps/dashboard/src/@/actions/deleteTeam.ts around lines 13 to 25, improve
error handling by mapping common HTTP failure statuses to more descriptive error
messages instead of throwing a generic error. Keep the Promise<void> signature
and rely on server-side permission enforcement, but update the error block to
check res.status and throw specific errors like "Unauthorized" for 401,
"Forbidden" for 403, or a detailed message from the response text for other
failures. This will allow the UI toast.promise to display clearer messages
automatically.
[Dashboard] Feature: Add Team Creation and Deletion
Notes for the reviewer
This PR adds the ability for users to create and delete teams from the dashboard. It includes:
How to test
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation