playchoo-manager-nextjs/REFACTORING_EXAMPLE.md

Refactoring Example: Before & After

Current Implementation (Problematic)

// page.tsx - 840+ lines, multiple concerns mixed
export default function BookingDetailPage() {
  // 14+ useState declarations
  const [bookingData, setBookingData] = useState<BookingResponseOk | null>(null);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState('');
  const [fatalError, setFatalError] = useState('');
  const [success, setSuccess] = useState('');
  const [actionLoading, setActionLoading] = useState(false);
  const [showPicker, setShowPicker] = useState(false);
  const [joinPosition, setJoinPosition] = useState<number | null>(null);
  const [showLoginModal, setShowLoginModal] = useState(false);
  const [localJoinPositions, setLocalJoinPositions] = useState<number[]>([]);
  const [forceSwap, setForceSwap] = useState(false);
  const [recordedSwaps, setRecordedSwaps] = useState<[number, number][]>([]);
  const [isProcessingSwap, setIsProcessingSwap] = useState(false);
  // ... more states

  // Complex inline fetch logic
  const fetchBooking = React.useCallback(() => {
    setLoading(true);
    setFatalError('');
    apiFetch(`/booking/${slot_id}`)
      .then(async (res) => {
        // 30+ lines of response handling
      })
      .catch(() => setFatalError(t('Network error')))
      .finally(() => setLoading(false));
  }, [slot_id]);

  // Inline action handlers in JSX
  return (
    <SwapGroupsPanel
      onApprove={async (approvalId: number) => {
        setActionLoading(true);
        try {
          const response = await apiFetch(`/booking/${slot_id}/swap-approval/${approvalId}`, {
            method: 'PATCH',
          });
          if (response.ok) {
            setSuccess(t('Approved!'));
            fetchBooking();
          } else {
            const data = await response.json();
            setError(data.message || t('Failed'));
          }
        } catch {
          setError(t('Network error'));
        } finally {
          setActionLoading(false);
        }
      }}
    />
  );
}

Problems:

• ❌ Can't test logic without rendering component
• ❌ State can be inconsistent (loading=false but still fetching)
• ❌ Hard to add features (retry, optimistic updates, undo)
• ❌ Error handling duplicated everywhere
• ❌ No type safety for errors

---

Improved Implementation

1. Create Types

// types/booking.ts
export type Result<T, E = BookingError> =
  | { status: 'idle' }
  | { status: 'loading' }
  | { status: 'success'; data: T }
  | { status: 'error'; error: E };

// types/errors.ts
export class BookingError extends Error {
  constructor(
    message: string,
    public readonly code: string,
    public readonly severity: 'fatal' | 'recoverable',
    public readonly retryable: boolean = false
  ) {
    super(message);
    this.name = 'BookingError';
  }

  static networkError(message: string): BookingError {
    return new BookingError(message, 'NETWORK_ERROR', 'recoverable', true);
  }

  static notFound(slotId: string): BookingError {
    return new BookingError(`Booking ${slotId} not found`, 'NOT_FOUND', 'fatal', false);
  }

  static serverError(message: string): BookingError {
    return new BookingError(message, 'SERVER_ERROR', 'recoverable', true);
  }
}

2. Create Service Layer

// services/BookingService.ts
export class BookingService {
  constructor(private readonly apiClient: typeof apiFetch) {}

  async getBooking(slotId: string): Promise<BookingData> {
    try {
      const response = await this.apiClient(`/booking/${slotId}`);

      if (!response.ok) {
        if (response.status === 404) {
          throw BookingError.notFound(slotId);
        }
        const data = await response.json().catch(() => ({}));
        throw BookingError.serverError(data.message || 'Failed to load booking');
      }

      return response.json();
    } catch (error) {
      if (error instanceof BookingError) throw error;
      throw BookingError.networkError('Network error while loading booking');
    }
  }

  async approveSwap(slotId: string, approvalId: number): Promise<void> {
    try {
      const response = await this.apiClient(
        `/booking/${slotId}/swap-approval/${approvalId}`,
        { method: 'PATCH' }
      );

      if (!response.ok) {
        const data = await response.json().catch(() => ({}));
        throw BookingError.serverError(data.message || 'Failed to approve swap');
      }
    } catch (error) {
      if (error instanceof BookingError) throw error;
      throw BookingError.networkError('Network error while approving swap');
    }
  }

