Account for legacy Business plans #2521
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2521:
To re-run the tests, please comment |
WalkthroughThe changes primarily add the "business legacy" plan to the list of allowed or required plans for various API endpoints, access control checks, and capability logic throughout the codebase. Some files adjust the order of plan names, and a PATCH endpoint for partner sales is removed. Additionally, a new script for exporting partner links to CSV is introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API_Endpoint
participant Auth_Middleware
User->>API_Endpoint: Request (e.g., GET /api/customers)
API_Endpoint->>Auth_Middleware: Check requiredPlan (includes "business legacy")
Auth_Middleware-->>API_Endpoint: Allow or deny based on plan
API_Endpoint-->>User: Response (data or error)
sequenceDiagram
participant Script
participant Prisma_DB
participant FileSystem
Script->>Prisma_DB: Query links (by programId, partnerId)
Prisma_DB-->>Script: Return link records
Script->>FileSystem: Write CSV (via, partner_id)
FileSystem-->>Script: File written (framer_partner_links.csv)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/scripts/framer/export-links.ts (1)
6-34: 🛠️ Refactor suggestionAdd error handling & ensure Prisma disconnect
main()is invoked without a.catch, and the Prisma client is never disconnected—Node processes can hang on open connections.async function main() { const links = await prisma.link.findMany({ ... }); ... fs.writeFileSync( "framer_partner_links.csv", ... ); + + await prisma.$disconnect(); } -main(); +main().catch((err) => { + console.error(err); + prisma.$disconnect().finally(() => process.exit(1)); +});This guarantees clean shutdown and surfaces failures in CI.
♻️ Duplicate comments (3)
apps/web/app/api/folders/[folderId]/route.ts (2)
99-107: Same concern as above – see earlier comment.
170-178: Same concern as above – see earlier comment.apps/web/app/(ee)/api/track/lead/route.ts (1)
308-316: Plan list mirrors the earlier events route; same centralization suggestion applies.
🧹 Nitpick comments (14)
apps/web/app/api/folders/[folderId]/users/route.ts (1)
79-79: Extract repeated plan lists to maintain DRY.Adding
"business legacy"here is correct, but the same array of plans is duplicated across many routes. Consider extracting these into a shared constant (e.g.ALLOWED_FOLDER_PLANS) to avoid drift when plan tiers change.apps/web/app/api/folders/route.ts (2)
41-41: Extract shared requiredPlan array for consistency.The insertion of
"business legacy"looks good. To reduce duplication and ensure uniform updates, factor these plan arrays into a single exported constant used by both GET and POST handlers.
143-143: Extract shared requiredPlan array for consistency.Similarly here, the
"business legacy"addition is valid. Pull the repeated plan list into a shared constant to streamline future modifications.apps/web/app/api/folders/access-requests/route.ts (1)
25-25: Extract repeated plan lists to maintain DRY.Including
"business legacy"is correct for access requests. As the same plan sequence recurs across many endpoints, factor this into a sharedREQUIRED_PLANSconstant to reduce duplication.apps/web/lib/auth/workspace.ts (1)
43-53: Add legacy plan to global workspace auth defaults
Including"business legacy"in the defaultrequiredPlanensures any route that doesn’t override plan checks will honor the legacy tier.Nitpick: Consider a consistent ordering of business tiers (e.g., legacy → plus → extra → max) across all configurations for readability.
apps/web/app/(ee)/api/events/route.ts (1)
88-96: Centralize plan-gating arrays to avoid future driftNearly every route now repeats the same long
requiredPlanliteral. Consider importing a shared constant (e.g.BUSINESS_PLANSorPAID_PLANS) or a helper such asgetBusinessPlans()so that adding/removing a tier only touches one file.-import { withWorkspace } from "@/lib/auth"; +import { withWorkspace } from "@/lib/auth"; +import { BUSINESS_PLANS } from "@/lib/plans"; // ← single source-of-truth ... - requiredPlan: [ - "business", - "business legacy", - "business plus", - "business extra", - "business max", - "advanced", - "enterprise", - ], + requiredPlan: BUSINESS_PLANS,apps/web/app/(ee)/api/partners/route.ts (1)
32-39: Nit: keep plan arrays consistently orderedRe-ordering
"business plus"is harmless but makes diff-noise across routes. Adopting alphabetical or tier-ascending order (or, better, a shared constant) keeps future PRs tidy.apps/web/lib/plan-capabilities.ts (1)
12-13: Centralise plan-exclusion lists to avoid future drift
["free", "pro", "business legacy"]is now the third hard-coded list in this file alone (see lines 8-11).
When a new tier is introduced the risk of missing one of these literals is high.- !!plan && !["free", "pro", "business legacy"].includes(plan), + !!plan && !EXCLUDED_FROM_PROGRAM.includes(plan),…and export
EXCLUDED_FROM_PROGRAM(or a more generalPLAN_EXCLUSIONS) from a single source of truth under@/lib/plans.apps/web/app/(ee)/api/stripe/integration/route.ts (1)
96-105: DuplicaterequiredPlanarrays: extract a reusable constantThe same eight-item array appears in many routes. Consider:
import { BUSINESS_WRITE_PLANS } from "@/lib/plans"; // … requiredPlan: BUSINESS_WRITE_PLANS,A shared constant improves readability and prevents inconsistencies when plans change.
apps/web/app/(ee)/api/shopify/integration/callback/route.ts (1)
88-97: Keep plan lists consistent via a shared enum/constantSame comment as for the Stripe route: hard-coding plan strings in every handler is brittle.
apps/web/app/(ee)/api/customers/[id]/route.ts (1)
42-50: Consolidate plan gating logicThree identical
requiredPlanblocks in one file (GET/PATCH/DELETE). Refactor to:const CUSTOMER_PLANS = BUSINESS_WRITE_PLANS; // from shared constant // … requiredPlan: CUSTOMER_PLANS,This reduces noise and keeps authorisation rules aligned across methods.
Also applies to: 143-151, 180-188
apps/web/app/api/webhooks/[webhookId]/route.ts (1)
41-49: Authorisation lists are drifting – move to configWebhooks now have three separate copies of the plan array. A central definition (e.g.,
ALLOWED_WEBHOOK_PLANS) will make future tier additions a one-line change.Also applies to: 250-259, 320-329
apps/web/app/api/webhooks/route.ts (1)
49-58: Extract plan list to a shared constant to avoid copy-paste driftThe same
requiredPlanarray (now with “business legacy”) appears verbatim across many endpoints. Keeping these ad-hoc lists in every route is brittle—next time a plan tier is renamed/added you must hunt through multiple files.Consider:
- requiredPlan: [ - "business", - "business legacy", - "business plus", - "business extra", - "business max", - "advanced", - "enterprise", - ], + requiredPlan: ALL_BUSINESS_PLANS, // import from a single source of truthCentralising the tiers (e.g.
@/lib/plan/constants.ts) eliminates duplication and guarantees consistency.Also applies to: 192-201
apps/web/app/(ee)/api/customers/route.ts (1)
136-144: Same plan list duplication issue as aboveThis route repeats the tier list twice. Please reuse the shared constant once it’s introduced to keep customer APIs in sync with other endpoints.
Also applies to: 209-217
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/web/app/(ee)/api/customers/[id]/route.ts(3 hunks)apps/web/app/(ee)/api/customers/route.ts(2 hunks)apps/web/app/(ee)/api/events/export/route.ts(1 hunks)apps/web/app/(ee)/api/events/route.ts(1 hunks)apps/web/app/(ee)/api/partners/count/route.ts(1 hunks)apps/web/app/(ee)/api/partners/export/route.ts(1 hunks)apps/web/app/(ee)/api/partners/route.ts(1 hunks)apps/web/app/(ee)/api/partners/sales/route.ts(0 hunks)apps/web/app/(ee)/api/shopify/integration/callback/route.ts(1 hunks)apps/web/app/(ee)/api/stripe/integration/route.ts(1 hunks)apps/web/app/(ee)/api/track/lead/route.ts(1 hunks)apps/web/app/(ee)/api/track/sale/route.ts(1 hunks)apps/web/app/api/folders/[folderId]/route.ts(3 hunks)apps/web/app/api/folders/[folderId]/users/route.ts(1 hunks)apps/web/app/api/folders/access-requests/route.ts(1 hunks)apps/web/app/api/folders/permissions/route.ts(1 hunks)apps/web/app/api/folders/route.ts(2 hunks)apps/web/app/api/webhooks/[webhookId]/events/route.ts(1 hunks)apps/web/app/api/webhooks/[webhookId]/route.ts(3 hunks)apps/web/app/api/webhooks/route.ts(2 hunks)apps/web/lib/auth/workspace.ts(1 hunks)apps/web/lib/plan-capabilities.ts(1 hunks)apps/web/lib/types.ts(1 hunks)apps/web/scripts/framer/export-links.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/(ee)/api/partners/sales/route.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (9)
apps/web/app/(ee)/api/partners/count/route.ts (1)
165-165: Align plan ordering in access control.The repositioning of
"business plus"immediately after"business"matches the ordering in related partner endpoints. No functional impact detected.apps/web/app/(ee)/api/partners/export/route.ts (1)
81-81: Align plan ordering in access control.Moving
"business plus"to follow"business"brings this export endpoint into line with the other partner routes. No issues found.apps/web/app/api/folders/permissions/route.ts (1)
51-59:✅ Verification successful
Add legacy business plan to folder permissions access
Including"business legacy"alongside existing business tiers correctly grants legacy customers access to folder permissions. Ensure thePlanPropsunion inapps/web/lib/types.tsis updated to include this new plan name.
🏁 Script executed:
#!/bin/bash # Verify "business legacy" is present in the PlanProps type rg -n "\"business legacy\"" apps/web/lib/types.tsLength of output: 74
All set: “business legacy” is included
Verified that"business legacy"exists in thePlanPropsunion (apps/web/lib/types.ts:264). No further changes needed.apps/web/app/(ee)/api/events/export/route.ts (1)
118-126: Include "business legacy" for event export
Adding"business legacy"here aligns this endpoint’s plan requirements with the rest of the suite.apps/web/app/(ee)/api/track/sale/route.ts (1)
277-285: Authorize legacy business plan for sale tracking
The"business legacy"plan is now included in the POST/api/track/saleaccess control list, matching other endpoints.apps/web/app/api/webhooks/[webhookId]/events/route.ts (1)
30-39: Authorize legacy business plan for webhook events
Adding"business legacy"here extends webhook event log access to legacy business customers, consistent with other API routes.apps/web/app/api/folders/[folderId]/route.ts (1)
29-37:✅ Verification successful
Double-check capability matrix for “business legacy”
accessLevelvalidation relies ongetPlanCapabilities(workspace.plan).
Please verify thatplan-capabilities.tstreats"business legacy"the same as current Business tiers for folder-permission management; otherwise users on this legacy tier will hit the “Upgrade to Business” error path.
🏁 Script executed:
#!/usr/bin/env bash # Does canManageFolderPermissions include "business legacy"? rg -A3 -n 'canManageFolderPermissions' apps/web/lib/plan-capabilities.tsLength of output: 348
Business legacy is covered by
canManageFolderPermissions
The capability is defined as:canManageFolderPermissions: !!plan && !["free", "pro"].includes(plan)Since
"business legacy"isn’t in["free", "pro"], it’s granted the same permissions as other business tiers. No changes required.< 8000 summary>apps/web/lib/types.ts (1)
260-270:❓ Verification inconclusive
plansenum extended – compile-time fallout looks good, but scan for exhaustive checksThe new literal is correctly inserted. Make sure any
switch(plan)/if (…)logic that enumerates all plans now handles"business legacy"explicitly to avoid falling into default branches.
🏁 Script executed:
#!/usr/bin/env bash # Find potentially exhaustive switches. rg -n 'switch *\(.*plan' --glob '*.ts'Length of output: 40
Let’s extend our search to include
.tsxfiles and also look for anyif (plan === “…”)comparisons:
🏁 Script executed:
#!/usr/bin/env bash # Search for switch statements on `plan` in both .ts and .tsx rg -n 'switch\s*\(\s*plan' --glob '*.{ts,tsx}' # Search for explicit if-checks comparing `plan` to string literals rg -n -E 'if\s*\(\s*plan\s*===\s*"' --glob '*.{ts,tsx}'Length of output: 503
Let’s correct the quoting and include .tsx files. Also scan for any direct
plan ===checks:#!/usr/bin/env bash # Search for `switch (plan)` in .ts and .tsx rg -n 'switch\s*\(\s*plan' --glob '*.{ts,tsx}' # Search for explicit `if (plan === "...")` checks rg -n 'if\s*\(\s*plan\s*===\s*"' --glob '*.{ts,tsx}' # Catch any other direct comparisons `plan === "…"` rg -n 'plan\s*===\s*"' --glob '*.{ts,tsx}'
Confirm exhaustive plan-based logic
I didn’t find any
switch(plan)orif (plan === “…”)checks in your.ts/.tsxfiles—please manually verify that any places enumeratingplans(e.g. viaswitchorif/else if) have been updated to handle the new"business legacy"value and won’t fall through to a default case.To assist your verification, you can run:
# Look for switch statements on `plan` rg -n 'switch\s*\(\s*plan' --glob '*.{ts,tsx}' # Look for direct plan comparisons rg -n 'if\s*\(\s*plan\s*===\s*' --glob '*.{ts,tsx}'apps/web/lib/plan-capabilities.ts (1)
8-13: Confirm “business legacy” is only blocked forcanManageProgramEvery other capability still grants access to the legacy tier.
Please double-check that this asymmetry is intentional; otherwise the legacy tier could unexpectedly edit folders, customers, etc.
| links.map((link) => ({ | ||
| via: link.key, | ||
| parnter_id: link.partnerId, | ||
| })), |
There was a problem hiding this comment.
Typo in CSV header will break downstream consumers
parnter_id is misspelled.
- parnter_id: link.partnerId,
+ partner_id: link.partnerId,Fix before the CSV is consumed by external tooling.
📝 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.
| links.map((link) => ({ | |
| via: link.key, | |
| parnter_id: link.partnerId, | |
| })), | |
| links.map((link) => ({ | |
| via: link.key, | |
| partner_id: link.partnerId, | |
| })), |
🤖 Prompt for AI Agents
In apps/web/scripts/framer/export-links.ts around lines 25 to 28, the CSV header
key 'parnter_id' is misspelled. Correct the typo by changing 'parnter_id' to
'partner_id' to ensure downstream consumers and external tooling can correctly
process the CSV data.
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2521:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2521:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2521:
To re-run the tests, please comment |
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2521:
To re-run the tests, please comment |
Summary by CodeRabbit
New Features
Changes
Bug Fixes
Chores
Revert