feat: add user activity timeline to admin user details page

[mchestr]

Summary

• Adds a timeline view showing Discord commands and media marks on the admin user details page (/admin/users/[userId])
• Timeline displays activities chronologically (most recent first) with pagination
• Uses discriminated union types for type-safe rendering of different activity types

Changes

New Files

• components/admin/users/user-activity-timeline.tsx - Client component with timeline UI and pagination
• actions/__tests__/user-queries.test.ts - Server action tests (12 tests)
• components/__tests__/user-activity-timeline.test.tsx - Component tests (23 tests)

Modified Files

• types/admin.ts - Added activity type definitions (DiscordCommandActivity, MediaMarkActivity, UserActivityItem, UserActivityTimelineData)
• actions/user-queries.ts - Added getUserActivityTimeline server action
• actions/users.ts - Exported new server action
• app/admin/users/[userId]/page.tsx - Integrated timeline section
• __tests__/utils/test-builders.ts - Added test builders for activity types

Features

• Discord commands: Shows command name, status badge, response time, channel type
• Media marks: Shows mark type badge, formatted media title, source (discord/web)
• Pagination: 10 items per page with TanStack Query for client-side pagination
• Badges: Color-coded status badges following existing Discord activity patterns

Test plan

• Run unit tests: npm test user-activity-timeline (23 tests pass)
• Run server action tests: npm test actions/__tests__/user-queries (12 tests pass)
• Build passes: npm run build
• Lint passes: npm run lint
• Manual testing: View user details page with activity data

🤖 Generated with Claude Code

[claude]

Code Review: User Activity Timeline Feature

Summary

This PR adds a user activity timeline to the admin user details page, showing Discord commands and media marks in chronological order. Overall, this is high-quality, well-tested code that follows the project's conventions. Great work!

✅ Strengths

1. Excellent Test Coverage

• 35 total tests (12 server action tests + 23 component tests)
• Comprehensive coverage of edge cases, pagination, sorting, transformations
• Tests properly use data-testid attributes (following project conventions)
• Good use of test builders for maintainability

2. Type Safety

• Discriminated union types (UserActivityItem) for type-safe rendering
• Proper TypeScript with no any types
• Type guards used correctly in component rendering

3. Follows Project Conventions

• ✅ Server Actions for data fetching (not API routes)
• ✅ Client component only where needed ('use client')
• ✅ Tailwind utility classes (no custom CSS)
• ✅ Proper error handling with logging
• ✅ Uses existing UI components (Pagination)
• ✅ Server component async/await pattern in page
• ✅ Parallel data fetching with Promise.all

4. Good UX Design

• Timeline visualization with colored dots
• Badge-based status indicators
• Relative timestamps
• Pagination with clear controls
• Empty state handling

⚠️ Issues Found

🔴 Critical: Pagination Bug

Location: actions/user-queries.ts:449-454

take: pageSize * 2, // Fetch extra to merge properly

Problem: The pagination logic is broken. You're fetching pageSize * 2 items from each source, merging them, then slicing. This means:

• If pageSize = 10, you fetch 20 Discord + 20 media marks = 40 items
• After sorting, you slice 10 items from page 1
• But the actual page 2 data was never fetched - you only have the first 20 from each source
• Users will see wrong/missing data on later pages

Example Bug:

• User has 50 Discord commands, 50 media marks (100 total)
• Page 1: Shows correct 10 items
• Page 2: Shows 10 items, but they're from the first 40 items only (20 Discord + 20 media), missing items 41-100

Correct Approach:
You need to fetch all items (or implement cursor-based pagination). Options:

Option A: Fetch All (simple, works for reasonable data sizes)

const [discordLogs, mediaMarks, discordCount, mediaMarkCount] = await Promise.all([
  prisma.discordCommandLog.findMany({
    where: { userId },
    orderBy: { createdAt: "desc" },
  }),
  prisma.userMediaMark.findMany({
    where: { userId },
    orderBy: { markedAt: "desc" },
  }),
  // ... counts
])

Option B: Database-Level Merge (efficient, but complex)
Use a raw SQL query with UNION to merge and paginate at the database level.

Option C: Limit Maximum (pragmatic)
Document a max limit (e.g., "showing last 500 activities") and fetch that many.

For most users, Option A is fine unless you expect thousands of activities per user.

🟡 Medium: Potential Performance Issue

