Fix/revenue dashboard bugs

backend/app/api/v1/dashboard.py

@@ -1,25 +1,64 @@
-from fastapi import APIRouter, Depends, HTTPException
-from typing import Dict, Any
+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
+from app.services.reservations import RevenueDataUnavailable
 from app.core.auth import authenticate_request as get_current_user
 
 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,
+    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"
+    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",
+        )
 
-    revenue_data = await get_revenue_summary(property_id, tenant_id)
+    if (month is None) != (year is None):
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="month and year must be supplied together",
+        )
 
-    total_revenue_float = float(revenue_data['total'])
+    try:
+        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.
+        raise HTTPException(
+            status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+            detail="Revenue data is temporarily unavailable",
+        ) from e
 
     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']
     }

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()

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:

backend/app/services/cache.py

@@ -1,29 +1,80 @@
 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__)
+
 # Initialize Redis client (typically configured centrally).
 redis_client = redis.Redis.from_url(os.getenv("REDIS_URL", "redis://localhost:6379/0"))
 
-async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any]:
+CACHE_TTL_SECONDS = 300
+
+
+def build_cache_key(
+    property_id: str,
+    tenant_id: str,
+    month: Optional[int] = None,
+    year: Optional[int] = None,
+) -> 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.
+
+    The period is part of the key so a month's figure is never served as the
+    all-time total.
+    """
+    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,
+    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 = f"revenue:{property_id}"
-    
+    cache_key = build_cache_key(property_id, tenant_id, month, year)
+
     # 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
-
+    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
-    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