[Dashboard] Rename getMemberById to getMemberByAccountId and add onboarding banner #7334
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe changes primarily involve renaming the function Changes
Sequence Diagram(s)Team Member Fetching (After Refactor)sequenceDiagram
participant UI
participant API
UI->>API: getMemberByAccountId(teamSlug, accountId)
API-->>UI: Returns member data or null
Team Onboarding Requirement Check (New Logic)sequenceDiagram
participant UI
participant AccountService
participant TeamMemberService
UI->>AccountService: getValidAccount(pagePath)
AccountService-->>UI: account
UI->>TeamMemberService: getMemberByAccountId(team.slug, account.id)
TeamMemberService-->>UI: member
UI->>UI: Check member existence and role
UI->>UI: Return onboarding required status based on team.isOnboarded
TeamPlanBadge Click HandlingsequenceDiagram
participant User
participant TeamPlanBadge
participant Tracker
participant Router
User->>TeamPlanBadge: Click badge
alt Plan is "free"
TeamPlanBadge->>Tracker: track("ShowPlans")
TeamPlanBadge->>Router: navigate("/settings/billing")
else Plan is not "free"
TeamPlanBadge->>TeamPlanBadge: No navigation
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ 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. |
jnsdls
force-pushed
the
_Dashboard_Rename_getMemberById_to_getMemberByAccountId_and_add_onboarding_banner
branch
from
fc062de to
1b95215
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7334 +/- ##
=======================================
Coverage 55.57% 55.57%
=======================================
Files 909 909
Lines 58675 58675
Branches 4160 4160
=======================================
Hits 32609 32609
Misses 25959 25959
Partials 107 107
🚀 New features to boost your workflow:
|
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
apps/dashboard/src/@/api/team-members.ts (1)
51-54: Add an explicit return type for stronger type-safety
getMemberByAccountIdcurrently relies on type inference. Declaring an explicit return type helps TS catch accidental changes (e.g. returningnull) and makes the API crystal-clear to consumers.-export async function getMemberByAccountId( - teamSlug: string, - accountId: string, -) { +export async function getMemberByAccountId( + teamSlug: string, + accountId: string, +): Promise<TeamMember | undefined> {apps/dashboard/src/app/(app)/account/page.tsx (1)
28-28: Potential N-API calls per page load – consider batchingInside
teams.mapevery team results in its own network call togetMemberByAccountId.
For users in many teams this can create noticeable latency (N + 1 pattern).
If the backend supports it, fetch all memberships in a single call or reuse the list already available ingetTeams().apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/settings/page.tsx (1)
40-40: Repeat of the N + 1 membership fetchSame performance concern as in
account/page.tsx: one request per team.
Consider a bulk endpoint or cachinggetMemberByAccountIdresults across iterations.apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx (1)
33-35: Avoid double account fetch insidehasToCompleteTeamOnboarding
hasToCompleteTeamOnboardinginternally callsgetValidAccount, incurring an extra cookie read / API hit that this layout doesn’t otherwise need.
Given you already haveauthTokenand could cheaply fetch the account once here, consider passing the account (or member) into the helper to eliminate redundant work on every team page navigation.No functional bug — just an optional perf tidy-up.
apps/dashboard/src/app/(app)/get-started/team/[team_slug]/layout.tsx (1)
38-41: Guard against clock skew & future-datedcreatedAtIf
team.createdAtis (accidentally) in the future or the server clock differs,
differenceInDaysbecomes negative and the banner will never render.
A smallMath.max(…, 0)keeps intent intact:-const shouldShowOnboardingBanner = - differenceInDays(new Date(), new Date(team.createdAt)) > 3; +const shouldShowOnboardingBanner = + Math.max(differenceInDays(new Date(), new Date(team.createdAt)), 0) > 3;apps/dashboard/src/app/(app)/login/onboarding/isOnboardingRequired.ts (1)
11-24: Edge-case:getValidAccountcan redirect — consider early bailout
getValidAccountmay throw a Next.js redirect for unauthenticated users.
BecausehasToCompleteTeamOnboardingis itself awaited inside layouts after the auth-token check, the redirect could still bubble up and override the intended “/login” route.If that’s undesirable, wrap the call and fall back to
false:- const account = await getValidAccount(pagePath); + const account = await getValidAccount(pagePath).catch(() => null); + if (!account) { + return false; + }Keeps control flow predictable and avoids double redirects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/dashboard/src/@/api/team-members.ts(1 hunks)apps/dashboard/src/app/(app)/account/page.tsx(2 hunks)apps/dashboard/src/app/(app)/components/TeamPlanBadge.tsx(2 hunks)apps/dashboard/src/app/(app)/get-started/team/[team_slug]/layout.tsx(3 hunks)apps/dashboard/src/app/(app)/login/onboarding/isOnboardingRequired.ts(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/billing/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/invoices/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/settings/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx (1)
Learnt from: jnsdls
PR: thirdweb-dev/js#6929
File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/page.tsx:14-19
Timestamp: 2025-05-21T05:17:31.283Z
Learning: In Next.js server components, the `params` object can sometimes be a Promise that needs to be awaited, despite type annotations suggesting otherwise. In apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/page.tsx, it's necessary to await the params object before accessing its properties.
🧬 Code Graph Analysis (7)
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/invoices/page.tsx (1)
apps/dashboard/src/@/api/team-members.ts (1)
getMemberByAccountId(51-75)
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/page.tsx (1)
apps/dashboard/src/@/api/team-members.ts (1)
getMemberByAccountId(51-75)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/settings/page.tsx (1)
apps/dashboard/src/@/api/team-members.ts (1)
getMemberByAccountId(51-75)
apps/dashboard/src/@/api/team-members.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)/team/[team_slug]/layout.tsx (3)
apps/dashboard/src/app/(app)/api/lib/getAuthToken.ts (1)
getAuthToken(6-14)apps/dashboard/src/@/api/team.ts (1)
getTeamBySlug(11-30)apps/dashboard/src/app/(app)/login/onboarding/isOnboardingRequired.ts (1)
hasToCompleteTeamOnboarding(11-24)
apps/dashboard/src/app/(app)/account/page.tsx (1)
apps/dashboard/src/@/api/team-members.ts (1)
getMemberByAccountId(51-75)
apps/dashboard/src/app/(app)/components/TeamPlanBadge.tsx (1)
apps/dashboard/src/hooks/analytics/useTrack.ts (1)
useTrack(13-45)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/invoices/page.tsx (1)
3-3: Import rename looks goodImport path was updated consistently with the new API name. No further action needed.
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/page.tsx (1)
2-2: Import update acknowledgedThe renamed helper is referenced correctly.
apps/dashboard/src/app/(app)/account/page.tsx (1)
2-2: Consistent import renameNo issues here.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/settings/page.tsx (1)
3-3: Import rename looks correctNothing to flag.
apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx (1)
20-23: Nice micro-optimisation withPromise.allFetching the token and the team in parallel shaves one RTT off the critical path.
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/invoices/page.tsx
Show resolved
Hide resolved
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/page.tsx
Show resolved
Hide resolved
apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/billing/page.tsx
Show resolved
Hide resolved
jnsdls
force-pushed
the
_Dashboard_Rename_getMemberById_to_getMemberByAccountId_and_add_onboarding_banner
branch
from
1b95215 to
c269398
Compare
jnsdls
force-pushed
the
_Dashboard_Rename_getMemberById_to_getMemberByAccountId_and_add_onboarding_banner
branch
from
c269398 to
f0dacda
Compare
🚨 BugBot couldn't runSomething went wrong. Try again by commenting "bugbot run", or contact support (requestId: serverGenReqId_0bf8c088-9ad0-40ad-bf16-5123b7522382). |
jnsdls
force-pushed
the
_Dashboard_Rename_getMemberById_to_getMemberByAccountId_and_add_onboarding_banner
branch
from
f0dacda to
30301d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/dashboard/src/@/api/team.ts (1)
6-6: Use path alias instead of deep relative import for consistencyEvery other internal import in this module leverages the
@alias. Falling back to a two-level../../…path makes the file brittle to directory moves and breaks visual grepping consistency.-import { getValidAccount } from "../../app/(app)/account/settings/getAccount"; +import { getValidAccount } from "@/app/(app)/account/settings/getAccount";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/dashboard/src/@/api/team-members.ts(2 hunks)apps/dashboard/src/@/api/team.ts(2 hunks)apps/dashboard/src/app/(app)/account/page.tsx(2 hunks)apps/dashboard/src/app/(app)/components/TeamPlanBadge.tsx(2 hunks)apps/dashboard/src/app/(app)/get-started/team/[team_slug]/layout.tsx(3 hunks)apps/dashboard/src/app/(app)/login/onboarding/isOnboardingRequired.ts(0 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/billing/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/invoices/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/settings/page.tsx(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/src/app/(app)/login/onboarding/isOnboardingRequired.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/invoices/page.tsx
- apps/dashboard/src/app/(app)/account/page.tsx
- apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/settings/page.tsx
- apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/page.tsx
- apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx
- apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/settings/billing/page.tsx
- apps/dashboard/src/@/api/team-members.ts
- apps/dashboard/src/app/(app)/get-started/team/[team_slug]/layout.tsx
- apps/dashboard/src/app/(app)/components/TeamPlanBadge.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/src/@/api/team.ts (2)
apps/dashboard/src/app/(app)/account/settings/getAccount.ts (1)
getValidAccount(44-53)apps/dashboard/src/@/api/team-members.ts (1)
getMemberByAccountId(51-78)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
jnsdls
force-pushed
the
_Dashboard_Rename_getMemberById_to_getMemberByAccountId_and_add_onboarding_banner
branch
from
30301d9 to
2849782
Compare
jnsdls
deleted the
_Dashboard_Rename_getMemberById_to_getMemberByAccountId_and_add_onboarding_banner
branch
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ BugBot reviewed your changes and found no bugs!
BugBot free trial expires on June 17, 2025
You have used $0.00 of your $100.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Rename
getMemberByIdtogetMemberByAccountIdfor clarityThis PR renames the
getMemberByIdfunction togetMemberByAccountIdto better reflect its purpose, as it retrieves team members by their account ID rather than a generic member ID.Changes:
getMemberByIdtogetMemberByAccountIdand updated all referenceshasToCompleteTeamOnboardingfunction to check if user is team ownerTeamPlanBadgecomponent to useuseDashboardRouterinstead of Next.js LinkThese changes improve code clarity and enhance the onboarding experience for team owners.
PR-Codex overview
This PR primarily focuses on refactoring the onboarding logic for team members, specifically changing the way member data is fetched and introducing a new onboarding check for teams.
Detailed summary
getMemberByIdtogetMemberByAccountIdfor clarity.getMemberByAccountId.isTeamOnboardingCompletefunction.hasToCompleteTeamOnboardingto check onboarding status.Summary by CodeRabbit
New Features
Refactor