  async cancelSwap(slotId: string, swapGroupId: string): Promise<void> {
    try {
      const response = await this.apiClient(
        `/booking/${slotId}/swap-group/${swapGroupId}`,
        { method: 'DELETE' }
      );

      if (!response.ok) {
        const data = await response.json().catch(() => ({}));
        throw BookingError.serverError(data.message || 'Failed to cancel swap');
      }
    } catch (error) {
      if (error instanceof BookingError) throw error;
      throw BookingError.networkError('Network error while cancelling swap');
    }
  }
}

3. Create Custom Hooks

// hooks/useBookingData.ts
export function useBookingData(slotId: string, service: BookingService) {
  const [result, setResult] = useState<Result<BookingData>>({ status: 'loading' });

  const fetch = useCallback(async () => {
    setResult({ status: 'loading' });

    try {
      const data = await service.getBooking(slotId);
      setResult({ status: 'success', data });
    } catch (error) {
      setResult({
        status: 'error',
        error: error instanceof BookingError ? error : BookingError.networkError('Unknown error')
      });
    }
  }, [slotId, service]);

  useEffect(() => {
    fetch();
  }, [fetch]);

  return { result, refetch: fetch };
}

// hooks/useSwapActions.ts
export function useSwapActions(
  slotId: string,
  service: BookingService,
  onSuccess: () => void
) {
  const [result, setResult] = useState<Result<void>>({ status: 'idle' });
  const { t } = useTranslation();

  const approveSwap = useCallback(async (approvalId: number) => {
    setResult({ status: 'loading' });

    try {
      await service.approveSwap(slotId, approvalId);
      setResult({ status: 'success', data: undefined });
      onSuccess();
    } catch (error) {
      setResult({
        status: 'error',
        error: error instanceof BookingError ? error : BookingError.networkError(t('Unknown error'))
      });
    }
  }, [slotId, service, onSuccess, t]);

  const cancelSwap = useCallback(async (swapGroupId: string) => {
    setResult({ status: 'loading' });

    try {
      await service.cancelSwap(slotId, swapGroupId);
      setResult({ status: 'success', data: undefined });
      onSuccess();
    } catch (error) {
      setResult({
        status: 'error',
        error: error instanceof BookingError ? error : BookingError.networkError(t('Unknown error'))
      });
    }
  }, [slotId, service, onSuccess, t]);

  const clearError = useCallback(() => {
    setResult({ status: 'idle' });
  }, []);

  return { result, approveSwap, cancelSwap, clearError };
}

// hooks/useUIState.ts
type UIState = {
  modals: {
    picker: boolean;
    login: boolean;
    matchType: boolean;
    punctuality: boolean;
  };
};

type UIAction =
  | { type: 'OPEN_MODAL'; modal: keyof UIState['modals'] }
  | { type: 'CLOSE_MODAL'; modal: keyof UIState['modals'] }
  | { type: 'CLOSE_ALL_MODALS' };

function uiReducer(state: UIState, action: UIAction): UIState {
  switch (action.type) {
    case 'OPEN_MODAL':
      return { ...state, modals: { ...state.modals, [action.modal]: true } };
    case 'CLOSE_MODAL':
      return { ...state, modals: { ...state.modals, [action.modal]: false } };
    case 'CLOSE_ALL_MODALS':
      return { ...state, modals: { picker: false, login: false, matchType: false, punctuality: false } };
    default:
      return state;
  }
}

export function useUIState() {
  return useReducer(uiReducer, {
    modals: { picker: false, login: false, matchType: false, punctuality: false }
  });
}

4. Refactored Component