Location: actions/user-queries.ts:445-458

The current implementation always fetches data from both tables even when:

• One table might be empty
• The count queries could tell us which sources to fetch from

Optimization:

// Check counts first
const [discordCount, mediaMarkCount] = await Promise.all([
  prisma.discordCommandLog.count({ where: { userId } }),
  prisma.userMediaMark.count({ where: { userId } }),
])

if (discordCount === 0 && mediaMarkCount === 0) {
  return { items: [], total: 0, page, pageSize, totalPages: 0 }
}

// Only fetch from non-empty tables
const promises = []
if (discordCount > 0) {
  promises.push(prisma.discordCommandLog.findMany(...))
}
if (mediaMarkCount > 0) {
  promises.push(prisma.userMediaMark.findMany(...))
}

This is a minor optimization but could save database queries.

🟡 Medium: Badge Component Duplication

Location: components/admin/users/user-activity-timeline.tsx:15-95

The getStatusBadge and getMarkTypeBadge functions have significant duplication. Each badge is nearly identical JSX.

Suggestion: Extract to a reusable badge component (following DRY principle):

const Badge = ({ 
  color, 
  text 
}: { 
  color: 'green' | 'red' | 'yellow' | 'orange' | 'blue' | 'cyan' | 'amber' | 'slate'
  text: string 
}) => (
  <span className={`px-1.5 py-0.5 bg-${color}-500/20 text-${color}-400 text-xs font-medium rounded`}>
    {text}
  </span>
)

However, note that Tailwind's JIT compiler may not pick up dynamic classes. You might need to use the existing Badge component from components/ui/badge.tsx if available, or use a clsx approach with explicit class mappings.

🟢 Minor: Missing Logger Import

Location: actions/user-queries.ts:507

logger.error("Error fetching user activity timeline", error, { userId })

I don't see the logger imported at the top of the file in the diff. Please verify it's imported:

import { createLogger } from '@/lib/utils/logger'
const logger = createLogger('user-queries')

🟢 Minor: Component Size

Location: components/admin/users/user-activity-timeline.tsx

The file is 254 lines. While not terrible, it's approaching the recommended max (~200-300 lines). Consider extracting:

• ActivityItem → separate file activity-item.tsx
• Badge functions → activity-badges.tsx

This would improve readability and testability.

🔍 Code Quality Notes

Good Practices Observed:

• ✅ Proper use of discriminated unions for type safety
• ✅ Descriptive test names
• ✅ Empty state handling
• ✅ Accessibility attributes (data-testid)
• ✅ Error handling with logging
• ✅ Conditional rendering (timeline only shows when data exists)

Minor Suggestions:

1. Consider adding loading states - The component shows "Loading activity..." but could show a skeleton UI
2. Response time formatting - Consider formatting responseTimeMs for better readability (e.g., "2.5s" for 2500ms)
3. Command args truncation - The truncate class is good, but consider a tooltip or expandable view for long commands

🧪 Testing

Tests are excellent! Just ensure:

• Manual testing with large datasets (100+ activities) to verify pagination bug
• Test with users who have only Discord commands or only media marks
• Test pagination navigation (especially pages 2+)

📋 Security & Performance

• ✅ Admin-only access properly enforced via requireAdmin()
• ✅ No SQL injection risk (using Prisma)
• ✅ No XSS risk (React escapes by default)
• ⚠️ Performance: Fix pagination bug (could cause unnecessary data fetching)

Recommendation

Status: Needs Changes (due to pagination bug)

Please fix the critical pagination issue before merging. The rest of the code is solid. Once the pagination is corrected, this will be a great addition to the admin tools.

---

Priority:

1. 🔴 Fix pagination logic (critical bug)
2. 🟡 Verify logger import exists
3. 🟡 Consider performance optimization for empty tables
4. 🟢 Consider refactoring badge components (optional)

[claude]

Code Review: User Activity Timeline Feature

Summary

This is a well-implemented feature that adds an activity timeline to the admin user details page. The PR demonstrates strong adherence to the project's coding standards with excellent test coverage (35 total tests), proper type safety, and good architectural patterns.

✅ Strengths

1. Excellent Test Coverage

• 23 component tests covering all rendering scenarios, badge types, formatting, and pagination
• 12 server action tests covering data fetching, sorting, transformation, and error handling
• Tests follow project conventions with proper mocking and realistic user interactions
• Good use of test builders for consistent test data creation

