feat: add user selection functionality for suggestions and update API… - #39
Hidden character warning
Conversation
… to include analystId
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds analyst assignment support for suggestions: frontend introduces a UserSelector to set analystId per suggestion and threads _currentUser; backend expands admin list selection fields and updates admin update mutation to accept optional analystId, defaulting to the current user when not provided. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin User
participant UI as AdminSuggestionsPage
participant Sel as UserSelector
participant API as tRPC: suggestions
participant DB as Database
Admin->>UI: Open admin suggestions
UI->>API: listAdmin()
API->>DB: SELECT id,...,analystId, user{...}, analyst{...}
DB-->>API: Rows
API-->>UI: Suggestions with analyst and user data
Admin->>Sel: Open analyst selector
Sel->>API: user.listAll()
API->>DB: SELECT users
DB-->>API: Users
API-->>Sel: Users list
Admin->>Sel: Choose analyst (or "Não atribuído")
Sel-->>UI: onValueChange(analystId|null)
UI->>API: updateAdmin({ id, ..., analystId? })
API->>DB: UPDATE suggestion SET analystId = input.analystId ?? ctx.auth.userId
DB-->>API: OK
API-->>UI: Updated suggestion
UI-->>Admin: Reflect updated responsible analyst
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File ( |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(authenticated)/admin/suggestions/page.tsx (1)
367-383: Preserve null for analystId; don’t coerce to undefinedCoercing
nulltoundefinedbreaks unassigning semantics and, combined with the server’s current fallback, will reassign to the current admin. Keepnullintact.Apply:
- if (updates.analystId !== undefined) {- updateData.analystId = updates.analystId ?? undefined- }+ if (updates.analystId !== undefined) {+ // Keep null to explicitly clear assignment; undefined means no change+ updateData.analystId = updates.analystId+ }
🧹 Nitpick comments (6)
src/server/api/routers/suggestions.ts (1)
110-127: Good switch to explicit select; consider trimming unused fields to reduce payloadNice move to an explicit
selectfor admin list; this typically improves performance and keeps types tight. If the UI ultimately displays the user's sector (user.setor) instead of the submitted sector, consider droppingsubmittedSectorfrom the selection to cut payload, or align the mapping to avoid confusion.Example minimal trim:
select: { id: true, ideaNumber: true, submittedName: true, - submittedSector: true, isNameVisible: true, description: true, contribution: true, dateRef: true, impact: true, capacity: true, effort: true, finalScore: true, finalClassification: true, status: true, rejectionReason: true, analystId: true, createdAt: true, user: { select: { firstName: true, lastName: true, email: true, setor: true, }, }, analyst: { select: { firstName: true, lastName: true, email: true, }, }, },src/app/(authenticated)/admin/suggestions/page.tsx (5)
44-45: Type captures the new responsibility concept; document tri-state intentDefining
analystId: string | nullinSuggestionLocalis fine. For clarity and to align with the server change, ensure the app treats:
- undefined: no change
- string: assign to that user
- null: unassign
Would you like me to sweep the components for places where
analystIdis constructed to ensure they adhere to this tri-state convention?
222-222: _currentUser is fetched but never used; avoid prop drilling
_currentUseris passed down multiple levels but not used. Either remove it to reduce churn, or leverage it (e.g., to show “Você” next to the current user in UserSelector).
612-612: Remove unused _currentUser prop passed through components
_currentUseris threaded throughIdeasAccordionandSuggestionItembut unused. Trim these props to simplify component signatures and avoid unnecessary rerenders.High-level diff outline:
- _currentUser={_currentUser}And in component props:
- _currentUser: RouterOutputs["user"]["me"] | undefinedAlso applies to: 630-630, 689-689, 701-701, 779-779, 806-806
1040-1044: Unassign flow and UX: ensure it works end-to-end and consider disabling while savingThis call path must allow setting
analystIdto null. With the client/server fixes above, the “Não atribuído” option will clear the assignment. Consider temporarily disabling the selector while the update mutation for this suggestion is in-flight to avoid rapid toggles causing racey UI.Example (conceptual; needs plumbing of a per-suggestion pending state):
- <UserSelector+ <UserSelector value={s.analystId} - onValueChange={(userId: string | null) => {+ onValueChange={(userId: string | null) => { update(s.id, { analystId: userId }) }} - />+ disabled={/* pending state for this suggestion's update */}+ />
129-215: Ensure “Não atribuído” shows selected state, improve typing, scalability, and confirm security– Selected state
The “Não atribuído” item should display a checkmark whenvalue === null, matching the behavior of other rows.– Typing
UpdategetUserDisplayNameto use(typeof allUsers)[number]so the parameter type can never beundefined.– Scalability
Currentlyapi.user.listAll.useQuery()fetches every user on mount. If the user count grows, consider switching to a server-side search or paginated endpoint to keep the popover performant.– Security
Verified thatuser.listAllis implemented withadminProcedureinsrc/server/api/routers/user.ts(lines 21–23), so only admins can call it.Apply these optional refactors:
- const getUserDisplayName = (user: typeof allUsers[0]) => {+ const getUserDisplayName = (user: (typeof allUsers)[number]) => { if (user.firstName && user.lastName) { return `${user.firstName} ${user.lastName}` } return user.email }<CommandItem onSelect={() => { onValueChange(null) setOpen(false) }} className="text-muted-foreground" > - <Check className="mr-2 h-4 w-4 opacity-0" />+ <Check className={`mr-2 h-4 w-4 ${value === null ? "opacity-100" : "opacity-0"}`} /> Não atribuído </CommandItem>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/app/(authenticated)/admin/suggestions/page.tsx(15 hunks)src/server/api/routers/suggestions.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/(authenticated)/admin/suggestions/page.tsx (4)
src/trpc/react.tsx (2)
api(23-23)RouterOutputs(37-37)src/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)src/components/ui/button.tsx (1)
Button(56-56)src/components/ui/command.tsx (6)
Command(144-144)CommandInput(146-146)CommandList(147-147)CommandEmpty(148-148)CommandGroup(149-149)CommandItem(150-150)
🔇 Additional comments (5)
src/server/api/routers/suggestions.ts (1)
162-162: Comment update improves clarityThe updated doc comment calling out analyst handling is helpful for future readers.
src/app/(authenticated)/admin/suggestions/page.tsx (4)
18-18: Icon import looks goodImports are consistent with usage in this file.
20-21: Command/Popover UI imports are appropriateMatches the new UserSelector implementation below.
113-113: Mapping analystId from DB is correctDirect pass-through matches the server
select.
313-338: Classification pools: good memoized mappingMapping tRPC results to local
ClassItem[]is straightforward and readable.
| kpis: z.array(z.string()).optional(), | ||
| status: StatusEnum.optional(), | ||
| rejectionReason: z.string().optional(), | ||
| analystId: z.string().optional(), |
There was a problem hiding this comment.
Allow null to enable unassigning an analyst (tri-state: string | null | undefined)
Right now analystId is z.string().optional(). That makes it impossible to intentionally clear the responsible analyst: sending null fails validation; omitting the field (undefined) currently triggers a fallback (see Line 240) that will force-assign the current admin. Support null so the client can unassign.
Apply:
- analystId: z.string().optional(),+ // string = assign to user; null = unassign; undefined = do not change+ analystId: z.string().nullable().optional(),📝 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.
| analystId: z.string().optional(), | |
| // string = assign to user; null = unassign; undefined = do not change | |
| analystId: z.string().nullable().optional(), |
🤖 Prompt for AI Agents
In src/server/api/routers/suggestions.ts around line 172, the Zod schema for
analystId should accept null so clients can explicitly unassign; change the
schema to allow string | null | undefined (e.g., make it nullable and optional).
Also adjust the handling around line 240 so the "fallback to current admin" only
runs when analystId is strictly undefined (preserve existing behavior for
omitted field) and treat a null value as an explicit unassign (do not replace it
with the current admin).
| status: input.status, | ||
| rejectionReason: input.rejectionReason, | ||
| analystId: ctx.auth.userId, | ||
| analystId: input.analystId ?? ctx.auth.userId, // Usar analystId fornecido ou fallback para usuário atual |
There was a problem hiding this comment.
Do not force-assign analyst on every admin update; honor undefined vs null
analystId: input.analystId ?? ctx.auth.userId reassigns the analyst to the current admin on any update that omits analystId. It also prevents unassigning even if the client sends null (because ?? coalesces null). This will silently overwrite the responsible analyst during unrelated updates (status, scores, etc.).
Use tri-state semantics:
- undefined: don't touch analystId
- string: assign to that user
- null: clear the assignment
Apply:
- analystId: input.analystId ?? ctx.auth.userId, // Usar analystId fornecido ou fallback para usuário atual+ // Only modify when provided; allow null to clear+ ...(input.analystId !== undefined ? { analystId: input.analystId } : {}),Optionally, if you want a one-time default assignment when there is no analyst yet, gate it explicitly:
// Before update, fetch current analystIdconstcurrent=awaitctx.db.suggestion.findUnique({where: {id: input.id},select: {analystId: true}});// In data:
...(input.analystId!==undefined
? {analystId: input.analystId}
: current?.analystId ? {} : {/* decide whether to set a default here or leave null */}),🤖 Prompt for AI Agents
In src/server/api/routers/suggestions.ts around line 240, the current use of
"analystId: input.analystId ?? ctx.auth.userId" force-assigns the analyst on
every update and treats null as undefined; change this to tri-state handling:
fetch the current suggestion's analystId first, then when building the update
data only include analystId if input.analystId !== undefined (if string, set it;
if null, set it to null to clear), and do not fall back to ctx.auth.userId
implicitly — if you want a one-time default when there is no analyst, gate that
explicitly based on the fetched current value.
Uh oh!
There was an error while loading. Please reload this page.
… to include analystId
Summary by CodeRabbit
New Features
Improvements