diff --git a/REFACTOR_PLAN.md b/REFACTOR_PLAN.md deleted file mode 100644 index 76cb3ba..0000000 --- a/REFACTOR_PLAN.md +++ /dev/null @@ -1,244 +0,0 @@ -# Refactoring Plan: Extract Business Logic from Routes - -## Goal -Remove all business/domain logic from route handlers. Routes should only: -1. Receive HTTP requests -2. Call service methods -3. Map responses using mappers -4. Return HTTP responses - -## Current State Analysis - -### Routes with Business Logic - -#### 1. `routes/auth.py` -**Business Logic:** -- `register()`: Invite validation, user creation, invite marking, role assignment -- `get_default_role()`: Database query (should use repository) - -**Action:** Create `AuthService` with: -- `register_user()` - handles entire registration flow -- `login_user()` - handles authentication and token creation - -#### 2. `routes/invites.py` -**Business Logic:** -- `check_invite()`: Invite validation logic -- `get_my_invites()`: Database query + response building -- `create_invite()`: Invite creation with collision retry logic -- `list_all_invites()`: Query building, filtering, pagination -- `revoke_invite()`: Revocation business logic - -**Action:** Use existing `InviteService` (already exists but not fully used): -- Move `check_invite()` logic to `InviteService.check_invite_validity()` -- Move `create_invite()` logic to `InviteService.create_invite()` -- Move `revoke_invite()` logic to `InviteService.revoke_invite()` -- Add `InviteService.get_user_invites()` for `get_my_invites()` -- Add `InviteService.list_invites()` for `list_all_invites()` - -#### 3. `routes/profile.py` -**Business Logic:** -- `get_godfather_email()`: Database query (should use repository) -- `get_profile()`: Data retrieval and response building -- `update_profile()`: Validation and field updates - -**Action:** Create `ProfileService` with: -- `get_profile()` - retrieves profile with godfather email -- `update_profile()` - validates and updates profile fields - -#### 4. `routes/availability.py` -**Business Logic:** -- `get_availability()`: Query, grouping by date, transformation -- `set_availability()`: Slot overlap validation, time ordering validation, deletion, creation -- `copy_availability()`: Source validation, copying logic, atomic transaction handling - -**Action:** Create `AvailabilityService` with: -- `get_availability_for_range()` - gets and groups availability -- `set_availability_for_date()` - validates slots and replaces availability -- `copy_availability()` - copies availability from one date to others - -#### 5. `routes/audit.py` -**Business Logic:** -- `get_price_history()`: Database query -- `fetch_price_now()`: Price fetching, duplicate timestamp handling -- `_to_price_history_response()`: Mapping (should use mapper) - -**Action:** Create `PriceService` with: -- `get_recent_prices()` - gets recent price history -- `fetch_and_store_price()` - fetches from Bitfinex and stores (handles duplicates) -- Move `_to_price_history_response()` to `PriceHistoryMapper` - -#### 6. `routes/exchange.py` -**Business Logic:** -- `get_available_slots()`: Query, slot expansion logic -- Enum validation (acceptable - this is input validation at route level) - -**Action:** -- Move slot expansion logic to `ExchangeService` or `AvailabilityService` -- Keep enum validation in route (it's input validation, not business logic) - -## Implementation Plan - -### Phase 1: Create Missing Services -1. ✅ `ExchangeService` (already exists) -2. ✅ `InviteService` (already exists, needs expansion) -3. ❌ `AuthService` (needs creation) -4. ❌ `ProfileService` (needs creation) -5. ❌ `AvailabilityService` (needs creation) -6. ❌ `PriceService` (needs creation) - -### Phase 2: Expand Existing Services -1. Expand `InviteService`: - - Add `get_user_invites()` - - Add `list_invites()` with pagination - - Ensure all methods use repositories - -### Phase 3: Update Routes to Use Services -1. `routes/auth.py` → Use `AuthService` -2. `routes/invites.py` → Use `InviteService` consistently -3. `routes/profile.py` → Use `ProfileService` -4. `routes/availability.py` → Use `AvailabilityService` -5. `routes/audit.py` → Use `PriceService` -6. `routes/exchange.py` → Move slot expansion to service - -### Phase 4: Clean Up -1. Remove all direct database queries from routes -2. Remove all business logic from routes -3. Replace all `HTTPException` with custom exceptions -4. Ensure all mappers are used consistently -5. Remove helper functions from routes (move to services/repositories) - -## File Structure After Refactoring - -``` -backend/ -├── routes/ -│ ├── auth.py # Only HTTP handling, calls AuthService -│ ├── invites.py # Only HTTP handling, calls InviteService -│ ├── profile.py # Only HTTP handling, calls ProfileService -│ ├── availability.py # Only HTTP handling, calls AvailabilityService -│ ├── audit.py # Only HTTP handling, calls PriceService -│ └── exchange.py # Only HTTP handling, calls ExchangeService -├── services/ -│ ├── __init__.py -│ ├── auth.py # NEW: Registration, login logic -│ ├── invite.py # EXISTS: Expand with missing methods -│ ├── profile.py # NEW: Profile CRUD operations -│ ├── availability.py # NEW: Availability management -│ ├── price.py # NEW: Price fetching and history -│ └── exchange.py # EXISTS: Already good, minor additions -├── repositories/ -│ └── ... (already good) -└── mappers/ - └── ... (add PriceHistoryMapper) -``` - -## Detailed Service Specifications - -### AuthService -```python -class AuthService: - async def register_user( - self, - email: str, - password: str, - invite_identifier: str - ) -> tuple[User, str]: # Returns (user, token) - """Register new user with invite validation.""" - - async def login_user( - self, - email: str, - password: str - ) -> tuple[User, str]: # Returns (user, token) - """Authenticate user and create token.""" -``` - -### ProfileService -```python -class ProfileService: - async def get_profile(self, user: User) -> ProfileResponse: - """Get user profile with godfather email.""" - - async def update_profile( - self, - user: User, - data: ProfileUpdate - ) -> ProfileResponse: - """Validate and update profile fields.""" -``` - -### AvailabilityService -```python -class AvailabilityService: - async def get_availability_for_range( - self, - from_date: date, - to_date: date - ) -> AvailabilityResponse: - """Get availability grouped by date.""" - - async def set_availability_for_date( - self, - target_date: date, - slots: list[TimeSlot] - ) -> AvailabilityDay: - """Validate and set availability for a date.""" - - async def copy_availability( - self, - source_date: date, - target_dates: list[date] - ) -> AvailabilityResponse: - """Copy availability from source to target dates.""" -``` - -### PriceService -```python -class PriceService: - async def get_recent_prices(self, limit: int = 20) -> list[PriceHistory]: - """Get recent price history.""" - - async def fetch_and_store_price(self) -> PriceHistory: - """Fetch price from Bitfinex and store (handles duplicates).""" -``` - -### InviteService (Expansion) -```python -class InviteService: - # Existing methods... - - async def get_user_invites(self, user_id: int) -> list[Invite]: - """Get all invites for a user.""" - - async def list_invites( - self, - page: int, - per_page: int, - status_filter: str | None = None, - godfather_id: int | None = None - ) -> PaginatedInviteRecords: - """List invites with pagination and filtering.""" -``` - -## Testing Strategy -1. Ensure all existing tests pass after each service creation -2. Add service-level unit tests -3. Keep route tests focused on HTTP concerns (status codes, response formats) -4. Move business logic tests to service tests - -## Migration Order -1. **PriceService** (simplest, least dependencies) -2. **AvailabilityService** (self-contained) -3. **ProfileService** (simple CRUD) -4. **AuthService** (more complex, but isolated) -5. **InviteService expansion** (already exists, just expand) -6. **ExchangeService** (slot expansion logic) - -## Success Criteria -- ✅ No `await db.execute()` calls in routes -- ✅ No business validation logic in routes -- ✅ No data transformation logic in routes -- ✅ All routes are thin wrappers around service calls -- ✅ All tests pass -- ✅ Code is more testable and maintainable - diff --git a/frontend/app/admin/invites/page.tsx b/frontend/app/admin/invites/page.tsx index abe1912..751e3a6 100644 --- a/frontend/app/admin/invites/page.tsx +++ b/frontend/app/admin/invites/page.tsx @@ -4,6 +4,7 @@ import { useEffect, useState, useCallback } from "react"; import { Permission } from "../../auth-context"; import { adminApi } from "../../api"; import { Header } from "../../components/Header"; +import { StatusBadge } from "../../components/StatusBadge"; import { useRequireAuth } from "../../hooks/useRequireAuth"; import { useMutation } from "../../hooks/useMutation"; import { components } from "../../generated/api"; @@ -15,7 +16,6 @@ import { paginationStyles, formStyles, buttonStyles, - badgeStyles, utilityStyles, } from "../../styles/shared"; @@ -119,16 +119,16 @@ export default function AdminInvitesPage() { return new Date(dateStr).toLocaleString(); }; - const getStatusBadgeStyle = (status: string) => { + const getStatusBadgeVariant = (status: string): "ready" | "success" | "error" | undefined => { switch (status) { case READY: - return badgeStyles.badgeReady; + return "ready"; case SPENT: - return badgeStyles.badgeSuccess; + return "success"; case REVOKED: - return badgeStyles.badgeError; + return "error"; default: - return {}; + return undefined; } }; @@ -237,11 +237,9 @@ export default function AdminInvitesPage() { {record.identifier} {record.godfather_email} - + {record.status} - + {record.used_by_email || "-"} {formatDate(record.created_at)} diff --git a/frontend/app/admin/trades/page.tsx b/frontend/app/admin/trades/page.tsx index 4fb9372..bfc19b7 100644 --- a/frontend/app/admin/trades/page.tsx +++ b/frontend/app/admin/trades/page.tsx @@ -5,18 +5,13 @@ import { Permission } from "../../auth-context"; import { adminApi } from "../../api"; import { Header } from "../../components/Header"; import { SatsDisplay } from "../../components/SatsDisplay"; +import { EmptyState } from "../../components/EmptyState"; +import { ConfirmationButton } from "../../components/ConfirmationButton"; import { useRequireAuth } from "../../hooks/useRequireAuth"; import { components } from "../../generated/api"; import { formatDateTime } from "../../utils/date"; -import { formatEur, getTradeStatusDisplay } from "../../utils/exchange"; -import { - layoutStyles, - typographyStyles, - bannerStyles, - badgeStyles, - buttonStyles, - tradeCardStyles, -} from "../../styles/shared"; +import { formatEur } from "../../utils/exchange"; +import { layoutStyles, typographyStyles, bannerStyles, tradeCardStyles } from "../../styles/shared"; type AdminExchangeResponse = components["schemas"]["AdminExchangeResponse"]; @@ -197,15 +192,14 @@ export default function AdminTradesPage() { )} {isLoadingTrades ? ( -
Loading trades...
+ ) : trades.length === 0 ? ( -
- {activeTab === "upcoming" ? "No upcoming trades." : "No trades found."} -
+ ) : (
{trades.map((trade) => { - const status = getTradeStatusDisplay(trade.status); const isBuy = trade.direction === "buy"; const isPast = new Date(trade.slot_start) <= new Date(); const canComplete = trade.status === "booked" && isPast && activeTab === "past"; @@ -281,83 +275,72 @@ export default function AdminTradesPage() {
- - {status.text} - + {/* Actions */}
- {confirmAction?.id === trade.public_id ? ( + {canComplete && ( <> - - - - ) : ( - <> - {canComplete && ( - <> - - - - )} - {trade.status === "booked" && ( - - )} + onConfirm={() => handleAction(trade.public_id, "complete")} + onCancel={() => setConfirmAction(null)} + onActionClick={() => + setConfirmAction({ + id: trade.public_id, + type: "complete", + }) + } + actionLabel="Complete" + isLoading={actioningIds.has(trade.public_id)} + confirmVariant="success" + confirmButtonStyle={styles.successButton} + actionButtonStyle={styles.successButton} + /> + handleAction(trade.public_id, "no_show")} + onCancel={() => setConfirmAction(null)} + onActionClick={() => + setConfirmAction({ + id: trade.public_id, + type: "no_show", + }) + } + actionLabel="No Show" + isLoading={actioningIds.has(trade.public_id)} + confirmVariant="primary" + actionButtonStyle={styles.warningButton} + /> )} + {trade.status === "booked" && ( + handleAction(trade.public_id, "cancel")} + onCancel={() => setConfirmAction(null)} + onActionClick={() => + setConfirmAction({ + id: trade.public_id, + type: "cancel", + }) + } + actionLabel="Cancel" + isLoading={actioningIds.has(trade.public_id)} + confirmVariant="danger" + confirmButtonStyle={styles.dangerButton} + /> + )}
diff --git a/frontend/app/components/ConfirmationButton.tsx b/frontend/app/components/ConfirmationButton.tsx new file mode 100644 index 0000000..eb75f08 --- /dev/null +++ b/frontend/app/components/ConfirmationButton.tsx @@ -0,0 +1,97 @@ +import { buttonStyles } from "../styles/shared"; + +interface ConfirmationButtonProps { + /** Whether confirmation mode is active */ + isConfirming: boolean; + /** Callback when user confirms the action */ + onConfirm: () => void; + /** Callback when user cancels the confirmation */ + onCancel: () => void; + /** Callback when user clicks the initial action button (to enter confirmation mode) */ + onActionClick: () => void; + /** Label for the initial action button */ + actionLabel: string; + /** Label for the confirm button (default: "Confirm") */ + confirmLabel?: string; + /** Label for the cancel button (default: "No") */ + cancelLabel?: string; + /** Whether the action is in progress (shows "..." on confirm button) */ + isLoading?: boolean; + /** Style variant for the confirm button */ + confirmVariant?: "danger" | "success" | "primary"; + /** Custom style for the action button */ + actionButtonStyle?: React.CSSProperties; + /** Custom style for the confirm button */ + confirmButtonStyle?: React.CSSProperties; +} + +/** + * Confirmation button component that shows a two-step confirmation flow. + * Initially shows an action button. When clicked, shows Confirm/Cancel buttons. + */ +export function ConfirmationButton({ + isConfirming, + onConfirm, + onCancel, + onActionClick, + actionLabel, + confirmLabel = "Confirm", + cancelLabel = "No", + isLoading = false, + confirmVariant = "primary", + actionButtonStyle, + confirmButtonStyle, +}: ConfirmationButtonProps) { + if (isConfirming) { + let confirmStyle: React.CSSProperties = buttonStyles.primaryButton; + if (confirmVariant === "danger") { + confirmStyle = { + ...buttonStyles.primaryButton, + background: "rgba(239, 68, 68, 0.9)", + borderColor: "rgba(239, 68, 68, 1)", + }; + } else if (confirmVariant === "success") { + confirmStyle = { + ...buttonStyles.primaryButton, + background: "rgba(34, 197, 94, 0.9)", + borderColor: "rgba(34, 197, 94, 1)", + }; + } + + return ( + <> + + + + ); + } + + return ( + + ); +} diff --git a/frontend/app/components/EmptyState.tsx b/frontend/app/components/EmptyState.tsx new file mode 100644 index 0000000..44cf5af --- /dev/null +++ b/frontend/app/components/EmptyState.tsx @@ -0,0 +1,50 @@ +import { utilityStyles } from "../styles/shared"; + +interface EmptyStateProps { + /** Message to display when empty */ + message: string; + /** Optional hint/subtitle text */ + hint?: string; + /** Show loading state instead of empty message */ + isLoading?: boolean; + /** Optional action element (e.g., link or button) */ + action?: React.ReactNode; + /** Custom style override */ + style?: React.CSSProperties; +} + +/** + * Standardized empty state component. + * Displays a message when there's no data, or a loading state. + */ +export function EmptyState({ message, hint, isLoading, action, style }: EmptyStateProps) { + if (isLoading) { + return
Loading...
; + } + + return ( +
+

{message}

+ {hint &&

{hint}

} + {action &&
{action}
} +
+ ); +} + +const styles: Record = { + emptyText: { + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "rgba(255, 255, 255, 0.6)", + fontSize: "1rem", + margin: 0, + }, + emptyHint: { + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "rgba(255, 255, 255, 0.4)", + fontSize: "0.85rem", + marginTop: "0.5rem", + }, + action: { + marginTop: "1rem", + }, +}; diff --git a/frontend/app/components/StatusBadge.tsx b/frontend/app/components/StatusBadge.tsx new file mode 100644 index 0000000..4b50d4c --- /dev/null +++ b/frontend/app/components/StatusBadge.tsx @@ -0,0 +1,52 @@ +import { badgeStyles } from "../styles/shared"; +import { getTradeStatusDisplay } from "../utils/exchange"; + +type StatusBadgeVariant = "success" | "error" | "ready"; + +interface StatusBadgeProps { + /** Status text to display */ + children: React.ReactNode; + /** Optional variant for simple status badges */ + variant?: StatusBadgeVariant; + /** Trade status (uses getTradeStatusDisplay for styling) */ + tradeStatus?: string; + /** Custom style override */ + style?: React.CSSProperties; +} + +/** + * Standardized status badge component. + * Can be used with a variant prop for simple badges, or tradeStatus prop for trade-specific styling. + */ +export function StatusBadge({ children, variant, tradeStatus, style }: StatusBadgeProps) { + let badgeStyle: React.CSSProperties = { ...badgeStyles.badge }; + + if (tradeStatus) { + // Use trade status display utility for trade-specific badges + const statusDisplay = getTradeStatusDisplay(tradeStatus); + badgeStyle = { + ...badgeStyle, + background: statusDisplay.bgColor, + color: statusDisplay.textColor, + }; + } else if (variant) { + // Use variant styles for simple badges + switch (variant) { + case "success": + badgeStyle = { ...badgeStyle, ...badgeStyles.badgeSuccess }; + break; + case "error": + badgeStyle = { ...badgeStyle, ...badgeStyles.badgeError }; + break; + case "ready": + badgeStyle = { ...badgeStyle, ...badgeStyles.badgeReady }; + break; + } + } + + return ( + + {tradeStatus ? getTradeStatusDisplay(tradeStatus).text : children} + + ); +} diff --git a/frontend/app/invites/page.tsx b/frontend/app/invites/page.tsx index 080c187..952fbc3 100644 --- a/frontend/app/invites/page.tsx +++ b/frontend/app/invites/page.tsx @@ -3,12 +3,13 @@ import { useState } from "react"; import { invitesApi } from "../api"; import { PageLayout } from "../components/PageLayout"; +import { StatusBadge } from "../components/StatusBadge"; import { useRequireAuth } from "../hooks/useRequireAuth"; import { useAsyncData } from "../hooks/useAsyncData"; import { components } from "../generated/api"; import constants from "../../../shared/constants.json"; import { Permission } from "../auth-context"; -import { cardStyles, typographyStyles, badgeStyles, buttonStyles } from "../styles/shared"; +import { cardStyles, typographyStyles, buttonStyles } from "../styles/shared"; // Use generated type from OpenAPI schema type Invite = components["schemas"]["UserInviteResponse"]; @@ -67,10 +68,10 @@ export default function InvitesPage() { {invites.length === 0 ? ( -
-

You don't have any invites yet.

-

Contact an admin if you need invite codes to share.

-
+ ) : (
{/* Ready Invites */} @@ -107,9 +108,7 @@ export default function InvitesPage() {
{invite.identifier}
- - Used - + Used by {invite.used_by_email}
@@ -126,9 +125,7 @@ export default function InvitesPage() { {revokedInvites.map((invite) => (
{invite.identifier}
- - Revoked - + Revoked
))}
diff --git a/frontend/app/trades/[id]/page.tsx b/frontend/app/trades/[id]/page.tsx index 78d25db..bd6cef7 100644 --- a/frontend/app/trades/[id]/page.tsx +++ b/frontend/app/trades/[id]/page.tsx @@ -6,15 +6,15 @@ import { Permission } from "../../auth-context"; import { tradesApi } from "../../api"; import { Header } from "../../components/Header"; import { SatsDisplay } from "../../components/SatsDisplay"; +import { StatusBadge } from "../../components/StatusBadge"; import { useRequireAuth } from "../../hooks/useRequireAuth"; import { useAsyncData } from "../../hooks/useAsyncData"; import { formatDateTime } from "../../utils/date"; -import { formatEur, getTradeStatusDisplay } from "../../utils/exchange"; +import { formatEur } from "../../utils/exchange"; import { layoutStyles, typographyStyles, bannerStyles, - badgeStyles, buttonStyles, tradeCardStyles, } from "../../styles/shared"; @@ -78,7 +78,6 @@ export default function TradeDetailPage() { ); } - const status = getTradeStatusDisplay(trade.status); const isBuy = trade.direction === "buy"; return ( @@ -98,15 +97,7 @@ export default function TradeDetailPage() {
Status: - - {status.text} - +
Time: diff --git a/frontend/app/trades/page.tsx b/frontend/app/trades/page.tsx index 6284d15..bd35ba5 100644 --- a/frontend/app/trades/page.tsx +++ b/frontend/app/trades/page.tsx @@ -6,12 +6,14 @@ import { Permission } from "../auth-context"; import { tradesApi } from "../api"; import { PageLayout } from "../components/PageLayout"; import { SatsDisplay } from "../components/SatsDisplay"; +import { StatusBadge } from "../components/StatusBadge"; +import { EmptyState } from "../components/EmptyState"; import { useRequireAuth } from "../hooks/useRequireAuth"; import { useAsyncData } from "../hooks/useAsyncData"; import { useMutation } from "../hooks/useMutation"; import { formatDateTime } from "../utils/date"; -import { formatEur, getTradeStatusDisplay } from "../utils/exchange"; -import { typographyStyles, badgeStyles, buttonStyles, tradeCardStyles } from "../styles/shared"; +import { formatEur } from "../utils/exchange"; +import { typographyStyles, tradeCardStyles } from "../styles/shared"; export default function TradesPage() { const router = useRouter(); @@ -71,14 +73,16 @@ export default function TradesPage() {

View and manage your Bitcoin trades

{isLoadingTrades ? ( -
Loading trades...
+ ) : trades.length === 0 ? ( -
-

You don't have any trades yet.

- - Start trading - -
+ + Start trading + + } + /> ) : ( <> {/* Upcoming Trades */} @@ -87,7 +91,6 @@ export default function TradesPage() {

Upcoming ({upcomingTrades.length})

{upcomingTrades.map((trade) => { - const status = getTradeStatusDisplay(trade.status); const isBuy = trade.direction === "buy"; return (
@@ -137,55 +140,21 @@ export default function TradesPage() { /BTC
- - {status.text} - +
{trade.status === "booked" && ( - <> - {confirmCancelId === trade.public_id ? ( - <> - - - - ) : ( - - )} - + handleCancel(trade.public_id)} + onCancel={() => setConfirmCancelId(null)} + onActionClick={() => setConfirmCancelId(trade.public_id)} + actionLabel="Cancel" + isLoading={cancellingId === trade.public_id} + confirmVariant="danger" + confirmButtonStyle={styles.confirmButton} + /> )}