2. Type Safety & Architecture

• Discriminated union types (UserActivityItem) provide excellent type safety
• Type guards work correctly for rendering different activity types
• Server Components by default with 'use client' only where needed
• Proper use of Server Actions following CLAUDE.md guidelines

3. Performance Considerations

• Parallel data fetching for Discord logs and media marks (Promise.all)
• TanStack Query integration with proper caching (staleTime: 30000)
• Server-side pagination to avoid loading excessive data
• Initial data passed to avoid unnecessary refetch on first render

4. Code Quality

• Clean separation of concerns (timeline logic, rendering, badge utilities)
• Consistent styling with existing patterns (dark theme, badge colors)
• Good accessibility with semantic HTML and test IDs
• Follows Tailwind utility-first approach

5. User Experience

• Chronological sorting (most recent first) with clear visual timeline
• Color-coded badges for quick status recognition
• Relative timestamps for better context
• Pagination for large activity sets

⚠️ Issues & Recommendations

1. CRITICAL: Pagination Logic Bug

Location: actions/user-queries.ts:449-497

The pagination implementation has a critical flaw that will cause incorrect results:

// ❌ INCORRECT: Fetches 2x pageSize from each source, then paginates
take: pageSize * 2  // Line 449, 454

// Later applies pagination to merged results
const paginatedItems = allActivities.slice(offset, offset + pageSize)

Problem: This approach doesn't correctly paginate merged data sources.

Example scenario:

• Page 1, pageSize 10
• User has 50 Discord commands and 50 media marks (100 total activities)
• Code fetches 20 Discord + 20 media marks, merges (40 items), returns items 0-10
• Missing 60 activities that should be included in the sort!

Fix: Use database-level pagination with cursor-based approach or fetch ALL data (with reasonable limits):

// Option 1: Fetch all with reasonable limit
const MAX_ACTIVITIES = 1000
const [discordLogs, mediaMarks, discordCount, mediaMarkCount] = await Promise.all([
  prisma.discordCommandLog.findMany({
    where: { userId },
    orderBy: { createdAt: "desc" },
    take: MAX_ACTIVITIES,
  }),
  prisma.userMediaMark.findMany({
    where: { userId },
    orderBy: { markedAt: "desc" },
    take: MAX_ACTIVITIES,
  }),
  // ... counts
])

// Then merge, sort, and paginate in memory

// Option 2: Use raw SQL with UNION and proper pagination
// (More complex but handles unlimited data)

This is a must-fix before merging as it will produce incorrect results for users with many activities.

2. Performance: Inefficient for Large Datasets

Location: actions/user-queries.ts:489-497

Even with the pagination fix, the current approach fetches ALL activities into memory, sorts them, then slices. For users with thousands of activities, this is inefficient.

Recommendation: Consider a hybrid approach:

• For users with < 1000 total activities: Current in-memory approach (with fix above)
• For users with > 1000 activities: Use raw SQL with UNION ALL to merge and paginate at the database level

Example:

if (total > 1000) {
  // Use raw SQL with UNION ALL and proper LIMIT/OFFSET
  const activities = await prisma.$queryRaw`
    SELECT * FROM (
      SELECT id, created_at as timestamp, 'discord_command' as type, ...
      FROM discord_command_log WHERE user_id = ${userId}
      UNION ALL
      SELECT id, marked_at as timestamp, 'media_mark' as type, ...
      FROM user_media_mark WHERE user_id = ${userId}
    ) activities
    ORDER BY timestamp DESC
    LIMIT ${pageSize} OFFSET ${offset}
  `
}

3. Missing Error State in Component

Location: components/admin/users/user-activity-timeline.tsx:207-228

The component handles loading and empty states but not error states from the query.

Fix:

const { data, isLoading, isError } = useQuery({
  // ... existing config
})

if (isError) {
  return (
    <div className="p-8 text-center text-red-400">
      Failed to load activity timeline
    </div>
  )
}

4. Missing Null Check

Location: components/admin/users/user-activity-timeline.tsx:166-170

The code checks item.responseTimeMs != null which is good, but also displays when it's 0. Consider if 0ms is a valid display value or if it should be hidden.

{item.responseTimeMs != null && item.responseTimeMs > 0 && (
  <span className="text-xs text-slate-500">
    {item.responseTimeMs}ms
  </span>
)}

5. Component Extraction Opportunity

Location: components/admin/users/user-activity-timeline.tsx:15-95

The badge functions (getStatusBadge, getMarkTypeBadge) have significant duplication. Consider extracting to a shared utility or creating a reusable Badge component.

Suggestion:

// components/ui/status-badge.tsx
type BadgeVariant = 'success' | 'error' | 'warning' | 'info' | 'neutral'

export function StatusBadge({ 
  variant, 
  children 
}: { 
  variant: BadgeVariant
  children: React.ReactNode 
}) {
  const styles = {
    success: 'bg-green-500/20 text-green-400',
    error: 'bg-red-500/20 text-red-400',
    warning: 'bg-yellow-500/20 text-yellow-400',
    // ... etc
  }
  
  return (
    <span className={cn('px-1.5 py-0.5 text-xs font-medium rounded', styles[variant])}>
      {children}
    </span>
  )
}

6. Minor: Type Inference Could Be Better

Location: components/admin/users/user-activity-timeline.tsx:97

The function parameter type is verbose. TypeScript can infer this:

// Current
function formatMediaTitle(item: UserActivityItem & { type: "media_mark" })

// Suggestion: Use Extract utility type
function formatMediaTitle(item: Extract<UserActivityItem, { type: "media_mark" }>)

7. Accessibility: Missing ARIA Labels

Location: Timeline component

The timeline dots and visual indicators lack ARIA labels for screen readers.

Suggestion:

<div 
  className="absolute left-0 top-1.5 w-6 h-6..."
  aria-label={isDiscord ? "Discord command" : "Media mark"}
  role="presentation"
>

🔒 Security Review

✅ No security concerns identified:

• Proper admin authorization check (requireAdmin())
• Uses Prisma parameterized queries (SQL injection safe)
• No user input rendered without escaping
• No sensitive data exposed in client components

🧪 Test Coverage Analysis

Excellent coverage overall, but consider adding:

1. Integration test for the full flow (server action + component)
2. Edge case test for users with exactly pageSize activities
3. Error recovery test for what happens when server action returns null
4. Performance test for large datasets (if applicable)

📊 Performance Impact

• Database queries: 4 queries per page load (2 findMany + 2 counts) - well optimized with Promise.all
• Memory usage: Currently loads 2x pageSize items - minimal impact
• Client bundle: Small increase (~8KB estimated for component + types)

🎯 Recommendations Priority

