Update table implementation #2459
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes consolidate imports of the Changes
Sequence Diagram(s)sequenceDiagram
participant PageClient as New Program Page Client
participant WorkspaceAPI as Workspace Data API
participant Navigation as Next.js Navigation
PageClient->>WorkspaceAPI: mutateWorkspace()
WorkspaceAPI-->>PageClient: updated workspace data
PageClient->>Navigation: redirect to new program page (using defaultProgramId)
sequenceDiagram
participant ProgramsPage as Programs Page
participant DB as Database
participant Navigation as Next.js Navigation
ProgramsPage->>DB: fetch workspace with defaultProgramId only
DB-->>ProgramsPage: workspace data with defaultProgramId
ProgramsPage->>ProgramsPage: if no defaultProgramId, redirect to /dashboard
ProgramsPage->>Navigation: redirect to defaultProgramId program page
sequenceDiagram
participant ProgramAuth as ProgramAuth Component
participant WorkspaceAPI as Workspace Data API
participant Navigation as Next.js Navigation
ProgramAuth->>WorkspaceAPI: mutateWorkspace() if no defaultProgramId
WorkspaceAPI-->>ProgramAuth: updated workspace data
ProgramAuth->>ProgramAuth: check partnersEnabled and defaultProgramId
alt defaultProgramId missing and partnersEnabled true
ProgramAuth->>Navigation: redirect to /[slug]/programs/new
else defaultProgramId missing and partnersEnabled false
ProgramAuth->>Navigation: redirect to /[slug]
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the table components, updates the column-editing button export, removes the columns prop from Table, and streamlines partner onboarding and schema fields.
- Refactor Table API by dropping the
columnsprop - Switch
EditColumnsButtonto a named export and update imports - Remove
earningsfrom partner schema and simplify onboarding action
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/table/table.tsx | Removed the columns prop from Table component signature |
| packages/ui/src/table/index.ts | Re-exported edit-columns-button alongside other table exports |
| packages/ui/src/table/edit-columns-button.tsx | Changed EditColumnsButton to a named export and updated imports |
| apps/web/lib/zod/schemas/partners.ts | Deleted earnings field from EnrolledPartnerSchema |
| apps/web/lib/actions/partners/onboard-program.ts | Removed redirect logic after creating a program |
| apps/web/app/.../programs/page.tsx | Switched redirect to use defaultProgramId |
| apps/web/app/.../customer-table.tsx, events-table.tsx, etc. | Updated imports to use named EditColumnsButton |
Comments suppressed due to low confidence (3)
packages/ui/src/table/table.tsx:288
- Dropping the
columnsprop is a breaking change; update any documentation, TypeScript definitions, and bump the package version accordingly to signal consumers about the API change.
columns,
apps/web/lib/zod/schemas/partners.ts:242
- The
earningsfield was removed from the schema—ensure any dependent data transformations or UI logic that relied on this field are covered by tests and updated to prevent runtime errors.
earnings: z.number().default(0)
apps/web/lib/actions/partners/onboard-program.ts:32
- Removing the redirect and program ID assignment breaks the onboarding flow—users won’t be navigated to the new program. Consider restoring the redirect with the created program’s ID or explicitly handling navigation in the caller.
return;
| import { Popover } from "../popover"; | ||
|
|
||
| export default function EditColumnsButton({ table }: { table: Table<any> }) { | ||
| export function EditColumnsButton({ table }: { table: Table<any> }) { |
There was a problem hiding this comment.
[nitpick] Since EditColumnsButton switched from default to named export, update any README, component docs, or Storybook stories to reflect the new import syntax.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx(2 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/page.tsx(1 hunks)apps/web/lib/actions/partners/onboard-program.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/lib/actions/partners/onboard-program.ts (1)
apps/web/lib/actions/partners/create-program.ts (1)
createProgram(17-181)
apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx (1)
apps/web/lib/actions/partners/onboard-program.ts (1)
onboardProgramAction(12-39)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
apps/web/lib/actions/partners/onboard-program.ts (1)
28-32: Good refactoring to separate navigation concerns.The removal of server-side redirect logic aligns well with the coordinated changes across the codebase. Server actions now focus purely on data mutations while client-side components handle navigation using the workspace's
defaultProgramId.apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/page.tsx (1)
15-15: Excellent optimization to reduce data fetching.Fetching only
defaultProgramIdinstead of the full programs list is more efficient and aligns with the simplified navigation logic.apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx (2)
13-13: Good addition for client-side navigation.Adding the
redirectimport supports the shift from server-side to client-side navigation control.
26-26: Correctly accessing defaultProgramId from workspace context.This aligns with the pattern of using
defaultProgramIdfor navigation throughout the application.
apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/scripts/partners/delete-program.ts (1)
6-58: 🛠️ Refactor suggestionAdd error handling for production safety.
The script lacks error handling, which could lead to silent failures or partial completion in production environments.
Consider adding try-catch blocks and logging:
async function main() { + try { // existing operations... console.log("Successfully deleted program", deletedProgram); + } catch (error) { + console.error("Failed to delete program:", error); + process.exit(1); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx(1 hunks)apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/support/form.tsx(1 hunks)apps/web/lib/actions/partners/onboard-program.ts(1 hunks)apps/web/lib/zod/schemas/partners.ts(1 hunks)apps/web/scripts/partners/delete-program.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/support/form.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/lib/actions/partners/onboard-program.ts
- apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx
- apps/web/lib/zod/schemas/partners.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
apps/web/scripts/partners/delete-program.ts (1)
9-9: LGTM - Updated target program ID.The program ID update is straightforward and appears to target a different program for deletion.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/[programId]/auth.tsx(1 hunks)apps/web/scripts/partners/delete-program.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/scripts/partners/delete-program.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/[programId]/auth.tsx (3)
11-11: LGTM: Import addition is correct.The addition of
useEffectto the React imports is necessary for the new hook implementation below.
14-21: Good practice: Descriptive function renaming.Renaming
mutatetomutateWorkspaceimproves code readability and makes the function's purpose clearer.
40-46:✅ Verification successful
Verify the removal of empty string check.
The redirect logic was simplified by removing the check for
defaultProgramId === "". This could be problematic if the API can return an empty string instead of null/undefined for missing program IDs.Please verify that
defaultProgramIdis never returned as an empty string from the API:
🏁 Script executed:
#!/bin/bash # Description: Search for empty string checks and API responses related to defaultProgramId # Expected: Confirm whether empty string handling is still needed # Search for empty string checks on defaultProgramId rg -A 3 -B 3 'defaultProgramId.*==.*""' # Search for API endpoints or responses that might return defaultProgramId rg -A 5 -B 5 'defaultProgramId'Length of output: 29538
Empty string check on
defaultProgramIdis unnecessary
Based on the Prisma schema and update scripts,defaultProgramIdis only ever assigned a valid program ID ornull—it cannot be an empty string. You can safely rely on the simplified redirect logic.
- packages/prisma/schema/workspace.prisma:
defaultProgramId String?(no empty-string default)- apps/web/scripts/partners/{create-program.ts, delete-program.ts} & backfill-default-program-ids.ts only set
defaultProgramIdto an ID ornull
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/[programId]/auth.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/lib/actions/partners/create-program.ts (1)
146-158:⚠️ Potential issuePartners are invited twice (await + deferred)
The partners are invited immediately with
await Promise.all(...)and again insidewaitUntil(lines 162-172).
This results in:
- duplicate partner records,
- duplicate links,
- duplicate email notifications.
Remove one of the two calls (usually the awaited one) or gate the second call behind a feature flag/condition.
-// invite the partners -if (partners && partners.length > 0) { - await Promise.all( - partners.map((partner) => - invitePartner({ ... }), - ), - ); -}
🧹 Nitpick comments (1)
apps/web/lib/actions/partners/create-program.ts (1)
160-172: Non-promise values passed toPromise.allSettledInside
Promise.allSettledyou spread the result of the conditional partners map which is OK, but ifpartnersis[]you pass an empty array (fine), whereas the uploaded-logo deletion expression can resolve to booleanfalse, also fine.
However, readability suffers and consumers of the settled results must now handle heterogeneous value types.Consider normalising the list so every entry is a
Promise, e.g.:const tasks: Promise<unknown>[] = []; if (partners?.length) tasks.push(...partners.map(invitePartner)); tasks.push( prisma.program.update({ ... }), uploadedLogo && isStored(uploadedLogo) ? storage.delete(...) : Promise.resolve() ); waitUntil(Promise.allSettled(tasks));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/[programId]/auth.tsx(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/page.tsx(1 hunks)apps/web/lib/actions/partners/create-program.ts(3 hunks)apps/web/lib/actions/partners/onboard-program.ts(1 hunks)apps/web/scripts/partners/delete-program.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/lib/actions/partners/onboard-program.ts
- apps/web/scripts/partners/delete-program.ts
- apps/web/app/(ee)/app.dub.co/(new-program)/[slug]/programs/new/overview/page-client.tsx
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/page.tsx
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/programs/[programId]/auth.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
apps/web/lib/actions/partners/create-program.ts (1)
134-138: 🛠️ Refactor suggestion
programOnboarding: undefinedmay not remove the key in JSONSetting a JSON column to
{ …, programOnboarding: undefined }often serialises tonull, leaving the key behind.
If the intention is to delete the key, build a new object without the property:- store: { - ...store, - programOnboarding: undefined, - }, + store: Object.fromEntries( + Object.entries(store).filter(([k]) => k !== "programOnboarding"), + ),Alternatively, use the
unsetoperator if supported by your Prisma connector.Likely an incorrect or invalid review comment.
Summary by CodeRabbit