feat(admin): Contacts CRM Portal — Phase 1: Backend + Enhanced Table#152
Merged
Systemsaholic merged 7 commits intomainfrom Apr 2, 2026
Merged
feat(admin): Contacts CRM Portal — Phase 1: Backend + Enhanced Table#152Systemsaholic merged 7 commits intomainfrom
Systemsaholic merged 7 commits intomainfrom
Conversation
Adds contactType (lead/client) and contactStatus (multi-select) to ContactFilterDto, service WHERE clause, shared types, and frontend hook. Also wires sortOrder, hasPassport, passportExpiring to the frontend hook. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Indexes on trip_travelers.contact_id and contact_relationships.contact_id1/contact_id2 for batch nextTrip and relationshipCount queries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Shows 👥 count badge with lazy-loaded tooltip listing relationship labels on hover. Click handler for opening relationship manager. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Batch-computes nextTripName/Date and relationshipCount for the contacts list. Uses DISTINCT ON and UNION ALL to avoid N+1 queries. New fields are optional on ContactListItemDto (extends ContactResponseDto) to avoid breaking embedded contact references. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Checkboxes for bulk select, sortable headers, status/type badges, next trip, birthday highlight, relationship indicator, and tags. Replaces the basic 6-column table. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sort state stored in filters, bulk selection via Set<string>, view toggle (table/kanban with ToggleGroup, kanban placeholder for Phase 3), selected count indicator, clear selection on filter change. Wires enhanced 10-column ContactsTable with all new props. Bumps default page size to 25 and shows "Page X of Y" in pagination. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughContacts list UI was refactored into a client table with multi-selection, sortable columns, and new computed columns (nextTrip, relationshipCount). Backend API, service, DTOs, and DB migrations were updated to support new filters, computed per-contact fields, and indexes; frontend hooks and page wiring were adapted for selection and sorting. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Contacts Page
participant Table as ContactsTable (client)
participant API as /api/contacts
participant Service as ContactsService
participant DB as Database
Page->>Table: render with filters, sort, selection handlers
Table->>API: GET /contacts?filters...&sortBy=&sortOrder=
API->>Service: resolve findAll(filters, pagination)
Service->>DB: query contacts (with WHERE filters)
Service->>DB: batch query next trips (DISTINCT ON by contact_id)
Service->>DB: batch query relationship counts (union counts)
DB-->>Service: contact rows + computed values
Service-->>API: ContactListItemDto[] (with nextTrip / relationshipCount)
API-->>Table: paginated response
Table-->>Page: render rows, update selection/sort events
Page->>Table: onSortChange/onSelectionChange -> re-query
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/admin/src/app/contacts/_components/relationship-indicator.tsx (1)
36-46: Consider handling query error state in tooltip.If
useRelationshipsfails,relationshipsremainsundefinedand the tooltip perpetually shows "Loading...". Consider checking for error state:💡 Suggested improvement
+ const { data: relationships, isError } = useRelationships( hovered && count > 0 ? contactId : null ) - const tooltipLines = relationships + const tooltipLines = isError + ? ['Failed to load'] + : relationships ? relationships.map((r) => { // ... }) : ['Loading...']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/contacts/_components/relationship-indicator.tsx` around lines 36 - 46, The tooltipLines logic currently treats undefined relationships as a perpetual "Loading..." state; update it to handle the error and empty cases returned by useRelationships by checking the hook's error (e.g., error from useRelationships) and empty-array responses: if error is present set tooltipLines to ['Error loading relationships'] (or include brief error.message), if relationships is an empty array set tooltipLines to ['No relationships'], otherwise map relationships as currently done (preserve contactId, labelForContact1/2 and relatedContact usage). Ensure you reference and use the same identifiers tooltipLines, relationships, useRelationships, contactId, and relatedContact when making this change.apps/admin/src/app/contacts/page.tsx (1)
46-59: Minor: View toggle may flash on initial load.The component renders with the default
'table'view before theuseEffectreads fromlocalStorage. Consider initializing state lazily to avoid the flash:💡 Alternative initialization
- const [view, setView] = useState<ContactsView>('table') + const [view, setView] = useState<ContactsView>(() => { + if (typeof window !== 'undefined') { + const stored = localStorage.getItem('contacts-view-preference') + if (stored === 'table' || stored === 'kanban') return stored + } + return 'table' + }) - useEffect(() => { - const stored = localStorage.getItem('contacts-view-preference') - if (stored === 'table' || stored === 'kanban') { - setView(stored) - } - }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/contacts/page.tsx` around lines 46 - 59, The page currently flashes the default 'table' view because view is set synchronously to 'table' and only updated in the useEffect after mount; to fix, initialize the view state lazily by reading localStorage in the state initializer (instead of relying on the first useEffect) so the correct view is set before first render, remove the redundant initial useEffect that reads localStorage, and keep the existing effect that persists view changes; update references to setView, handleViewChange and the second useEffect ([view]) accordingly.packages/shared-types/src/api/contacts.types.ts (1)
238-239: Consider using a typed array forcontactStatusfilter.
contactTypeuses a strict literal union ('lead' | 'client'), butcontactStatusis loosely typed asstring[]. For consistency and type safety, consider defining a status type alias and reusing it:+export type ContactStatusValue = 'prospecting' | 'quoted' | 'booked' | 'traveling' | 'returned' | 'awaiting_next' | 'inactive' + export interface ContactFilterDto { // ... contactType?: 'lead' | 'client' - contactStatus?: string[] + contactStatus?: ContactStatusValue[]This would catch invalid status values at compile time and align with the status types used in
CreateContactDto(line 71) andUpdateContactDto(line 167).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-types/src/api/contacts.types.ts` around lines 238 - 239, Define a typed alias for contact statuses (e.g., ContactStatus) matching the statuses used in CreateContactDto and UpdateContactDto, then replace the loose contactStatus: string[] with contactStatus?: ContactStatus[] so the filter uses the same strict union as contactType and enforces valid status values at compile time (update the type alias usage wherever contactStatus is referenced).apps/admin/src/app/contacts/_components/contacts-table.tsx (2)
72-90: Consider addingaria-sortfor accessibility.The sortable header is functional, but screen readers would benefit from knowing the current sort state:
♿ Suggested accessibility enhancement
+ const ariaSort = sortBy === field ? (sortOrder === 'asc' ? 'ascending' : 'descending') : undefined + return ( <button className="flex items-center gap-1 hover:text-foreground" + aria-sort={ariaSort} onClick={() =>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/contacts/_components/contacts-table.tsx` around lines 72 - 90, The header button lacks an aria-sort attribute which helps screen readers announce the current sort state; update the button in contacts-table.tsx (the element rendering the sortable header using props field, sortBy, sortOrder and onSortChange) to include aria-sort set to 'ascending' when sortBy === field && sortOrder === 'asc', 'descending' when sortBy === field && sortOrder === 'desc', and 'none' otherwise so assistive tech can convey the sort direction.
409-413: Avoid non-null assertion with extracted variable.The
!assertion on line 411 is safe given the condition, but can be avoided:♻️ Cleaner pattern
- {(contact.tags?.length ?? 0) > 3 && ( - <Badge variant="secondary" className="text-xs"> - +{contact.tags!.length - 3} - </Badge> - )} + {contact.tags && contact.tags.length > 3 && ( + <Badge variant="secondary" className="text-xs"> + +{contact.tags.length - 3} + </Badge> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/contacts/_components/contacts-table.tsx` around lines 409 - 413, Extract the tag count into a local variable (e.g., const tagsLength = contact.tags?.length ?? 0) inside the ContactsTable component (contacts-table.tsx) and use that variable both in the conditional and in the Badge content instead of using the non-null assertion contact.tags!.length; update the conditional to (tagsLength > 3) and render +{tagsLength - 3} so there is no need for the `!` operator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/admin/src/app/contacts/_components/contacts-table.tsx`:
- Around line 359-361: The contacts-table component risks throwing when
rendering contact.nextTripDate; add a defensive helper (e.g.,
safeFormatDate(dateStr: string | null | undefined, formatStr: string): string |
null) that returns null for falsy/invalid dates and catches exceptions, then
replace direct format(new Date(contact.nextTripDate), ...) calls in
ContactsTable (the component rendering contact.nextTripDate and the other
occurrence) with safeFormatDate(contact.nextTripDate, 'MMM d'); additionally,
when safeFormatDate catches an error, call Sentry.captureException(error) or
processLogger to record the malformed value for monitoring.
In `@apps/admin/src/app/contacts/page.tsx`:
- Around line 216-222: The Retry Button rendered inside the error branch (the
JSX block that checks error) is not wired to refetch contacts; add an onClick
handler to that Button which calls the data-refetching function returned by your
data hook (e.g. refetch from useQuery, mutate/revalidate from useSWR, or a local
fetchContacts function) so it retries loading; if the current component doesn’t
expose a refetch function, wrap the contacts fetch call in a stateful async
function (e.g. fetchContacts or loadContacts) and invoke it from Button onClick,
and also disable the button or show a loading state while the retry is in
progress.
---
Nitpick comments:
In `@apps/admin/src/app/contacts/_components/contacts-table.tsx`:
- Around line 72-90: The header button lacks an aria-sort attribute which helps
screen readers announce the current sort state; update the button in
contacts-table.tsx (the element rendering the sortable header using props field,
sortBy, sortOrder and onSortChange) to include aria-sort set to 'ascending' when
sortBy === field && sortOrder === 'asc', 'descending' when sortBy === field &&
sortOrder === 'desc', and 'none' otherwise so assistive tech can convey the sort
direction.
- Around line 409-413: Extract the tag count into a local variable (e.g., const
tagsLength = contact.tags?.length ?? 0) inside the ContactsTable component
(contacts-table.tsx) and use that variable both in the conditional and in the
Badge content instead of using the non-null assertion contact.tags!.length;
update the conditional to (tagsLength > 3) and render +{tagsLength - 3} so there
is no need for the `!` operator.
In `@apps/admin/src/app/contacts/_components/relationship-indicator.tsx`:
- Around line 36-46: The tooltipLines logic currently treats undefined
relationships as a perpetual "Loading..." state; update it to handle the error
and empty cases returned by useRelationships by checking the hook's error (e.g.,
error from useRelationships) and empty-array responses: if error is present set
tooltipLines to ['Error loading relationships'] (or include brief
error.message), if relationships is an empty array set tooltipLines to ['No
relationships'], otherwise map relationships as currently done (preserve
contactId, labelForContact1/2 and relatedContact usage). Ensure you reference
and use the same identifiers tooltipLines, relationships, useRelationships,
contactId, and relatedContact when making this change.
In `@apps/admin/src/app/contacts/page.tsx`:
- Around line 46-59: The page currently flashes the default 'table' view because
view is set synchronously to 'table' and only updated in the useEffect after
mount; to fix, initialize the view state lazily by reading localStorage in the
state initializer (instead of relying on the first useEffect) so the correct
view is set before first render, remove the redundant initial useEffect that
reads localStorage, and keep the existing effect that persists view changes;
update references to setView, handleViewChange and the second useEffect ([view])
accordingly.
In `@packages/shared-types/src/api/contacts.types.ts`:
- Around line 238-239: Define a typed alias for contact statuses (e.g.,
ContactStatus) matching the statuses used in CreateContactDto and
UpdateContactDto, then replace the loose contactStatus: string[] with
contactStatus?: ContactStatus[] so the filter uses the same strict union as
contactType and enforces valid status values at compile time (update the type
alias usage wherever contactStatus is referenced).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d329c1d-2a9f-486a-9548-f0d83cf7425f
📒 Files selected for processing (10)
apps/admin/src/app/contacts/_components/contacts-table.tsxapps/admin/src/app/contacts/_components/relationship-indicator.tsxapps/admin/src/app/contacts/page.tsxapps/admin/src/hooks/use-contacts.tsapps/api/src/contacts/contacts.controller.tsapps/api/src/contacts/contacts.service.tsapps/api/src/contacts/dto/contact-filter.dto.tspackages/database/src/migrations/20260402201359_add_contacts_crm_indexes.sqlpackages/database/src/migrations/meta/_journal.jsonpackages/shared-types/src/api/contacts.types.ts
Comment on lines
+359
to
+361
| {contact.nextTripDate && ( | ||
| <span className="text-xs text-ash-500"> | ||
| {format(new Date(contact.nextTripDate), 'MMM d')} |
There was a problem hiding this comment.
Date parsing could throw on malformed data.
date-fns format() throws a RangeError if passed an invalid date. While the backend should validate date strings, defensive parsing would prevent a single malformed record from crashing the table.
🛡️ Suggested defensive helper
function safeFormatDate(dateStr: string | null | undefined, formatStr: string): string | null {
if (!dateStr) return null
try {
const date = new Date(dateStr)
if (isNaN(date.getTime())) return null
return format(date, formatStr)
} catch {
return null
}
}Then use:
- {format(new Date(contact.nextTripDate), 'MMM d')}
+ {safeFormatDate(contact.nextTripDate, 'MMM d')}As per coding guidelines, admin application components should capture runtime errors. Consider logging to Sentry when date parsing fails to detect data quality issues.
Also applies to: 380-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/contacts/_components/contacts-table.tsx` around lines 359
- 361, The contacts-table component risks throwing when rendering
contact.nextTripDate; add a defensive helper (e.g., safeFormatDate(dateStr:
string | null | undefined, formatStr: string): string | null) that returns null
for falsy/invalid dates and catches exceptions, then replace direct format(new
Date(contact.nextTripDate), ...) calls in ContactsTable (the component rendering
contact.nextTripDate and the other occurrence) with
safeFormatDate(contact.nextTripDate, 'MMM d'); additionally, when safeFormatDate
catches an error, call Sentry.captureException(error) or processLogger to record
the malformed value for monitoring.
Comment on lines
+216
to
+222
| {error ? ( | ||
| <div className="text-center py-12"> | ||
| <p className="text-destructive mb-4"> | ||
| Failed to load contacts. Please try again. | ||
| </p> | ||
| <Button variant="outline">Retry</Button> | ||
| </div> |
There was a problem hiding this comment.
Retry button is not functional.
The Retry button in the error state doesn't trigger any action. Wire it to refetch the data:
🔧 Proposed fix
+ const { data, isLoading, error, refetch } = useContacts(filters)
...
- <Button variant="outline">Retry</Button>
+ <Button variant="outline" onClick={() => refetch()}>Retry</Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/contacts/page.tsx` around lines 216 - 222, The Retry
Button rendered inside the error branch (the JSX block that checks error) is not
wired to refetch contacts; add an onClick handler to that Button which calls the
data-refetching function returned by your data hook (e.g. refetch from useQuery,
mutate/revalidate from useSWR, or a local fetchContacts function) so it retries
loading; if the current component doesn’t expose a refetch function, wrap the
contacts fetch call in a stateful async function (e.g. fetchContacts or
loadContacts) and invoke it from Button onClick, and also disable the button or
show a loading state while the retry is in progress.
Drizzle's sql template can't pass JS arrays to ANY(). Switched to IN() with sql.join() for the nextTrip and relationshipCount batch queries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Transforms the contacts page from a basic 6-column table into a CRM-ready portal with smart data, sorting, bulk selection, and a pipeline view toggle.
Backend Changes
contactType(lead/client) andcontactStatus(multi-select) added to ContactFilterDto, service WHERE clause, shared types, and frontend hooknextTripName,nextTripDate,relationshipCount— batched per-page to avoid N+1trip_travelers.contact_id,contact_relationships.contact_id1/id2for computed field performancesortOrder,hasPassport,passportExpiringto API (previously missing)Frontend Changes
New Components
RelationshipIndicator— 👥 count badge with lazy-loaded tooltipTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit