Resolve org leads name from user info table #422
Conversation
WalkthroughThese changes introduce automatic name enrichment for organization leads by fetching user information via a batch API call. Lead name fields are made optional in the data model, validation rules are updated to accept ACM email domains, and the UI now uses a dedicated NameOptionalUserCard component for rendering leads instead of manual rendering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/ui/pages/organization/ManageOrganizationForm.tsx (2)
81-81: Remove unusednewLeadNamestate and related operations.The
newLeadNamestate variable is no longer needed since the Lead Name input field was removed. The variable is declared, cleared in useEffect, set when adding leads (always to empty string), and cleared after adding - but serves no purpose.Apply this diff to clean up the dead code:
- const [newLeadName, setNewLeadName] = useState(""); const [newLeadEmail, setNewLeadEmail] = useState(""); const [newLeadTitle, setNewLeadTitle] = useState(""); const [newLeadNonVoting, setNewLeadNonVoting] = useState(false);setCurrentLeads([]); setToAdd([]); setToRemove([]); - setNewLeadName(""); setNewLeadEmail(""); setNewLeadTitle(""); setNewLeadNonVoting(false);setToAdd((prev) => [ ...prev, { - name: newLeadName.trim(), + name: "", username: newLeadEmail.trim(), title: newLeadTitle.trim(), nonVotingMember: newLeadNonVoting, }, ]); - setNewLeadName(""); setNewLeadEmail("");Also applies to: 137-137, 180-180, 187-187
617-625: Improve confirmation modal display for leads without names.The confirmation modal displays
lead.namewhich will be an empty string for newly added leads. This results in a display like "() - Title" which may be confusing. Consider displaying the email address instead when name is not available.Apply this diff to improve the display:
<Text key={lead.username} fz="sm"> - - {lead.name} ({lead.username}) - {lead.title} + - {lead.name || lead.username} ({lead.username}) - {lead.title} {lead.nonVotingMember && (Or simplify to only show email and title:
<Text key={lead.username} fz="sm"> - - {lead.name} ({lead.username}) - {lead.title} + - {lead.username} - {lead.title} {lead.nonVotingMember && (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
src/api/functions/organizations.ts(3 hunks)src/common/types/organizations.ts(2 hunks)src/ui/pages/organization/ManageOrganizationForm.test.tsx(22 hunks)src/ui/pages/organization/ManageOrganizationForm.tsx(3 hunks)
🧰 Additional context used
🪛 ESLint
src/api/functions/organizations.ts
[error] 28-28: Unexpected use of file extension "js" for "./uin.js"
(import/extensions)
src/common/types/organizations.ts
[error] 9-9: Replace (email)·=>·email.endsWith('@illinois.edu')·||·email.endsWith('@acm.illinois.edu' with ··(email)·=>⏎········email.endsWith("@illinois.edu")·||·email.endsWith("@acm.illinois.edu"
(prettier/prettier)
[error] 28-28: Replace ·title:·z.string().min(1)·}) with ⏎··title:·z.string().min(1),⏎});
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests
- GitHub Check: Build Application
🔇 Additional comments (7)
src/common/types/organizations.ts (1)
28-28: LGTM - Schema change aligns with PR objectives.Making the name field optional in
enforcedOrgLeadEntrycorrectly supports the automatic name enrichment workflow where names are resolved from user info rather than manually entered.src/ui/pages/organization/ManageOrganizationForm.test.tsx (2)
9-9: LGTM - Test infrastructure updated correctly.The addition of
UserResolverProviderwrapper withresolutionDisabledproperly supports the newNameOptionalUserCardcomponent usage in the UI. The wrapper is consistently applied across all test cases.Also applies to: 40-47
715-715: Test expectation aligns with UI implementation.The test correctly expects
name: ""since the Lead Name input field was removed from the UI, andnewLeadNamewill always be an empty string.src/api/functions/organizations.ts (2)
112-134: LGTM - Name enrichment logic is well-implemented.The batch user info resolution approach is efficient and the name construction handles missing data gracefully. The logic correctly:
- Extracts emails from leads
- Batch fetches user information
- Constructs display names from firstName/lastName when available
- Returns undefined when no name data exists
114-118: The premise of this review comment is incorrect.
batchGetUserInfodoes not throw errors to its caller. The function catches all errors internally (lines 308-313 insrc/api/functions/uin.ts) and logs them as warnings without rethrowing. It always returns aRecordof user information, which may be partial or empty if batches fail, but the function itself never propagates an error.The existing call at line 114 in
src/api/functions/organizations.tsis already handling partial failures gracefully by design—failures in fetching user info simply result in incomplete data being returned rather than an exception being thrown. This is the intended behavior.Likely an incorrect or invalid review comment.
src/ui/pages/organization/ManageOrganizationForm.tsx (2)
282-289: LGTM - Lead rendering updated to use NameOptionalUserCard.The rendering logic correctly uses
NameOptionalUserCardwith the lead's email (username) and properly maintains the Non-Voting badge display.
146-146: LGTM - Validation updated to match new requirements.The validation correctly requires only email and title, aligning with the removal of the name input field.
Summary by CodeRabbit
New Features
Improvements