-
Notifications
You must be signed in to change notification settings - Fork 561
[Dashboard] Rename getMemberById to getMemberByAccountId and add onboarding banner #7334
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
[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. |
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 📦
|
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: 5
🧹 Nitpick comments (6)
apps/dashboard/src/@/api/team-members.ts (1)
51-54
: Add an explicit return type for stronger type-safety
getMemberByAccountId
currently 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.map
every 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 cachinggetMemberByAccountId
results across iterations.apps/dashboard/src/app/(app)/team/[team_slug]/layout.tsx (1)
33-35
: Avoid double account fetch insidehasToCompleteTeamOnboarding
hasToCompleteTeamOnboarding
internally callsgetValidAccount
, incurring an extra cookie read / API hit that this layout doesn’t otherwise need.
Given you already haveauthToken
and 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-datedcreatedAt
If
team.createdAt
is (accidentally) in the future or the server clock differs,
differenceInDays
becomes 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:getValidAccount
can redirect — consider early bailout
getValidAccount
may throw a Next.js redirect for unauthenticated users.
BecausehasToCompleteTeamOnboarding
is 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.all
Fetching 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
1b95215
to
c269398
Compare
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). |
f0dacda
to
30301d9
Compare
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
🧹 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)
30301d9
to
2849782
Compare
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.
✅ 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
getMemberById
togetMemberByAccountId
for clarityThis PR renames the
getMemberById
function togetMemberByAccountId
to better reflect its purpose, as it retrieves team members by their account ID rather than a generic member ID.Changes:
getMemberById
togetMemberByAccountId
and updated all referenceshasToCompleteTeamOnboarding
function to check if user is team ownerTeamPlanBadge
component to useuseDashboardRouter
instead 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
getMemberById
togetMemberByAccountId
for clarity.getMemberByAccountId
.isTeamOnboardingComplete
function.hasToCompleteTeamOnboarding
to check onboarding status.Summary by CodeRabbit
New Features
Refactor