From 740b546bec63dce5491ebb3f38d09aec1e7e7c73 Mon Sep 17 00:00:00 2001 From: El Houssaine Izem Date: Tue, 10 Nov 2026 18:00:42 +0000 Subject: [PATCH] Fix revenue dashboard: tenant isolation, timezone months, cent rounding Tenant isolation (Ocean Rentals seeing another company's revenue): - Include tenant_id in the revenue cache key; prop-001 exists for two tenants - Fix DatabasePool: build URL from settings.database_url, drop sync-only QueuePool, make get_session usable with `async with`, initialize once - Add sqlalchemy[asyncio] so greenlet is installed - Reuse the shared db_pool instead of creating a pool per request - Remove tenant-blind mock fallback; return 503 when the DB is unavailable - Reject users without a tenant (403) instead of a shared default tenant - Add GET /dashboard/properties; frontend lists only the tenant's properties Wrong March totals (Sunset Properties): - Compute month boundaries in the property's timezone, converted to UTC - Implement calculate_monthly_revenue (was a placeholder returning 0), scoped by tenant_id Totals off by a few cents: - Round the final total to cents with Decimal (ROUND_HALF_UP) on the server Also clear the stale error in RevenueSummary when switching property. --- backend/app/api/v1/dashboard.py | 48 ++++- backend/app/core/database_pool.py | 24 ++- backend/app/main.py | 8 + backend/app/services/cache.py | 3 +- backend/app/services/reservations.py | 229 +++++++++++++-------- backend/requirements.txt | 2 +- frontend/src/components/Dashboard.tsx | 38 +++- frontend/src/components/RevenueSummary.tsx | 1 + frontend/src/lib/secureApi.ts | 5 + 9 files changed, 236 insertions(+), 122 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e3..9f6a0b1821 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,25 +1,55 @@ from fastapi import APIRouter, Depends, HTTPException -from typing import Dict, Any +from decimal import Decimal, ROUND_HALF_UP +from typing import Dict, Any, List from app.services.cache import get_revenue_summary +from app.services.reservations import get_tenant_properties from app.core.auth import authenticate_request as get_current_user router = APIRouter() + +def _require_tenant(current_user) -> str: + """Every request must belong to a tenant. Never fall back to a shared default tenant.""" + tenant_id = getattr(current_user, "tenant_id", None) + if not tenant_id: + raise HTTPException(status_code=403, detail="No tenant assigned to this user") + return tenant_id + + +@router.get("/dashboard/properties") +async def get_dashboard_properties( + current_user: dict = Depends(get_current_user) +) -> List[Dict[str, Any]]: + """Lists only the properties of the logged-in user's tenant.""" + tenant_id = _require_tenant(current_user) + try: + return await get_tenant_properties(tenant_id) + except Exception as e: + print(f"Database error listing properties (tenant: {tenant_id}): {e}") + raise HTTPException(status_code=503, detail="Property data is temporarily unavailable") + + @router.get("/dashboard/summary") async def get_dashboard_summary( property_id: str, current_user: dict = Depends(get_current_user) ) -> Dict[str, Any]: - - tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant" - - revenue_data = await get_revenue_summary(property_id, tenant_id) - - total_revenue_float = float(revenue_data['total']) - + + tenant_id = _require_tenant(current_user) + + try: + revenue_data = await get_revenue_summary(property_id, tenant_id) + except Exception as e: + print(f"Database error for {property_id} (tenant: {tenant_id}): {e}") + raise HTTPException(status_code=503, detail="Revenue data is temporarily unavailable") + + # Round once, on the final total, to whole cents (amounts are stored with 3 decimals). + # Decimal keeps the math exact; only the rounded value is turned into a JSON number. + total_revenue = Decimal(revenue_data['total']).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP) + return { "property_id": revenue_data['property_id'], - "total_revenue": total_revenue_float, + "total_revenue": float(total_revenue), "currency": revenue_data['currency'], "reservations_count": revenue_data['count'] } diff --git a/backend/app/core/database_pool.py b/backend/app/core/database_pool.py index d638dfcfe3..d4b41a4cda 100644 --- a/backend/app/core/database_pool.py +++ b/backend/app/core/database_pool.py @@ -1,6 +1,5 @@ import asyncio from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker -from sqlalchemy.pool import QueuePool import logging from ..config import settings @@ -10,16 +9,23 @@ class DatabasePool: def __init__(self): self.engine = None self.session_factory = None - + self._init_lock = asyncio.Lock() + async def initialize(self): - """Initialize database connection pool""" + """Initialize database connection pool (only once)""" + async with self._init_lock: + if self.engine is not None: + return # Already initialized, reuse the same pool + self._create_engine() + + def _create_engine(self): try: - # Create async engine with connection pooling - database_url = f"postgresql+asyncpg://{settings.supabase_db_user}:{settings.supabase_db_password}@{settings.supabase_db_host}:{settings.supabase_db_port}/{settings.supabase_db_name}" - + # settings.database_url is "postgresql://...", async SQLAlchemy needs the asyncpg driver + database_url = settings.database_url.replace("postgresql://", "postgresql+asyncpg://", 1) + + # No poolclass: async engines use AsyncAdaptedQueuePool by default (QueuePool is sync-only) self.engine = create_async_engine( database_url, - poolclass=QueuePool, pool_size=20, # Number of connections to maintain max_overflow=30, # Additional connections when needed pool_pre_ping=True, # Validate connections @@ -45,8 +51,8 @@ async def close(self): if self.engine: await self.engine.dispose() - async def get_session(self) -> AsyncSession: - """Get database session from pool""" + def get_session(self) -> AsyncSession: + """Get database session from pool. Use it as: `async with db_pool.get_session() as session:`""" if not self.session_factory: raise Exception("Database pool not initialized") return self.session_factory() diff --git a/backend/app/main.py b/backend/app/main.py index 00734b2fa3..e0add463a3 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -136,6 +136,14 @@ async def lifespan(app: FastAPI): except Exception as e: logger.warning(f"⚠️ Error closing connection pool: {e}") + # Close the shared database pool used by the revenue services + try: + from .core.database_pool import db_pool + + await db_pool.close() + except Exception as e: + logger.warning(f"⚠️ Error closing database pool: {e}") + app = FastAPI( title="Auth Skeleton API", diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957f..41a64d8285 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -10,7 +10,8 @@ async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any """ Fetches revenue summary, utilizing caching to improve performance. """ - cache_key = f"revenue:{property_id}" + # Include the tenant: the same property_id can exist for different tenants + cache_key = f"revenue:{tenant_id}:{property_id}" # Try to get from cache cached = await redis_client.get(cache_key) diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00abe..7c00059fa4 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,109 +1,156 @@ -from datetime import datetime +from datetime import datetime, timezone as dt_timezone from decimal import Decimal -from typing import Dict, Any, List +from typing import Dict, Any, List, Optional, Tuple +from zoneinfo import ZoneInfo -async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: + +def get_month_bounds_utc(year: int, month: int, timezone: str = "UTC") -> Tuple[datetime, datetime]: """ - Calculates revenue for a specific month. + Returns [start, end) of a month in the property's local time, converted to UTC. + Example: March in Europe/Paris starts at 2024-02-29 23:00 UTC, not 2024-03-01 00:00 UTC. """ + local_tz = ZoneInfo(timezone) - start_date = datetime(year, month, 1) + # Midnight on the 1st of this month and of next month, in the property's time zone + start_local = datetime(year, month, 1, tzinfo=local_tz) if month < 12: - end_date = datetime(year, month + 1, 1) + end_local = datetime(year, month + 1, 1, tzinfo=local_tz) else: - end_date = datetime(year + 1, 1, 1) - - print(f"DEBUG: Querying revenue for {property_id} from {start_date} to {end_date}") + end_local = datetime(year + 1, 1, 1, tzinfo=local_tz) - # SQL Simulation (This would be executed against the actual DB) - query = """ - SELECT SUM(total_amount) as total - FROM reservations - WHERE property_id = $1 - AND tenant_id = $2 - AND check_in_date >= $3 - AND check_in_date < $4 + # check_in_date is stored in UTC, so compare against UTC bounds + return start_local.astimezone(dt_timezone.utc), end_local.astimezone(dt_timezone.utc) + + +async def calculate_monthly_revenue( + property_id: str, + tenant_id: str, + month: int, + year: int, + db_session=None, + timezone: Optional[str] = None, +) -> Decimal: """ - - # In production this query executes against a database session. - # result = await db.fetch_val(query, property_id, tenant_id, start_date, end_date) - # return result or Decimal('0') - - return Decimal('0') # Placeholder for now until DB connection is finalized + Calculates revenue for one property, for one tenant, for one month. + - db_session: a SQLAlchemy AsyncSession. If None, one is opened from DatabasePool. + - timezone: the property's time zone. If None, it is read from the properties table. + """ + if db_session is not None: + return await _fetch_monthly_revenue(db_session, property_id, tenant_id, month, year, timezone) + + # No session given: open one from the shared pool + from app.core.database_pool import db_pool + + await db_pool.initialize() + if not db_pool.session_factory: + raise Exception("Database pool not available") + + async with db_pool.get_session() as session: + return await _fetch_monthly_revenue(session, property_id, tenant_id, month, year, timezone) + + +async def _fetch_monthly_revenue(session, property_id: str, tenant_id: str, month: int, year: int, timezone: Optional[str]) -> Decimal: + """Runs the monthly revenue query on an open session.""" + from sqlalchemy import text + + # Look up the property's time zone if the caller didn't pass it + if timezone is None: + tz_result = await session.execute( + text("SELECT timezone FROM properties WHERE id = :property_id AND tenant_id = :tenant_id"), + {"property_id": property_id, "tenant_id": tenant_id}, + ) + timezone = tz_result.scalar() + if timezone is None: + # This tenant has no such property, so it has no revenue for it + return Decimal("0") + + start_date, end_date = get_month_bounds_utc(year, month, timezone) + + print(f"DEBUG: Querying revenue for {property_id} (tenant: {tenant_id}) from {start_date} to {end_date}") + + # Filter by tenant too: the same property_id can exist for several tenants + query = text(""" + SELECT SUM(total_amount) AS total + FROM reservations + WHERE property_id = :property_id + AND tenant_id = :tenant_id + AND check_in_date >= :start_date + AND check_in_date < :end_date + """) + + result = await session.execute(query, { + "property_id": property_id, + "tenant_id": tenant_id, + "start_date": start_date, + "end_date": end_date, + }) + + # SUM() returns NULL when there are no reservations, so fall back to 0 + total = result.scalar() + return total if total is not None else Decimal("0") + +async def get_tenant_properties(tenant_id: str) -> List[Dict[str, Any]]: + """Returns only this tenant's properties (id, name, timezone).""" + from app.core.database_pool import db_pool + from sqlalchemy import text + + await db_pool.initialize() + if not db_pool.session_factory: + raise Exception("Database pool not available") + + async with db_pool.get_session() as session: + result = await session.execute( + text("SELECT id, name, timezone FROM properties WHERE tenant_id = :tenant_id ORDER BY id"), + {"tenant_id": tenant_id}, + ) + return [{"id": row.id, "name": row.name, "timezone": row.timezone} for row in result] + async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]: """ Aggregates revenue from database. + Errors are raised, never replaced by fake numbers: showing wrong revenue is worse than showing an error. """ - try: - # Import database pool - from app.core.database_pool import DatabasePool - - # Initialize pool if needed - db_pool = DatabasePool() - await db_pool.initialize() - - if db_pool.session_factory: - async with db_pool.get_session() as session: - # Use SQLAlchemy text for raw SQL - from sqlalchemy import text - - query = text(""" - SELECT - property_id, - SUM(total_amount) as total_revenue, - COUNT(*) as reservation_count - FROM reservations - WHERE property_id = :property_id AND tenant_id = :tenant_id - GROUP BY property_id - """) - - result = await session.execute(query, { - "property_id": property_id, - "tenant_id": tenant_id - }) - row = result.fetchone() - - if row: - total_revenue = Decimal(str(row.total_revenue)) - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": str(total_revenue), - "currency": "USD", - "count": row.reservation_count - } - else: - # No reservations found for this property - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": "0.00", - "currency": "USD", - "count": 0 - } - else: - raise Exception("Database pool not available") - - except Exception as e: - print(f"Database error for {property_id} (tenant: {tenant_id}): {e}") - - # Create property-specific mock data for testing when DB is unavailable - # This ensures each property shows different figures - mock_data = { - 'prop-001': {'total': '1000.00', 'count': 3}, - 'prop-002': {'total': '4975.50', 'count': 4}, - 'prop-003': {'total': '6100.50', 'count': 2}, - 'prop-004': {'total': '1776.50', 'count': 4}, - 'prop-005': {'total': '3256.00', 'count': 3} - } - - mock_property_data = mock_data.get(property_id, {'total': '0.00', 'count': 0}) - + from app.core.database_pool import db_pool + from sqlalchemy import text + + # Reuse the one shared pool (creating a new pool per request leaks connections) + await db_pool.initialize() + if not db_pool.session_factory: + raise Exception("Database pool not available") + + async with db_pool.get_session() as session: + query = text(""" + SELECT + property_id, + SUM(total_amount) as total_revenue, + COUNT(*) as reservation_count + FROM reservations + WHERE property_id = :property_id AND tenant_id = :tenant_id + GROUP BY property_id + """) + + result = await session.execute(query, { + "property_id": property_id, + "tenant_id": tenant_id + }) + row = result.fetchone() + + if row: + total_revenue = Decimal(str(row.total_revenue)) + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": str(total_revenue), + "currency": "USD", + "count": row.reservation_count + } + + # No reservations found for this property (for this tenant) return { "property_id": property_id, - "tenant_id": tenant_id, - "total": mock_property_data['total'], + "tenant_id": tenant_id, + "total": "0.00", "currency": "USD", - "count": mock_property_data['count'] + "count": 0 } diff --git a/backend/requirements.txt b/backend/requirements.txt index 6b777d2aaa..06cb8bd4a6 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -18,7 +18,7 @@ pytz redis>=5.0.0 requests sendgrid -sqlalchemy>=2.0.0 +sqlalchemy[asyncio]>=2.0.0 supabase tenacity>=8.0.0 uvicorn[standard] diff --git a/frontend/src/components/Dashboard.tsx b/frontend/src/components/Dashboard.tsx index a21bba404e..bbbbf97984 100644 --- a/frontend/src/components/Dashboard.tsx +++ b/frontend/src/components/Dashboard.tsx @@ -1,16 +1,29 @@ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { RevenueSummary } from "./RevenueSummary"; +import { SecureAPI } from "../lib/secureApi"; -const PROPERTIES = [ - { id: 'prop-001', name: 'Beach House Alpha' }, - { id: 'prop-002', name: 'City Apartment Downtown' }, - { id: 'prop-003', name: 'Country Villa Estate' }, - { id: 'prop-004', name: 'Lakeside Cottage' }, - { id: 'prop-005', name: 'Urban Loft Modern' } -]; +interface Property { + id: string; + name: string; +} const Dashboard: React.FC = () => { - const [selectedProperty, setSelectedProperty] = useState('prop-001'); + // Load the property list from the API so each client only sees their own properties + const [properties, setProperties] = useState([]); + const [selectedProperty, setSelectedProperty] = useState(''); + const [propertiesError, setPropertiesError] = useState(''); + + useEffect(() => { + SecureAPI.getDashboardProperties() + .then((list) => { + setProperties(list); + if (list.length > 0) setSelectedProperty(list[0].id); + }) + .catch((err) => { + console.error(err); + setPropertiesError('Failed to load properties'); + }); + }, []); return (
@@ -35,7 +48,7 @@ const Dashboard: React.FC = () => { onChange={(e) => setSelectedProperty(e.target.value)} className="block w-full sm:w-auto min-w-[200px] px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-blue-500 focus:border-blue-500 text-sm" > - {PROPERTIES.map((property) => ( + {properties.map((property) => ( @@ -46,7 +59,10 @@ const Dashboard: React.FC = () => {
- + {propertiesError && ( +
{propertiesError}
+ )} + {selectedProperty && }
diff --git a/frontend/src/components/RevenueSummary.tsx b/frontend/src/components/RevenueSummary.tsx index dbb6d0629d..5ac795c479 100644 --- a/frontend/src/components/RevenueSummary.tsx +++ b/frontend/src/components/RevenueSummary.tsx @@ -24,6 +24,7 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr useEffect(() => { const fetchRevenue = async () => { setLoading(true); + setError(''); // Clear an old error when loading a new property try { // Use SecureAPI to handle authentication automatically // We pass the simulatedTenant option which SecureAPI will attach as a header diff --git a/frontend/src/lib/secureApi.ts b/frontend/src/lib/secureApi.ts index f85f04c90a..7dc7ec125a 100644 --- a/frontend/src/lib/secureApi.ts +++ b/frontend/src/lib/secureApi.ts @@ -1468,6 +1468,11 @@ export class SecureAPIClient { return this.request(`/api/v1/dashboard/summary?${queryParams}`, requestOptions); } + // Only the logged-in tenant's properties + async getDashboardProperties(): Promise> { + return this.request('/api/v1/dashboard/properties'); + } + async uploadCompanyLogo(logo_url: string) { return this.request('/api/v1/company-settings/logo', { method: 'POST',