1. 🔴 MUST FIX: Pagination logic bug (chore(deps): bump actions/setup-node from 4 to 6 #1)
2. 🟡 SHOULD FIX: Error state handling (chore(deps): bump actions/checkout from 4 to 5 #3)
3. 🟢 NICE TO HAVE: Performance optimization for large datasets (chore(deps): bump docker/build-push-action from 5 to 6 #2), component extraction (chore(deps): bump react-dom and @types/react-dom #5)

Conclusion

This is a solid PR with excellent test coverage and adherence to project standards. The main blocker is the pagination bug which must be fixed before merging. Once addressed, this will be a valuable addition to the admin user details page.

Great work overall! 🎉

---

Reviewed using: Repository CLAUDE.md guidelines
Test Results: ✅ 23 component tests + ✅ 12 server action tests = 35 total tests passing

[claude]

Code Review - PR #170: User Activity Timeline

Overview

This PR adds a well-structured timeline view for user activities (Discord commands and media marks) on the admin user details page. The implementation demonstrates strong adherence to project conventions with excellent test coverage.

✅ Strengths

1. Architecture & Design

• ✅ Proper use of discriminated union types for type-safe activity rendering
• ✅ Server Actions used correctly for data fetching (not API routes)
• ✅ Client component appropriately marked with 'use client' for interactivity
• ✅ Smart use of initialData prop to avoid loading state on initial page load
• ✅ Clean separation: types, server actions, components, and tests are well-organized

2. Data Fetching & Performance

• ✅ Parallel fetching of Discord logs and media marks using Promise.all
• ✅ TanStack Query integration with proper initialData handling
• ✅ Reasonable staleTime: 30000 (30s) for client-side caching
• ✅ Admin access check via requireAdmin() in server action

3. Testing

• ✅ Exceptional test coverage: 35 total tests (23 component + 12 server action)
• ✅ Comprehensive test scenarios covering all badge types, edge cases, and error handling
• ✅ Proper use of test builders for maintainable test data
• ✅ Tests verify both transformation logic and UI rendering

4. UI/UX

• ✅ Uses existing Pagination component (following project guidelines)
• ✅ Consistent Tailwind styling matching project dark theme
• ✅ Good use of data-testid attributes for reliable E2E testing
• ✅ Accessible empty states and loading states

---

🔍 Issues & Recommendations

🐛 CRITICAL: Pagination Logic Bug

Location: actions/user-queries.ts:449-454

The current implementation fetches pageSize * 2 from each source, but pagination happens AFTER merging and sorting. This causes incorrect results on page 2+.

Problem: When navigating to page 2+ with mixed activity types, you'll get incorrect results because:

1. You fetch only the first 20 items (2 × pageSize) from each source
2. After merging and sorting, you paginate from this limited set
3. Older items from page 3+ may be missing entirely

Example Scenario:

• User has 50 Discord commands and 50 media marks (100 total activities)
• Page 1 (items 1-10): Works correctly
• Page 2 (items 11-20): BUG - You only have 20 Discord + 20 media items loaded, but after merging/sorting, you might need Discord item Refactor: Split large maintenance components exceeding file size guidelines #35 or media mark Add database index for MaintenanceCandidate queries #40

Recommended Fix: Remove the take: pageSize * 2 limit and fetch all activities, then paginate in memory. For most users with reasonable activity counts (< 1000), this is acceptable. If you need to optimize for very large datasets later, consider database-level UNION queries or cursor-based pagination.

---

⚠️ Minor Issues

1. Type Safety Enhancement
Consider making status/markType union types instead of strings for stricter type safety in types/admin.ts

2. Code Duplication in Badge Functions
getStatusBadge() and getMarkTypeBadge() have repetitive badge JSX. Consider extracting common logic (though Tailwind limitations with dynamic classes make this challenging).

3. Missing Test Coverage
Add a test for pagination beyond page 1 to catch the bug mentioned above.

---

📊 Test Coverage Assessment

Server Action Tests (actions/tests/user-queries.test.ts):

• ✅ Admin access verification
• ✅ Empty state handling
• ✅ Single-source scenarios
• ✅ Sorting verification
• ✅ Transformation accuracy
• ✅ Pagination metadata calculation
• ✅ Error handling
• ⚠️ Missing: Pagination beyond page 1

Component Tests (components/tests/user-activity-timeline.test.tsx):

• ✅ All badge variants (4 command statuses, 6 mark types)
• ✅ Media title formatting
• ✅ Command arguments display
• ✅ Empty/loading states
• ✅ Pagination controls

---

🔒 Security Review

• ✅ Admin authorization enforced via requireAdmin()
• ✅ No SQL injection risk (using Prisma)
• ✅ No XSS vulnerabilities (React escapes output)
• ✅ No user input validation needed (userId from URL)
• ✅ No sensitive data exposure

---

🚀 Performance Considerations

Current State: Fetching pageSize * 2 from each source is memory-efficient for small datasets but breaks pagination for page 2+

Scaling Recommendations:

1. Short-term: Fetch all activities (acceptable if most users have < 1000 activities)
2. Long-term: If this becomes a bottleneck, consider database view with pre-merged timeline or cursor-based pagination

---

📋 Summary

Category	Rating	Notes
Code Quality	⭐⭐⭐⭐⭐	Excellent structure, type safety, conventions
Test Coverage	⭐⭐⭐⭐½	Outstanding (35 tests), missing multi-page test
Performance	⭐⭐⭐	Pagination bug for page 2+
Security	⭐⭐⭐⭐⭐	Proper admin checks, no vulnerabilities
UX	⭐⭐⭐⭐⭐	Clean, accessible, consistent

Recommendation: ✅ Approve with required fix for pagination bug

---

🔧 Action Items

Before Merge (Required):

1. Fix pagination logic to handle page 2+ correctly
2. Add test case for multi-page pagination

Future Enhancements (Optional):

• Consider stricter typing for status/markType enums
• Refactor badge component duplication
• Add performance monitoring if datasets grow large

---

Great work on this feature! The code quality and test coverage are exemplary. The pagination bug is the only blocker preventing merge.

🤖 Generated with Claude Code