// page.tsx - Now clean and focused
export default function BookingDetailPage() {
  const { slot_id } = useEnhancedParams();
  const { t } = useTranslation();

  // Services
  const bookingService = useMemo(() => new BookingService(apiFetch), []);

  // Data fetching
  const { result: bookingResult, refetch } = useBookingData(slot_id, bookingService);
  const { result: actionResult, approveSwap, cancelSwap, clearError } = useSwapActions(
    slot_id,
    bookingService,
    refetch
  );

  // UI state
  const [uiState, dispatch] = useUIState();

  // Extract data safely
  const booking = bookingResult.status === 'success' ? bookingResult.data : null;
  const fatalError = bookingResult.status === 'error' ? bookingResult.error : null;
  const actionError = actionResult.status === 'error' ? actionResult.error : null;
  const isLoading = bookingResult.status === 'loading';

  // Loading state
  if (isLoading) {
    return (
      <div className="min-h-screen bg-gray-100">
        <div className="container mx-auto px-4 py-8">
          <BookingDetailSkeleton />
        </div>
      </div>
    );
  }

  // Fatal error state
  if (fatalError) {
    return (
      <div className="min-h-screen bg-gray-100">
        <div className="container mx-auto px-4 py-8">
          <div className="max-w-4xl mx-auto">
            <Card>
              <div className="py-12">
                <ErrorMessage message={fatalError.message} />
                {fatalError.retryable && (
                  <button onClick={refetch} className="mt-4">
                    {t('Retry')}
                  </button>
                )}
              </div>
            </Card>
          </div>
        </div>
      </div>
    );
  }

  // Not found
  if (!booking) return <NotFound />;

  // Main render
  return (
    <>
      <div className="min-h-screen bg-gray-100">
        <div className="container mx-auto px-4 py-8">
          <BookingCourtCard booking={booking} />

          {booking.pending_position_swap_groups?.length > 0 && (
            <SwapGroupsPanel
              swapGroups={booking.pending_position_swap_groups}
              players={booking.players}
              currentRemoteMemberId={getCurrentUser(booking)?.remote_member_id}
              onApprove={approveSwap}
              onCancel={cancelSwap}
            />
          )}
        </div>
      </div>

      {/* Error Modal */}
      <ErrorModal
        isOpen={!!actionError}
        onClose={clearError}
        message={actionError?.message || ''}
        title={actionError?.code === 'NETWORK_ERROR' ? t('Network Error') : t('Error')}
      />
    </>
  );
}

---

Benefits Summary

Before (Current)

• ❌ 840+ lines in one file
• ❌ 14+ useState hooks
• ❌ Mixed concerns
• ❌ Hard to test
• ❌ String-based errors
• ❌ Duplicate error handling
• ❌ No retry logic

After (Improved)

• ✅ ~150 lines in component
• ✅ 3 main state hooks (Result types)
• ✅ Clear separation of concerns
• ✅ Fully testable
• ✅ Typed errors with metadata
• ✅ Centralized error handling
• ✅ Easy to add retry/undo

Test Examples

// Before: Can't test without rendering
// Need to mock entire React component tree

// After: Can test in isolation
describe('BookingService', () => {
  it('should throw NotFoundError for 404', async () => {
    const mockApi = jest.fn().mockResolvedValue({ ok: false, status: 404 });
    const service = new BookingService(mockApi);

    await expect(service.getBooking('123'))
      .rejects
      .toThrow(BookingError);
  });
});

describe('useSwapActions', () => {
  it('should set error state on failure', async () => {
    const { result } = renderHook(() =>
      useSwapActions('123', mockService, jest.fn())
    );

    await act(() => result.current.approveSwap(456));

    expect(result.current.result.status).toBe('error');
  });
});

Adding New Features

// Before: Need to modify massive component, add more state

// After: Just extend the hook
export function useSwapActions(/* ... */) {
  // ... existing code

  // Add retry logic
  const retryLast = useCallback(() => {
    if (lastAction) {
      lastAction();
    }
  }, [lastAction]);

  // Add optimistic updates
  const optimisticApprove = useCallback((approvalId: number) => {
    // Update UI immediately
    updateLocalState(approvalId);

    // Then sync with server
    approveSwap(approvalId).catch(() => {
      // Rollback on error
      revertLocalState(approvalId);
    });
  }, [approveSwap]);

  return { /* ... */, retryLast, optimisticApprove };
}

---

Migration Path

1. Week 1: Create types and service layer (no breaking changes)
2. Week 2: Create custom hooks (parallel to existing code)
3. Week 3: Update component to use new hooks
4. Week 4: Remove old code, add tests

Risk: Low - Each step is backwards compatible Effort: ~4 days of development Value: High - Much easier to maintain and extend