From bfdf2635cbeca1df91213fbec6229b036f0f8ab8 Mon Sep 17 00:00:00 2001 From: Ansab Rehman Date: Mon, 12 Oct 2026 18:59:28 +0500 Subject: [PATCH 1/6] fix(cache): scope revenue cache key by tenant_id Both tenants seed a property with id prop-001, so a key of revenue:{property_id} let whichever tenant loaded first serve its totals to the other for the 300s TTL. --- backend/app/services/cache.py | 46 +++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957..3a65efb23 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -1,29 +1,55 @@ import json +import logging import redis.asyncio as redis from typing import Dict, Any import os +logger = logging.getLogger(__name__) + # Initialize Redis client (typically configured centrally). redis_client = redis.Redis.from_url(os.getenv("REDIS_URL", "redis://localhost:6379/0")) +CACHE_TTL_SECONDS = 300 + + +def build_cache_key(property_id: str, tenant_id: str) -> str: + """ + Builds the revenue cache key. + + The tenant is part of the key because property ids are only unique within a + tenant: properties.id is PRIMARY KEY (id, tenant_id), so two tenants can + both own a 'prop-001'. Keying on property_id alone lets one tenant's totals + be served to another. + """ + return f"revenue:{tenant_id}:{property_id}" + + 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}" - + cache_key = build_cache_key(property_id, tenant_id) + # Try to get from cache - cached = await redis_client.get(cache_key) - if cached: - return json.loads(cached) - + try: + cached = await redis_client.get(cache_key) + if cached: + return json.loads(cached) + except Exception as e: + # A cache outage must not take the dashboard down; fall through and + # calculate from the database instead. + logger.warning(f"Cache read failed for {cache_key}: {e}") + # Revenue calculation is delegated to the reservation service. from app.services.reservations import calculate_total_revenue - + # Calculate revenue result = await calculate_total_revenue(property_id, tenant_id) - + # Cache the result for 5 minutes - await redis_client.setex(cache_key, 300, json.dumps(result)) - + try: + await redis_client.setex(cache_key, CACHE_TTL_SECONDS, json.dumps(result)) + except Exception as e: + logger.warning(f"Cache write failed for {cache_key}: {e}") + return result From 254b4f396b55714dadb4b2cd3ba627402dc42a0f Mon Sep 17 00:00:00 2001 From: Ansab Rehman Date: Mon, 12 Oct 2026 19:04:09 +0500 Subject: [PATCH 2/6] fix(db): build async engine from settings.database_url DatabasePool read settings.supabase_db_* which config.py never defines, so initialize() swallowed an AttributeError and left session_factory as None. Also drops QueuePool (invalid for create_async_engine), makes get_session an async context manager to match its call sites, and adds the sqlalchemy[asyncio] extra so greenlet is present. --- backend/app/core/database_pool.py | 101 ++++++++++++++++++++---------- backend/requirements.txt | 2 +- 2 files changed, 70 insertions(+), 33 deletions(-) diff --git a/backend/app/core/database_pool.py b/backend/app/core/database_pool.py index d638dfcfe..aef2f41ab 100644 --- a/backend/app/core/database_pool.py +++ b/backend/app/core/database_pool.py @@ -1,55 +1,92 @@ import asyncio +from contextlib import asynccontextmanager from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker -from sqlalchemy.pool import QueuePool import logging from ..config import settings logger = logging.getLogger(__name__) + +def build_async_database_url(database_url: str) -> str: + """ + Normalises a libpq-style URL to the asyncpg driver SQLAlchemy needs. + + docker-compose supplies DATABASE_URL as postgresql://..., which + create_async_engine rejects because it resolves to the sync psycopg2 + driver. + """ + if database_url.startswith("postgresql+"): + return database_url + if database_url.startswith("postgresql://"): + return database_url.replace("postgresql://", "postgresql+asyncpg://", 1) + if database_url.startswith("postgres://"): + return database_url.replace("postgres://", "postgresql+asyncpg://", 1) + return database_url + + 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""" - 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}" - - 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 - pool_recycle=3600, # Recycle connections every hour - echo=False # Set to True for SQL debugging - ) - - self.session_factory = async_sessionmaker( - bind=self.engine, - class_=AsyncSession, - expire_on_commit=False - ) - - logger.info("✅ Database connection pool initialized") - - except Exception as e: - logger.error(f"❌ Database pool initialization failed: {e}") - self.engine = None - self.session_factory = None - + # Idempotent: callers initialize per request, and building a new engine + # each time would leak connection pools. + if self.session_factory: + return + + async with self._init_lock: + if self.session_factory: + return + + try: + database_url = build_async_database_url(settings.database_url) + + # No poolclass: create_async_engine selects AsyncAdaptedQueuePool. + # Passing the sync QueuePool here raises at engine creation. + self.engine = create_async_engine( + database_url, + pool_size=settings.database_pool_size, + max_overflow=settings.database_max_overflow, + pool_timeout=settings.database_pool_timeout, + pool_pre_ping=True, # Validate connections + pool_recycle=settings.database_pool_recycle, + echo=False # Set to True for SQL debugging + ) + + self.session_factory = async_sessionmaker( + bind=self.engine, + class_=AsyncSession, + expire_on_commit=False + ) + + logger.info("✅ Database connection pool initialized") + + except Exception as e: + # Do not leave the pool silently unusable: callers that treat a + # missing session_factory as "no data" would report wrong + # figures rather than an outage. + logger.error(f"❌ Database pool initialization failed: {e}") + self.engine = None + self.session_factory = None + raise + async def close(self): """Close database connections""" if self.engine: await self.engine.dispose() - - async def get_session(self) -> AsyncSession: + self.engine = None + self.session_factory = None + + @asynccontextmanager + async def get_session(self): """Get database session from pool""" if not self.session_factory: raise Exception("Database pool not initialized") - return self.session_factory() + async with self.session_factory() as session: + yield session # Global database pool instance db_pool = DatabasePool() diff --git a/backend/requirements.txt b/backend/requirements.txt index 6b777d2aa..06cb8bd4a 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] From a1ea6d1a5379f8fdbea0c460542cdb0930e3591e Mon Sep 17 00:00:00 2001 From: Ansab Rehman Date: Mon, 12 Oct 2026 19:07:18 +0500 Subject: [PATCH 3/6] fix(revenue): fail loudly instead of serving mock totals calculate_total_revenue caught every DB error and returned a hardcoded, tenant-blind mock_data table, so the dashboard showed invented figures while the connection pool was broken. Errors now raise RevenueDataUnavailable and surface as 503. --- backend/app/api/v1/dashboard.py | 13 ++- backend/app/services/reservations.py | 115 +++++++++++---------------- 2 files changed, 57 insertions(+), 71 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e..a57a4551a 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,6 +1,7 @@ -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, status from typing import Dict, Any from app.services.cache import get_revenue_summary +from app.services.reservations import RevenueDataUnavailable from app.core.auth import authenticate_request as get_current_user router = APIRouter() @@ -13,7 +14,15 @@ async def get_dashboard_summary( tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant" - revenue_data = await get_revenue_summary(property_id, tenant_id) + try: + revenue_data = await get_revenue_summary(property_id, tenant_id) + except RevenueDataUnavailable as e: + # Report the outage. A dashboard that quietly shows a stand-in figure + # is worse than one that admits it cannot answer. + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Revenue data is temporarily unavailable", + ) from e total_revenue_float = float(revenue_data['total']) diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00ab..67f2e641d 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,7 +1,19 @@ +import logging from datetime import datetime from decimal import Decimal from typing import Dict, Any, List +logger = logging.getLogger(__name__) + + +class RevenueDataUnavailable(RuntimeError): + """ + Raised when revenue cannot be read from the database. + + Callers must surface this as an error. Returning a plausible-looking + number in its place is what hid the broken connection pool for so long. + """ + async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: """ Calculates revenue for a specific month. @@ -35,75 +47,40 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, """ Aggregates revenue from database. """ + from app.core.database_pool import db_pool + from sqlalchemy import text + + query = text(""" + SELECT + COALESCE(SUM(total_amount), 0) as total_revenue, + COUNT(*) as reservation_count + FROM reservations + WHERE property_id = :property_id AND tenant_id = :tenant_id + """) + 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") - + async with db_pool.get_session() as session: + result = await session.execute(query, { + "property_id": property_id, + "tenant_id": tenant_id + }) + row = result.fetchone() 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}) - - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": mock_property_data['total'], - "currency": "USD", - "count": mock_property_data['count'] - } + logger.error(f"Database error for {property_id} (tenant: {tenant_id}): {e}") + raise RevenueDataUnavailable( + f"Could not read revenue for {property_id}" + ) from e + + # An aggregate without GROUP BY always returns exactly one row, so a + # property with no reservations reports 0.00 rather than falling through + # to a branch that could invent a figure. + 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 + } From e2161f13b721786fe77218b92db625bff7428fab Mon Sep 17 00:00:00 2001 From: Ansab Rehman Date: Mon, 12 Oct 2026 19:12:19 +0500 Subject: [PATCH 4/6] fix(revenue): bucket months in the property timezone Month boundaries were naive UTC datetimes compared against TIMESTAMPTZ, ignoring properties.timezone. A 2024-02-29 23:30Z check-in is 1 March in Europe/Paris, so 1250.00 fell out of March and prop-001 reported 1000.00 instead of 2250.00. calculate_monthly_revenue was also a stub with no tenant_id parameter; it now executes and is reachable via optional month/year query params. --- backend/app/api/v1/dashboard.py | 14 +++- backend/app/services/cache.py | 39 ++++++++-- backend/app/services/reservations.py | 108 +++++++++++++++++++-------- 3 files changed, 119 insertions(+), 42 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index a57a4551a..433c4b643 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,5 +1,5 @@ -from fastapi import APIRouter, Depends, HTTPException, status -from typing import Dict, Any +from fastapi import APIRouter, Depends, HTTPException, Query, status +from typing import Dict, Any, Optional from app.services.cache import get_revenue_summary from app.services.reservations import RevenueDataUnavailable from app.core.auth import authenticate_request as get_current_user @@ -9,13 +9,21 @@ @router.get("/dashboard/summary") async def get_dashboard_summary( property_id: str, + month: Optional[int] = Query(None, ge=1, le=12), + year: Optional[int] = Query(None, ge=2000, le=2100), current_user: dict = Depends(get_current_user) ) -> Dict[str, Any]: tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant" + if (month is None) != (year is None): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="month and year must be supplied together", + ) + try: - revenue_data = await get_revenue_summary(property_id, tenant_id) + revenue_data = await get_revenue_summary(property_id, tenant_id, month, year) except RevenueDataUnavailable as e: # Report the outage. A dashboard that quietly shows a stand-in figure # is worse than one that admits it cannot answer. diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index 3a65efb23..35b8ed7e1 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -1,7 +1,7 @@ import json import logging import redis.asyncio as redis -from typing import Dict, Any +from typing import Dict, Any, Optional import os logger = logging.getLogger(__name__) @@ -12,7 +12,12 @@ CACHE_TTL_SECONDS = 300 -def build_cache_key(property_id: str, tenant_id: str) -> str: +def build_cache_key( + property_id: str, + tenant_id: str, + month: Optional[int] = None, + year: Optional[int] = None, +) -> str: """ Builds the revenue cache key. @@ -20,15 +25,29 @@ def build_cache_key(property_id: str, tenant_id: str) -> str: tenant: properties.id is PRIMARY KEY (id, tenant_id), so two tenants can both own a 'prop-001'. Keying on property_id alone lets one tenant's totals be served to another. + + The period is part of the key so a month's figure is never served as the + all-time total. """ - return f"revenue:{tenant_id}:{property_id}" + key = f"revenue:{tenant_id}:{property_id}" + if month is not None and year is not None: + key = f"{key}:{year}-{month:02d}" + return key -async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any]: +async def get_revenue_summary( + property_id: str, + tenant_id: str, + month: Optional[int] = None, + year: Optional[int] = None, +) -> Dict[str, Any]: """ Fetches revenue summary, utilizing caching to improve performance. + + Supplying month and year scopes the figure to that calendar month in the + property's own timezone; otherwise the all-time total is returned. """ - cache_key = build_cache_key(property_id, tenant_id) + cache_key = build_cache_key(property_id, tenant_id, month, year) # Try to get from cache try: @@ -41,10 +60,16 @@ async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any logger.warning(f"Cache read failed for {cache_key}: {e}") # Revenue calculation is delegated to the reservation service. - from app.services.reservations import calculate_total_revenue + from app.services.reservations import ( + calculate_monthly_revenue, + calculate_total_revenue, + ) # Calculate revenue - result = await calculate_total_revenue(property_id, tenant_id) + if month is not None and year is not None: + result = await calculate_monthly_revenue(property_id, tenant_id, month, year) + else: + result = await calculate_total_revenue(property_id, tenant_id) # Cache the result for 5 minutes try: diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 67f2e641d..2917f0acc 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -14,40 +14,92 @@ class RevenueDataUnavailable(RuntimeError): number in its place is what hid the broken connection pool for so long. """ -async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: + +async def _fetch_revenue_row(query, params: Dict[str, Any], context: str): + """ + Runs a revenue aggregate and returns its single row. + """ + from app.core.database_pool import db_pool + + try: + await db_pool.initialize() + async with db_pool.get_session() as session: + result = await session.execute(query, params) + return result.fetchone() + except Exception as e: + logger.error(f"Database error for {context}: {e}") + raise RevenueDataUnavailable(f"Could not read revenue for {context}") from e + + +def _month_bounds(month: int, year: int): """ - Calculates revenue for a specific month. + Returns the half-open [start, end) local month boundaries. """ + if not 1 <= month <= 12: + raise ValueError(f"month must be between 1 and 12, got {month}") start_date = datetime(year, month, 1) if month < 12: end_date = datetime(year, month + 1, 1) else: end_date = datetime(year + 1, 1, 1) - - print(f"DEBUG: Querying revenue for {property_id} from {start_date} to {end_date}") + return start_date, end_date - # 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 + +async def calculate_monthly_revenue(property_id: str, tenant_id: str, month: int, year: int) -> Dict[str, Any]: """ - - # 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 a specific month, in the property's own timezone. + + A booking belongs to the month the guest checked in locally, which is what + the merchant reconciles against. check_in_date is TIMESTAMPTZ, so comparing + it to naive UTC boundaries misfiles any check-in that falls on either side + of local midnight: a 2024-02-29 23:30Z arrival is 1 March in Europe/Paris. + Converting with AT TIME ZONE p.timezone uses the zone the schema already + records per property. + """ + from sqlalchemy import text + + start_date, end_date = _month_bounds(month, year) + + query = text(""" + SELECT + COALESCE(SUM(r.total_amount), 0) as total_revenue, + COUNT(*) as reservation_count + FROM reservations r + JOIN properties p + ON p.id = r.property_id AND p.tenant_id = r.tenant_id + WHERE r.property_id = :property_id + AND r.tenant_id = :tenant_id + AND (r.check_in_date AT TIME ZONE p.timezone) >= :start_date + AND (r.check_in_date AT TIME ZONE p.timezone) < :end_date + """) + + row = await _fetch_revenue_row( + query, + { + "property_id": property_id, + "tenant_id": tenant_id, + "start_date": start_date, + "end_date": end_date, + }, + f"{property_id} (tenant: {tenant_id}, {year}-{month:02d})", + ) + + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": str(Decimal(str(row.total_revenue))), + "currency": "USD", + "count": row.reservation_count, + "month": month, + "year": year, + } + async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]: """ Aggregates revenue from database. """ - from app.core.database_pool import db_pool from sqlalchemy import text query = text(""" @@ -58,19 +110,11 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, WHERE property_id = :property_id AND tenant_id = :tenant_id """) - try: - await db_pool.initialize() - async with db_pool.get_session() as session: - result = await session.execute(query, { - "property_id": property_id, - "tenant_id": tenant_id - }) - row = result.fetchone() - except Exception as e: - logger.error(f"Database error for {property_id} (tenant: {tenant_id}): {e}") - raise RevenueDataUnavailable( - f"Could not read revenue for {property_id}" - ) from e + row = await _fetch_revenue_row( + query, + {"property_id": property_id, "tenant_id": tenant_id}, + f"{property_id} (tenant: {tenant_id})", + ) # An aggregate without GROUP BY always returns exactly one row, so a # property with no reservations reports 0.00 rather than falling through From 733bb94cb141f3ba06ca14533e2d4395754fc5fc Mon Sep 17 00:00:00 2001 From: Ansab Rehman Date: Mon, 12 Oct 2026 19:19:15 +0500 Subject: [PATCH 5/6] fix(revenue): keep money in Decimal, not float total_amount is NUMERIC(10,3), so a total can land on a half cent. float(total) on the API and Math.round(x*100)/100 on the client both resolved those downwards: 0.005 displayed as 0.00. Totals are now quantized once with ROUND_HALF_UP and sent as an exact string. --- backend/app/api/v1/dashboard.py | 21 +++++++++++++++--- frontend/src/components/RevenueSummary.tsx | 25 ++++++++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 433c4b643..fb70b1ed6 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,3 +1,4 @@ +from decimal import Decimal, ROUND_HALF_UP from fastapi import APIRouter, Depends, HTTPException, Query, status from typing import Dict, Any, Optional from app.services.cache import get_revenue_summary @@ -6,6 +7,22 @@ router = APIRouter() +CENTS = Decimal("0.01") + + +def to_currency_string(amount: str) -> str: + """ + Rounds an exact amount to cents and renders it as a string. + + reservations.total_amount is NUMERIC(10,3), so a total can land on a half + cent. Going through float to do that rounding resolves half cents + downwards -- 0.005 becomes 0.00 rather than 0.01 -- because the binary + value sits just below the midpoint. Rounding the Decimal explicitly makes + the policy half-up, and sending a string keeps JSON parsing from putting + the result back into a float. + """ + return str(Decimal(amount).quantize(CENTS, rounding=ROUND_HALF_UP)) + @router.get("/dashboard/summary") async def get_dashboard_summary( property_id: str, @@ -32,11 +49,9 @@ async def get_dashboard_summary( detail="Revenue data is temporarily unavailable", ) from e - total_revenue_float = float(revenue_data['total']) - return { "property_id": revenue_data['property_id'], - "total_revenue": total_revenue_float, + "total_revenue": to_currency_string(revenue_data['total']), "currency": revenue_data['currency'], "reservations_count": revenue_data['count'] } diff --git a/frontend/src/components/RevenueSummary.tsx b/frontend/src/components/RevenueSummary.tsx index dbb6d0629..249b7b560 100644 --- a/frontend/src/components/RevenueSummary.tsx +++ b/frontend/src/components/RevenueSummary.tsx @@ -3,11 +3,28 @@ import { SecureAPI } from '../lib/secureApi'; interface RevenueData { property_id: string; - total_revenue: number; + // Money arrives as an exact decimal string. Typing this as number would + // hand it straight back to binary floating point. + total_revenue: string; currency: string; reservations_count: number; } +// An amount the API has already rounded to cents, e.g. "2250.00". +const EXACT_CURRENCY = /^-?\d+\.\d{2}$/; + +/** + * Groups thousands and keeps both cents, working on the digits themselves so + * the value is never parsed into a float. + */ +const formatMoney = (amount: string): string => { + const negative = amount.startsWith('-'); + const [whole, fraction = ''] = (negative ? amount.slice(1) : amount).split('.'); + const cents = `${fraction}00`.slice(0, 2); + const grouped = whole.replace(/\B(?=(\d{3})+(?!\d))/g, ','); + return `${negative ? '-' : ''}${grouped}.${cents}`; +}; + interface RevenueSummaryProps { propertyId?: string; debugTenant?: string; @@ -61,7 +78,7 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr if (error) return
{error}
; if (!data) return null; - const displayTotal = Math.round(data.total_revenue * 100) / 100; + const displayTotal = formatMoney(data.total_revenue); return (
@@ -78,7 +95,7 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr

Total Revenue

- {data.currency} {displayTotal.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 })} + {data.currency} {displayTotal} {/* Fake trend indicator for premium feel */} @@ -104,7 +121,7 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr {/* Precision Warning Area */}
- {Math.abs(data.total_revenue - displayTotal) > 0.000001 && showRaw && ( + {!EXACT_CURRENCY.test(data.total_revenue) && showRaw && (
From e383a85cb0ff9b86d4b090ecb15e8a18f2802f9e Mon Sep 17 00:00:00 2001 From: Ansab Rehman Date: Mon, 12 Oct 2026 19:24:22 +0500 Subject: [PATCH 6/6] fix(auth): never guess a tenant resolve_tenant_id returned 'tenant-a' for any unknown user and the dashboard fell back to 'default_tenant', so an unrecognised account received Sunset Properties' revenue. Both now refuse to guess and the endpoint returns 403. Also removes the client-controlled X-Simulated-Tenant header the backend rightly ignored. --- backend/app/api/v1/dashboard.py | 9 +++++- backend/app/core/tenant_resolver.py | 33 +++++++++++++--------- frontend/src/components/RevenueSummary.tsx | 12 +++----- frontend/src/lib/secureApi.ts | 21 +++++++------- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index fb70b1ed6..a0b75888f 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -31,7 +31,14 @@ async def get_dashboard_summary( current_user: dict = Depends(get_current_user) ) -> Dict[str, Any]: - tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant" + tenant_id = getattr(current_user, "tenant_id", None) + if not tenant_id: + # Without a known tenant there is no safe row set to return. The old + # "default_tenant" fallback resolved to real data belonging to someone. + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No tenant is associated with this account", + ) if (month is None) != (year is None): raise HTTPException( diff --git a/backend/app/core/tenant_resolver.py b/backend/app/core/tenant_resolver.py index db09a4629..362d18830 100644 --- a/backend/app/core/tenant_resolver.py +++ b/backend/app/core/tenant_resolver.py @@ -68,28 +68,33 @@ def resolve_tenant_from_user(user_data: dict) -> Optional[str]: return None + # Known accounts and the tenant each one belongs to. + EMAIL_TENANTS = { + "sunset@propertyflow.com": "tenant-a", + "ocean@propertyflow.com": "tenant-b", + "candidate@propertyflow.com": "tenant-a", + } + @staticmethod - async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] = None) -> str: + async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] = None) -> Optional[str]: """ Resolve tenant ID for a user. - + Args: user_id: User ID user_email: User email - + Returns: - Tenant ID + Tenant ID, or None when the user's tenant cannot be established. + Returning a default here would silently grant one tenant's data to + every unrecognised account. """ - # Fallback mapping by known user email. - if user_email == "sunset@propertyflow.com": - return "tenant-a" - if user_email == "ocean@propertyflow.com": - return "tenant-b" - if user_email == "candidate@propertyflow.com": - return "tenant-a" - - # Default fallback - return "tenant-a" + tenant_id = TenantResolver.EMAIL_TENANTS.get((user_email or "").lower()) + if tenant_id: + return tenant_id + + logger.warning(f"Could not resolve a tenant for {user_email!r} (user_id={user_id})") + return None @staticmethod async def update_user_tenant_metadata(user_id: str, tenant_id: str) -> None: diff --git a/frontend/src/components/RevenueSummary.tsx b/frontend/src/components/RevenueSummary.tsx index 249b7b560..b73b0d43b 100644 --- a/frontend/src/components/RevenueSummary.tsx +++ b/frontend/src/components/RevenueSummary.tsx @@ -27,25 +27,21 @@ const formatMoney = (amount: string): string => { interface RevenueSummaryProps { propertyId?: string; - debugTenant?: string; showRaw?: boolean; } -export const RevenueSummary: React.FC = ({ propertyId = 'prop-001', debugTenant, showRaw }) => { +export const RevenueSummary: React.FC = ({ propertyId = 'prop-001', showRaw }) => { const [data, setData] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(''); - const activeTenant = debugTenant || 'candidate'; - useEffect(() => { const fetchRevenue = async () => { setLoading(true); try { - // Use SecureAPI to handle authentication automatically - // We pass the simulatedTenant option which SecureAPI will attach as a header + // The signed-in user's tenant comes from the bearer token that + // SecureAPI attaches; the client does not get to name it. const response = await SecureAPI.getDashboardSummary(propertyId, { - simulatedTenant: activeTenant, timestamp: Date.now() }); setData(response); @@ -58,7 +54,7 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr }; fetchRevenue(); - }, [propertyId, activeTenant]); + }, [propertyId]); if (loading) { return ( diff --git a/frontend/src/lib/secureApi.ts b/frontend/src/lib/secureApi.ts index f85f04c90..159324739 100644 --- a/frontend/src/lib/secureApi.ts +++ b/frontend/src/lib/secureApi.ts @@ -1450,22 +1450,23 @@ export class SecureAPIClient { // ============= DASHBOARD API ============= /** - * Get dashboard summary with optional simulation header + * Get dashboard summary for a property. + * + * The tenant is taken from the bearer token on the server. It is never sent + * from here: a caller-supplied tenant header would let anyone read another + * company's revenue by editing one request. */ - async getDashboardSummary(propertyId: string, options?: { simulatedTenant?: string, timestamp?: number }) { + async getDashboardSummary(propertyId: string, options?: { month?: number, year?: number, timestamp?: number }) { const queryParams = new URLSearchParams({ property_id: propertyId }); + if (options?.month && options?.year) { + queryParams.append('month', options.month.toString()); + queryParams.append('year', options.year.toString()); + } if (options?.timestamp) { queryParams.append('_t', options.timestamp.toString()); } - const requestOptions: RequestInit = {}; - if (options?.simulatedTenant) { - requestOptions.headers = { - 'X-Simulated-Tenant': options.simulatedTenant - }; - } - - return this.request(`/api/v1/dashboard/summary?${queryParams}`, requestOptions); + return this.request(`/api/v1/dashboard/summary?${queryParams}`); } async uploadCompanyLogo(logo_url: string) {