-
Notifications
You must be signed in to change notification settings - Fork 1
fix: enrich dashboard roles with missing assignedUserName on initial load #287
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughConsolidated role enrichment logic by introducing new utility functions to populate missing assignedUserName values for chart roles and team roles across API routers. Both dashboard and public-view routers now delegate enrichment to shared utilities in organization-members, replacing inline conditional logic with unified calls. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/api/routers/public-view.ts (1)
122-126: Missing role enrichment for public dashboard view.The
getDashboardByShareTokenprocedure includesroles: true(line 104) but does not callenrichChartRolesWithUserNamesbefore returning, unlike the authenticated dashboard queries indashboard.ts. This means public dashboard views will still show "Unassigned" for roles with a missingassignedUserName.🔎 Proposed fix
+import { + enrichChartRolesWithUserNames, + enrichRolesWithUserNames, + getOrganizationDirectoryId, +} from "@/server/api/utils/organization-members"; -import { - enrichRolesWithUserNames, - getOrganizationDirectoryId, -} from "@/server/api/utils/organization-members";Then update the enrichment flow:
+ // Enrich roles with missing assignedUserName + const directoryId = await getOrganizationDirectoryId(team.organizationId); + const chartsWithUserNames = await enrichChartRolesWithUserNames( + dashboardCharts, + team.organizationId, + directoryId, + ); + // Enrich charts with goal progress and value labels const enrichedCharts = await enrichChartsWithGoalProgress( - dashboardCharts, + chartsWithUserNames, ctx.db, );
🧹 Nitpick comments (1)
src/server/api/utils/organization-members.ts (1)
166-202: LGTM!Efficient implementation that fetches organization members once for all charts rather than per-chart. The nested mapping correctly handles the
chart.metric.rolesstructure.The role mapping logic (lines 190-198) is duplicated from
enrichRolesWithUserNames. You could optionally extract a helper or reuseenrichRolesWithUserNamesinternally:roles: await enrichRolesWithUserNames(chart.metric.roles, organizationId, directoryId),However, this would require passing the already-fetched
nameMapto avoid redundant API calls, so the current approach is acceptable for clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/server/api/routers/dashboard.tssrc/server/api/routers/public-view.tssrc/server/api/utils/organization-members.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Use TypeScript 5.9 with strict type checking for all frontend and backend code
Use Next.js 15 App Router with tRPC 11 and TanStack Query for API communication. For server components, use direct tRPC calls (
import { api } from '@/trpc/server') for 10x faster performance. For client components, use React hooks with TanStack Query (import { api } from '@/trpc/react').
Files:
src/server/api/utils/organization-members.tssrc/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All TypeScript/JavaScript files should use ESLint and Prettier for code quality, with import sorting via
@trivago/prettier-plugin-sort-importsusing inline type imports.
Files:
src/server/api/utils/organization-members.tssrc/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (GEMINI.md)
src/server/api/routers/**/*.ts: In tRPC procedures, never assume authentication; always use helpers likegetTeamAndVerifyAccessto verify access before operations
Be careful when modifying team save logic related toEditSessionmodel, which handles locking for concurrent team editing
src/server/api/routers/**/*.ts: Always useprotectedProcedureorworkspaceProcedurefor tRPC routes that require authentication. Include authorization checks using helpers from@/server/api/utils/authorizationto verify resources belong to the user's organization.
For new tRPC procedures, useworkspaceProcedurefor org-scoped operations, call authorization helpers for resource verification, and invalidate cache tags after mutations usinginvalidateCacheByTags(ctx.db, [tagName]).
Files:
src/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
src/**/*{metric,role,dashboard}*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
When linking/unlinking roles to metrics, update both TanStack Query caches:
role.getByTeamIdcache anddashboard.getDashboardChartscache. Useuse-optimistic-role-update.tshook for consistent cache management across both dashboards and canvas.
Files:
src/server/api/routers/dashboard.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/**/*{metric,role,dashboard}*.{ts,tsx} : When linking/unlinking roles to metrics, update both TanStack Query caches: `role.getByTeamId` cache and `dashboard.getDashboardCharts` cache. Use `use-optimistic-role-update.ts` hook for consistent cache management across both dashboards and canvas.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/app/teams/[teamId]/**/*role*.{ts,tsx} : For role nodes in the team canvas, store only the `roleId` in node data, not the full role object. Fetch display data from TanStack Query cache using the `useRoleData(roleId)` pattern to keep nodes lightweight and cache-efficient.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/app/teams/[teamId]/**/*role*node*.tsx : Consolidate duplicated `role-node.tsx` and `public-role-node.tsx` by extracting a shared `RoleNodeTemplate` component with an `isEditable` prop.
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/**/*{metric,role,dashboard}*.{ts,tsx} : When linking/unlinking roles to metrics, update both TanStack Query caches: `role.getByTeamId` cache and `dashboard.getDashboardCharts` cache. Use `use-optimistic-role-update.ts` hook for consistent cache management across both dashboards and canvas.
Applied to files:
src/server/api/utils/organization-members.tssrc/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/app/teams/[teamId]/**/*role*.{ts,tsx} : For role nodes in the team canvas, store only the `roleId` in node data, not the full role object. Fetch display data from TanStack Query cache using the `useRoleData(roleId)` pattern to keep nodes lightweight and cache-efficient.
Applied to files:
src/server/api/utils/organization-members.tssrc/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/app/teams/[teamId]/**/*role*node*.tsx : Consolidate duplicated `role-node.tsx` and `public-role-node.tsx` by extracting a shared `RoleNodeTemplate` component with an `isEditable` prop.
Applied to files:
src/server/api/utils/organization-members.tssrc/server/api/routers/public-view.ts
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Avoid modifying `enrichNodesWithRoleData` flow without understanding the complete canvas serialization logic for saving/loading React Flow nodes to the database
Applied to files:
src/server/api/utils/organization-members.tssrc/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/**/*dashboard*metric*card*.tsx : Consolidate duplicated `dashboard-metric-card.tsx` and `public-dashboard-metric-card.tsx` by adding a `readOnly` mode instead of maintaining separate components.
Applied to files:
src/server/api/routers/dashboard.ts
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : React Flow nodes must store minimal data (e.g., just `roleId`); fetch full Role data from TanStack Query cache in the Node component to keep canvas and sidebars in sync
Applied to files:
src/server/api/routers/dashboard.tssrc/server/api/routers/public-view.ts
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/server/api/routers/**/*.ts : For new tRPC procedures, use `workspaceProcedure` for org-scoped operations, call authorization helpers for resource verification, and invalidate cache tags after mutations using `invalidateCacheByTags(ctx.db, [tagName])`.
Applied to files:
src/server/api/routers/dashboard.ts
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to src/server/api/routers/**/*.ts : Always use `protectedProcedure` or `workspaceProcedure` for tRPC routes that require authentication. Include authorization checks using helpers from `@/server/api/utils/authorization` to verify resources belong to the user's organization.
Applied to files:
src/server/api/routers/dashboard.ts
📚 Learning: 2025-12-25T20:38:49.083Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T20:38:49.083Z
Learning: Applies to **/*.{ts,tsx} : Use Next.js 15 App Router with tRPC 11 and TanStack Query for API communication. For server components, use direct tRPC calls (`import { api } from '@/trpc/server'`) for 10x faster performance. For client components, use React hooks with TanStack Query (`import { api } from '@/trpc/react'`).
Applied to files:
src/server/api/routers/dashboard.ts
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/server/api/routers/**/*.ts : Be careful when modifying team save logic related to `EditSession` model, which handles locking for concurrent team editing
Applied to files:
src/server/api/routers/public-view.ts
🧬 Code graph analysis (2)
src/server/api/routers/dashboard.ts (2)
src/server/api/utils/organization-members.ts (1)
enrichChartRolesWithUserNames(170-202)src/server/api/utils/enrich-charts-with-goal-progress.ts (1)
enrichChartsWithGoalProgress(60-141)
src/server/api/routers/public-view.ts (1)
src/server/api/utils/organization-members.ts (2)
getOrganizationDirectoryId(117-128)enrichRolesWithUserNames(140-164)
🔇 Additional comments (7)
src/server/api/routers/dashboard.ts (3)
12-12: LGTM!The import of
enrichChartRolesWithUserNamescorrectly brings in the shared utility for role enrichment.
50-57: LGTM!Role enrichment is correctly applied before goal progress enrichment. The flow ensures
assignedUserNameis populated before charts are returned to the client.
98-105: LGTM!Consistent enrichment pattern with
getAllDashboardChartsWithData. Both dashboard queries now properly populate missingassignedUserNamevalues.src/server/api/routers/public-view.ts (2)
11-14: LGTM!Import updated to use the shared
enrichRolesWithUserNamesutility, removing the now-unnecessary individual imports.
55-63: LGTM!Clean refactor using the shared
enrichRolesWithUserNamesutility. The public view correctly fetches the directory ID separately since it lacks workspace context.src/server/api/utils/organization-members.ts (2)
129-134: LGTM!Minimal type definition that captures the required fields for enrichment. The generic constraint pattern (
<T extends RoleWithUser>) allows the functions to work with full role objects while only requiring these two fields.
136-164: LGTM!Well-implemented enrichment function with:
- Early return optimization to avoid unnecessary WorkOS API calls
- Correct preservation of original role properties via spread
- Sensible fallback (
User ${id.substring(0, 8)}) for unknown users
Summary
Fixes roles in the dashboard drawer's Roles tab showing "Unassigned" when they have
assignedUserIdbutassignedUserNameis null in the database.Changes
enrichRolesWithUserNamesandenrichChartRolesWithUserNamesshared utilitiesSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.