diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e17d0d..2d400de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,6 @@ jobs: - name: MyPy type check run: mypy app/ --ignore-missing-imports - continue-on-error: true # SQLAlchemy Column[T] vs T false positives until models use Mapped[T] # ────────────────────────────────────────── # Backend: Tests @@ -149,7 +148,7 @@ jobs: run: npm ci - name: Run tests - run: npx vitest run --passWithNoTests + run: npx vitest run # ────────────────────────────────────────── # Frontend: Build diff --git a/AUDIT.md b/AUDIT.md new file mode 100644 index 0000000..20d71e7 --- /dev/null +++ b/AUDIT.md @@ -0,0 +1,381 @@ +# Codebase Audit & Optimization Plan + +**Date**: 2026-02-26 +**Scope**: Full-stack audit — backend (Python/FastAPI), frontend (React/TypeScript), infrastructure (Docker, CI, migrations) + +--- + +## Executive Summary + +The codebase has a solid multi-tenant architecture foundation, but suffers from significant code duplication (both backend and frontend), monolithic files, inconsistent patterns, and missing performance optimizations. Below is the full diagnostic organized by severity, followed by a prioritized remediation plan. + +--- + +## PART 1: DIAGNOSTIC + +--- + +### A. BACKEND (Python / FastAPI) + +#### A1. Code Duplication — Response Builders (HIGH) + +Multiple routers implement near-identical `build_*_response()` helper functions that manually convert ORM objects to dicts: + +| Location | Pattern | +|----------|---------| +| `app/routers/users.py:29-85` | `build_user_response()` — manual dict from User | +| `app/routers/staff.py:35-60` | `build_staff_response()` — same pattern | +| `app/routers/equipment.py:124-135` | `build_equipment_response()` — same pattern | +| `app/routers/vacations.py:26-41` | `build_vacation_response()` — same pattern | +| `app/routers/notes.py:24-38` | `build_note_response()` — same pattern | + +**Fix**: Create a shared response builder utility or use Pydantic `model_dump()` with `response_model` on route decorators consistently. + +#### A2. Duplicated User/Site Relationship Queries (HIGH) + +Repeated across `users.py`, `staff.py`, `vacations.py`: +```python +sites = user.sites if hasattr(user, "sites") and user.sites else [] +sorted_sites = sorted(sites, key=lambda s: s.id) +site_ids = [s.id for s in sorted_sites] +site_names = [s.name for s in sorted_sites] +``` + +**Fix**: Add a helper method to the User model (e.g., `get_sorted_site_info()`). + +#### A3. Staff Name Building (MEDIUM) + +Repeated in `vacations.py:29-30`, `notes.py:26-27`, `staff.py:48-52` — manually concatenating `first_name + last_name`. The User model already has a `full_name` property at `user.py:107-109`. + +**Fix**: Use `user.full_name` consistently everywhere. + +#### A4. N+1 Query Problem (HIGH) + +`app/routers/notes.py:68-79` — iterates over notes and executes a separate query per note to load related staff: +```python +for note in notes: + if note.staff_id: + staff_result = await db.execute(select(User).where(User.id == note.staff_id)) +``` + +**Fix**: Use `selectinload(Note.staff)` on the initial query. + +#### A5. Missing Eager Loading (MEDIUM) + +Several routers lack `selectinload()` for common relationships: +- `equipment.py:145+` — Equipment list doesn't eager-load Site +- `projects.py:241-246` — Project detail doesn't eager-load all needed relationships +- Various site-related queries + +#### A6. Oversized Router Files (HIGH) + +| File | Lines | Problem | +|------|-------|---------| +| `app/routers/mpp_import.py` | 1,172 | Complex import logic, no separation | +| `app/routers/projects.py` | 1,064 | CRUD + tree building + assignments mixed | +| `app/routers/admin.py` | 1,046 | Admin auth + tenant CRUD mixed | +| `app/routers/sites.py` | 712 | Sites + bank holidays + company events | + +**Fix**: Extract into sub-modules (e.g., `projects/crud.py`, `projects/tree.py`, `admin/auth.py`, `admin/tenants.py`). + +#### A7. Inconsistent Error Handling (MEDIUM) + +- `admin.py:47` — complex exception handling with JSON parsing +- `equipment.py:72-80` — inconsistent detail messages +- `notes.py:138` — minimal error details +- `main.py:428-439` — global `Exception` handler catches everything (masks bugs) + +**Fix**: Create a custom exception hierarchy with consistent error response shapes. + +#### A8. Dual Authentication Patterns (MEDIUM) + +- Regular auth: session-based (`middleware/auth.py:18-124`) +- Admin auth: cookie-based with express-session JSON parsing (`admin.py:64-138`) +- Both duplicate session lookup, expiration checking, and user validation + +**Fix**: Unify into a single auth service with configurable scopes. + +#### A9. Inconsistent Datetime Serialization (MEDIUM) + +Two approaches exist: +- `main.py:22-41` — `CustomJSONResponse` with `format_datetime_js()` (no timezone offset) +- `schemas/base.py:82-100` — `serialize_datetime_js()` (applies timezone offset) + +**Fix**: Consolidate to one approach. + +#### A10. No Pagination (MEDIUM) + +All list endpoints (`GET /projects`, `GET /users`, `GET /equipment`) return full result sets with no limit/offset. + +**Fix**: Add standardized pagination (offset/limit or cursor-based) to all list endpoints. + +#### A11. Missing Database Indexes (MEDIUM) + +Common lookup patterns lack indexes: +- `User.email` (auth lookups) +- `tenants.database_name` (connection routing) +- `organization_sso_config.enabled` (login flow) + +#### A12. `print()` Instead of `logging` (LOW) + +15+ `print()` statements in production code (`main.py:110-124`, `tenant_manager.py:68`, `middleware/tenant.py:96`). No structured logging, no log levels. + +**Fix**: Replace with Python `logging` module throughout. + +#### A13. Connection Pool Inconsistency (LOW) + +Three different pool configurations with no documented rationale: +- `database.py:37-49` — `pool_size=20, max_overflow=10` +- `master_db.py:51-54` — `pool_size=10, max_overflow=5` +- `tenant_manager.py:193-200` — `pool_size=5, max_overflow=5` + +--- + +### B. FRONTEND (React / TypeScript) + +#### B1. Assignment Modal Duplication (HIGH) + +- `components/modals/StaffAssignmentModal.tsx` (341 lines) +- `components/modals/EquipmentAssignmentModal.tsx` (244 lines) + +~90% code duplication: same form state, same lookup logic, same lifecycle patterns, same CSS file. + +**Fix**: Refactor into a generic `>` component with prop-driven differences. + +#### B2. Recursive Subphase Search Duplication (MEDIUM) + +The same recursive search logic is duplicated in 3+ files: +- `StaffAssignmentModal.tsx:69-79` +- `EquipmentAssignmentModal.tsx:52-65` +- `SubphaseModal.tsx:18-35` + +**Fix**: Extract to `utils/subphaseUtils.ts` with `findSubphaseById()`, `findPhaseContainingSubphase()`. + +#### B3. Admin Modal Duplication (MEDIUM) + +- `admin/modals/CreateAdminModal.tsx` (151 lines) +- `admin/modals/CreateOrganizationModal.tsx` (156 lines) + +Both share identical form structure, validation, and error handling. + +**Fix**: Extract shared form/modal primitives. + +#### B4. Monolithic State Store (HIGH) + +`stores/appStore.ts` — **1,106 lines** with 50+ state properties and 60+ actions: +- Auth, data, UI, what-if mode, critical path, custom columns, and expansion states all in one store +- Lines 810-858: critical path calculation embedded in store +- Lines 867-900: what-if mode snapshot/replay logic +- Lines 532-577: custom column filtering + +**Fix**: Split into focused stores: `projectStore`, `resourceStore`, `viewStore`, `whatIfStore`, `customColumnStore`. + +#### B5. UIStore Overloaded (MEDIUM) + +`stores/uiStore.ts` — 724 lines with 48 modal type variants (lines 25-48), mixed tooltip/editing/modal state. + +**Fix**: Separate modal state management from transient UI state. + +#### B6. `any` Types in API Layer (HIGH) + +17+ instances of `any` in API endpoints: +- `api/endpoints/auth.ts:11,33,47` +- `api/endpoints/projects.ts:136,147,166+` + +```typescript +function transformUser(apiUser: any): User { ... } +function transformProject(project: any): Project { ... } +``` + +**Fix**: Create proper API response interfaces (e.g., `APIUser`, `APIProject`) and use them in transform functions. + +#### B7. Repetitive API Transform Functions (MEDIUM) + +`api/endpoints/projects.ts:136-333` — `transformProject`, `transformPhase`, `transformSubphase` follow identical mapping patterns with no reusable helpers. + +#### B8. Console Logs in Production (HIGH) + +50+ `console.log` statements across: +- `hooks/useDataLoader.ts:63,75-82,101-102,108,110,113,116,122,134,143` +- `hooks/useWebSocket.ts` — multiple +- `api/client.ts:106,165,188` +- `components/gantt/Timeline/ProjectTimeline.tsx` + +**Fix**: Remove or gate behind `import.meta.env.DEV`. + +#### B9. No Code Splitting for Modals (MEDIUM) + +`components/modals/ModalContainer.tsx` eagerly imports and renders all 22 modals simultaneously. No `React.lazy()`. + +**Fix**: Lazy-load modals on demand. + +#### B10. Zero `React.memo()` Usage (MEDIUM) + +No memoized components despite 199 files. Large list/row components (SubphaseRow, ProjectRow, assignment rows) re-render on any parent state change. + +**Fix**: Memoize row-level components in lists/tables. + +#### B11. Oversized Components (MEDIUM) + +| File | Lines | +|------|-------| +| `gantt/Timeline/ProjectTimeline.tsx` | 1,104 | +| `gantt/ProjectPanel/SubphaseRow.tsx` | 550 | +| `gantt/ProjectPanel/ProjectRow.tsx` | 549 | + +**Fix**: Break into smaller focused components. + +#### B12. Missing Barrel Exports for Modals (LOW) + +`components/modals/` has no `index.ts`. Other directories (`common/`, `hooks/`, `api/`) properly have barrel exports. + +#### B13. Store Selectors Pull Entire Branches (MEDIUM) + +```typescript +// Triggers re-render on ANY store change: +const { staff, projects, currentSite } = useAppStore(); +// Should be: +const staff = useAppStore((s) => s.staff); +``` + +--- + +### C. INFRASTRUCTURE + +#### C1. Default Admin Credentials in Repo (CRITICAL) + +`setup_databases.sql:123-132` contains a hardcoded bcrypt hash for password "admin": +```sql +INSERT INTO admin_users (email, password_hash, name, role, active) +VALUES ('admin@milestone.local', '$2b$12$LQv3c1yqBWVHxkd0LHAkCOY...', ...) +``` + +**Fix**: Remove default admin from SQL; rely on `app/scripts/init_db.py` with interactive setup. + +#### C2. Source Maps in Production Build (MEDIUM) + +`frontend/vite.config.ts:53` — `sourcemap: true` adds 30-50% bundle size and exposes source code. + +**Fix**: `sourcemap: process.env.NODE_ENV === 'development'` + +#### C3. Fragile SQL Migration Parsing (MEDIUM) + +`migrations/run_migration_master.py:57-61` splits SQL on `;` — breaks `DO $$ ... END $$;` blocks (documented gotcha in CLAUDE.md). + +**Fix**: Use proper SQL parsing or single-statement-per-file convention. + +#### C4. CI Allows No Tests to Pass (MEDIUM) + +`.github/workflows/ci.yml:152` — `npx vitest run --passWithNoTests` means CI passes even if all tests are deleted. + +**Fix**: Remove `--passWithNoTests`, ensure tests exist. + +#### C5. MyPy Errors Ignored in CI (MEDIUM) + +`.github/workflows/ci.yml:47` — `continue-on-error: true` for mypy. Real type errors can be merged. + +#### C6. Minimal Test Coverage (HIGH) + +Only 3 test files (`conftest.py`, `test_config.py`, `test_models.py`) for ~71 Python modules. Estimated <5% coverage. No integration tests for multi-tenant routing, admin APIs, SSO flows. + +#### C7. Deploy Script Deletes Before Verifying Build (MEDIUM) + +`deploy-react.sh:21-26` — removes `public/` contents before verifying the new build succeeded. + +**Fix**: Build to temp dir, verify, then atomic swap. + +#### C8. Docker Image Includes Java Unconditionally (LOW) + +`Dockerfile:48` — `default-jre-headless` adds ~150-200MB for MPP import, even when not needed. + +**Fix**: Make Java optional via `ARG BUILD_WITH_JAVA=false`. + +#### C9. Dev Compose Files Run `npm install` Every Startup (LOW) + +`docker-compose.dev.yml:64`, `docker-compose.react-dev.yml:65` — reinstalls node_modules on every container start. + +**Fix**: Use `npm ci` with proper volume caching. + +--- + +## PART 2: PRIORITIZED REMEDIATION PLAN + +### Phase 1 — Critical & Quick Wins (Week 1) + +| # | Task | Impact | Files | +|---|------|--------|-------| +| 1 | Remove hardcoded default admin from `setup_databases.sql` | Security | `setup_databases.sql` | +| 2 | Remove 50+ `console.log` statements from frontend | Performance/Security | `hooks/`, `api/`, `components/` | +| 3 | Fix N+1 query in `notes.py` with `selectinload` | Performance | `app/routers/notes.py` | +| 4 | Add proper API response types, eliminate `any` | Type Safety | `frontend/src/api/endpoints/` | +| 5 | Replace `print()` with `logging` module in backend | Observability | `app/main.py`, `app/services/`, `app/middleware/` | + +### Phase 2 — Code Deduplication (Weeks 2-3) + +| # | Task | Impact | Files | +|---|------|--------|-------| +| 6 | Extract shared response builder utility for backend routers | Maintainability | New `app/services/response_builder.py`, all routers | +| 7 | Merge StaffAssignmentModal + EquipmentAssignmentModal into generic component | Saves ~300 lines | `frontend/src/components/modals/` | +| 8 | Extract recursive subphase search to shared utility | Eliminates 3x duplication | New `frontend/src/utils/subphaseUtils.ts` | +| 9 | Use `user.full_name` property consistently | Consistency | `vacations.py`, `notes.py`, `staff.py` | +| 10 | Consolidate datetime serialization to single approach | Consistency | `app/main.py`, `app/schemas/base.py` | + +### Phase 3 — Architecture Refactoring (Weeks 3-5) + +| # | Task | Impact | Files | +|---|------|--------|-------| +| 11 | Split `appStore.ts` (1,106 lines) into focused stores | Testability, Performance | `frontend/src/stores/` | +| 12 | Split large routers (`projects.py`, `admin.py`, `mpp_import.py`) into sub-modules | Maintainability | `app/routers/` | +| 13 | Unify authentication logic (regular + admin) | Security, Maintainability | `app/middleware/auth.py`, `app/routers/admin.py` | +| 14 | Add pagination to all list endpoints | Performance | All routers with GET list endpoints | +| 15 | Create custom exception hierarchy with consistent error responses | Reliability | New `app/exceptions.py`, all routers | + +### Phase 4 — Performance Optimization (Weeks 5-6) + +| # | Task | Impact | Files | +|---|------|--------|-------| +| 16 | Add `React.memo()` to row-level components | Render Performance | `SubphaseRow`, `ProjectRow`, assignment rows | +| 17 | Lazy-load modals with `React.lazy()` | Bundle Size | `ModalContainer.tsx`, all 22 modals | +| 18 | Add `selectinload()` to remaining eager-load gaps | Query Performance | `equipment.py`, `projects.py`, `sites.py` | +| 19 | Add database indexes for common lookups | Query Performance | New migration file | +| 20 | Disable source maps in production Vite build | Bundle Size | `frontend/vite.config.ts` | +| 21 | Fix store selectors to use atomic selectors | Render Performance | All components using `useAppStore()` | + +### Phase 5 — Testing & CI Hardening (Weeks 6-8) + +| # | Task | Impact | Files | +|---|------|--------|-------| +| 22 | Add integration tests for critical backend flows (auth, multi-tenant, CRUD) | Reliability | `tests/` | +| 23 | Remove `--passWithNoTests` from CI | CI Integrity | `.github/workflows/ci.yml` | +| 24 | Fix MyPy `continue-on-error` in CI | Type Safety | `.github/workflows/ci.yml` | +| 25 | Fix SQL migration parsing for `DO $$` blocks | Correctness | `migrations/run_migration_master.py` | +| 26 | Make deploy script atomic (build, verify, swap) | Reliability | `deploy-react.sh` | + +### Phase 6 — Polish (Ongoing) + +| # | Task | Impact | Files | +|---|------|--------|-------| +| 27 | Split oversized frontend components (`ProjectTimeline` 1,104 lines) | Maintainability | `gantt/Timeline/`, `gantt/ProjectPanel/` | +| 28 | Add barrel exports for modals directory | DX | `components/modals/index.ts` | +| 29 | Standardize connection pool config with documentation | Clarity | `app/database.py`, `app/config.py` | +| 30 | Make Docker Java optional via build arg | Image Size | `Dockerfile` | + +--- + +## Summary Statistics + +| Category | Issues Found | +|----------|-------------| +| Code Duplication (Backend) | 5 major patterns | +| Code Duplication (Frontend) | 4 major patterns | +| Performance Issues | 10+ | +| Security Issues | 2 (1 critical) | +| Type Safety Issues | 17+ `any` types + missing types | +| Architecture Issues | 6 oversized files (>700 lines each) | +| Infrastructure Issues | 9 | +| **Total actionable items** | **30 in remediation plan** | + +**Files most in need of refactoring**: +- Backend: `projects.py` (1,064), `admin.py` (1,046), `mpp_import.py` (1,172) +- Frontend: `appStore.ts` (1,106), `ProjectTimeline.tsx` (1,104), `uiStore.ts` (724) +- Infrastructure: `setup_databases.sql`, `deploy-react.sh`, `ci.yml` diff --git a/app/database.py b/app/database.py index a187785..ac5d163 100644 --- a/app/database.py +++ b/app/database.py @@ -3,6 +3,7 @@ Uses async SQLAlchemy for PostgreSQL connections. """ +import logging from collections.abc import AsyncGenerator from contextlib import asynccontextmanager @@ -17,6 +18,8 @@ from app.config import get_settings +logger = logging.getLogger(__name__) + class Base(DeclarativeBase): """Base class for all SQLAlchemy models.""" @@ -154,7 +157,7 @@ async def init_db() -> None: async with engine.begin() as conn: await conn.run_sync(lambda _: None) # Simple connectivity test - print("Database connection established successfully") + logger.info("Database connection established successfully") async def close_db() -> None: @@ -166,7 +169,7 @@ async def close_db() -> None: _engine = None _async_session_factory = None - print("Database connections closed") + logger.info("Database connections closed") # --------------------------------------------------------- diff --git a/app/exceptions.py b/app/exceptions.py new file mode 100644 index 0000000..d15299b --- /dev/null +++ b/app/exceptions.py @@ -0,0 +1,88 @@ +""" +Custom exception hierarchy for consistent error responses. + +Usage: + from app.exceptions import NotFoundError, ForbiddenError, ConflictError + + raise NotFoundError("User", user_id) + raise ForbiddenError("Admin access required") + raise ConflictError("A user with this email already exists") + raise ValidationError("Invalid slug format") + +These exceptions are caught by the handler registered in main.py and +converted to consistent JSON error responses with the shape: + {"error": "", "detail": ""} +""" + +from fastapi import HTTPException, status + + +class AppError(HTTPException): + """Base application error with a default status code.""" + + status_code: int = status.HTTP_500_INTERNAL_SERVER_ERROR + + def __init__(self, message: str, detail: str | None = None): + super().__init__( + status_code=self.__class__.status_code, + detail=message, + ) + self.extra_detail = detail + + +class NotFoundError(AppError): + """Resource not found (404).""" + + status_code = status.HTTP_404_NOT_FOUND + + def __init__(self, resource: str, resource_id: int | str | None = None): + if resource_id is not None: + message = f"{resource} not found (id={resource_id})" + else: + message = f"{resource} not found" + super().__init__(message) + + +class ForbiddenError(AppError): + """Forbidden access (403).""" + + status_code = status.HTTP_403_FORBIDDEN + + def __init__(self, message: str = "Access denied"): + super().__init__(message) + + +class UnauthorizedError(AppError): + """Unauthorized access (401).""" + + status_code = status.HTTP_401_UNAUTHORIZED + + def __init__(self, message: str = "Authentication required"): + super().__init__(message) + + +class ConflictError(AppError): + """Resource conflict (409).""" + + status_code = status.HTTP_409_CONFLICT + + def __init__(self, message: str): + super().__init__(message) + + +class ValidationError(AppError): + """Validation error (400).""" + + status_code = status.HTTP_400_BAD_REQUEST + + def __init__(self, message: str): + super().__init__(message) + + +class ServiceError(AppError): + """Internal service error (500).""" + + status_code = status.HTTP_500_INTERNAL_SERVER_ERROR + + def __init__(self, message: str = "Internal server error"): + super().__init__(message) diff --git a/app/main.py b/app/main.py index 670345b..ed50b03 100644 --- a/app/main.py +++ b/app/main.py @@ -3,9 +3,10 @@ """ import json +import logging from collections.abc import AsyncGenerator from contextlib import asynccontextmanager -from datetime import UTC, date, datetime +from datetime import date, datetime from pathlib import Path from typing import Any @@ -17,51 +18,22 @@ from app import __version__ from app.config import get_settings from app.database import close_db, init_db +from app.schemas.base import serialize_date_simple, serialize_datetime_js +logger = logging.getLogger(__name__) -def format_datetime_js(dt: datetime) -> str: - """ - Format datetime to match JavaScript's toISOString(). - - Node.js outputs: "2025-12-08T09:01:16.715Z" - """ - if dt is None: - return None - - if dt.tzinfo is None: - # Assume UTC for naive datetimes - formatted = dt.strftime("%Y-%m-%dT%H:%M:%S") - ms = dt.microsecond // 1000 - return f"{formatted}.{ms:03d}Z" - else: - # Convert to UTC and format - utc_dt = dt.astimezone(UTC) - formatted = utc_dt.strftime("%Y-%m-%dT%H:%M:%S") - ms = utc_dt.microsecond // 1000 - return f"{formatted}.{ms:03d}Z" - - -def format_date_js(d: date) -> str: - """ - Format date to match Node.js date serialization. - Node.js stores dates as timestamps at midnight UTC, which when - serialized becomes the previous day at 23:00:00.000Z due to timezone. +def custom_json_serializer(obj: Any) -> Any: + """Custom JSON serializer for non-standard types. - For consistency, we'll output as ISO date string since that's what - the database actually stores, and it's cleaner. + Delegates to the canonical serializers in app.schemas.base to ensure + consistent datetime/date formatting across dict-based responses and + Pydantic model responses. """ - if d is None: - return None - return d.isoformat() - - -def custom_json_serializer(obj: Any) -> Any: - """Custom JSON serializer for non-standard types.""" if isinstance(obj, datetime): - return format_datetime_js(obj) + return serialize_datetime_js(obj) if isinstance(obj, date): - return format_date_js(obj) + return serialize_date_simple(obj) raise TypeError(f"Object of type {type(obj)} is not JSON serializable") @@ -107,21 +79,21 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: """ # Startup settings = get_settings() - print(f"Starting Milestone API v{__version__}") - print(f"Mode: {'Multi-Tenant' if settings.multi_tenant else 'Single-Tenant'}") - print(f"Debug: {settings.debug}") + logger.info("Starting Milestone API v%s", __version__) + logger.info("Mode: %s", "Multi-Tenant" if settings.multi_tenant else "Single-Tenant") + logger.info("Debug: %s", settings.debug) # Log proxy configuration if settings.https_proxy or settings.http_proxy or settings.proxy_pac_url: - print("Proxy Configuration:") + logger.info("Proxy Configuration:") if settings.https_proxy: - print(f" HTTPS_PROXY: {settings.https_proxy}") + logger.info(" HTTPS_PROXY: %s", settings.https_proxy) if settings.http_proxy: - print(f" HTTP_PROXY: {settings.http_proxy}") + logger.info(" HTTP_PROXY: %s", settings.http_proxy) if settings.proxy_pac_url: - print(f" PROXY_PAC_URL: {settings.proxy_pac_url}") + logger.info(" PROXY_PAC_URL: %s", settings.proxy_pac_url) else: - print("Proxy Configuration: None") + logger.info("Proxy Configuration: None") # Initialize database connections if settings.multi_tenant: @@ -132,7 +104,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: await master_db.init_db() await master_db.verify_admin_exists() tenant_connection_manager.start_cleanup_task() - print("Master database initialized") + logger.info("Master database initialized") else: # Single-tenant: connect to the default tenant database await init_db() @@ -149,7 +121,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: await tenant_connection_manager.close_all() await master_db.close() - print("Milestone API shutdown complete") + logger.info("Milestone API shutdown complete") def create_app() -> FastAPI: @@ -191,7 +163,7 @@ def create_app() -> FastAPI: CORSMiddleware, allow_origins=allowed_origins, allow_credentials=True, - allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], + allow_methods=["GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS"], allow_headers=["Content-Type", "Accept", "Authorization"], ) @@ -377,16 +349,20 @@ async def serve_admin(): return JSONResponse(status_code=404, content={"error": "Admin panel not found"}) # SPA fallback - serve index.html for non-API routes - @app.get("/{full_path:path}") + # Handle all methods to return 404 instead of 405 for unmatched routes + @app.api_route("/{full_path:path}", methods=["GET", "POST", "PUT", "DELETE", "PATCH"]) async def serve_spa(request: Request, full_path: str): """Serve the SPA frontend for non-API routes.""" - # Debug: log all requests hitting this endpoint - print(f"[SPA Fallback] path={full_path}, method={request.method}") + logger.debug("SPA Fallback: path=%s, method=%s", full_path, request.method) # Don't serve index.html for API routes if full_path.startswith("api/"): return JSONResponse(status_code=404, content={"error": "Not found"}) + # Only serve the SPA for GET requests + if request.method != "GET": + return JSONResponse(status_code=404, content={"error": "Not found"}) + # Don't serve index.html for WebSocket endpoint # Handle both /ws and /t/{tenant}/ws patterns if ( @@ -395,7 +371,7 @@ async def serve_spa(request: Request, full_path: str): or full_path.endswith("/ws") or "/ws" in full_path ): - print(f"[SPA Fallback] WebSocket path detected: {full_path}, returning 426") + logger.debug("SPA Fallback: WebSocket path detected: %s, returning 426", full_path) return JSONResponse( status_code=426, # Upgrade Required content={"error": "WebSocket endpoint - use ws:// or wss:// protocol"}, @@ -428,10 +404,7 @@ async def no_frontend(): @app.exception_handler(Exception) async def global_exception_handler(request: Request, exc: Exception): """Handle uncaught exceptions.""" - import traceback - - print(f"Unhandled exception on {request.method} {request.url.path}: {exc}") - traceback.print_exc() + logger.exception("Unhandled exception on %s %s", request.method, request.url.path) # Don't leak internal error details to clients return JSONResponse( status_code=500, @@ -446,17 +419,17 @@ def create_wrapped_app(): app = create_app() settings = get_settings() - print(f"[App Startup] MULTI_TENANT setting = {settings.multi_tenant}") + logger.info("MULTI_TENANT setting = %s", settings.multi_tenant) if settings.multi_tenant: - print("[App Startup] Wrapping app with TenantMiddleware") + logger.info("Wrapping app with TenantMiddleware") from app.middleware.tenant import TenantMiddleware # Wrap the entire app with tenant middleware # This ensures URL rewriting happens BEFORE FastAPI routing return TenantMiddleware(app) - print("[App Startup] Running in single-tenant mode (no middleware)") + logger.info("Running in single-tenant mode (no middleware)") return app diff --git a/app/middleware/tenant.py b/app/middleware/tenant.py index 23ebe1d..33c8477 100644 --- a/app/middleware/tenant.py +++ b/app/middleware/tenant.py @@ -5,6 +5,7 @@ Resolves tenant from URL and attaches tenant context to request state. """ +import logging import re from datetime import datetime from typing import Any @@ -18,6 +19,8 @@ from app.services.master_db import master_db from app.services.tenant_manager import tenant_connection_manager +logger = logging.getLogger(__name__) + # Simple in-memory cache for tenant lookups # Store primitive values, not ORM objects, to avoid detached session issues _tenant_cache: dict[str, tuple[dict[str, Any], datetime]] = {} @@ -93,16 +96,18 @@ class TenantMiddleware: def __init__(self, app: ASGIApp): self.app = app - print("[TenantMiddleware] Initialized - wrapping app") + logger.info("TenantMiddleware initialized - wrapping app") async def __call__(self, scope: Scope, receive: Receive, send: Send): # Debug: log every request type - print(f"[TenantMiddleware] Request: type={scope['type']}, path={scope.get('path', 'N/A')}") + logger.debug( + "TenantMiddleware request: type=%s, path=%s", scope["type"], scope.get("path", "N/A") + ) # SKIP WebSocket connections - they handle tenant resolution themselves # This avoids ASGI scope/state issues with WebSocket connections if scope["type"] == "websocket": - print("[TenantMiddleware] Skipping WebSocket - handler will resolve tenant") + logger.debug("TenantMiddleware skipping WebSocket - handler will resolve tenant") await self.app(scope, receive, send) return @@ -115,7 +120,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send): # Debug logging for WebSocket if scope["type"] == "websocket": - print(f"[TenantMiddleware] WebSocket request: path={path}") + logger.debug("TenantMiddleware WebSocket request: path=%s", path) # Fast path: Skip if not a tenant URL if not path.startswith("/t/"): @@ -131,8 +136,10 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send): # Debug logging if scope["type"] == "websocket": - print( - f"[TenantMiddleware] WebSocket tenant extraction: slug={slug}, remaining={remaining_path}" + logger.debug( + "TenantMiddleware WebSocket tenant extraction: slug=%s, remaining=%s", + slug, + remaining_path, ) if not slug: @@ -193,7 +200,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send): engine = await tenant_connection_manager.get_pool_from_info(tenant_info) state["tenant_engine"] = engine except Exception as e: - print(f"Tenant DB connection error: {e}") + logger.error("Tenant DB connection error: %s", e) # For WebSocket, let handler deal with it if scope["type"] == "websocket": await self.app(scope, receive, send) @@ -218,10 +225,14 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send): # Debug logging for WebSocket if scope["type"] == "websocket": - print(f"[TenantMiddleware] WebSocket path rewritten: {path} -> {remaining_path}") - print(f"[TenantMiddleware] WebSocket new_scope path: {new_scope.get('path')}") - print( - f"[TenantMiddleware] WebSocket state: tenant_slug={state.get('tenant_slug')}, has_engine={state.get('tenant_engine') is not None}" + logger.debug( + "TenantMiddleware WebSocket path rewritten: %s -> %s", path, remaining_path + ) + logger.debug("TenantMiddleware WebSocket new_scope path: %s", new_scope.get("path")) + logger.debug( + "TenantMiddleware WebSocket state: tenant_slug=%s, has_engine=%s", + state.get("tenant_slug"), + state.get("tenant_engine") is not None, ) # Also update root_path if needed for URL generation @@ -230,10 +241,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send): await self.app(new_scope, receive, send) except Exception as e: - print(f"Tenant middleware error: {e}") - import traceback - - traceback.print_exc() + logger.exception("Tenant middleware error: %s", e) # For WebSocket, let it fail naturally if scope["type"] == "websocket": await self.app(scope, receive, send) diff --git a/app/models/note.py b/app/models/note.py index 62d86ef..8c2b415 100644 --- a/app/models/note.py +++ b/app/models/note.py @@ -20,6 +20,7 @@ if TYPE_CHECKING: from app.models.site import Site + from app.models.user import User class Note(Base): @@ -43,8 +44,9 @@ class Note(Base): type: Mapped[str] = mapped_column(String(50), default="general", nullable=False) created_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow, nullable=False) - # Relationships - no back_populates since Site.notes relationship removed + # Relationships site: Mapped["Site"] = relationship("Site") + staff: Mapped["User | None"] = relationship("User", foreign_keys=[staff_id], lazy="noload") def __repr__(self) -> str: return f"" diff --git a/app/routers/admin/__init__.py b/app/routers/admin/__init__.py new file mode 100644 index 0000000..137b8b2 --- /dev/null +++ b/app/routers/admin/__init__.py @@ -0,0 +1,20 @@ +""" +Admin API Router package. + +Splits the admin router into focused sub-modules: +- auth: Admin authentication (login, logout, session management) +- tenants: Tenant CRUD, provisioning, database management +- users: Admin user CRUD (superadmin only) +""" + +from fastapi import APIRouter + +from app.routers.admin.auth import router as auth_router +from app.routers.admin.tenants import router as tenants_router +from app.routers.admin.users import router as users_router + +router = APIRouter(prefix="/admin", tags=["Admin"]) + +router.include_router(auth_router) +router.include_router(tenants_router) +router.include_router(users_router) diff --git a/app/routers/admin/auth.py b/app/routers/admin/auth.py new file mode 100644 index 0000000..10cc60e --- /dev/null +++ b/app/routers/admin/auth.py @@ -0,0 +1,230 @@ +""" +Admin authentication routes and dependencies. + +Provides: +- Login / logout / session endpoints +- get_current_admin / require_superadmin dependencies +""" + +import json +import logging +import time +from datetime import datetime + +from fastapi import APIRouter, Depends, HTTPException, Request, Response, status +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.config import get_settings +from app.models.tenant import AdminSession, AdminUser +from app.schemas.tenant import ( + AdminLoginRequest, + AdminLoginResponse, + AdminMeResponse, + AdminUserInfo, + ChangeAdminPasswordRequest, +) +from app.services.auth import validate_admin_session +from app.services.encryption import generate_password, hash_password, verify_password +from app.services.master_db import get_master_db + +logger = logging.getLogger(__name__) +settings = get_settings() +router = APIRouter() + + +# --------------------------------------------------------- +# Auth Dependencies (used by tenants.py and users.py too) +# --------------------------------------------------------- + + +async def get_admin_session_id(request: Request) -> str | None: + """Extract admin session ID from cookie.""" + cookie = request.cookies.get("admin_session") + if not cookie: + return None + return cookie + + +async def get_current_admin( + request: Request, + db: AsyncSession = Depends(get_master_db), +) -> AdminUser: + """Get current authenticated admin user. + + Uses the shared validate_admin_session service for session lookup/validation. + """ + session_id = await get_admin_session_id(request) + + if not session_id: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Admin authentication required", + ) + + admin = await validate_admin_session(db, session_id) + if not admin: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid or expired session", + ) + + return admin + + +async def get_current_admin_optional( + request: Request, + db: AsyncSession = Depends(get_master_db), +) -> AdminUser | None: + """Get current admin if authenticated, None otherwise.""" + try: + return await get_current_admin(request, db) + except HTTPException: + return None + + +async def require_superadmin( + admin: AdminUser = Depends(get_current_admin), +) -> AdminUser: + """Require superadmin role.""" + if not admin.is_superadmin: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Superadmin access required", + ) + return admin + + +# --------------------------------------------------------- +# Auth Routes +# --------------------------------------------------------- + + +@router.post("/auth/login", response_model=AdminLoginResponse) +async def admin_login( + data: AdminLoginRequest, + response: Response, + db: AsyncSession = Depends(get_master_db), +): + """Admin login.""" + result = await db.execute(select(AdminUser).where(AdminUser.email == data.email)) + admin = result.scalar_one_or_none() + + if not admin or not verify_password(data.password, admin.password_hash): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid credentials", + ) + + if not admin.is_active: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Admin account is disabled", + ) + + session_id = generate_password(64) + expires_ms = int((time.time() + 86400) * 1000) + + sess_data = json.dumps( + { + "admin_user_id": admin.id, + "email": admin.email, + "role": admin.role, + } + ) + + session = AdminSession( + sid=session_id, + sess=sess_data, + expired=expires_ms, + ) + db.add(session) + + admin.last_login = datetime.utcnow() + + await db.commit() + + response.set_cookie( + key="admin_session", + value=session_id, + max_age=86400, + httponly=True, + samesite="lax", + secure=settings.secure_cookies, + path="/", + ) + + return AdminLoginResponse( + success=True, + user=AdminUserInfo( + id=admin.id, + email=admin.email, + name=admin.name, + role=admin.role, + ), + must_change_password=admin.must_change_password == 1, + ) + + +@router.post("/auth/logout") +async def admin_logout( + response: Response, + request: Request, + db: AsyncSession = Depends(get_master_db), +): + """Admin logout.""" + session_id = await get_admin_session_id(request) + + if session_id: + result = await db.execute(select(AdminSession).where(AdminSession.sid == session_id)) + session = result.scalar_one_or_none() + if session: + await db.delete(session) + await db.commit() + + response.delete_cookie(key="admin_session", path="/") + + return {"success": True} + + +@router.get("/auth/me", response_model=AdminMeResponse) +async def admin_me( + admin: AdminUser | None = Depends(get_current_admin_optional), +): + """Get current admin session.""" + if admin: + return AdminMeResponse( + user=AdminUserInfo( + id=admin.id, + email=admin.email, + name=admin.name, + role=admin.role, + ) + ) + return AdminMeResponse(user=None) + + +@router.post("/auth/change-password") +async def change_admin_password( + data: ChangeAdminPasswordRequest, + admin: AdminUser = Depends(get_current_admin), + db: AsyncSession = Depends(get_master_db), +): + """Change current admin user's password.""" + if not verify_password(data.current_password, admin.password_hash): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Current password is incorrect", + ) + + if len(data.new_password) < 8: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="New password must be at least 8 characters", + ) + + admin.password_hash = hash_password(data.new_password) + admin.must_change_password = 0 + await db.commit() + + return {"success": True, "message": "Password changed successfully"} diff --git a/app/routers/admin.py b/app/routers/admin/tenants.py similarity index 58% rename from app/routers/admin.py rename to app/routers/admin/tenants.py index fb31057..2131bcc 100644 --- a/app/routers/admin.py +++ b/app/routers/admin/tenants.py @@ -1,46 +1,29 @@ """ -Admin API Router. - -Handles multi-tenant administration: -- Admin authentication -- Tenant CRUD operations -- Database provisioning -- System statistics -- Admin user management +Tenant management routes. + +Provides CRUD operations for tenants, provisioning, and system stats. """ -import json +import logging import sys import time -from datetime import datetime +import uuid -from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response, status +from fastapi import APIRouter, Depends, HTTPException, Query, Request from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload from app.config import get_settings -from app.models.tenant import AdminSession, AdminUser, Tenant, TenantAuditLog, TenantCredentials +from app.models.tenant import AdminUser, Tenant, TenantAuditLog, TenantCredentials +from app.routers.admin.auth import get_current_admin from app.schemas.tenant import ( - AdminLoginRequest, - AdminLoginResponse, - AdminMeResponse, - AdminUserCreate, - AdminUserInfo, - AdminUserUpdate, - ChangeAdminPasswordRequest, ResetAdminPasswordRequest, TenantCreate, TenantProvisionRequest, TenantUpdate, ) -from app.services.encryption import ( - decrypt, - encrypt, - generate_password, - hash_password, - verify_password, -) +from app.services.encryption import decrypt, encrypt, generate_password from app.services.master_db import get_master_db from app.services.tenant_manager import tenant_connection_manager from app.services.tenant_provisioner import ( @@ -49,280 +32,32 @@ provision_tenant_database, ) -router = APIRouter(prefix="/admin", tags=["Admin"]) +logger = logging.getLogger(__name__) settings = get_settings() +router = APIRouter() # Start time for uptime calculation _start_time = time.time() # --------------------------------------------------------- -# Admin Auth Middleware -# --------------------------------------------------------- - - -async def get_admin_session_id(request: Request) -> str | None: - """Extract admin session ID from cookie.""" - cookie = request.cookies.get("admin_session") - if not cookie: - return None - return cookie - - -async def get_current_admin( - request: Request, - db: AsyncSession = Depends(get_master_db), -) -> AdminUser: - """ - Get current authenticated admin user. - - Uses Node.js admin_sessions format: - - sid: TEXT PRIMARY KEY (session ID) - - sess: TEXT (JSON blob with {admin_user_id, email, ...}) - - expired: BIGINT (Unix timestamp in milliseconds) - """ - session_id = await get_admin_session_id(request) - - if not session_id: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Admin authentication required", - ) - - # Find session (Node.js format) - now_ms = int(time.time() * 1000) # Current time in milliseconds - result = await db.execute( - select(AdminSession) - .where(AdminSession.sid == session_id) - .where(AdminSession.expired > now_ms) - ) - session = result.scalar_one_or_none() - - if not session: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid or expired session", - ) - - # Parse session data from JSON blob - try: - sess_data = json.loads(session.sess) - admin_user_id = sess_data.get("admin_user_id") - if not admin_user_id: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid session data", - ) - except (json.JSONDecodeError, KeyError): - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid session data format", - ) from None - - # Get admin user - result = await db.execute(select(AdminUser).where(AdminUser.id == admin_user_id)) - admin = result.scalar_one_or_none() - - if not admin: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Admin user not found", - ) - - if not admin.is_active: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Admin account is disabled", - ) - - return admin - - -async def get_current_admin_optional( - request: Request, - db: AsyncSession = Depends(get_master_db), -) -> AdminUser | None: - """Get current admin if authenticated, None otherwise.""" - try: - return await get_current_admin(request, db) - except HTTPException: - return None - - -async def require_superadmin( - admin: AdminUser = Depends(get_current_admin), -) -> AdminUser: - """Require superadmin role.""" - if not admin.is_superadmin: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Superadmin access required", - ) - return admin - - -# --------------------------------------------------------- -# Admin Authentication -# --------------------------------------------------------- - - -@router.post("/auth/login", response_model=AdminLoginResponse) -async def admin_login( - data: AdminLoginRequest, - response: Response, - db: AsyncSession = Depends(get_master_db), -): - """Admin login.""" - # Find admin by email - result = await db.execute(select(AdminUser).where(AdminUser.email == data.email)) - admin = result.scalar_one_or_none() - - # Verify credentials - if not admin or not verify_password(data.password, admin.password_hash): - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid credentials", - ) - - if not admin.is_active: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Admin account is disabled", - ) - - # Create session in Node.js format - session_id = generate_password(64) - expires_ms = int((time.time() + 86400) * 1000) # 24 hours from now in milliseconds - - # Session data as JSON blob (Node.js format) - sess_data = json.dumps( - { - "admin_user_id": admin.id, - "email": admin.email, - "role": admin.role, - } - ) - - session = AdminSession( - sid=session_id, - sess=sess_data, - expired=expires_ms, - ) - db.add(session) - - # Update last login - admin.last_login = datetime.utcnow() - - await db.commit() - - # Set cookie - response.set_cookie( - key="admin_session", - value=session_id, - max_age=86400, # 24 hours - httponly=True, - samesite="lax", - secure=settings.secure_cookies, - path="/", - ) - - return AdminLoginResponse( - success=True, - user=AdminUserInfo( - id=admin.id, - email=admin.email, - name=admin.name, - role=admin.role, - ), - must_change_password=admin.must_change_password == 1, - ) - - -@router.post("/auth/logout") -async def admin_logout( - response: Response, - request: Request, - db: AsyncSession = Depends(get_master_db), -): - """Admin logout.""" - session_id = await get_admin_session_id(request) - - if session_id: - result = await db.execute(select(AdminSession).where(AdminSession.sid == session_id)) - session = result.scalar_one_or_none() - if session: - await db.delete(session) - await db.commit() - - response.delete_cookie(key="admin_session", path="/") - - return {"success": True} - - -@router.get("/auth/me", response_model=AdminMeResponse) -async def admin_me( - admin: AdminUser | None = Depends(get_current_admin_optional), -): - """Get current admin session.""" - if admin: - return AdminMeResponse( - user=AdminUserInfo( - id=admin.id, - email=admin.email, - name=admin.name, - role=admin.role, - ) - ) - return AdminMeResponse(user=None) - - -@router.post("/auth/change-password") -async def change_admin_password( - data: ChangeAdminPasswordRequest, - admin: AdminUser = Depends(get_current_admin), - db: AsyncSession = Depends(get_master_db), -): - """Change current admin user's password.""" - # Verify current password - if not verify_password(data.current_password, admin.password_hash): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Current password is incorrect", - ) - - if len(data.new_password) < 8: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="New password must be at least 8 characters", - ) - - admin.password_hash = hash_password(data.new_password) - admin.must_change_password = 0 - await db.commit() - - return {"success": True, "message": "Password changed successfully"} - - -# --------------------------------------------------------- -# Tenant Management +# Helpers # --------------------------------------------------------- async def add_audit_log( db: AsyncSession, - tenant_id, # UUID + tenant_id, action: str, details: dict | None = None, actor: str | None = None, ): """Add an audit log entry.""" - import uuid - log = TenantAuditLog( id=uuid.uuid4(), tenant_id=tenant_id, action=action, - details=details, # JSONB column, pass dict directly + details=details, actor=actor, ) db.add(log) @@ -331,7 +66,7 @@ async def add_audit_log( def tenant_to_response(tenant: Tenant, db_status: dict | None = None) -> dict: """Convert tenant model to response dict.""" return { - "id": str(tenant.id), # UUID to string + "id": str(tenant.id), "name": tenant.name, "slug": tenant.slug, "database_name": tenant.database_name, @@ -345,7 +80,6 @@ def tenant_to_response(tenant: Tenant, db_status: dict | None = None) -> dict: "created_at": tenant.created_at, "updated_at": tenant.updated_at, "database_status": db_status, - # Organization fields "organization_id": str(tenant.organization_id) if tenant.organization_id else None, "organization_name": tenant.organization.name if tenant.organization else None, "required_group_ids": tenant.required_group_ids or [], @@ -353,6 +87,11 @@ def tenant_to_response(tenant: Tenant, db_status: dict | None = None) -> dict: } +# --------------------------------------------------------- +# Tenant CRUD +# --------------------------------------------------------- + + @router.get("/tenants") async def list_tenants( include_archived: bool = False, @@ -360,7 +99,6 @@ async def list_tenants( db: AsyncSession = Depends(get_master_db), ): """List all tenants.""" - query = select(Tenant).options( selectinload(Tenant.credentials), selectinload(Tenant.organization), @@ -374,7 +112,6 @@ async def list_tenants( result = await db.execute(query) tenants = result.scalars().all() - # Get database status for each tenant response = [] for tenant in tenants: db_status = None @@ -419,7 +156,6 @@ async def get_tenant( if not tenant: raise HTTPException(status_code=404, detail="Tenant not found") - # Get database status db_status = None if tenant.credentials: try: @@ -438,7 +174,6 @@ async def get_tenant( response = tenant_to_response(tenant, db_status) - # Add audit log (limited to 20 entries) response["audit_log"] = [ { "id": str(log.id), @@ -461,31 +196,25 @@ async def create_tenant( ): """Create a new tenant.""" import re - import uuid - # Validate slug format if not re.match(r"^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$", data.slug): raise HTTPException( status_code=400, detail="Invalid slug format. Use lowercase letters, numbers, and hyphens only.", ) - # Check reserved slugs reserved = ["admin", "api", "app", "www", "mail", "ftp", "localhost", "test", "demo"] if data.slug in reserved: raise HTTPException(status_code=400, detail="This slug is reserved") - # Check if slug exists existing = await db.execute(select(Tenant).where(Tenant.slug == data.slug)) if existing.scalar_one_or_none(): raise HTTPException(status_code=409, detail="A tenant with this slug already exists") - # Generate database credentials db_name = f"milestone_{data.slug.replace('-', '_')}" db_user = f"milestone_{data.slug.replace('-', '_')}_user" db_password = generate_password(32) - # Create tenant with UUID tenant = Tenant( id=uuid.uuid4(), name=data.name, @@ -500,9 +229,8 @@ async def create_tenant( max_projects=data.max_projects, ) db.add(tenant) - await db.flush() # Get tenant ID + await db.flush() - # Store encrypted credentials try: encrypted_pw = encrypt(db_password) except ValueError as e: @@ -513,7 +241,6 @@ async def create_tenant( ) db.add(credentials) - # Add audit log await add_audit_log( db, tenant.id, @@ -555,11 +282,9 @@ async def update_tenant( if not tenant: raise HTTPException(status_code=404, detail="Tenant not found") - # Update fields update_fields = data.model_dump(exclude_unset=True, by_alias=False) changes = {} - # Validate organization_id if being set if "organization_id" in update_fields and update_fields["organization_id"]: org_result = await db.execute( select(Organization).where(Organization.id == update_fields["organization_id"]) @@ -567,7 +292,6 @@ async def update_tenant( if not org_result.scalar_one_or_none(): raise HTTPException(status_code=404, detail="Organization not found") - # Validate group_membership_mode if "group_membership_mode" in update_fields: if update_fields["group_membership_mode"] not in ("any", "all"): raise HTTPException( @@ -584,7 +308,6 @@ async def update_tenant( await db.commit() - # Refresh with organization relationship result = await db.execute( select(Tenant).where(Tenant.id == tenant_id).options(selectinload(Tenant.organization)) ) @@ -600,11 +323,7 @@ async def update_tenant_status( admin: AdminUser = Depends(get_current_admin), db: AsyncSession = Depends(get_master_db), ): - """ - Update tenant status. - - Valid statuses: active, suspended, archived - """ + """Update tenant status. Valid statuses: active, suspended, archived.""" body = await request.json() new_status = body.get("status") @@ -630,12 +349,11 @@ async def update_tenant_status( actor=admin.email, ) - # If suspending or archiving, close any active connections if new_status != "active": try: await tenant_connection_manager._close_pool(tenant.slug) except Exception: - pass # Ignore errors closing connections + pass await db.commit() await db.refresh(tenant) @@ -659,27 +377,20 @@ async def delete_tenant( if not tenant: raise HTTPException(status_code=404, detail="Tenant not found") - # Check if tenant is suspended (required before deletion) if tenant.status == "active": raise HTTPException( status_code=400, detail="Tenant must be suspended before deletion. Active tenants cannot be deleted.", ) - # Drop database if requested if delete_database and tenant.credentials: try: - print(f"Dropping database for tenant {tenant.slug}: {tenant.database_name}") + logger.info("Dropping database for tenant %s: %s", tenant.slug, tenant.database_name) await drop_tenant_database(tenant.database_name, tenant.database_user) - print(f"Successfully dropped database: {tenant.database_name}") - except Exception as e: - print(f"Error dropping database: {e}") - # Continue with tenant deletion even if database drop fails - import traceback - - traceback.print_exc() + logger.info("Successfully dropped database: %s", tenant.database_name) + except Exception: + logger.exception("Error dropping database for tenant %s", tenant.slug) - # Delete tenant record (cascades to credentials and audit log) await db.delete(tenant) await db.commit() @@ -691,6 +402,11 @@ async def delete_tenant( } +# --------------------------------------------------------- +# Provisioning +# --------------------------------------------------------- + + @router.post("/tenants/{tenant_id}/provision") async def provision_tenant( tenant_id: str, @@ -710,19 +426,17 @@ async def provision_tenant( if not tenant.credentials: raise HTTPException(status_code=400, detail="Tenant credentials not found") - # Get password db_password = decrypt(tenant.credentials.encrypted_password) - - # Admin email and password admin_email = data.admin_email if data and data.admin_email else tenant.admin_email admin_password = data.admin_password if data and data.admin_password else None - print(f"Provision request: data={data}, tenant.admin_email={tenant.admin_email}") - print( - f"Using admin_email={admin_email}, admin_password={'***' if admin_password else 'will be generated'}" + logger.info( + "Provisioning tenant %s: admin_email=%s, password=%s", + tenant.slug, + admin_email, + "provided" if admin_password else "will be generated", ) - # Provision database try: prov_result = await provision_tenant_database( tenant_id=tenant.id, @@ -732,14 +446,12 @@ async def provision_tenant( admin_email=admin_email, admin_password=admin_password, ) - print("Tenant database provisioned successfully") + logger.info("Tenant database provisioned successfully: %s", tenant.slug) except Exception as e: raise HTTPException(status_code=500, detail=f"Provisioning failed: {e}") from e - # Update tenant status tenant.status = "active" - # Add audit log await add_audit_log( db, tenant.id, @@ -777,10 +489,8 @@ async def reset_tenant_admin_password( if not tenant.credentials: raise HTTPException(status_code=400, detail="Tenant credentials not found") - # Decrypt password first tenant_password = decrypt(tenant.credentials.encrypted_password) - # Check database is accessible db_status = await check_tenant_database( tenant.database_name, tenant.database_user, @@ -793,11 +503,9 @@ async def reset_tenant_admin_password( detail="Tenant database is not accessible. Provision it first.", ) - # Generate new password new_password = data.password if data and data.password else generate_password(16) admin_email = data.email if data and data.email else tenant.admin_email - # Connect to tenant database and update password import asyncpg conn = await asyncpg.connect( @@ -809,7 +517,6 @@ async def reset_tenant_admin_password( ) try: - # Update admin user password result = await conn.execute( "UPDATE users SET password = $1 WHERE email = $2 AND role = 'admin'", new_password, @@ -817,7 +524,6 @@ async def reset_tenant_admin_password( ) if result == "UPDATE 0": - # Try any admin row = await conn.fetchrow("SELECT email FROM users WHERE role = 'admin' LIMIT 1") if row: admin_email = row["email"] @@ -834,7 +540,6 @@ async def reset_tenant_admin_password( finally: await conn.close() - # Add audit log await add_audit_log( db, tenant.id, @@ -893,7 +598,6 @@ async def get_system_stats( """Get system statistics.""" import psutil - # Tenant stats result = await db.execute(select(Tenant.status, func.count(Tenant.id)).group_by(Tenant.status)) status_counts = {row[0]: row[1] for row in result.all()} @@ -905,10 +609,8 @@ async def get_system_stats( "archived": status_counts.get("archived", 0), } - # Connection stats pool_stats = tenant_connection_manager.get_stats() - # System stats process = psutil.Process() memory_info = process.memory_info() @@ -924,123 +626,3 @@ async def get_system_stats( "python_version": sys.version, }, } - - -# --------------------------------------------------------- -# Admin User Management (Superadmin only) -# --------------------------------------------------------- - - -@router.get("/users") -async def list_admin_users( - admin: AdminUser = Depends(require_superadmin), - db: AsyncSession = Depends(get_master_db), -): - """List all admin users.""" - result = await db.execute(select(AdminUser).order_by(AdminUser.email)) - users = result.scalars().all() - - return [ - { - "id": u.id, - "email": u.email, - "name": u.name, - "role": u.role, - "active": u.active, - "created_at": u.created_at, - "last_login": u.last_login, - } - for u in users - ] - - -@router.post("/users", status_code=201) -async def create_admin_user( - data: AdminUserCreate, - admin: AdminUser = Depends(require_superadmin), - db: AsyncSession = Depends(get_master_db), -): - """Create an admin user.""" - # Check if email exists - existing = await db.execute(select(AdminUser).where(AdminUser.email == data.email)) - if existing.scalar_one_or_none(): - raise HTTPException(status_code=409, detail="Email already exists") - - user = AdminUser( - email=data.email, - password_hash=hash_password(data.password), - name=data.name, - role=data.role, - active=1, - ) - db.add(user) - await db.commit() - await db.refresh(user) - - return { - "id": user.id, - "email": user.email, - "name": user.name, - "role": user.role, - "active": user.active, - "created_at": user.created_at, - } - - -@router.put("/users/{user_id}") -async def update_admin_user( - user_id: int, - data: AdminUserUpdate, - admin: AdminUser = Depends(require_superadmin), - db: AsyncSession = Depends(get_master_db), -): - """Update an admin user.""" - result = await db.execute(select(AdminUser).where(AdminUser.id == user_id)) - user = result.scalar_one_or_none() - - if not user: - raise HTTPException(status_code=404, detail="User not found") - - if data.name is not None: - user.name = data.name - if data.role is not None: - user.role = data.role - if data.active is not None: - user.active = 1 if data.active else 0 - if data.password is not None: - user.password_hash = hash_password(data.password) - - await db.commit() - await db.refresh(user) - - return { - "id": user.id, - "email": user.email, - "name": user.name, - "role": user.role, - "active": user.active, - "created_at": user.created_at, - "last_login": user.last_login, - } - - -@router.delete("/users/{user_id}") -async def delete_admin_user( - user_id: int, - admin: AdminUser = Depends(require_superadmin), - db: AsyncSession = Depends(get_master_db), -): - """Delete an admin user.""" - if user_id == admin.id: - raise HTTPException(status_code=400, detail="Cannot delete yourself") - - result = await db.execute(select(AdminUser).where(AdminUser.id == user_id)) - user = result.scalar_one_or_none() - - if not user: - raise HTTPException(status_code=404, detail="User not found") - - await db.delete(user) - await db.commit() - - return {"success": True} diff --git a/app/routers/admin/users.py b/app/routers/admin/users.py new file mode 100644 index 0000000..83c91ad --- /dev/null +++ b/app/routers/admin/users.py @@ -0,0 +1,132 @@ +""" +Admin user management routes (superadmin only). +""" + +import logging + +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.tenant import AdminUser +from app.routers.admin.auth import require_superadmin +from app.schemas.tenant import AdminUserCreate, AdminUserUpdate +from app.services.encryption import hash_password +from app.services.master_db import get_master_db + +logger = logging.getLogger(__name__) +router = APIRouter() + + +@router.get("/users") +async def list_admin_users( + admin: AdminUser = Depends(require_superadmin), + db: AsyncSession = Depends(get_master_db), +): + """List all admin users.""" + result = await db.execute(select(AdminUser).order_by(AdminUser.email)) + users = result.scalars().all() + + return [ + { + "id": u.id, + "email": u.email, + "name": u.name, + "role": u.role, + "active": u.active, + "created_at": u.created_at, + "last_login": u.last_login, + } + for u in users + ] + + +@router.post("/users", status_code=201) +async def create_admin_user( + data: AdminUserCreate, + admin: AdminUser = Depends(require_superadmin), + db: AsyncSession = Depends(get_master_db), +): + """Create an admin user.""" + existing = await db.execute(select(AdminUser).where(AdminUser.email == data.email)) + if existing.scalar_one_or_none(): + raise HTTPException(status_code=409, detail="Email already exists") + + user = AdminUser( + email=data.email, + password_hash=hash_password(data.password), + name=data.name, + role=data.role, + active=1, + ) + db.add(user) + await db.commit() + await db.refresh(user) + + return { + "id": user.id, + "email": user.email, + "name": user.name, + "role": user.role, + "active": user.active, + "created_at": user.created_at, + } + + +@router.put("/users/{user_id}") +async def update_admin_user( + user_id: int, + data: AdminUserUpdate, + admin: AdminUser = Depends(require_superadmin), + db: AsyncSession = Depends(get_master_db), +): + """Update an admin user.""" + result = await db.execute(select(AdminUser).where(AdminUser.id == user_id)) + user = result.scalar_one_or_none() + + if not user: + raise HTTPException(status_code=404, detail="User not found") + + if data.name is not None: + user.name = data.name + if data.role is not None: + user.role = data.role + if data.active is not None: + user.active = 1 if data.active else 0 + if data.password is not None: + user.password_hash = hash_password(data.password) + + await db.commit() + await db.refresh(user) + + return { + "id": user.id, + "email": user.email, + "name": user.name, + "role": user.role, + "active": user.active, + "created_at": user.created_at, + "last_login": user.last_login, + } + + +@router.delete("/users/{user_id}") +async def delete_admin_user( + user_id: int, + admin: AdminUser = Depends(require_superadmin), + db: AsyncSession = Depends(get_master_db), +): + """Delete an admin user.""" + if user_id == admin.id: + raise HTTPException(status_code=400, detail="Cannot delete yourself") + + result = await db.execute(select(AdminUser).where(AdminUser.id == user_id)) + user = result.scalar_one_or_none() + + if not user: + raise HTTPException(status_code=404, detail="User not found") + + await db.delete(user) + await db.commit() + + return {"success": True} diff --git a/app/routers/admin_organizations.py b/app/routers/admin_organizations.py index bb31cdf..206b764 100644 --- a/app/routers/admin_organizations.py +++ b/app/routers/admin_organizations.py @@ -16,10 +16,9 @@ from sqlalchemy.orm import selectinload from app.models.organization import Organization, OrganizationSSOConfig -from app.models.tenant import Tenant - -# Import admin auth dependency from admin router -from app.routers.admin import AdminUser, add_audit_log, get_current_admin +from app.models.tenant import AdminUser, Tenant +from app.routers.admin.auth import get_current_admin +from app.routers.admin.tenants import add_audit_log from app.schemas.organization import ( OrganizationCreate, OrganizationDetailResponse, diff --git a/app/routers/auth.py b/app/routers/auth.py index b745912..241f5b1 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -5,6 +5,8 @@ Matches the Node.js API at /api/auth exactly. """ +import logging + import httpx from fastapi import APIRouter, Depends, HTTPException, Request, Response, status from fastapi.responses import RedirectResponse @@ -34,6 +36,8 @@ from app.services.encryption import hash_user_password, password_needs_upgrade, verify_user_password from app.services.session import SessionService +logger = logging.getLogger(__name__) + router = APIRouter() settings = get_settings() @@ -284,7 +288,7 @@ async def get_sso_config_public( result = await db.execute(select(SSOConfig).where(SSOConfig.id == 1)) config = result.scalar_one_or_none() except Exception as e: - print(f"SSO config query failed: {e}") + logger.error("SSO config query failed: %s", e) return {"enabled": 0} if not config: @@ -316,10 +320,7 @@ async def get_sso_config_full( result = await db.execute(select(SSOConfig).where(SSOConfig.id == 1)) config = result.scalar_one_or_none() except Exception as e: - print(f"SSO config query failed: {e}") - import traceback - - traceback.print_exc() + logger.exception("SSO config query failed: %s", e) return {"id": 1, "enabled": 0} if not config: @@ -356,13 +357,13 @@ async def update_sso_config_new( Requires admin authentication. Matches: PUT /api/sso/config """ - print(f"SSO Update received: {data}") + logger.info("SSO Update received: %s", data) result = await db.execute(select(SSOConfig).where(SSOConfig.id == 1)) config = result.scalar_one_or_none() if not config: - print("Creating new SSO config record") + logger.info("Creating new SSO config record") config = SSOConfig(id=1) db.add(config) @@ -387,7 +388,7 @@ async def update_sso_config_new( await db.commit() - print(f"SSO config saved: enabled={config.enabled}, tenant_id={config.tenant_id}") + logger.info("SSO config saved: enabled=%s, tenant_id=%s", config.enabled, config.tenant_id) # Return { success: true } to match Node.js return {"success": True} @@ -694,7 +695,9 @@ async def sso_callback( token_response = await client.post(token_url, data=token_data) if token_response.status_code != 200: - print(f"Token exchange failed: {token_response.status_code} {token_response.text}") + logger.error( + "Token exchange failed: %s %s", token_response.status_code, token_response.text + ) return RedirectResponse( url="/?sso_error=Failed+to+exchange+authorization+code", status_code=302 ) @@ -709,7 +712,7 @@ async def sso_callback( ) if graph_response.status_code != 200: - print(f"Graph API failed: {graph_response.status_code} {graph_response.text}") + logger.error("Graph API failed: %s %s", graph_response.status_code, graph_response.text) return RedirectResponse(url="/?sso_error=Failed+to+fetch+user+info", status_code=302) user_info = graph_response.json() diff --git a/app/routers/equipment.py b/app/routers/equipment.py index 9e4e3df..eebb799 100644 --- a/app/routers/equipment.py +++ b/app/routers/equipment.py @@ -6,7 +6,7 @@ """ from fastapi import APIRouter, Depends, HTTPException, Query -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -15,6 +15,7 @@ from app.models.equipment import Equipment, EquipmentAssignment from app.models.site import Site from app.models.user import User +from app.schemas.base import PaginationParams from app.schemas.equipment import ( EquipmentAssignmentResponse, EquipmentAssignmentUpdate, @@ -135,32 +136,47 @@ def build_equipment_response(equipment: Equipment) -> dict: } -@router.get("/equipment", response_model=list[EquipmentResponse]) +@router.get("/equipment") async def get_equipment( siteId: int | None = Query(None), includeAllSites: bool | None = Query(False), + pagination: PaginationParams = Depends(), db: AsyncSession = Depends(get_db), user: User = Depends(get_current_user), ): """ - Get equipment list. + Get equipment list with pagination. If siteId provided, returns equipment for that site. If includeAllSites=true, returns all equipment. Matches: GET /api/equipment """ - query = select(Equipment).where(Equipment.active == 1) + base_query = select(Equipment).where(Equipment.active == 1) if not includeAllSites and siteId: - query = query.where(Equipment.site_id == siteId) + base_query = base_query.where(Equipment.site_id == siteId) - query = query.options(selectinload(Equipment.site)).order_by(Equipment.name) + # Get total count + count_result = await db.execute(select(func.count()).select_from(base_query.subquery())) + total = count_result.scalar() or 0 + + query = ( + base_query.options(selectinload(Equipment.site)) + .order_by(Equipment.name) + .offset(pagination.offset) + .limit(pagination.limit) + ) result = await db.execute(query) equipment_list = result.scalars().all() - return [build_equipment_response(e) for e in equipment_list] + return { + "items": [build_equipment_response(e) for e in equipment_list], + "total": total, + "offset": pagination.offset, + "limit": pagination.limit, + } @router.get("/equipment/all", response_model=list[EquipmentResponse]) diff --git a/app/routers/mpp_import.py b/app/routers/mpp_import.py index 1774869..79ceebe 100644 --- a/app/routers/mpp_import.py +++ b/app/routers/mpp_import.py @@ -171,7 +171,7 @@ def find_mpxj_jar() -> str | None: for filename in os.listdir(mpxj_dir): if filename.endswith(".jar") and "mpxj" in filename.lower(): jar_path = os.path.join(mpxj_dir, filename) - logger.info(f"Found MPXJ JAR: {jar_path}") + logger.info("Found MPXJ JAR: %s", jar_path) return jar_path # Also check in a 'lib' subdirectory @@ -180,16 +180,16 @@ def find_mpxj_jar() -> str | None: for filename in os.listdir(lib_dir): if filename.endswith(".jar"): jar_path = os.path.join(lib_dir, filename) - logger.info(f"Found JAR in lib: {jar_path}") + logger.info("Found JAR in lib: %s", jar_path) return jar_path - logger.warning(f"No JAR file found in mpxj package at {mpxj_dir}") + logger.warning("No JAR file found in mpxj package at %s", mpxj_dir) return None except ImportError: logger.warning("mpxj Python package not installed") return None except Exception as e: - logger.warning(f"Error finding MPXJ JAR: {e}") + logger.warning("Error finding MPXJ JAR: %s", e) return None @@ -220,7 +220,7 @@ def get_mpxj_classpath() -> str: if jar_files: classpath = os.pathsep.join(jar_files) - logger.info(f"MPXJ classpath: {classpath}") + logger.info("MPXJ classpath: %s", classpath) return classpath logger.warning("No JAR files found in mpxj package") @@ -229,7 +229,7 @@ def get_mpxj_classpath() -> str: logger.warning("mpxj Python package not installed - install with: pip install mpxj") return "" except Exception as e: - logger.warning(f"Error building MPXJ classpath: {e}") + logger.warning("Error building MPXJ classpath: %s", e) return "" @@ -250,9 +250,9 @@ def ensure_jvm_started() -> None: try: import mpxj - logger.info(f"mpxj package loaded from: {mpxj.__file__}") + logger.info("mpxj package loaded from: %s", mpxj.__file__) except ImportError as e: - logger.error(f"mpxj package not installed: {e}") + logger.error("mpxj package not installed: %s", e) raise HTTPException( status_code=500, detail=f"""mpxj Python package not installed. @@ -270,11 +270,11 @@ def ensure_jvm_started() -> None: jvm_path = None if java_home: - logger.info(f"Found JAVA_HOME: {java_home}") + logger.info("Found JAVA_HOME: %s", java_home) os.environ["JAVA_HOME"] = java_home jvm_path = find_jvm_library(java_home) if jvm_path: - logger.info(f"Found JVM library: {jvm_path}") + logger.info("Found JVM library: %s", jvm_path) try: # Start JVM - mpxj has already added its JARs to the classpath @@ -298,7 +298,7 @@ def ensure_jvm_started() -> None: logger.info("MPXJ loaded successfully (net.sf.mpxj)") except ImportError as e: - logger.error(f"MPXJ not accessible: {e}") + logger.error("MPXJ not accessible: %s", e) raise HTTPException( status_code=500, detail=f"""MPXJ Java library not found in classpath. @@ -313,7 +313,7 @@ def ensure_jvm_started() -> None: except Exception as e: error_msg = str(e) - logger.error(f"Failed to start JVM: {error_msg}") + logger.error("Failed to start JVM: %s", error_msg) # Provide helpful error message if "libjvm.so" in error_msg or "JVMNotFoundException" in str(type(e)): @@ -841,7 +841,7 @@ async def test_mpp_import( except HTTPException: raise except Exception as e: - logger.error(f"MPP availability check failed: {e}") + logger.error("MPP availability check failed: %s", e) return JSONResponse( status_code=500, content={ @@ -902,7 +902,7 @@ async def import_mpp_file( except HTTPException: raise except Exception as e: - logger.error(f"MPP import failed: {e}") + logger.error("MPP import failed: %s", e) return JSONResponse( status_code=500, content={"success": False, "error": "Failed to parse MPP file"}, @@ -950,10 +950,6 @@ async def import_project_full( Matches: POST /api/import/project """ - import logging - - logger = logging.getLogger(__name__) - # Parse site_id from string parsed_site_id: int | None = None if site_id and site_id.strip(): @@ -962,8 +958,7 @@ async def import_project_full( except ValueError: pass - logger.info(f"Import request received: {file.filename}, site_id: {parsed_site_id}") - print(f"Import request received: {file.filename}, site_id: {parsed_site_id}") + logger.info("Import request received: %s, site_id: %s", file.filename, parsed_site_id) if not file.filename: raise HTTPException(status_code=400, detail="No file provided") @@ -971,8 +966,7 @@ async def import_project_full( filename_lower = file.filename.lower() content = await file.read() - logger.info(f"File size: {len(content)} bytes") - print(f"File size: {len(content)} bytes") + logger.info("File size: %d bytes", len(content)) if not content: raise HTTPException(status_code=400, detail="Empty file") @@ -1143,10 +1137,7 @@ async def create_subphases_for_parent( except HTTPException: raise except Exception as e: - import traceback - - logger.error(f"Import failed: {str(e)}") - logger.error(traceback.format_exc()) + logger.exception("Import failed: %s", e) await db.rollback() error_msg = str(e) diff --git a/app/routers/notes.py b/app/routers/notes.py index 0152a7a..e4e9046 100644 --- a/app/routers/notes.py +++ b/app/routers/notes.py @@ -8,6 +8,7 @@ from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import selectinload from app.database import get_db from app.middleware.auth import get_current_user, require_superuser @@ -23,8 +24,7 @@ def build_note_response(note: Note) -> dict: """Build note response dict.""" - staff_name = None - # Note: staff relationship not defined in Note model, query separately if needed + staff_name = note.staff.full_name if note.staff else None return { "id": note.id, @@ -53,7 +53,7 @@ async def get_notes( Matches: GET /api/notes """ - query = select(Note).where(Note.site_id == siteId) + query = select(Note).where(Note.site_id == siteId).options(selectinload(Note.staff)) if startDate: query = query.where(Note.date >= startDate) @@ -65,20 +65,7 @@ async def get_notes( result = await db.execute(query) notes = result.scalars().all() - # Fetch staff names for notes with staff_id - response = [] - for note in notes: - note_dict = build_note_response(note) - - if note.staff_id: - staff_result = await db.execute(select(User).where(User.id == note.staff_id)) - staff = staff_result.scalar_one_or_none() - if staff: - note_dict["staff_name"] = f"{staff.first_name} {staff.last_name}".strip() - - response.append(note_dict) - - return response + return [build_note_response(note) for note in notes] @router.post("/notes", response_model=NoteResponse, status_code=201) @@ -106,16 +93,12 @@ async def create_note( await db.commit() await db.refresh(note) - note_dict = build_note_response(note) - - # Fetch staff name if applicable + # Eager-load staff for response if note.staff_id: staff_result = await db.execute(select(User).where(User.id == note.staff_id)) - staff = staff_result.scalar_one_or_none() - if staff: - note_dict["staff_name"] = f"{staff.first_name} {staff.last_name}".strip() + note.staff = staff_result.scalar_one_or_none() - return note_dict + return build_note_response(note) @router.delete("/notes/{note_id}") diff --git a/app/routers/projects.py b/app/routers/projects.py index f52e041..b9c1d7f 100644 --- a/app/routers/projects.py +++ b/app/routers/projects.py @@ -5,6 +5,7 @@ """ import json +import logging from fastapi import APIRouter, Depends, HTTPException, Query, Request from sqlalchemy import and_, func, or_, select, update @@ -21,13 +22,13 @@ from app.models.project import Project, ProjectPhase, ProjectSubphase from app.models.site import Site from app.models.user import User +from app.schemas.base import PaginationParams from app.schemas.project import ( PhaseCreate, PhaseReorderRequest, PhaseUpdate, ProjectCreate, ProjectDetailResponse, - ProjectListResponse, ProjectUpdate, SubphaseCreate, SubphaseReorderRequest, @@ -35,6 +36,8 @@ ) from app.websocket.broadcast import broadcast_change +logger = logging.getLogger(__name__) + router = APIRouter() @@ -141,25 +144,45 @@ def build_subphase_tree( # --------------------------------------------------------- -@router.get("/projects", response_model=list[ProjectListResponse]) +@router.get("/projects") async def get_projects( siteId: int | None = Query(None), includeOtherSites: str | None = Query(None), archived: str | None = Query(None), + pagination: PaginationParams = Depends(), db: AsyncSession = Depends(get_db_readonly), user: User = Depends(get_current_user), ): """ - Get list of projects. + Get list of projects with pagination. Query params: - siteId: Filter by site - includeOtherSites: If 'true', show all sites but mask external project details - archived: 'true' for archived only, 'all' for all, default is non-archived + - offset: Number of items to skip (default 0) + - limit: Max items to return (default 50, max 200) Matches: GET /api/projects """ - # Build base query with joins + # Build base filter query + base_filter = select(Project.id) + + # Filter by archived status + if archived == "true": + base_filter = base_filter.where(Project.archived == 1) + elif archived != "all": + base_filter = base_filter.where(or_(Project.archived == 0, Project.archived.is_(None))) + + # Filter by site + if siteId and includeOtherSites != "true": + base_filter = base_filter.where(Project.site_id == int(siteId)) + + # Get total count + count_result = await db.execute(select(func.count()).select_from(base_filter.subquery())) + total = count_result.scalar() or 0 + + # Build paginated query with joins query = ( select( Project, @@ -170,17 +193,15 @@ async def get_projects( .outerjoin(User, Project.pm_id == User.id) ) - # Filter by archived status if archived == "true": query = query.where(Project.archived == 1) elif archived != "all": query = query.where(or_(Project.archived == 0, Project.archived.is_(None))) - # Filter by site if siteId and includeOtherSites != "true": query = query.where(Project.site_id == int(siteId)) - query = query.order_by(Project.start_date) + query = query.order_by(Project.start_date).offset(pagination.offset).limit(pagination.limit) result = await db.execute(query) rows = result.all() @@ -235,7 +256,12 @@ async def get_projects( } ) - return projects + return { + "items": projects, + "total": total, + "offset": pagination.offset, + "limit": pagination.limit, + } @router.get("/projects/{project_id}", response_model=ProjectDetailResponse) @@ -367,8 +393,17 @@ async def query_equipment(): equipment_rows = equipment_result.all() # Log all query times - print( - f"PROJECT {project_id}: q1={int((t1 - t0) * 1000)}ms q2={int((t2 - t1) * 1000)}ms q3={int((t3 - t2) * 1000)}ms q4={int((t4 - t3) * 1000)}ms q5={int((t5 - t4) * 1000)}ms q6={int((t6 - t5) * 1000)}ms q7={int((t7 - t6) * 1000)}ms TOTAL={int((t7 - t0) * 1000)}ms" + logger.debug( + "PROJECT %s: q1=%dms q2=%dms q3=%dms q4=%dms q5=%dms q6=%dms q7=%dms TOTAL=%dms", + project_id, + int((t1 - t0) * 1000), + int((t2 - t1) * 1000), + int((t3 - t2) * 1000), + int((t4 - t3) * 1000), + int((t5 - t4) * 1000), + int((t6 - t5) * 1000), + int((t7 - t6) * 1000), + int((t7 - t0) * 1000), ) # Convert to list with attributes @@ -460,8 +495,13 @@ def __init__(self, assignment, staff_name, staff_role): ] t8 = time.perf_counter() - print( - f"PROJECT {project_id} TIMING: q6={int((t6 - t5) * 1000)}ms q7={int((t7 - t6) * 1000)}ms proc={int((t8 - t7) * 1000)}ms TOTAL={int((t8 - t0) * 1000)}ms" + logger.debug( + "PROJECT %s TIMING: q6=%dms q7=%dms proc=%dms TOTAL=%dms", + project_id, + int((t6 - t5) * 1000), + int((t7 - t6) * 1000), + int((t8 - t7) * 1000), + int((t8 - t0) * 1000), ) return { diff --git a/app/routers/settings.py b/app/routers/settings.py index c3b32c1..1dca3b5 100644 --- a/app/routers/settings.py +++ b/app/routers/settings.py @@ -6,6 +6,8 @@ It matches the Node.js API at /api/settings exactly. """ +import logging + from fastapi import APIRouter, Depends from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -17,6 +19,8 @@ from app.schemas.auth import SSOConfigResponse, SSOConfigUpdate from app.schemas.settings import SettingsResponse, SettingUpdate +logger = logging.getLogger(__name__) + router = APIRouter() @@ -54,7 +58,7 @@ async def get_sso_settings( config = result.scalar_one_or_none() except Exception as e: # Table might not exist - return default disabled config - print(f"SSO config query failed (table may not exist): {e}") + logger.warning("SSO config query failed (table may not exist): %s", e) return SSOConfigResponse( enabled=False, configured=False, diff --git a/app/routers/sites.py b/app/routers/sites.py index e39c719..7b300f9 100644 --- a/app/routers/sites.py +++ b/app/routers/sites.py @@ -5,6 +5,7 @@ Matches the Node.js API at /api/sites exactly. """ +import logging from datetime import datetime import httpx @@ -27,6 +28,8 @@ SiteUpdate, ) +logger = logging.getLogger(__name__) + router = APIRouter() @@ -444,7 +447,9 @@ async def fetch_and_store_bank_holidays( country_code = site.country_code region_code = site.region_code - print(f"Fetching holidays for site {site_id}, country={country_code}, region={region_code}") + logger.info( + "Fetching holidays for site %s, country=%s, region=%s", site_id, country_code, region_code + ) settings = get_settings() current_year = datetime.now().year @@ -457,7 +462,7 @@ async def fetch_and_store_bank_holidays( proxy_url = await get_proxy_for_url(settings.nager_api_url) if proxy_url: - print(f"Using proxy: {proxy_url}") + logger.info("Using proxy: %s", proxy_url) # Add authentication to proxy URL if credentials provided if settings.proxy_username and settings.proxy_password: from urllib.parse import urlparse, urlunparse @@ -474,38 +479,43 @@ async def fetch_and_store_bank_holidays( ) ) proxy_url = auth_proxy - print(f"Proxy authentication enabled for user: {settings.proxy_username}") + logger.info("Proxy authentication enabled for user: %s", settings.proxy_username) # SSL verification - can use custom CA cert for corporate proxies ssl_verify: bool | str = settings.proxy_verify_ssl if settings.proxy_ca_cert: - print(f"Using custom CA certificate: {settings.proxy_ca_cert}") + logger.info("Using custom CA certificate: %s", settings.proxy_ca_cert) ssl_verify = settings.proxy_ca_cert elif not settings.proxy_verify_ssl: - print("SSL verification disabled for proxy") + logger.info("SSL verification disabled for proxy") async with httpx.AsyncClient(timeout=10.0, proxy=proxy_url, verify=ssl_verify) as client: for year in years: try: url = f"{settings.nager_api_url}/PublicHolidays/{year}/{country_code}" - print(f"Fetching: {url}") + logger.info("Fetching: %s", url) response = await client.get(url) if response.status_code != 200: - print( - f"Failed to fetch holidays for {country_code}/{year}: HTTP {response.status_code}" + logger.warning( + "Failed to fetch holidays for %s/%s: HTTP %s", + country_code, + year, + response.status_code, ) # Debug: show response headers and body for non-200 responses - print(f" Response headers: {dict(response.headers)}") + logger.debug(" Response headers: %s", dict(response.headers)) try: body = response.text[:500] # First 500 chars - print(f" Response body: {body}") + logger.debug(" Response body: %s", body) except Exception: pass continue holidays_data = response.json() - print(f"Received {len(holidays_data)} holidays for {country_code}/{year}") + logger.info( + "Received %d holidays for %s/%s", len(holidays_data), country_code, year + ) for h in holidays_data: # Filter by region if specified @@ -539,20 +549,19 @@ async def fetch_and_store_bank_holidays( # Don't rollback the whole transaction, just skip this one # It might be a duplicate await db.rollback() - print(f"Skipped holiday {h['date']} {h.get('localName', h['name'])}: {e}") + logger.debug( + "Skipped holiday %s %s: %s", h["date"], h.get("localName", h["name"]), e + ) continue except httpx.RequestError as e: - print(f"Request error fetching holidays for {country_code}/{year}: {e}") + logger.error("Request error fetching holidays for %s/%s: %s", country_code, year, e) continue except Exception as e: - print(f"Error fetching holidays for {country_code}/{year}: {e}") - import traceback - - traceback.print_exc() + logger.exception("Error fetching holidays for %s/%s: %s", country_code, year, e) continue - print(f"Added {total_added} holidays for site {site_id}") + logger.info("Added %d holidays for site %s", total_added, site_id) # Update last fetch timestamp site.last_holiday_fetch = datetime.utcnow() diff --git a/app/routers/staff.py b/app/routers/staff.py index 22a8aed..a2cbef0 100644 --- a/app/routers/staff.py +++ b/app/routers/staff.py @@ -28,6 +28,7 @@ StaffDetailResponse, StaffResponse, ) +from app.services.response_builders import build_skills_list, get_max_capacity router = APIRouter() @@ -38,25 +39,18 @@ def build_staff_row(user: User, site: Site | None = None) -> dict: Returns one row per user-site combination. """ - # Build skills list from user's skills relationship - skills = [] - if hasattr(user, "skills") and user.skills: - skills = [ - {"id": skill.id, "name": skill.name, "color": skill.color} for skill in user.skills - ] - return { "id": user.id, "first_name": user.first_name, "last_name": user.last_name, - "name": f"{user.first_name} {user.last_name}", + "name": user.full_name, "role": user.job_title, # Node.js uses job_title as 'role' "email": user.email, "active": user.active, - "max_capacity": user.max_capacity if hasattr(user, "max_capacity") else 100, + "max_capacity": get_max_capacity(user), "site_id": site.id if site else None, "site_name": site.name if site else None, - "skills": skills, + "skills": build_skills_list(user), } diff --git a/app/routers/users.py b/app/routers/users.py index 8b307f2..9dea22b 100644 --- a/app/routers/users.py +++ b/app/routers/users.py @@ -6,7 +6,7 @@ """ from fastapi import APIRouter, Depends, HTTPException, Query -from sqlalchemy import delete, select +from sqlalchemy import delete, func, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -15,6 +15,7 @@ from app.models.site import Site from app.models.skill import Skill, UserSkill from app.models.user import User, UserSite +from app.schemas.base import PaginationParams from app.schemas.user import ( StaffCreate, StaffUpdate, @@ -22,94 +23,77 @@ UserResponse, ) from app.services.encryption import hash_user_password +from app.services.response_builders import build_skills_list, build_user_base, get_sorted_sites router = APIRouter() def build_user_list_response(user: User) -> dict: """Build user response for list view (includes job_title, skills, and site_names).""" - sites = user.sites if hasattr(user, "sites") and user.sites else [] - # Sort by site ID to match Node.js behavior - sorted_sites = sorted(sites, key=lambda s: s.id) - - # Build skills list - skills = [] - if hasattr(user, "skills") and user.skills: - skills = [ - {"id": skill.id, "name": skill.name, "color": skill.color} for skill in user.skills - ] - + sorted_sites = get_sorted_sites(user) return { - "id": user.id, - "email": user.email, - "first_name": user.first_name, - "last_name": user.last_name, - "job_title": user.job_title, - "role": user.role, - "max_capacity": user.max_capacity if hasattr(user, "max_capacity") else 100, - "active": user.active, + **build_user_base(user), "is_system": user.is_system if user.is_system is not None else 0, "created_at": user.created_at, "site_ids": [s.id for s in sorted_sites], "site_names": [s.name for s in sorted_sites], - "skills": skills, + "skills": build_skills_list(user), } def build_user_detail_response(user: User) -> dict: """Build user response for detail view (has sites array, no site_names).""" - sites = user.sites if hasattr(user, "sites") and user.sites else [] - - # Build skills list - skills = [] - if hasattr(user, "skills") and user.skills: - skills = [ - {"id": skill.id, "name": skill.name, "color": skill.color} for skill in user.skills - ] - + sorted_sites = get_sorted_sites(user) return { - "id": user.id, - "email": user.email, - "first_name": user.first_name, - "last_name": user.last_name, - "job_title": user.job_title, - "role": user.role, - "max_capacity": user.max_capacity if hasattr(user, "max_capacity") else 100, - "active": user.active, + **build_user_base(user), "is_system": user.is_system if user.is_system is not None else 0, "created_at": user.created_at, - "site_ids": [s.id for s in sites], - "sites": [{"id": s.id, "name": s.name} for s in sites], - "skills": skills, + "site_ids": [s.id for s in sorted_sites], + "sites": [{"id": s.id, "name": s.name} for s in sorted_sites], + "skills": build_skills_list(user), } -@router.get("/users", response_model=list[UserResponse]) +@router.get("/users") async def get_users( include_disabled: bool = Query(False, alias="includeDisabled"), + pagination: PaginationParams = Depends(), db: AsyncSession = Depends(get_db), user: User = Depends(require_superuser), ): """ - Get all users. + Get all users with pagination. Requires admin or superuser authentication. Matches: GET /api/users """ + base_query = select(User) + + if not include_disabled: + base_query = base_query.where(User.active == 1) + + # Get total count + count_result = await db.execute(select(func.count()).select_from(base_query.subquery())) + total = count_result.scalar() or 0 + + # Paginated query query = ( - select(User) - .options(selectinload(User.sites)) + base_query.options(selectinload(User.sites)) .options(selectinload(User.skills)) .order_by(User.last_name, User.first_name) + .offset(pagination.offset) + .limit(pagination.limit) ) - if not include_disabled: - query = query.where(User.active == 1) - result = await db.execute(query) users = result.unique().scalars().all() - return [build_user_list_response(u) for u in users] + return { + "items": [build_user_list_response(u) for u in users], + "total": total, + "offset": pagination.offset, + "limit": pagination.limit, + } @router.get("/users/{user_id}", response_model=UserDetailResponse) diff --git a/app/routers/vacations.py b/app/routers/vacations.py index 505cd98..a277551 100644 --- a/app/routers/vacations.py +++ b/app/routers/vacations.py @@ -25,9 +25,7 @@ def build_vacation_response(vacation: Vacation, can_edit: bool = False) -> dict: """Build vacation response dict.""" - staff_name = "" - if vacation.staff: - staff_name = f"{vacation.staff.first_name} {vacation.staff.last_name}".strip() + staff_name = vacation.staff.full_name if vacation.staff else "" return { "id": vacation.id, diff --git a/app/schemas/base.py b/app/schemas/base.py index 9b39c64..90e6ed0 100644 --- a/app/schemas/base.py +++ b/app/schemas/base.py @@ -33,10 +33,13 @@ import calendar from datetime import date, datetime, timedelta -from typing import Annotated +from typing import Annotated, Generic, TypeVar +from fastapi import Query from pydantic import BaseModel, ConfigDict, PlainSerializer +T = TypeVar("T") + def is_dst_europe(dt: datetime | date) -> bool: """ @@ -144,3 +147,36 @@ class BaseSchema(BaseModel): model_config = ConfigDict( from_attributes=True, ) + + +# --------------------------------------------------------------------------- +# Pagination +# --------------------------------------------------------------------------- + + +class PaginationParams: + """FastAPI dependency for pagination query parameters. + + Usage:: + + @router.get("/items") + async def list_items(pagination: PaginationParams = Depends()): + query = select(Item).offset(pagination.offset).limit(pagination.limit) + """ + + def __init__( + self, + offset: int = Query(0, ge=0, description="Number of items to skip"), + limit: int = Query(50, ge=1, le=200, description="Max items to return"), + ): + self.offset = offset + self.limit = limit + + +class PaginatedResponse(BaseSchema, Generic[T]): + """Paginated response wrapper.""" + + items: list[T] + total: int + offset: int + limit: int diff --git a/app/services/auth.py b/app/services/auth.py new file mode 100644 index 0000000..9bece31 --- /dev/null +++ b/app/services/auth.py @@ -0,0 +1,63 @@ +""" +Shared authentication utilities. + +Centralizes common patterns between tenant auth (middleware/auth.py) +and admin auth (routers/admin/auth.py). Each still handles its own +session format, but this module provides the shared validation logic. +""" + +import json +import logging +import time + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.tenant import AdminSession, AdminUser + +logger = logging.getLogger(__name__) + + +async def validate_admin_session( + db: AsyncSession, + session_id: str, +) -> AdminUser | None: + """ + Validate an admin session and return the AdminUser, or None. + + Encapsulates the common logic of: + 1. Look up session by ID + 2. Check expiration + 3. Parse session JSON + 4. Load and validate the admin user + + Returns None instead of raising exceptions — callers decide + how to handle unauthenticated requests. + """ + now_ms = int(time.time() * 1000) + result = await db.execute( + select(AdminSession) + .where(AdminSession.sid == session_id) + .where(AdminSession.expired > now_ms) + ) + session = result.scalar_one_or_none() + + if not session: + return None + + try: + sess_data = json.loads(session.sess) + admin_user_id = sess_data.get("admin_user_id") + if not admin_user_id: + return None + except (json.JSONDecodeError, KeyError): + logger.warning("Invalid admin session data for sid=%s", session_id[:20]) + return None + + result = await db.execute(select(AdminUser).where(AdminUser.id == admin_user_id)) + admin = result.scalar_one_or_none() + + if not admin or not admin.is_active: + return None + + return admin diff --git a/app/services/master_db.py b/app/services/master_db.py index cca48ce..81d8066 100644 --- a/app/services/master_db.py +++ b/app/services/master_db.py @@ -147,7 +147,7 @@ async def _apply_pending_migrations(self): orgs_table_exists = result.scalar() if not orgs_table_exists: - print("Master DB: Creating organizations table...") + logger.info("Master DB: Creating organizations table...") await conn.execute( text( "CREATE TABLE organizations (" @@ -166,7 +166,7 @@ async def _apply_pending_migrations(self): "ON organizations(slug)" ) ) - print("Master DB: organizations table created") + logger.info("Master DB: organizations table created") # Check if organization_sso_config table exists result = await conn.execute( @@ -180,7 +180,7 @@ async def _apply_pending_migrations(self): sso_table_exists = result.scalar() if not sso_table_exists: - print("Master DB: Creating organization_sso_config table...") + logger.info("Master DB: Creating organization_sso_config table...") await conn.execute( text( "CREATE TABLE organization_sso_config (" @@ -198,7 +198,7 @@ async def _apply_pending_migrations(self): ")" ) ) - print("Master DB: organization_sso_config table created") + logger.info("Master DB: organization_sso_config table created") # Check if tenants.organization_id column exists result = await conn.execute( @@ -212,7 +212,7 @@ async def _apply_pending_migrations(self): has_org_id = result.scalar() if not has_org_id: - print("Master DB: Adding organization_id column to tenants...") + logger.info("Master DB: Adding organization_id column to tenants...") await conn.execute( text( "ALTER TABLE tenants ADD COLUMN organization_id UUID " @@ -225,7 +225,7 @@ async def _apply_pending_migrations(self): "ON tenants(organization_id)" ) ) - print("Master DB: organization_id column added") + logger.info("Master DB: organization_id column added") # Check if tenants.required_group_ids column exists result = await conn.execute( @@ -239,11 +239,11 @@ async def _apply_pending_migrations(self): has_group_ids = result.scalar() if not has_group_ids: - print("Master DB: Adding required_group_ids column to tenants...") + logger.info("Master DB: Adding required_group_ids column to tenants...") await conn.execute( text("ALTER TABLE tenants ADD COLUMN required_group_ids JSONB DEFAULT '[]'") ) - print("Master DB: required_group_ids column added") + logger.info("Master DB: required_group_ids column added") # Check if tenants.group_membership_mode column exists result = await conn.execute( @@ -257,13 +257,13 @@ async def _apply_pending_migrations(self): has_group_mode = result.scalar() if not has_group_mode: - print("Master DB: Adding group_membership_mode column to tenants...") + logger.info("Master DB: Adding group_membership_mode column to tenants...") await conn.execute( text( "ALTER TABLE tenants ADD COLUMN group_membership_mode VARCHAR(10) DEFAULT 'any'" ) ) - print("Master DB: group_membership_mode column added") + logger.info("Master DB: group_membership_mode column added") # Check if admin_users.must_change_password column exists result = await conn.execute( @@ -277,18 +277,20 @@ async def _apply_pending_migrations(self): has_must_change = result.scalar() if not has_must_change: - print("Master DB: Adding must_change_password column to admin_users...") + logger.info("Master DB: Adding must_change_password column to admin_users...") await conn.execute( text( "ALTER TABLE admin_users ADD COLUMN must_change_password INTEGER DEFAULT 0" ) ) - print("Master DB: must_change_password column added") + logger.info("Master DB: must_change_password column added") except Exception as e: - print(f"WARNING: Auto-migration failed: {e}") - print("The app will continue, but organization features may not work.") - print("Run manually: python migrations/run_migration_master.py add_organizations") + logger.warning("Auto-migration failed: %s", e) + logger.warning("The app will continue, but organization features may not work.") + logger.warning( + "Run manually: python migrations/run_migration_master.py add_organizations" + ) async def close(self): """Close the database connection.""" diff --git a/app/services/proxy.py b/app/services/proxy.py index f7e72ca..5678258 100644 --- a/app/services/proxy.py +++ b/app/services/proxy.py @@ -3,12 +3,15 @@ Supports PAC (Proxy Auto-Config) files and direct proxy URLs. """ +import logging import re import httpx from app.config import get_settings +logger = logging.getLogger(__name__) + # Cache for PAC file content and parsed proxy _pac_content: str | None = None _cached_proxy: str | None = None @@ -58,17 +61,17 @@ async def get_proxy_for_url(url: str) -> str | None: settings = get_settings() - # Debug: print all proxy-related settings - print(f"[Proxy] Debug - https_proxy: '{settings.https_proxy}'") - print(f"[Proxy] Debug - http_proxy: '{settings.http_proxy}'") - print(f"[Proxy] Debug - proxy_pac_url: '{settings.proxy_pac_url}'") + # Debug: log all proxy-related settings + logger.debug("https_proxy: '%s'", settings.https_proxy) + logger.debug("http_proxy: '%s'", settings.http_proxy) + logger.debug("proxy_pac_url: '%s'", settings.proxy_pac_url) # Check direct proxy settings first if settings.https_proxy: - print(f"[Proxy] Using HTTPS_PROXY: {settings.https_proxy}") + logger.info("Using HTTPS_PROXY: %s", settings.https_proxy) return settings.https_proxy if settings.http_proxy: - print(f"[Proxy] Using HTTP_PROXY: {settings.http_proxy}") + logger.info("Using HTTP_PROXY: %s", settings.http_proxy) return settings.http_proxy # Check PAC file @@ -89,18 +92,18 @@ async def get_proxy_for_url(url: str) -> str | None: if proxy: _cached_proxy = proxy - print(f"[Proxy] PAC file resolved to: {proxy}") + logger.info("PAC file resolved to: %s", proxy) return proxy else: _cached_proxy = "DIRECT" - print("[Proxy] PAC file indicates DIRECT connection") + logger.info("PAC file indicates DIRECT connection") return None else: - print(f"[Proxy] Failed to fetch PAC file: HTTP {response.status_code}") + logger.warning("Failed to fetch PAC file: HTTP %s", response.status_code) except Exception as e: - print(f"[Proxy] Error fetching PAC file: {e}") + logger.error("Error fetching PAC file: %s", e) - print("[Proxy] No proxy configured") + logger.debug("No proxy configured") return None diff --git a/app/services/response_builders.py b/app/services/response_builders.py new file mode 100644 index 0000000..f4f5057 --- /dev/null +++ b/app/services/response_builders.py @@ -0,0 +1,46 @@ +""" +Shared response builder utilities. + +Centralizes the common patterns for building API response dicts +from ORM models, eliminating duplication across routers. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from app.models.site import Site + from app.models.user import User + + +def get_sorted_sites(user: User) -> list[Site]: + """Get a user's sites sorted by ID (matches Node.js behavior).""" + sites = user.sites if hasattr(user, "sites") and user.sites else [] + return sorted(sites, key=lambda s: s.id) + + +def build_skills_list(user: User) -> list[dict]: + """Build a skills list from a user's skills relationship.""" + if not hasattr(user, "skills") or not user.skills: + return [] + return [{"id": skill.id, "name": skill.name, "color": skill.color} for skill in user.skills] + + +def get_max_capacity(user: User) -> int: + """Get user's max capacity with fallback.""" + return user.max_capacity if hasattr(user, "max_capacity") else 100 + + +def build_user_base(user: User) -> dict: + """Build common user fields shared across list/detail/staff views.""" + return { + "id": user.id, + "email": user.email, + "first_name": user.first_name, + "last_name": user.last_name, + "job_title": user.job_title, + "role": user.role, + "max_capacity": get_max_capacity(user), + "active": user.active, + } diff --git a/app/services/sso.py b/app/services/sso.py index 4fb1fb2..85ec652 100644 --- a/app/services/sso.py +++ b/app/services/sso.py @@ -8,6 +8,7 @@ - SSO callback URL construction """ +import logging from typing import TYPE_CHECKING, Any, Optional from urllib.parse import urlencode @@ -18,6 +19,8 @@ from app.config import get_settings from app.services.encryption import decrypt +logger = logging.getLogger(__name__) + if TYPE_CHECKING: from app.models.tenant import Tenant @@ -106,8 +109,8 @@ async def fetch_user_groups(self, access_token: str, max_groups: int = 500) -> l if response.status_code != 200: # Log error but don't fail - groups might just not be available - print( - f"Warning: Failed to fetch groups: {response.status_code} {response.text}" + logger.warning( + "Failed to fetch groups: %s %s", response.status_code, response.text ) break diff --git a/app/services/tenant_manager.py b/app/services/tenant_manager.py index baedb39..d46b30a 100644 --- a/app/services/tenant_manager.py +++ b/app/services/tenant_manager.py @@ -6,6 +6,7 @@ """ import asyncio +import logging from datetime import datetime from typing import Any @@ -21,6 +22,8 @@ from app.models.tenant import Tenant, TenantCredentials from app.services.encryption import decrypt +logger = logging.getLogger(__name__) + class TenantConnectionManager: """ @@ -65,7 +68,7 @@ async def _cleanup_idle_pools(self): for slug in to_remove: await self._close_pool(slug) - print(f"Closed idle tenant pool: {slug}") + logger.info("Closed idle tenant pool: %s", slug) async def _close_pool(self, slug: str): """Close a specific tenant's connection pool.""" @@ -91,7 +94,7 @@ async def _run_auto_migrations(self, conn, slug: str): """) ) if not result.fetchone(): - print(f"Auto-migration: Adding is_system column to tenant {slug}...") + logger.info("Auto-migration: Adding is_system column to tenant %s...", slug) await conn.execute(text("ALTER TABLE users ADD COLUMN is_system INTEGER DEFAULT 0")) # Mark first admin as system user await conn.execute( @@ -101,9 +104,9 @@ async def _run_auto_migrations(self, conn, slug: str): """) ) await conn.commit() - print(f"Auto-migration: Added is_system column to tenant {slug}") + logger.info("Auto-migration: Added is_system column to tenant %s", slug) except Exception as e: - print(f"Auto-migration warning for {slug}: {e}") + logger.warning("Auto-migration warning for %s: %s", slug, e) # Don't fail if migration has issues - continue with connection async def get_pool(self, tenant: Tenant, credentials: TenantCredentials) -> AsyncEngine: @@ -142,8 +145,12 @@ async def get_pool(self, tenant: Tenant, credentials: TenantCredentials) -> Asyn f"@{host}:{port}/{tenant.database_name}" ) - print( - f"Connecting to tenant DB: {tenant.database_name} as {tenant.database_user}@{host}:{port}" + logger.info( + "Connecting to tenant DB: %s as %s@%s:%s", + tenant.database_name, + tenant.database_user, + host, + port, ) # Create engine @@ -163,7 +170,7 @@ async def get_pool(self, tenant: Tenant, credentials: TenantCredentials) -> Asyn # Auto-migration: Add is_system column if missing await self._run_auto_migrations(conn, slug) - print(f"Tenant pool created: {slug}") + logger.info("Tenant pool created: %s", slug) except Exception as e: await engine.dispose() raise ConnectionError( @@ -211,8 +218,12 @@ async def get_pool_from_info(self, tenant_info: dict[str, Any]) -> AsyncEngine: f"@{host}:{port}/{tenant_info['database_name']}" ) - print( - f"Connecting to tenant DB: {tenant_info['database_name']} as {tenant_info['database_user']}@{host}:{port}" + logger.info( + "Connecting to tenant DB: %s as %s@%s:%s", + tenant_info["database_name"], + tenant_info["database_user"], + host, + port, ) # Create engine @@ -232,7 +243,7 @@ async def get_pool_from_info(self, tenant_info: dict[str, Any]) -> AsyncEngine: # Auto-migration: Add is_system column if missing await self._run_auto_migrations(conn, slug) - print(f"Tenant pool created: {slug}") + logger.info("Tenant pool created: %s", slug) except Exception as e: await engine.dispose() raise ConnectionError( diff --git a/app/services/tenant_provisioner.py b/app/services/tenant_provisioner.py index 6e7441f..cdf8020 100644 --- a/app/services/tenant_provisioner.py +++ b/app/services/tenant_provisioner.py @@ -5,6 +5,7 @@ Schema matches Node.js application exactly. """ +import logging import re from typing import Any @@ -13,6 +14,8 @@ from app.config import get_settings from app.services.encryption import generate_password, hash_password +logger = logging.getLogger(__name__) + def _validate_identifier(name: str) -> str: """Validate a SQL identifier (database name, username) to prevent injection.""" @@ -42,7 +45,7 @@ async def get_admin_connection() -> asyncpg.Connection: password = settings.pg_admin_password or settings.db_password database = "postgres" # Connect to default database for admin operations - print(f"Admin connection: user={user}, host={host}:{port}, database={database}") + logger.info("Admin connection: user=%s, host=%s:%s, database=%s", user, host, port, database) return await asyncpg.connect( host=host, @@ -436,8 +439,6 @@ async def provision_tenant_database( Returns dict with admin credentials. """ - import traceback - conn = None tenant_conn = None @@ -450,7 +451,7 @@ async def provision_tenant_database( admin_password_hash = hash_password(admin_password) - print(f"Provisioning tenant database: {database_name}") + logger.info("Provisioning tenant database: %s", database_name) # Validate identifiers to prevent SQL injection _validate_identifier(database_user) @@ -460,25 +461,23 @@ async def provision_tenant_database( # Create database user try: await conn.execute(f"CREATE USER \"{database_user}\" WITH PASSWORD '{safe_password}'") - print(f" Created user: {database_user}") + logger.info(" Created user: %s", database_user) except asyncpg.DuplicateObjectError: # User exists, update password await conn.execute(f"ALTER USER \"{database_user}\" WITH PASSWORD '{safe_password}'") - print(f" Updated password for existing user: {database_user}") + logger.info(" Updated password for existing user: %s", database_user) except Exception as e: - print(f" Error creating user: {e}") - traceback.print_exc() + logger.exception(" Error creating user: %s", e) raise # Create database try: await conn.execute(f'CREATE DATABASE "{database_name}" OWNER "{database_user}"') - print(f" Created database: {database_name}") + logger.info(" Created database: %s", database_name) except asyncpg.DuplicateDatabaseError: - print(f" Database already exists: {database_name}") + logger.info(" Database already exists: %s", database_name) except Exception as e: - print(f" Error creating database: {e}") - traceback.print_exc() + logger.exception(" Error creating database: %s", e) raise # Grant privileges @@ -487,8 +486,7 @@ async def provision_tenant_database( f'GRANT ALL PRIVILEGES ON DATABASE "{database_name}" TO "{database_user}"' ) except Exception as e: - print(f" Error granting privileges: {e}") - traceback.print_exc() + logger.exception(" Error granting privileges: %s", e) raise await conn.close() @@ -496,7 +494,7 @@ async def provision_tenant_database( # Connect to the new database to create schema settings = get_settings() - print(f" Connecting to new database as {database_user}...") + logger.info(" Connecting to new database as %s...", database_user) tenant_conn = await asyncpg.connect( host=settings.db_host, port=settings.db_port, @@ -506,20 +504,20 @@ async def provision_tenant_database( ) # Create schema - print(" Creating schema tables...") + logger.info(" Creating schema tables...") schema_sql = get_tenant_schema_sql() await tenant_conn.execute(schema_sql) - print(" Created schema tables") + logger.info(" Created schema tables") # Seed data - print(" Seeding initial data...") + logger.info(" Seeding initial data...") await run_seed_data(tenant_conn, admin_email, admin_password_hash) - print(" Seeded initial data") + logger.info(" Seeded initial data") await tenant_conn.close() tenant_conn = None - print(f"Tenant database provisioned successfully: {database_name}") + logger.info("Tenant database provisioned successfully: %s", database_name) return { "database_name": database_name, @@ -529,8 +527,7 @@ async def provision_tenant_database( } except Exception as e: - print(f"PROVISIONING ERROR: {e}") - traceback.print_exc() + logger.exception("PROVISIONING ERROR: %s", e) raise finally: if conn: @@ -562,11 +559,11 @@ async def drop_tenant_database( # Drop database await conn.execute(f'DROP DATABASE IF EXISTS "{database_name}"') - print(f"Dropped database: {database_name}") + logger.info("Dropped database: %s", database_name) # Drop user await conn.execute(f'DROP USER IF EXISTS "{database_user}"') - print(f"Dropped user: {database_user}") + logger.info("Dropped user: %s", database_user) return True @@ -614,7 +611,7 @@ async def reset_tenant_admin_password( if result == "UPDATE 0": raise ValueError(f"Admin user not found: {admin_email}") - print(f"Reset password for {admin_email} in {database_name}") + logger.info("Reset password for %s in %s", admin_email, database_name) finally: await conn.close() @@ -649,7 +646,7 @@ async def test_tenant_connection( await conn.close() return True except Exception as e: - print(f"Connection test failed: {e}") + logger.error("Connection test failed: %s", e) return False diff --git a/app/websocket/handler.py b/app/websocket/handler.py index cd8d782..7e40713 100644 --- a/app/websocket/handler.py +++ b/app/websocket/handler.py @@ -34,35 +34,35 @@ async def get_user_from_session(session_id: str, db: AsyncSession) -> User | Non User if session is valid, None otherwise """ try: - print(f"[WS Auth] Looking up session: {session_id[:20]}...") + logger.debug("Looking up session: %s...", session_id[:20]) # Query the session result = await db.execute(select(Session).where(Session.sid == session_id)) session = result.scalar_one_or_none() if not session: - print("[WS Auth] Session not found in database") + logger.debug("Session not found in database") return None - print("[WS Auth] Session found, checking expiry...") + logger.debug("Session found, checking expiry...") # Check if session is expired now_ms = int(datetime.utcnow().timestamp() * 1000) if session.expired < now_ms: - print(f"[WS Auth] Session expired: {session.expired} < {now_ms}") + logger.debug("Session expired: %s < %s", session.expired, now_ms) return None - print("[WS Auth] Session valid, parsing data...") + logger.debug("Session valid, parsing data...") # Parse session data to get user ID sess_data = json.loads(session.sess) user_data = sess_data.get("user", {}) user_id = user_data.get("id") - print(f"[WS Auth] User ID from session: {user_id}") + logger.debug("User ID from session: %s", user_id) if not user_id: - print("[WS Auth] No user ID in session data") + logger.debug("No user ID in session data") return None # Get the user @@ -70,15 +70,14 @@ async def get_user_from_session(session_id: str, db: AsyncSession) -> User | Non user = result.scalar_one_or_none() if user: - print(f"[WS Auth] User found: {user.first_name} {user.last_name}") + logger.debug("User found: %s %s", user.first_name, user.last_name) else: - print(f"[WS Auth] User {user_id} not found in database") + logger.debug("User %s not found in database", user_id) return user except Exception as e: - print(f"[WS Auth] Error validating session: {e}") - logger.error(f"Error validating session: {e}") + logger.error("Error validating session: %s", e) return None @@ -120,8 +119,11 @@ def get_tenant_from_scope(websocket: WebSocket) -> str: tenant_slug = state.get("tenant_slug") path = websocket.scope.get("path", "/ws") - print( - f"[WS get_tenant_from_scope] path={path}, state keys={list(state.keys())}, tenant_slug={tenant_slug}" + logger.debug( + "get_tenant_from_scope: path=%s, state keys=%s, tenant_slug=%s", + path, + list(state.keys()), + tenant_slug, ) if tenant_slug: @@ -132,7 +134,7 @@ def get_tenant_from_scope(websocket: WebSocket) -> str: match = re.match(r"^/t/([a-z0-9][a-z0-9-]*)/ws", path) if match: - print(f"[WS get_tenant_from_scope] Using fallback path extraction: {match.group(1)}") + logger.debug("get_tenant_from_scope: Using fallback path extraction: %s", match.group(1)) return match.group(1) return "default" @@ -144,7 +146,7 @@ async def websocket_endpoint(websocket: WebSocket): WebSocket endpoint for single-tenant mode. Handles connections at /ws. """ - print("[WS] >>> websocket_endpoint (/ws) called <<<") + logger.debug("websocket_endpoint (/ws) called") await handle_websocket_connection(websocket, tenant_slug=None) @@ -154,7 +156,7 @@ async def websocket_tenant_endpoint(websocket: WebSocket, tenant_slug: str): WebSocket endpoint for multi-tenant mode. Handles connections at /t/{tenant}/ws. """ - print(f"[WS] >>> websocket_tenant_endpoint (/t/{tenant_slug}/ws) called <<<") + logger.debug("websocket_tenant_endpoint (/t/%s/ws) called", tenant_slug) await handle_websocket_connection(websocket, tenant_slug=tenant_slug) @@ -166,7 +168,7 @@ async def handle_websocket_connection(websocket: WebSocket, tenant_slug: str | N websocket: The WebSocket connection tenant_slug: Tenant slug from URL path (None for single-tenant) """ - logger.info(f"WebSocket connection attempt from {websocket.client}") + logger.info("WebSocket connection attempt from %s", websocket.client) # Get session cookie cookies = websocket.cookies @@ -180,7 +182,7 @@ async def handle_websocket_connection(websocket: WebSocket, tenant_slug: str | N # Determine tenant - use URL slug or "default" for single-tenant tenant_id = tenant_slug if tenant_slug else "default" - print(f"[WS] Tenant: {tenant_id}, Session: {session_id[:20]}...") + logger.debug("Tenant: %s, Session: %s...", tenant_id, session_id[:20]) # Validate session and get user try: @@ -236,13 +238,10 @@ async def handle_websocket_connection(websocket: WebSocket, tenant_slug: str | N user_id = user.id first_name = user.first_name last_name = user.last_name - print(f"[WS] Authenticated: user {user_id} ({first_name} {last_name})") + logger.info("Authenticated: user %s (%s %s)", user_id, first_name, last_name) except Exception as e: - logger.error(f"Database error during WebSocket auth: {e}") - import traceback - - traceback.print_exc() + logger.exception("Database error during WebSocket auth: %s", e) await websocket.accept() await websocket.close(code=4002, reason="Authentication error") return @@ -256,7 +255,7 @@ async def handle_websocket_connection(websocket: WebSocket, tenant_slug: str | N last_name=last_name, ) - print(f"[WS] Starting receive loop for user {user_id}") + logger.debug("Starting receive loop for user %s", user_id) try: while True: @@ -277,11 +276,11 @@ async def handle_websocket_connection(websocket: WebSocket, tenant_slug: str | N ) except json.JSONDecodeError: - logger.warning(f"Invalid JSON from user {user_id}") + logger.warning("Invalid JSON from user %s", user_id) except WebSocketDisconnect as e: - print(f"[WS] Disconnected: user {user_id}, code={getattr(e, 'code', 'N/A')}") + logger.info("Disconnected: user %s, code=%s", user_id, getattr(e, "code", "N/A")) await manager.disconnect(tenant_id, user_id) except Exception as e: - print(f"[WS] Error for user {user_id}: {e}") + logger.error("Error for user %s: %s", user_id, e) await manager.disconnect(tenant_id, user_id) diff --git a/app/websocket/manager.py b/app/websocket/manager.py index 124e210..53c8819 100644 --- a/app/websocket/manager.py +++ b/app/websocket/manager.py @@ -18,9 +18,6 @@ logger = logging.getLogger(__name__) -# Enable debug logging for websocket -logging.getLogger(__name__).setLevel(logging.DEBUG) - @dataclass class ConnectedUser: @@ -55,7 +52,7 @@ def __init__(self): self._connections: dict[str, dict[int, ConnectedUser]] = {} # Lock for thread-safe operations self._lock = asyncio.Lock() - print("[WS Manager] Initialized") + logger.info("WebSocket Manager initialized") async def connect( self, @@ -76,23 +73,27 @@ async def connect( last_name: User's last name """ await websocket.accept() - print( - f"[WS Manager] Connection accepted for user {user_id} ({first_name} {last_name}) in tenant '{tenant_id}'" + logger.info( + "Connection accepted for user %s (%s %s) in tenant '%s'", + user_id, + first_name, + last_name, + tenant_id, ) # Immediately send a test message to verify connection is alive try: await websocket.send_text('{"type":"ping","test":"immediate"}') - print(f"[WS Manager] Immediate ping sent successfully to user {user_id}") + logger.debug("Immediate ping sent successfully to user %s", user_id) except Exception as e: - print(f"[WS Manager] ERROR: Immediate ping failed for user {user_id}: {e}") + logger.error("Immediate ping failed for user %s: %s", user_id, e) return old_websocket_to_close = None # Use lock only for modifying the connections dict - keep it short! async with self._lock: - print(f"[WS Manager] Lock acquired for user {user_id}") + logger.debug("Lock acquired for user %s", user_id) if tenant_id not in self._connections: self._connections[tenant_id] = {} @@ -101,7 +102,7 @@ async def connect( if user_id in self._connections[tenant_id]: old_conn = self._connections[tenant_id][user_id] old_websocket_to_close = old_conn.websocket - print(f"[WS Manager] Will close existing connection for user {user_id}") + logger.debug("Will close existing connection for user %s", user_id) # Store new connection connected_user = ConnectedUser( @@ -112,42 +113,37 @@ async def connect( ) self._connections[tenant_id][user_id] = connected_user - print( - f"[WS Manager] Total connections in tenant '{tenant_id}': {len(self._connections[tenant_id])}" + logger.debug( + "Total connections in tenant '%s': %d", tenant_id, len(self._connections[tenant_id]) ) - print(f"[WS Manager] Connected users: {list(self._connections[tenant_id].keys())}") + logger.debug("Connected users: %s", list(self._connections[tenant_id].keys())) - print(f"[WS Manager] Lock released for user {user_id}") + logger.debug("Lock released for user %s", user_id) # Note: We used to close old connections here, but that can cause issues # with the close() call interfering with the new connection. # Old connections will time out naturally or be garbage collected. if old_websocket_to_close: - print(f"[WS Manager] Old connection exists for user {user_id} - will timeout naturally") + logger.debug("Old connection exists for user %s - will timeout naturally", user_id) - print(f"[WS Manager] About to send presence list to user {user_id}") - logger.info(f"WebSocket connected: user={user_id} tenant={tenant_id}") + logger.debug("About to send presence list to user %s", user_id) + logger.info("WebSocket connected: user=%s tenant=%s", user_id, tenant_id) # Check if connection is still open before sending if websocket.client_state.name != "CONNECTED": - print( - f"[WS Manager] WARNING: WebSocket not connected! State: {websocket.client_state.name}" - ) + logger.warning("WebSocket not connected! State: %s", websocket.client_state.name) await self.disconnect(tenant_id, user_id) return - print(f"[WS Manager] WebSocket state: {websocket.client_state.name}") + logger.debug("WebSocket state: %s", websocket.client_state.name) # Send current online users to the new connection (with safety check) try: - print("[WS Manager] Calling _send_presence_list...") + logger.debug("Calling _send_presence_list...") await self._send_presence_list(websocket, tenant_id) - print("[WS Manager] Presence list sent successfully") + logger.debug("Presence list sent successfully") except Exception as e: - print(f"[WS Manager] Failed to send presence list: {e}") - import traceback - - traceback.print_exc() + logger.exception("Failed to send presence list: %s", e) # Connection may have failed, clean up await self.disconnect(tenant_id, user_id) return @@ -170,16 +166,18 @@ async def disconnect(self, tenant_id: str, user_id: int) -> None: async with self._lock: if tenant_id in self._connections: user = self._connections[tenant_id].pop(user_id, None) - print(f"[WS Manager] Disconnected user {user_id} from tenant '{tenant_id}'") - print( - f"[WS Manager] Remaining connections in tenant '{tenant_id}': {len(self._connections.get(tenant_id, {}))}" + logger.debug("Disconnected user %s from tenant '%s'", user_id, tenant_id) + logger.debug( + "Remaining connections in tenant '%s': %d", + tenant_id, + len(self._connections.get(tenant_id, {})), ) # Clean up empty tenant rooms if not self._connections[tenant_id]: del self._connections[tenant_id] - logger.info(f"WebSocket disconnected: user={user_id} tenant={tenant_id}") + logger.info("WebSocket disconnected: user=%s tenant=%s", user_id, tenant_id) # Broadcast leave event if user: @@ -199,7 +197,7 @@ async def _send_presence_list(self, websocket: WebSocket, tenant_id: str) -> Non if tenant_id in self._connections: users = [user.to_presence_dict() for user in self._connections[tenant_id].values()] - print(f"[WS Manager] Sending presence list with {len(users)} users") + logger.debug("Sending presence list with %d users", len(users)) await self._send_json( websocket, { @@ -227,8 +225,12 @@ async def broadcast_to_tenant( connections = self._connections.get(tenant_id, {}).copy() recipients = [uid for uid in connections.keys() if uid != exclude_user] - print( - f"[WS Manager] Broadcasting {message.get('type')} to tenant '{tenant_id}', recipients: {recipients} (excluding: {exclude_user})" + logger.debug( + "Broadcasting %s to tenant '%s', recipients: %s (excluding: %s)", + message.get("type"), + tenant_id, + recipients, + exclude_user, ) for user_id, connected_user in connections.items(): @@ -237,10 +239,9 @@ async def broadcast_to_tenant( try: await self._send_json(connected_user.websocket, message) - print(f"[WS Manager] Sent to user {user_id}") + logger.debug("Sent to user %s", user_id) except Exception as e: - print(f"[WS Manager] Failed to send to user {user_id}: {e}") - logger.warning(f"Failed to send to user {user_id}: {e}") + logger.warning("Failed to send to user %s: %s", user_id, e) # Don't remove here - let the receive loop handle disconnection async def broadcast_change( @@ -267,8 +268,13 @@ async def broadcast_change( action: Action type (create, update, delete, move) summary: Optional human-readable summary """ - print( - f"[WS Manager] broadcast_change called: {entity_type}:{action} by user {user_id} ({user_name}) in tenant '{tenant_id}'" + logger.debug( + "broadcast_change called: %s:%s by user %s (%s) in tenant '%s'", + entity_type, + action, + user_id, + user_name, + tenant_id, ) await self.broadcast_to_tenant( @@ -294,21 +300,21 @@ async def _send_json(self, websocket: WebSocket, data: dict) -> None: try: # Check if WebSocket is still connected if websocket.client_state.name != "CONNECTED": - print( - f"[WS Manager] Cannot send - WebSocket not connected (state: {websocket.client_state.name})" + logger.debug( + "Cannot send - WebSocket not connected (state: %s)", websocket.client_state.name ) return await websocket.send_text(json.dumps(data)) except RuntimeError as e: # Handle "Cannot call send once close message has been sent" - print(f"[WS Manager] WebSocket already closed: {e}") + logger.debug("WebSocket already closed: %s", e) except Exception as e: - print(f"[WS Manager] Error sending to WebSocket: {e}") + logger.warning("Error sending to WebSocket: %s", e) def get_online_count(self, tenant_id: str) -> int: """Get number of online users in a tenant.""" count = len(self._connections.get(tenant_id, {})) - print(f"[WS Manager] get_online_count('{tenant_id}'): {count}") + logger.debug("get_online_count('%s'): %d", tenant_id, count) return count def get_online_users(self, tenant_id: str) -> list[dict]: diff --git a/deploy-react.sh b/deploy-react.sh index c8d5023..2afd412 100644 --- a/deploy-react.sh +++ b/deploy-react.sh @@ -17,17 +17,30 @@ if [ ! -d "$FRONTEND_DIST" ]; then exit 1 fi -# Remove old vanilla JS files completely -echo "Removing old vanilla JS files..." -rm -rf "$PUBLIC_DIR/admin" 2>/dev/null || true -rm -rf "$PUBLIC_DIR/css" 2>/dev/null || true -rm -rf "$PUBLIC_DIR/js" 2>/dev/null || true -rm -rf "$PUBLIC_DIR/assets" 2>/dev/null || true -rm -f "$PUBLIC_DIR/index.html" 2>/dev/null || true - -# Copy React build to public -echo "Copying React build..." -cp -r "$FRONTEND_DIST"/* "$PUBLIC_DIR/" +# Verify the build has index.html before touching public/ +if [ ! -f "$FRONTEND_DIST/index.html" ]; then + echo "Error: frontend/dist/index.html not found. Build may have failed." + exit 1 +fi + +# Deploy atomically: copy to temp dir, verify, then swap +TEMP_DIR="$SCRIPT_DIR/.public_deploy_tmp" +rm -rf "$TEMP_DIR" +mkdir -p "$TEMP_DIR" + +echo "Copying React build to staging directory..." +cp -r "$FRONTEND_DIST"/* "$TEMP_DIR/" + +# Preserve non-build assets (e.g., images uploaded at runtime) +if [ -d "$PUBLIC_DIR/img" ] && [ ! -d "$TEMP_DIR/img" ]; then + cp -r "$PUBLIC_DIR/img" "$TEMP_DIR/img" +fi + +# Swap: remove old public contents and move new ones in +echo "Swapping into public/..." +rm -rf "$PUBLIC_DIR/admin" "$PUBLIC_DIR/css" "$PUBLIC_DIR/js" "$PUBLIC_DIR/assets" "$PUBLIC_DIR/index.html" 2>/dev/null || true +cp -r "$TEMP_DIR"/* "$PUBLIC_DIR/" +rm -rf "$TEMP_DIR" echo "" echo "=== Deployment Complete ===" diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 9dd80ef..7b20e61 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,5 +1,6 @@ import { useEffect, useState } from 'react'; import { useAppStore } from './stores/appStore'; +import { useWhatIfStore } from './stores/whatIfStore'; import { useAuth } from './hooks/useAuth'; import { useDataLoader } from './hooks/useDataLoader'; import { LoadingScreen } from './components/screens/LoadingScreen'; @@ -14,7 +15,7 @@ import './App.css'; function App() { const { isLoading, isAuthenticated } = useAuth(); const { loadAllData, isLoading: isDataLoading } = useDataLoader(); - const whatIfMode = useAppStore((state) => state.whatIfMode); + const whatIfMode = useWhatIfStore((state) => state.whatIfMode); const instanceSettings = useAppStore((state) => state.instanceSettings); // Check if we're on the admin route diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index ee7e1c3..3ac7f3b 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -103,8 +103,6 @@ export async function apiRequest( const isAllowed = allowedPrefixes.some(prefix => url.includes(prefix)); if (!isAllowed) { - console.log(`[What If Mode] Queued ${method} request to ${url}`); - // Queue the operation for later execution when applying changes if (clientConfig.queueWhatIfOperation) { // Store the relative URL (without base) for replay diff --git a/frontend/src/api/endpoints/auth.ts b/frontend/src/api/endpoints/auth.ts index 4e45d4b..0f7df0b 100644 --- a/frontend/src/api/endpoints/auth.ts +++ b/frontend/src/api/endpoints/auth.ts @@ -8,19 +8,19 @@ import type { User, AuthResponse } from '@/types'; /** * Transform user from API camelCase to frontend snake_case */ -function transformUser(apiUser: any): User { +function transformUser(apiUser: Record): User { return { - id: apiUser.id, - email: apiUser.email, - first_name: apiUser.firstName, - last_name: apiUser.lastName, - name: apiUser.name, - job_title: apiUser.jobTitle, - role: apiUser.role, - max_capacity: apiUser.maxCapacity ?? apiUser.max_capacity ?? 100, + id: apiUser.id as number, + email: apiUser.email as string, + first_name: apiUser.firstName as string, + last_name: apiUser.lastName as string, + name: apiUser.name as string, + job_title: apiUser.jobTitle as string | undefined, + role: apiUser.role as User['role'], + max_capacity: (apiUser.maxCapacity ?? apiUser.max_capacity ?? 100) as number, active: true, - site_ids: apiUser.siteIds || [], - skills: apiUser.skills || [], + site_ids: (apiUser.siteIds || []) as number[], + skills: (apiUser.skills || []) as User['skills'], }; } @@ -30,7 +30,7 @@ function transformUser(apiUser: any): User { */ export async function checkAuth(): Promise<{ user: User | null }> { try { - const response = await apiGet<{ user: any }>('/api/auth/me'); + const response = await apiGet<{ user: Record | null }>('/api/auth/me'); if (response.user) { return { user: transformUser(response.user) }; } @@ -44,7 +44,7 @@ export async function checkAuth(): Promise<{ user: User | null }> { * Login with email and password */ export async function login(email: string, password: string): Promise { - const response = await apiPost<{ success: boolean; user: any }>('/api/auth/login', { email, password }); + const response = await apiPost<{ success: boolean; user: Record }>('/api/auth/login', { email, password }); return { user: transformUser(response.user), }; diff --git a/frontend/src/api/endpoints/equipment.ts b/frontend/src/api/endpoints/equipment.ts index 1107636..5d3d611 100644 --- a/frontend/src/api/endpoints/equipment.ts +++ b/frontend/src/api/endpoints/equipment.ts @@ -10,10 +10,11 @@ import type { Equipment } from '@/types'; * @param includeAllSites - If true, return equipment from all sites */ export async function getEquipment(includeAllSites = false): Promise { - const url = includeAllSites + const url = includeAllSites ? '/api/equipment?includeAllSites=true' : '/api/equipment'; - return apiGet(url); + const response = await apiGet<{ items: Equipment[]; total: number; offset: number; limit: number }>(url); + return response.items; } /** diff --git a/frontend/src/api/endpoints/projects.ts b/frontend/src/api/endpoints/projects.ts index b647219..db8c96f 100644 --- a/frontend/src/api/endpoints/projects.ts +++ b/frontend/src/api/endpoints/projects.ts @@ -27,7 +27,8 @@ import type { * @param archived - Filter: 'all', 'true', 'false' (default: active only) */ export async function getProjects(archived: 'all' | 'true' | 'false' = 'false'): Promise { - return apiGet(`/api/projects?archived=${archived}`); + const response = await apiGet<{ items: Project[]; total: number; offset: number; limit: number }>(`/api/projects?archived=${archived}`); + return response.items; } /** @@ -320,13 +321,16 @@ export async function deleteEquipmentAssignment(assignmentId: number): Promise): Project { + const phases = (project.phases ?? []) as Record[]; + const staffAssignments = (project.staff_assignments ?? project.staffAssignments ?? []) as Record[]; + const equipmentAssignments = (project.equipment_assignments ?? project.equipmentAssignments ?? []) as Record[]; return { - ...project, + ...(project as unknown as Project), // Ensure arrays exist and transform nested data - phases: (project.phases ?? []).map(transformPhase), - staffAssignments: (project.staff_assignments ?? project.staffAssignments ?? []).map(transformStaffAssignment), - equipmentAssignments: (project.equipment_assignments ?? project.equipmentAssignments ?? []).map(transformEquipmentAssignment), + phases: phases.map(transformPhase), + staffAssignments: staffAssignments.map(transformStaffAssignment), + equipmentAssignments: equipmentAssignments.map(transformEquipmentAssignment), }; } @@ -366,71 +370,77 @@ function getPhaseColorWithFallback(): string { } } -function transformPhase(phase: any): Phase { +function transformPhase(phase: Record): Phase { + const children = (phase.children ?? phase.subphases ?? []) as Record[]; + const staffAssignments = (phase.staff_assignments ?? phase.staffAssignments ?? []) as Record[]; + const equipmentAssignments = (phase.equipment_assignments ?? phase.equipmentAssignments ?? []) as Record[]; return { - id: phase.id, - project_id: phase.project_id, - name: phase.type || phase.name || 'Phase', // API returns 'type' for phases - start_date: phase.start_date, - end_date: phase.end_date, + id: phase.id as number, + project_id: phase.project_id as number, + name: (phase.type || phase.name || 'Phase') as string, // API returns 'type' for phases + start_date: phase.start_date as string, + end_date: phase.end_date as string, color: getPhaseColorWithFallback(), - order_index: phase.sort_order ?? 0, - completion: phase.completion ?? null, + order_index: (phase.sort_order ?? 0) as number, + completion: (phase.completion ?? null) as number | null, // Handle SQLite integer (0/1) and boolean is_milestone: Boolean(phase.is_milestone), - dependencies: phase.dependencies ?? [], - children: (phase.children ?? phase.subphases ?? []).map((s: any) => transformSubphase(s, 1)), - staffAssignments: (phase.staff_assignments ?? phase.staffAssignments ?? []).map(transformStaffAssignment), - equipmentAssignments: (phase.equipment_assignments ?? phase.equipmentAssignments ?? []).map(transformEquipmentAssignment), + dependencies: (phase.dependencies ?? []) as Phase['dependencies'], + children: children.map((s: Record) => transformSubphase(s, 1)), + staffAssignments: staffAssignments.map(transformStaffAssignment), + equipmentAssignments: equipmentAssignments.map(transformEquipmentAssignment), }; } -function transformSubphase(subphase: any, depth: number = 1): Subphase { - const actualDepth = subphase.depth ?? depth; +function transformSubphase(subphase: Record, depth: number = 1): Subphase { + const actualDepth = (subphase.depth ?? depth) as number; + const children = (subphase.children ?? []) as Record[]; + const staffAssignments = (subphase.staff_assignments ?? subphase.staffAssignments ?? []) as Record[]; + const equipmentAssignments = (subphase.equipment_assignments ?? subphase.equipmentAssignments ?? []) as Record[]; return { - id: subphase.id, - project_id: subphase.project_id, - parent_phase_id: subphase.parent_type === 'phase' ? subphase.parent_id : null, - parent_subphase_id: subphase.parent_type === 'subphase' ? subphase.parent_id : null, - name: subphase.name, - start_date: subphase.start_date, - end_date: subphase.end_date, + id: subphase.id as number, + project_id: subphase.project_id as number, + parent_phase_id: subphase.parent_type === 'phase' ? subphase.parent_id as number : null, + parent_subphase_id: subphase.parent_type === 'subphase' ? subphase.parent_id as number : null, + name: subphase.name as string, + start_date: subphase.start_date as string, + end_date: subphase.end_date as string, color: getDepthColorWithFallback(actualDepth), - order_index: subphase.sort_order ?? 0, - completion: subphase.completion ?? null, + order_index: (subphase.sort_order ?? 0) as number, + completion: (subphase.completion ?? null) as number | null, // Handle SQLite integer (0/1) and boolean is_milestone: Boolean(subphase.is_milestone), - dependencies: subphase.dependencies ?? [], - children: (subphase.children ?? []).map((s: any) => transformSubphase(s, actualDepth + 1)), - staffAssignments: (subphase.staff_assignments ?? subphase.staffAssignments ?? []).map(transformStaffAssignment), - equipmentAssignments: (subphase.equipment_assignments ?? subphase.equipmentAssignments ?? []).map(transformEquipmentAssignment), + dependencies: (subphase.dependencies ?? []) as Subphase['dependencies'], + children: children.map((s: Record) => transformSubphase(s, actualDepth + 1)), + staffAssignments: staffAssignments.map(transformStaffAssignment), + equipmentAssignments: equipmentAssignments.map(transformEquipmentAssignment), }; } -function transformStaffAssignment(assignment: any): StaffAssignment { +function transformStaffAssignment(assignment: Record): StaffAssignment { return { - id: assignment.id, - staff_id: assignment.staff_id, - staff_name: assignment.staff_name, - project_id: assignment.project_id, - phase_id: assignment.phase_id, - subphase_id: assignment.subphase_id, - allocation: assignment.allocation ?? assignment.allocation_percent ?? 100, - start_date: assignment.start_date, - end_date: assignment.end_date, + id: assignment.id as number, + staff_id: assignment.staff_id as number, + staff_name: assignment.staff_name as string | undefined, + project_id: assignment.project_id as number | null | undefined, + phase_id: assignment.phase_id as number | null | undefined, + subphase_id: assignment.subphase_id as number | null | undefined, + allocation: (assignment.allocation ?? assignment.allocation_percent ?? 100) as number, + start_date: assignment.start_date as string, + end_date: assignment.end_date as string, }; } -function transformEquipmentAssignment(assignment: any): EquipmentAssignment { +function transformEquipmentAssignment(assignment: Record): EquipmentAssignment { return { - id: assignment.id, - equipment_id: assignment.equipment_id, - equipment_name: assignment.equipment_name, - project_id: assignment.project_id, - phase_id: assignment.phase_id, - subphase_id: assignment.subphase_id, - start_date: assignment.start_date, - end_date: assignment.end_date, + id: assignment.id as number, + equipment_id: assignment.equipment_id as number, + equipment_name: assignment.equipment_name as string | undefined, + project_id: assignment.project_id as number | null | undefined, + phase_id: assignment.phase_id as number | null | undefined, + subphase_id: assignment.subphase_id as number | null | undefined, + start_date: assignment.start_date as string, + end_date: assignment.end_date as string, }; } @@ -451,5 +461,5 @@ export async function loadAllProjects(): Promise { ); // Transform to ensure correct property names - return fullProjects.map(transformProject); + return fullProjects.map(p => transformProject(p as unknown as Record)); } diff --git a/frontend/src/api/endpoints/users.ts b/frontend/src/api/endpoints/users.ts index 6bdca39..ef9dffb 100644 --- a/frontend/src/api/endpoints/users.ts +++ b/frontend/src/api/endpoints/users.ts @@ -10,10 +10,11 @@ import type { User, CreateUserRequest, UpdateUserRequest } from '@/types'; * @param includeDisabled - If true, include disabled users */ export async function getUsers(includeDisabled = false): Promise { - const url = includeDisabled + const url = includeDisabled ? '/api/users?includeDisabled=true' : '/api/users'; - return apiGet(url); + const response = await apiGet<{ items: User[]; total: number; offset: number; limit: number }>(url); + return response.items; } /** diff --git a/frontend/src/components/admin/AdminApp.tsx b/frontend/src/components/admin/AdminApp.tsx index a9ea1ef..ad031c1 100644 --- a/frontend/src/components/admin/AdminApp.tsx +++ b/frontend/src/components/admin/AdminApp.tsx @@ -27,7 +27,7 @@ export function AdminApp() { } } catch (err) { // Not authenticated - console.log('Admin not authenticated'); + // Not authenticated - no action needed } finally { setIsLoading(false); } diff --git a/frontend/src/components/common/OnlineUsers/OnlineUsers.tsx b/frontend/src/components/common/OnlineUsers/OnlineUsers.tsx index d6f191e..57ba617 100644 --- a/frontend/src/components/common/OnlineUsers/OnlineUsers.tsx +++ b/frontend/src/components/common/OnlineUsers/OnlineUsers.tsx @@ -54,9 +54,6 @@ export const OnlineUsers = memo(function OnlineUsers() { const { connectionState, onlineUsers, isConnected } = useWebSocketContext(); const [showDropdown, setShowDropdown] = useState(false); - // Debug logging - console.log('[OnlineUsers] connectionState:', connectionState, 'isConnected:', isConnected, 'users:', onlineUsers.length); - // Don't show anything if not connected or no other users if (!isConnected && connectionState !== 'connecting') { return null; diff --git a/frontend/src/components/gantt/ContextMenuContainer.tsx b/frontend/src/components/gantt/ContextMenuContainer.tsx index 49ef600..0952df6 100644 --- a/frontend/src/components/gantt/ContextMenuContainer.tsx +++ b/frontend/src/components/gantt/ContextMenuContainer.tsx @@ -9,6 +9,7 @@ import { useAppStore } from '@/stores/appStore'; import { useDataLoader } from '@/hooks/useDataLoader'; import { ContextMenu, type ContextMenuItem } from '@/components/common'; import { deleteProject, deletePhase, deleteSubphase, deleteStaffAssignment, deleteEquipmentAssignment } from '@/api'; +import type { Project, Phase, Subphase } from '@/types'; export function ContextMenuContainer() { const contextMenu = useUIStore((s) => s.contextMenu); @@ -56,7 +57,7 @@ export function ContextMenuContainer() { // Project context menu if (type === 'project') { - const project = target as any; + const project = target as Project; return [ { label: 'Edit Project', @@ -108,7 +109,7 @@ export function ContextMenuContainer() { // Phase context menu if (type === 'phase') { - const phase = target as any; + const phase = target as Phase; return [ { label: 'Edit Phase', @@ -160,7 +161,7 @@ export function ContextMenuContainer() { // Subphase context menu if (type === 'subphase') { - const subphase = target as any; + const subphase = target as Subphase; return [ { label: 'Edit Subphase', diff --git a/frontend/src/components/gantt/CustomColumns/CustomColumnCell.tsx b/frontend/src/components/gantt/CustomColumns/CustomColumnCell.tsx index ba1c1ae..01a6fba 100644 --- a/frontend/src/components/gantt/CustomColumns/CustomColumnCell.tsx +++ b/frontend/src/components/gantt/CustomColumns/CustomColumnCell.tsx @@ -6,7 +6,7 @@ */ import { useState, useRef, useEffect, useCallback } from 'react'; -import { useAppStore } from '@/stores/appStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { setCustomColumnValue as setCustomColumnValueApi, getValueKey } from '@/api'; import type { CustomColumn, CustomColumnEntityType } from '@/types'; import styles from './CustomColumnCell.module.css'; @@ -33,8 +33,8 @@ export function CustomColumnCell({ readOnly = false, childEntities = [], }: CustomColumnCellProps) { - const customColumnValues = useAppStore((s) => s.customColumnValues); - const setCustomColumnValueLocal = useAppStore((s) => s.setCustomColumnValue); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); + const setCustomColumnValueLocal = useCustomColumnStore((s) => s.setCustomColumnValue); const valueKey = getValueKey(column.id, entityType, entityId); const value = customColumnValues[valueKey] ?? null; diff --git a/frontend/src/components/gantt/CustomColumns/CustomColumnHeader.tsx b/frontend/src/components/gantt/CustomColumns/CustomColumnHeader.tsx index 00ced5b..6ca16f6 100644 --- a/frontend/src/components/gantt/CustomColumns/CustomColumnHeader.tsx +++ b/frontend/src/components/gantt/CustomColumns/CustomColumnHeader.tsx @@ -5,7 +5,7 @@ import { useRef, useState, useCallback, useEffect, useMemo } from 'react'; import { updateCustomColumn } from '@/api'; -import { useAppStore } from '@/stores/appStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import type { CustomColumn } from '@/types'; import styles from './CustomColumnHeader.module.css'; @@ -24,10 +24,10 @@ export function CustomColumnHeader({ const [isResizing, setIsResizing] = useState(false); const [showFilter, setShowFilter] = useState(false); - const customColumnValues = useAppStore((s) => s.customColumnValues); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); - const setCustomColumnFilter = useAppStore((s) => s.setCustomColumnFilter); - const clearCustomColumnFilter = useAppStore((s) => s.clearCustomColumnFilter); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); + const setCustomColumnFilter = useCustomColumnStore((s) => s.setCustomColumnFilter); + const clearCustomColumnFilter = useCustomColumnStore((s) => s.clearCustomColumnFilter); const activeFilter = customColumnFilters[column.id] || []; const hasActiveFilter = activeFilter.length > 0; diff --git a/frontend/src/components/gantt/GanttContainer.tsx b/frontend/src/components/gantt/GanttContainer.tsx index 10ba541..0a61688 100644 --- a/frontend/src/components/gantt/GanttContainer.tsx +++ b/frontend/src/components/gantt/GanttContainer.tsx @@ -6,6 +6,8 @@ import { useRef, useMemo, useState, useCallback } from 'react'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { ProjectPanel } from './ProjectPanel'; import { Timeline } from './Timeline'; import { useScrollSync } from './hooks/useScrollSync'; @@ -24,19 +26,19 @@ export function GanttContainer() { const projects = useAppStore((s) => s.projects); const currentSite = useAppStore((s) => s.currentSite); - const viewMode = useAppStore((s) => s.viewMode); - const currentDate = useAppStore((s) => s.currentDate); - const cellWidth = useAppStore((s) => s.cellWidth); const bankHolidayDates = useAppStore((s) => s.bankHolidayDates); const bankHolidays = useAppStore((s) => s.bankHolidays); const companyEventDates = useAppStore((s) => s.companyEventDates); const companyEvents = useAppStore((s) => s.companyEvents); - const customColumns = useAppStore((s) => s.customColumns); - const showAllCustomColumns = useAppStore((s) => s.showAllCustomColumns); - const hiddenCustomColumns = useAppStore((s) => s.hiddenCustomColumns); - const showStaffOverview = useAppStore((s) => s.showStaffOverview); - const showEquipmentOverview = useAppStore((s) => s.showEquipmentOverview); const currentUser = useAppStore((s) => s.currentUser); + const viewMode = useViewStore((s) => s.viewMode); + const currentDate = useViewStore((s) => s.currentDate); + const cellWidth = useViewStore((s) => s.cellWidth); + const showStaffOverview = useViewStore((s) => s.showStaffOverview); + const showEquipmentOverview = useViewStore((s) => s.showEquipmentOverview); + const customColumns = useCustomColumnStore((s) => s.customColumns); + const showAllCustomColumns = useCustomColumnStore((s) => s.showAllCustomColumns); + const hiddenCustomColumns = useCustomColumnStore((s) => s.hiddenCustomColumns); // Only superusers/admins can see overview panels const canShowOverview = currentUser?.role === 'superuser' || currentUser?.role === 'admin'; diff --git a/frontend/src/components/gantt/ProjectPanel/PhaseRow.tsx b/frontend/src/components/gantt/ProjectPanel/PhaseRow.tsx index c391904..1ffc434 100644 --- a/frontend/src/components/gantt/ProjectPanel/PhaseRow.tsx +++ b/frontend/src/components/gantt/ProjectPanel/PhaseRow.tsx @@ -6,6 +6,8 @@ import { memo, useMemo, useCallback, Fragment, useState } from 'react'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useUIStore } from '@/stores/uiStore'; import { useReorder } from '@/contexts/ReorderContext'; import { SubphaseRow } from './SubphaseRow'; @@ -105,12 +107,12 @@ export const PhaseRow = memo(function PhaseRow({ nameColumnWidth, criticalPathItems = new Set(), }: PhaseRowProps) { - const expandedPhases = useAppStore((s) => s.expandedPhases); - const togglePhaseExpanded = useAppStore((s) => s.togglePhaseExpanded); + const expandedPhases = useViewStore((s) => s.expandedPhases); + const togglePhaseExpanded = useViewStore((s) => s.togglePhaseExpanded); const staff = useAppStore((s) => s.staff); const equipment = useAppStore((s) => s.equipment); const currentUser = useAppStore((s) => s.currentUser); - const showAssignments = useAppStore((s) => s.showAssignments); + const showAssignments = useViewStore((s) => s.showAssignments); const projects = useAppStore((s) => s.projects); const setProjects = useAppStore((s) => s.setProjects); const openPhaseModal = useUIStore((s) => s.openPhaseModal); @@ -419,12 +421,12 @@ function PhaseSubphasesWithPhantom({ criticalPathItems: Set; }) { const phantomSiblingMode = useUIStore((s) => s.phantomSiblingMode); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); - const customColumnValues = useAppStore((s) => s.customColumnValues); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); const { completePhantom, cancelPhantom } = usePhantomSibling(); - + // Check if phantom mode is for a subphase under this phase - const showPhantomAfter = phantomSiblingMode?.type === 'subphase' && + const showPhantomAfter = phantomSiblingMode?.type === 'subphase' && phantomSiblingMode.projectId === projectId && phantomSiblingMode.parentType === 'phase' && phantomSiblingMode.parentId === phaseId diff --git a/frontend/src/components/gantt/ProjectPanel/ProjectPanel.tsx b/frontend/src/components/gantt/ProjectPanel/ProjectPanel.tsx index 2da61b4..77c9c8f 100644 --- a/frontend/src/components/gantt/ProjectPanel/ProjectPanel.tsx +++ b/frontend/src/components/gantt/ProjectPanel/ProjectPanel.tsx @@ -7,6 +7,8 @@ import { forwardRef, useState, useCallback, useRef, useEffect } from 'react'; import { ProjectRow } from './ProjectRow'; import { useUIStore } from '@/stores/uiStore'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useDataLoader } from '@/hooks'; import { ReorderProvider } from '@/contexts/ReorderContext'; import { CustomColumnModal, CustomColumnsManagerModal } from '@/components/modals'; @@ -26,14 +28,14 @@ export const ProjectPanel = forwardRef( const setActiveModal = useUIStore((s) => s.setActiveModal); const currentUser = useAppStore((s) => s.currentUser); const currentSite = useAppStore((s) => s.currentSite); - const customColumns = useAppStore((s) => s.customColumns); - const setCustomColumns = useAppStore((s) => s.setCustomColumns); - const showAssignments = useAppStore((s) => s.showAssignments); - const toggleShowAssignments = useAppStore((s) => s.toggleShowAssignments); - const showAllCustomColumns = useAppStore((s) => s.showAllCustomColumns); - const hiddenCustomColumns = useAppStore((s) => s.hiddenCustomColumns); - const setShowAllCustomColumns = useAppStore((s) => s.setShowAllCustomColumns); - const toggleCustomColumnVisibility = useAppStore((s) => s.toggleCustomColumnVisibility); + const showAssignments = useViewStore((s) => s.showAssignments); + const toggleShowAssignments = useViewStore((s) => s.toggleShowAssignments); + const customColumns = useCustomColumnStore((s) => s.customColumns); + const setCustomColumns = useCustomColumnStore((s) => s.setCustomColumns); + const showAllCustomColumns = useCustomColumnStore((s) => s.showAllCustomColumns); + const hiddenCustomColumns = useCustomColumnStore((s) => s.hiddenCustomColumns); + const setShowAllCustomColumns = useCustomColumnStore((s) => s.setShowAllCustomColumns); + const toggleCustomColumnVisibility = useCustomColumnStore((s) => s.toggleCustomColumnVisibility); const { refreshCustomColumns } = useDataLoader(); diff --git a/frontend/src/components/gantt/ProjectPanel/ProjectRow.tsx b/frontend/src/components/gantt/ProjectPanel/ProjectRow.tsx index fff1132..38f47fa 100644 --- a/frontend/src/components/gantt/ProjectPanel/ProjectRow.tsx +++ b/frontend/src/components/gantt/ProjectPanel/ProjectRow.tsx @@ -5,6 +5,8 @@ import { memo, useMemo, useCallback, Fragment, useState } from 'react'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useUIStore } from '@/stores/uiStore'; import { PhaseRow } from './PhaseRow'; import { AssignmentRow } from './AssignmentRow'; @@ -101,18 +103,19 @@ export const ProjectRow = memo(function ProjectRow({ siblingIds = [], index: _index = 0, }: ProjectRowProps) { - const expandedProjects = useAppStore((s) => s.expandedProjects); - const expandedPhases = useAppStore((s) => s.expandedPhases); - const expandedSubphases = useAppStore((s) => s.expandedSubphases); - const toggleProjectExpanded = useAppStore((s) => s.toggleProjectExpanded); - const expandProjectLevel = useAppStore((s) => s.expandProjectLevel); - const collapseProjectLevel = useAppStore((s) => s.collapseProjectLevel); + const expandedProjects = useViewStore((s) => s.expandedProjects); + const expandedPhases = useViewStore((s) => s.expandedPhases); + const expandedSubphases = useViewStore((s) => s.expandedSubphases); + const toggleProjectExpanded = useViewStore((s) => s.toggleProjectExpanded); + const expandProjectLevel = useViewStore((s) => s.expandProjectLevel); + const collapseProjectLevel = useViewStore((s) => s.collapseProjectLevel); const staff = useAppStore((s) => s.staff); const equipment = useAppStore((s) => s.equipment); - const showAssignments = useAppStore((s) => s.showAssignments); + const showAssignments = useViewStore((s) => s.showAssignments); const toggleCriticalPath = useAppStore((s) => s.toggleCriticalPath); const isCriticalPathEnabled = useAppStore((s) => s.isCriticalPathEnabled); const criticalPathItems = useAppStore((s) => s.criticalPathItems); + const projects = useAppStore((s) => s.projects); const openProjectModal = useUIStore((s) => s.openProjectModal); const isExpanded = expandedProjects.has(project.id); @@ -175,12 +178,12 @@ export const ProjectRow = memo(function ProjectRow({ const handleExpandLevel = (e: React.MouseEvent) => { e.stopPropagation(); - expandProjectLevel(project.id); + expandProjectLevel(project.id, projects); }; const handleCollapseLevel = (e: React.MouseEvent) => { e.stopPropagation(); - collapseProjectLevel(project.id); + collapseProjectLevel(project.id, projects); }; const handleCriticalPathToggle = (e: React.MouseEvent) => { @@ -470,8 +473,8 @@ function PhasesWithPhantom({ criticalPathItems: Set; }) { const phantomSiblingMode = useUIStore((s) => s.phantomSiblingMode); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); - const customColumnValues = useAppStore((s) => s.customColumnValues); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); const { completePhantom, cancelPhantom } = usePhantomSibling(); // Check if phantom mode is for a phase in this project diff --git a/frontend/src/components/gantt/ProjectPanel/SubphaseRow.tsx b/frontend/src/components/gantt/ProjectPanel/SubphaseRow.tsx index 4b06b02..2846f8e 100644 --- a/frontend/src/components/gantt/ProjectPanel/SubphaseRow.tsx +++ b/frontend/src/components/gantt/ProjectPanel/SubphaseRow.tsx @@ -7,6 +7,8 @@ import { memo, useMemo, useCallback, useState, Fragment } from 'react'; import { createPortal } from 'react-dom'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useUIStore } from '@/stores/uiStore'; import { useReorder } from '@/contexts/ReorderContext'; import { AssignmentRow } from './AssignmentRow'; @@ -113,12 +115,12 @@ export const SubphaseRow = memo(function SubphaseRow({ nameColumnWidth, criticalPathItems = new Set(), }: SubphaseRowProps) { - const expandedSubphases = useAppStore((s) => s.expandedSubphases); - const toggleSubphaseExpanded = useAppStore((s) => s.toggleSubphaseExpanded); + const expandedSubphases = useViewStore((s) => s.expandedSubphases); + const toggleSubphaseExpanded = useViewStore((s) => s.toggleSubphaseExpanded); const staff = useAppStore((s) => s.staff); const equipment = useAppStore((s) => s.equipment); const currentUser = useAppStore((s) => s.currentUser); - const showAssignments = useAppStore((s) => s.showAssignments); + const showAssignments = useViewStore((s) => s.showAssignments); const openSubphaseModal = useUIStore((s) => s.openSubphaseModal); const triggerScrollToDate = useUIStore((s) => s.triggerScrollToDate); const projects = useAppStore((s) => s.projects); @@ -467,8 +469,8 @@ function SubphasesWithPhantom({ criticalPathItems: Set; }) { const phantomSiblingMode = useUIStore((s) => s.phantomSiblingMode); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); - const customColumnValues = useAppStore((s) => s.customColumnValues); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); const { completePhantom, cancelPhantom } = usePhantomSibling(); // Check if phantom mode is for a subphase diff --git a/frontend/src/components/gantt/Timeline/DependencyLayer.tsx b/frontend/src/components/gantt/Timeline/DependencyLayer.tsx index 5dafc5e..78f587e 100644 --- a/frontend/src/components/gantt/Timeline/DependencyLayer.tsx +++ b/frontend/src/components/gantt/Timeline/DependencyLayer.tsx @@ -5,6 +5,8 @@ import { memo, useMemo, useEffect, useState, useRef, useCallback } from 'react'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useUIStore } from '@/stores/uiStore'; import { extractDependencies, @@ -15,7 +17,7 @@ import { import { calculateBarPosition } from '../utils/timeline'; import { getPhaseColor } from '@/utils/themeColors'; import type { TimelineCell } from '../utils/timeline'; -import type { Project, DependencyType } from '@/types'; +import type { Project, DependencyType, Subphase, Dependency } from '@/types'; import { DependencyPopup } from './DependencyPopup'; import { updatePhase, updateSubphase, loadAllProjects } from '@/api/endpoints/projects'; import styles from './DependencyLayer.module.css'; @@ -47,11 +49,11 @@ export const DependencyLayer = memo(function DependencyLayer({ cellWidth, rowPositions, }: DependencyLayerProps) { - // Get viewMode and setProjects from store - const viewMode = useAppStore((s) => s.viewMode); + // Get viewMode and setProjects from stores + const viewMode = useViewStore((s) => s.viewMode); const setProjects = useAppStore((s) => s.setProjects); - const showAssignments = useAppStore((s) => s.showAssignments); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); + const showAssignments = useViewStore((s) => s.showAssignments); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); // Get drag state to re-measure after drag ends const isDragging = useUIStore((s) => s.isDragging); @@ -314,11 +316,11 @@ export const DependencyLayer = memo(function DependencyLayer({ }; } else { // Update subphase recursively - const updateSubphaseInTree = (subphases: any[]): any[] => { + const updateSubphaseInTree = (subphases: Subphase[]): Subphase[] => { return subphases.map(sp => { if (sp.id === selectedDep.toId) { const newDeps = (sp.dependencies ?? []).filter( - (d: any) => !(d.id === selectedDep.fromId && d.type === selectedDep.type) + (d: Dependency) => !(d.id === selectedDep.fromId && d.type === selectedDep.type) ); return { ...sp, dependencies: newDeps }; } @@ -361,7 +363,7 @@ export const DependencyLayer = memo(function DependencyLayer({ } } else { // Find subphase - const findSubphase = (subphases: any[]): any | null => { + const findSubphase = (subphases: Subphase[]): Subphase | null => { for (const sp of subphases) { if (sp.id === selectedDep.toId) return sp; if (sp.children?.length) { @@ -372,7 +374,7 @@ export const DependencyLayer = memo(function DependencyLayer({ return null; }; - let targetSubphase: any = null; + let targetSubphase: Subphase | null = null; for (const phase of targetProject.phases ?? []) { targetSubphase = findSubphase(phase.children ?? []); if (targetSubphase) break; @@ -380,7 +382,7 @@ export const DependencyLayer = memo(function DependencyLayer({ if (targetSubphase) { const newDeps = (targetSubphase.dependencies ?? []).filter( - (d: any) => !(d.id === selectedDep.fromId && d.type === selectedDep.type) + (d: Dependency) => !(d.id === selectedDep.fromId && d.type === selectedDep.type) ); await updateSubphase(selectedDep.toId, { start_date: targetSubphase.start_date, diff --git a/frontend/src/components/gantt/Timeline/ProjectTimeline.tsx b/frontend/src/components/gantt/Timeline/ProjectTimeline.tsx index f288212..30f1461 100644 --- a/frontend/src/components/gantt/Timeline/ProjectTimeline.tsx +++ b/frontend/src/components/gantt/Timeline/ProjectTimeline.tsx @@ -8,9 +8,11 @@ import { ProjectBar } from './ProjectBar'; import { PhaseBar } from './PhaseBar'; import { calculateBarPosition } from '../utils'; import type { TimelineCell } from '../utils'; -import type { Project, Phase, Subphase, ViewMode } from '@/types'; +import type { Project, Phase, Subphase, ViewMode, StaffAssignment, EquipmentAssignment } from '@/types'; import { useUIStore } from '@/stores/uiStore'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useDragAndDrop } from '@/hooks/useDragAndDrop'; import { useResize } from '@/hooks/useResize'; import { useDependencyLinking } from '@/hooks/useDependencyLinking'; @@ -154,12 +156,12 @@ export const ProjectTimeline = memo(function ProjectTimeline({ const { openPhaseModal, openSubphaseModal, setEditingPhase, setEditingSubphase } = useUIStore(); const phantomSiblingMode = useUIStore((s) => s.phantomSiblingMode); - // Get viewMode and filter state from app store - const viewMode = useAppStore((s) => s.viewMode); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); - const customColumnValues = useAppStore((s) => s.customColumnValues); - const showAssignments = useAppStore((s) => s.showAssignments); - + // Get viewMode and filter state from stores + const viewMode = useViewStore((s) => s.viewMode); + const showAssignments = useViewStore((s) => s.showAssignments); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); + // Get critical path state const criticalPathEnabled = useAppStore((s) => s.criticalPathEnabled); const criticalPathItems = useAppStore((s) => s.criticalPathItems); @@ -626,7 +628,7 @@ const PhaseTimeline = memo(function PhaseTimeline({ })} {/* Phase-level assignments - use phase dates since phase assignments don't have their own - filtered */} - {showAssignments && (phase.staffAssignments ?? []).map((assignment: any) => ( + {showAssignments && (phase.staffAssignments ?? []).map((assignment: StaffAssignment) => (
))} - {showAssignments && (phase.equipmentAssignments ?? []).map((assignment: any) => ( + {showAssignments && (phase.equipmentAssignments ?? []).map((assignment: EquipmentAssignment) => (
( + {showAssignments && (subphase.staffAssignments ?? []).map((assignment: StaffAssignment) => (
))} - {showAssignments && (subphase.equipmentAssignments ?? []).map((assignment: any) => ( + {showAssignments && (subphase.equipmentAssignments ?? []).map((assignment: EquipmentAssignment) => (
s.endResourceDrag); const setProjects = useAppStore((s) => s.setProjects); const staff = useAppStore((s) => s.staff); - const ensureProjectExpanded = useAppStore((s) => s.ensureProjectExpanded); - const ensurePhaseExpanded = useAppStore((s) => s.ensurePhaseExpanded); - const ensureSubphaseExpanded = useAppStore((s) => s.ensureSubphaseExpanded); + const ensureProjectExpanded = useViewStore((s) => s.ensureProjectExpanded); + const ensurePhaseExpanded = useViewStore((s) => s.ensurePhaseExpanded); + const ensureSubphaseExpanded = useViewStore((s) => s.ensureSubphaseExpanded); const overlayRef = useRef(null); // Track position for preview bar diff --git a/frontend/src/components/gantt/Timeline/Timeline.tsx b/frontend/src/components/gantt/Timeline/Timeline.tsx index 4b53465..d29b70b 100644 --- a/frontend/src/components/gantt/Timeline/Timeline.tsx +++ b/frontend/src/components/gantt/Timeline/Timeline.tsx @@ -8,7 +8,7 @@ import { forwardRef, useRef, useEffect, useCallback, useState } from 'react'; import { useUIStore } from '@/stores/uiStore'; -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import { useCtrlScrollZoom } from '@/hooks/useCtrlScrollZoom'; import { TimelineHeader } from './TimelineHeader'; import { TimelineBody } from './TimelineBody'; @@ -41,10 +41,10 @@ export const Timeline = forwardRef( const clearScrollToDateTrigger = useUIStore((s) => s.clearScrollToDateTrigger); const zoomTrigger = useUIStore((s) => s.zoomTrigger); const clearZoomTrigger = useUIStore((s) => s.clearZoomTrigger); - const timelineScrollLeft = useAppStore((s) => s.timelineScrollLeft); - const setTimelineScrollLeft = useAppStore((s) => s.setTimelineScrollLeft); - const showStaffOverview = useAppStore((s) => s.showStaffOverview); - const showEquipmentOverview = useAppStore((s) => s.showEquipmentOverview); + const timelineScrollLeft = useViewStore((s) => s.timelineScrollLeft); + const setTimelineScrollLeft = useViewStore((s) => s.setTimelineScrollLeft); + const showStaffOverview = useViewStore((s) => s.showStaffOverview); + const showEquipmentOverview = useViewStore((s) => s.showEquipmentOverview); const hasRestoredScroll = useRef(false); const scrollSaveTimeout = useRef | null>(null); const previousViewModeRef = useRef(viewMode); diff --git a/frontend/src/components/gantt/Timeline/TimelineBody.tsx b/frontend/src/components/gantt/Timeline/TimelineBody.tsx index 1b9999e..3465cd5 100644 --- a/frontend/src/components/gantt/Timeline/TimelineBody.tsx +++ b/frontend/src/components/gantt/Timeline/TimelineBody.tsx @@ -4,7 +4,8 @@ */ import { forwardRef, useMemo, useRef, useCallback } from 'react'; -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { useUIStore } from '@/stores/uiStore'; import { ProjectTimeline } from './ProjectTimeline'; import { TodayLine } from './TodayLine'; @@ -27,12 +28,12 @@ interface TimelineBodyProps { export const TimelineBody = forwardRef( function TimelineBody({ cells, projects, cellWidth, totalWidth, viewMode }, ref) { - const expandedProjects = useAppStore((s) => s.expandedProjects); - const expandedPhases = useAppStore((s) => s.expandedPhases); - const expandedSubphases = useAppStore((s) => s.expandedSubphases); - const showAssignments = useAppStore((s) => s.showAssignments); - const customColumnFilters = useAppStore((s) => s.customColumnFilters); - const customColumnValues = useAppStore((s) => s.customColumnValues); + const expandedProjects = useViewStore((s) => s.expandedProjects); + const expandedPhases = useViewStore((s) => s.expandedPhases); + const expandedSubphases = useViewStore((s) => s.expandedSubphases); + const showAssignments = useViewStore((s) => s.showAssignments); + const customColumnFilters = useCustomColumnStore((s) => s.customColumnFilters); + const customColumnValues = useCustomColumnStore((s) => s.customColumnValues); const dragIndicator = useUIStore((s) => s.dragIndicator); const phantomSiblingMode = useUIStore((s) => s.phantomSiblingMode); const resourceDrag = useUIStore((s) => s.resourceDrag); diff --git a/frontend/src/components/gantt/utils/index.ts b/frontend/src/components/gantt/utils/index.ts index d09410b..840b04a 100644 --- a/frontend/src/components/gantt/utils/index.ts +++ b/frontend/src/components/gantt/utils/index.ts @@ -33,6 +33,7 @@ export { movePhasesWithProject, cloneProject, } from './autoCalculation'; +export type { PendingUpdate } from './autoCalculation'; export { calculateCriticalPath, diff --git a/frontend/src/components/layout/Header/DateNavigation.tsx b/frontend/src/components/layout/Header/DateNavigation.tsx index f95351e..0771cb4 100644 --- a/frontend/src/components/layout/Header/DateNavigation.tsx +++ b/frontend/src/components/layout/Header/DateNavigation.tsx @@ -3,15 +3,15 @@ * Controls for navigating the timeline (prev/next/today) */ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import { useUIStore } from '@/stores/uiStore'; import { addWeeks, addMonths, startOfWeek, startOfQuarter } from 'date-fns'; import styles from './DateNavigation.module.css'; export function DateNavigation() { - const viewMode = useAppStore((s) => s.viewMode); - const currentDate = useAppStore((s) => s.currentDate); - const setCurrentDate = useAppStore((s) => s.setCurrentDate); + const viewMode = useViewStore((s) => s.viewMode); + const currentDate = useViewStore((s) => s.currentDate); + const setCurrentDate = useViewStore((s) => s.setCurrentDate); const triggerScrollToToday = useUIStore((s) => s.triggerScrollToToday); // Navigate to previous/next period diff --git a/frontend/src/components/layout/Header/EquipmentOverviewToggle.tsx b/frontend/src/components/layout/Header/EquipmentOverviewToggle.tsx index e290140..a502cf4 100644 --- a/frontend/src/components/layout/Header/EquipmentOverviewToggle.tsx +++ b/frontend/src/components/layout/Header/EquipmentOverviewToggle.tsx @@ -5,13 +5,14 @@ */ import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import styles from './EquipmentOverviewToggle.module.css'; export function EquipmentOverviewToggle() { - const showEquipmentOverview = useAppStore((s) => s.showEquipmentOverview); - const toggleShowEquipmentOverview = useAppStore((s) => s.toggleShowEquipmentOverview); + const showEquipmentOverview = useViewStore((s) => s.showEquipmentOverview); + const toggleShowEquipmentOverview = useViewStore((s) => s.toggleShowEquipmentOverview); const currentUser = useAppStore((s) => s.currentUser); - const currentView = useAppStore((s) => s.currentView); + const currentView = useViewStore((s) => s.currentView); // Only show for admin/superuser and only in gantt view if (!currentUser || (currentUser.role !== 'admin' && currentUser.role !== 'superuser')) { diff --git a/frontend/src/components/layout/Header/Header.tsx b/frontend/src/components/layout/Header/Header.tsx index 878545d..d303b4c 100644 --- a/frontend/src/components/layout/Header/Header.tsx +++ b/frontend/src/components/layout/Header/Header.tsx @@ -1,5 +1,7 @@ import { useState, useEffect, useMemo, useRef } from 'react'; import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; +import { useWhatIfStore } from '@/stores/whatIfStore'; import { SiteSelector } from './SiteSelector'; import { DateNavigation } from './DateNavigation'; import { UserMenu } from './UserMenu'; @@ -28,16 +30,16 @@ const DEFAULT_CELL_WIDTH = 36; export function Header() { const currentUser = useAppStore((s) => s.currentUser); - const whatIfMode = useAppStore((s) => s.whatIfMode); - const currentView = useAppStore((s) => s.currentView); - const viewMode = useAppStore((s) => s.viewMode); - const setViewMode = useAppStore((s) => s.setViewMode); - const cellWidth = useAppStore((s) => s.cellWidth); - const setCellWidth = useAppStore((s) => s.setCellWidth); - const showStaffOverview = useAppStore((s) => s.showStaffOverview); - const showEquipmentOverview = useAppStore((s) => s.showEquipmentOverview); - const toggleShowStaffOverview = useAppStore((s) => s.toggleShowStaffOverview); - const toggleShowEquipmentOverview = useAppStore((s) => s.toggleShowEquipmentOverview); + const whatIfMode = useWhatIfStore((s) => s.whatIfMode); + const currentView = useViewStore((s) => s.currentView); + const viewMode = useViewStore((s) => s.viewMode); + const setViewMode = useViewStore((s) => s.setViewMode); + const cellWidth = useViewStore((s) => s.cellWidth); + const setCellWidth = useViewStore((s) => s.setCellWidth); + const showStaffOverview = useViewStore((s) => s.showStaffOverview); + const showEquipmentOverview = useViewStore((s) => s.showEquipmentOverview); + const toggleShowStaffOverview = useViewStore((s) => s.toggleShowStaffOverview); + const toggleShowEquipmentOverview = useViewStore((s) => s.toggleShowEquipmentOverview); const [theme, setThemeState] = useState(() => getTheme()); const [customLogoDark, setCustomLogoDark] = useState(''); diff --git a/frontend/src/components/layout/Header/SiteSelector.tsx b/frontend/src/components/layout/Header/SiteSelector.tsx index e1e5400..cee67e4 100644 --- a/frontend/src/components/layout/Header/SiteSelector.tsx +++ b/frontend/src/components/layout/Header/SiteSelector.tsx @@ -1,4 +1,5 @@ import { useAppStore } from '@/stores/appStore'; +import { useCustomColumnStore } from '@/stores/customColumnStore'; import { getBankHolidays, buildHolidayDateSet, getCustomColumnsWithValues, getCompanyEvents, buildEventDateSet } from '@/api'; import styles from './SiteSelector.module.css'; @@ -8,9 +9,9 @@ export function SiteSelector() { const setCurrentSite = useAppStore((s) => s.setCurrentSite); const setBankHolidays = useAppStore((s) => s.setBankHolidays); const setCompanyEvents = useAppStore((s) => s.setCompanyEvents); - const setCustomColumns = useAppStore((s) => s.setCustomColumns); - const setCustomColumnValues = useAppStore((s) => s.setCustomColumnValues); - const clearAllCustomColumnFilters = useAppStore((s) => s.clearAllCustomColumnFilters); + const setCustomColumns = useCustomColumnStore((s) => s.setCustomColumns); + const setCustomColumnValues = useCustomColumnStore((s) => s.setCustomColumnValues); + const clearAllCustomColumnFilters = useCustomColumnStore((s) => s.clearAllCustomColumnFilters); const currentUser = useAppStore((s) => s.currentUser); // Filter sites based on user's assigned sites (if not admin) @@ -51,7 +52,6 @@ export function SiteSelector() { const customColumnsData = await getCustomColumnsWithValues(site.id); setCustomColumns(customColumnsData.columns); setCustomColumnValues(customColumnsData.values); - console.log('[SiteSelector] Loaded custom columns for site:', site.name, customColumnsData.columns.length); } catch (err) { console.error('[SiteSelector] Failed to load custom columns:', err); } diff --git a/frontend/src/components/layout/Header/StaffOverviewToggle.tsx b/frontend/src/components/layout/Header/StaffOverviewToggle.tsx index 10ddcc7..a38a4b1 100644 --- a/frontend/src/components/layout/Header/StaffOverviewToggle.tsx +++ b/frontend/src/components/layout/Header/StaffOverviewToggle.tsx @@ -5,13 +5,14 @@ */ import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import styles from './StaffOverviewToggle.module.css'; export function StaffOverviewToggle() { - const showStaffOverview = useAppStore((s) => s.showStaffOverview); - const toggleShowStaffOverview = useAppStore((s) => s.toggleShowStaffOverview); + const showStaffOverview = useViewStore((s) => s.showStaffOverview); + const toggleShowStaffOverview = useViewStore((s) => s.toggleShowStaffOverview); const currentUser = useAppStore((s) => s.currentUser); - const currentView = useAppStore((s) => s.currentView); + const currentView = useViewStore((s) => s.currentView); // Only show for admin/superuser and only in gantt view if (!currentUser || (currentUser.role !== 'admin' && currentUser.role !== 'superuser')) { diff --git a/frontend/src/components/layout/Header/ViewModeControls.tsx b/frontend/src/components/layout/Header/ViewModeControls.tsx index 37f9985..e8ae82d 100644 --- a/frontend/src/components/layout/Header/ViewModeControls.tsx +++ b/frontend/src/components/layout/Header/ViewModeControls.tsx @@ -1,4 +1,4 @@ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import type { ViewMode } from '@/types'; import styles from './ViewModeControls.module.css'; @@ -10,8 +10,8 @@ const VIEW_MODES: { value: ViewMode; label: string }[] = [ ]; export function ViewModeControls() { - const viewMode = useAppStore((s) => s.viewMode); - const setViewMode = useAppStore((s) => s.setViewMode); + const viewMode = useViewStore((s) => s.viewMode); + const setViewMode = useViewStore((s) => s.setViewMode); return (
diff --git a/frontend/src/components/layout/Header/WhatIfToggle.tsx b/frontend/src/components/layout/Header/WhatIfToggle.tsx index 2f45bb0..5dc322c 100644 --- a/frontend/src/components/layout/Header/WhatIfToggle.tsx +++ b/frontend/src/components/layout/Header/WhatIfToggle.tsx @@ -1,18 +1,20 @@ import { useState } from 'react'; import { useAppStore } from '@/stores/appStore'; +import { useWhatIfStore } from '@/stores/whatIfStore'; import styles from './WhatIfToggle.module.css'; export function WhatIfToggle() { - const whatIfMode = useAppStore((s) => s.whatIfMode); - const whatIfPendingOperations = useAppStore((s) => s.whatIfPendingOperations); - const enterWhatIfMode = useAppStore((s) => s.enterWhatIfMode); - const exitWhatIfMode = useAppStore((s) => s.exitWhatIfMode); + const projects = useAppStore((s) => s.projects); + const whatIfMode = useWhatIfStore((s) => s.whatIfMode); + const whatIfPendingOperations = useWhatIfStore((s) => s.whatIfPendingOperations); + const enterWhatIfMode = useWhatIfStore((s) => s.enterWhatIfMode); + const exitWhatIfMode = useWhatIfStore((s) => s.exitWhatIfMode); const [isApplying, setIsApplying] = useState(false); const pendingCount = whatIfPendingOperations.length; const handleEnter = () => { - enterWhatIfMode(); + enterWhatIfMode(projects); }; const handleDiscard = () => { diff --git a/frontend/src/components/layout/Header/ZoomControls.tsx b/frontend/src/components/layout/Header/ZoomControls.tsx index 6df3f92..c321e81 100644 --- a/frontend/src/components/layout/Header/ZoomControls.tsx +++ b/frontend/src/components/layout/Header/ZoomControls.tsx @@ -2,7 +2,7 @@ * ZoomControls - Zoom in/out buttons and current zoom level indicator */ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import { useUIStore } from '@/stores/uiStore'; import styles from './ZoomControls.module.css'; @@ -13,8 +13,8 @@ const ZOOM_STEP = 4; const DEFAULT_CELL_WIDTH = 36; export function ZoomControls() { - const cellWidth = useAppStore((s) => s.cellWidth); - const setCellWidth = useAppStore((s) => s.setCellWidth); + const cellWidth = useViewStore((s) => s.cellWidth); + const setCellWidth = useViewStore((s) => s.setCellWidth); const triggerZoom = useUIStore((s) => s.triggerZoom); // Calculate zoom percentage relative to default diff --git a/frontend/src/components/layout/MainLayout.tsx b/frontend/src/components/layout/MainLayout.tsx index 36e80e6..4415917 100644 --- a/frontend/src/components/layout/MainLayout.tsx +++ b/frontend/src/components/layout/MainLayout.tsx @@ -3,7 +3,7 @@ * The primary application layout after authentication */ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import { useKeyboardShortcuts } from '@/hooks'; import { Header } from './Header'; import { Sidebar } from './Sidebar'; @@ -13,7 +13,7 @@ import { StaffView, EquipmentView, CrossSiteView, ArchivedView } from '@/compone import styles from './MainLayout.module.css'; export function MainLayout() { - const currentView = useAppStore((s) => s.currentView); + const currentView = useViewStore((s) => s.currentView); // Enable global keyboard shortcuts useKeyboardShortcuts(); diff --git a/frontend/src/components/layout/ResourcePanel/ResourceList.tsx b/frontend/src/components/layout/ResourcePanel/ResourceList.tsx index 6243670..9cc430a 100644 --- a/frontend/src/components/layout/ResourcePanel/ResourceList.tsx +++ b/frontend/src/components/layout/ResourcePanel/ResourceList.tsx @@ -1,13 +1,14 @@ import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import { ResourceCard } from './ResourceCard'; import styles from './ResourceList.module.css'; export function ResourceList() { - const currentResourceTab = useAppStore((s) => s.currentResourceTab); + const currentResourceTab = useViewStore((s) => s.currentResourceTab); const staff = useAppStore((s) => s.staff); const equipment = useAppStore((s) => s.equipment); const currentSite = useAppStore((s) => s.currentSite); - + // Subscribe to projects to pass to ResourceCards const projects = useAppStore((s) => s.projects); diff --git a/frontend/src/components/layout/ResourcePanel/ResourcePanel.tsx b/frontend/src/components/layout/ResourcePanel/ResourcePanel.tsx index 6a1b158..0f8c1dd 100644 --- a/frontend/src/components/layout/ResourcePanel/ResourcePanel.tsx +++ b/frontend/src/components/layout/ResourcePanel/ResourcePanel.tsx @@ -1,11 +1,11 @@ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import { ResourceTabs } from './ResourceTabs'; import { ResourceList } from './ResourceList'; import styles from './ResourcePanel.module.css'; export function ResourcePanel() { - const resourcePanelCollapsed = useAppStore((s) => s.resourcePanelCollapsed); - const setResourcePanelCollapsed = useAppStore((s) => s.setResourcePanelCollapsed); + const resourcePanelCollapsed = useViewStore((s) => s.resourcePanelCollapsed); + const setResourcePanelCollapsed = useViewStore((s) => s.setResourcePanelCollapsed); const toggleCollapsed = () => { setResourcePanelCollapsed(!resourcePanelCollapsed); diff --git a/frontend/src/components/layout/ResourcePanel/ResourceTabs.tsx b/frontend/src/components/layout/ResourcePanel/ResourceTabs.tsx index 975a5f9..08db397 100644 --- a/frontend/src/components/layout/ResourcePanel/ResourceTabs.tsx +++ b/frontend/src/components/layout/ResourcePanel/ResourceTabs.tsx @@ -1,4 +1,4 @@ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import type { ResourceTab } from '@/types'; import styles from './ResourceTabs.module.css'; @@ -8,8 +8,8 @@ const TABS: { value: ResourceTab; label: string }[] = [ ]; export function ResourceTabs() { - const currentResourceTab = useAppStore((s) => s.currentResourceTab); - const setCurrentResourceTab = useAppStore((s) => s.setCurrentResourceTab); + const currentResourceTab = useViewStore((s) => s.currentResourceTab); + const setCurrentResourceTab = useViewStore((s) => s.setCurrentResourceTab); return (
diff --git a/frontend/src/components/layout/Sidebar/Navigation.tsx b/frontend/src/components/layout/Sidebar/Navigation.tsx index 016ea8d..d626561 100644 --- a/frontend/src/components/layout/Sidebar/Navigation.tsx +++ b/frontend/src/components/layout/Sidebar/Navigation.tsx @@ -1,4 +1,4 @@ -import { useAppStore } from '@/stores/appStore'; +import { useViewStore } from '@/stores/viewStore'; import type { CurrentView } from '@/types'; import styles from './Navigation.module.css'; @@ -72,8 +72,8 @@ const NAV_ITEMS: NavItem[] = [ ]; export function Navigation({ collapsed = false }: NavigationProps) { - const currentView = useAppStore((s) => s.currentView); - const setCurrentView = useAppStore((s) => s.setCurrentView); + const currentView = useViewStore((s) => s.currentView); + const setCurrentView = useViewStore((s) => s.setCurrentView); return (