From 32b854cba63c6b7a8cf1728ec7b0dabc3e509326 Mon Sep 17 00:00:00 2001 From: liberto6 Date: Fri, 9 Oct 2026 18:05:54 +0200 Subject: [PATCH 1/4] fix: query the real database instead of mock data The DB pool built its URL from nonexistent supabase_db_* settings, so every query failed and calculate_total_revenue silently returned hardcoded mock totals keyed only by property_id. Build the asyncpg pool from settings.database_url, share it, and let DB errors surface. --- backend/app/core/database_pool.py | 46 ++++------ backend/app/services/reservations.py | 98 +++++++--------------- backend/tests/__init__.py | 0 backend/tests/conftest.py | 64 ++++++++++++++ backend/tests/test_dashboard_api.py | 31 +++++++ backend/tests/test_database_pool.py | 14 ++++ backend/tests/test_reservations_service.py | 17 ++++ 7 files changed, 174 insertions(+), 96 deletions(-) create mode 100644 backend/tests/__init__.py create mode 100644 backend/tests/conftest.py create mode 100644 backend/tests/test_dashboard_api.py create mode 100644 backend/tests/test_database_pool.py create mode 100644 backend/tests/test_reservations_service.py diff --git a/backend/app/core/database_pool.py b/backend/app/core/database_pool.py index d638dfcfe..d43fcdc0f 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 asyncpg import logging from ..config import settings @@ -8,53 +7,40 @@ class DatabasePool: def __init__(self): - self.engine = None - self.session_factory = None + self.pool = None 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 + self.pool = await asyncpg.create_pool( + settings.database_url, + min_size=1, + max_size=20 # Number of connections to maintain ) 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 + self.pool = None + raise async def close(self): """Close database connections""" - if self.engine: - await self.engine.dispose() + if self.pool: + await self.pool.close() + self.pool = None - async def get_session(self) -> AsyncSession: - """Get database session from pool""" - if not self.session_factory: + def get_session(self): + """Get database connection from pool (use with `async with`)""" + if not self.pool: raise Exception("Database pool not initialized") - return self.session_factory() + return self.pool.acquire() # Global database pool instance db_pool = DatabasePool() -async def get_db_session() -> AsyncSession: +async def get_db_session(): """Dependency to get database session""" async with db_pool.get_session() as session: yield session diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00ab..fcc2bf721 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -35,75 +35,41 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, """ Aggregates revenue from database. """ - try: - # Import database pool - from app.core.database_pool import DatabasePool - - # Initialize pool if needed - db_pool = DatabasePool() + # Import database pool + from app.core.database_pool import db_pool + + # Initialize pool if needed + if db_pool.pool is None: 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} + + async with db_pool.get_session() as conn: + query = """ + SELECT + property_id, + SUM(total_amount) as total_revenue, + COUNT(*) as reservation_count + FROM reservations + WHERE property_id = $1 AND tenant_id = $2 + GROUP BY property_id + """ + + row = await conn.fetchrow(query, property_id, tenant_id) + + 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"] } - - mock_property_data = mock_data.get(property_id, {'total': '0.00', 'count': 0}) - + else: + # No reservations found for this property 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/tests/__init__.py b/backend/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py new file mode 100644 index 000000000..2f07fad3f --- /dev/null +++ b/backend/tests/conftest.py @@ -0,0 +1,64 @@ +"""Shared test configuration. + +DB-backed tests run against a real Postgres loaded with database/schema.sql and +database/seed.sql. Point TEST_DATABASE_URL at it (default: localhost:5544, see +README of the test run command in the task notes). +""" +import os +from datetime import datetime, timezone + +TEST_DATABASE_URL = os.getenv( + "TEST_DATABASE_URL", "postgresql://postgres:postgres@localhost:5544/propertyflow" +) +# Must be set before any `app.*` import reads settings. +os.environ["DATABASE_URL"] = TEST_DATABASE_URL +os.environ.setdefault("REDIS_URL", "redis://localhost:6379/0") + +import fakeredis +import pytest_asyncio + +from app.core.database_pool import db_pool + + +@pytest_asyncio.fixture +async def fake_redis(monkeypatch): + from app.services import cache + + client = fakeredis.FakeAsyncRedis() + monkeypatch.setattr(cache, "redis_client", client) + yield client + await client.aclose() + + +@pytest_asyncio.fixture(autouse=True) +async def reset_db_pool(): + """Each test runs on its own event loop, so drop the shared pool after it.""" + yield + await db_pool.close() + db_pool.pool = None + + +@pytest_asyncio.fixture +async def extra_reservation(): + """Insert a reservation for the test and delete it afterwards.""" + inserted = [] + + async def _insert(res_id, property_id, tenant_id, amount, currency="USD", check_in=datetime(2024, 3, 10, 12, tzinfo=timezone.utc)): + if db_pool.pool is None: + await db_pool.initialize() + async with db_pool.get_session() as conn: + await conn.execute( + "INSERT INTO reservations (id, property_id, tenant_id, check_in_date, check_out_date," + " total_amount, currency) VALUES ($1, $2, $3, $4::timestamptz, $4::timestamptz + interval '2 days'," + " $5::numeric, $6)", + res_id, property_id, tenant_id, check_in, amount, currency, + ) + inserted.append(res_id) + + yield _insert + + if inserted: + if db_pool.pool is None: + await db_pool.initialize() + async with db_pool.get_session() as conn: + await conn.execute("DELETE FROM reservations WHERE id = ANY($1::text[])", inserted) diff --git a/backend/tests/test_dashboard_api.py b/backend/tests/test_dashboard_api.py new file mode 100644 index 000000000..0c8a669e2 --- /dev/null +++ b/backend/tests/test_dashboard_api.py @@ -0,0 +1,31 @@ +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from httpx import ASGITransport, AsyncClient + +from app.api.v1 import dashboard +from app.core.auth import authenticate_request +from app.models.auth import AuthenticatedUser + + +def make_app(tenant_id): + app = FastAPI() + app.include_router(dashboard.router, prefix="/api/v1") + user = AuthenticatedUser( + id="u1", email="u@x.com", permissions=[], cities=[], is_admin=False, tenant_id=tenant_id + ) + app.dependency_overrides[authenticate_request] = lambda: user + return app + + +def make_client(tenant_id): + return TestClient(make_app(tenant_id)) + + +@pytest.mark.asyncio +async def test_summary_returns_total_revenue_as_number(fake_redis): + app = make_app("tenant-a") + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + body = (await client.get("/api/v1/dashboard/summary", params={"property_id": "prop-001"})).json() + + assert body["total_revenue"] == 2250.0 diff --git a/backend/tests/test_database_pool.py b/backend/tests/test_database_pool.py new file mode 100644 index 000000000..d4f308551 --- /dev/null +++ b/backend/tests/test_database_pool.py @@ -0,0 +1,14 @@ +import pytest +from app.core.database_pool import DatabasePool + + +@pytest.mark.asyncio +async def test_pool_initializes_from_database_url_and_runs_queries(): + pool = DatabasePool() + await pool.initialize() + try: + assert pool.pool is not None + async with pool.get_session() as conn: + assert await conn.fetchval("SELECT COUNT(*) FROM tenants") == 2 + finally: + await pool.close() diff --git a/backend/tests/test_reservations_service.py b/backend/tests/test_reservations_service.py new file mode 100644 index 000000000..0988e0b24 --- /dev/null +++ b/backend/tests/test_reservations_service.py @@ -0,0 +1,17 @@ +from decimal import Decimal + +import pytest + +from app.core import database_pool +from app.core.database_pool import DatabasePool +from app.services import reservations + + +@pytest.mark.asyncio +async def test_database_failure_surfaces_instead_of_returning_mock_data(monkeypatch): + unreachable = DatabasePool() + monkeypatch.setattr(database_pool.settings, "database_url", "postgresql://x:x@127.0.0.1:1/none") + monkeypatch.setattr(reservations, "db_pool", unreachable, raising=False) + + with pytest.raises(Exception): + await reservations.calculate_total_revenue("prop-001", "tenant-a") From 7d9e8d73f2e18f32dfd7f557d35a612fa44ebd40 Mon Sep 17 00:00:00 2001 From: liberto6 Date: Fri, 9 Oct 2026 18:05:57 +0200 Subject: [PATCH 2/4] fix: isolate revenue cache and tenant resolution per tenant The revenue cache key was revenue:{property_id}, and prop-001 exists in two tenants, so one client could see the other's cached totals. Unresolved tenants also defaulted to tenant-a. Key the cache by tenant and reject users without a tenant with 403. --- backend/app/api/v1/dashboard.py | 4 +++- backend/app/core/tenant_resolver.py | 8 ++++---- backend/app/services/cache.py | 2 +- backend/tests/test_cache.py | 12 ++++++++++++ backend/tests/test_dashboard_api.py | 9 +++++++++ backend/tests/test_tenant_resolver.py | 9 +++++++++ 6 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 backend/tests/test_cache.py create mode 100644 backend/tests/test_tenant_resolver.py diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e..d5fd0a1bf 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -11,7 +11,9 @@ 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: + raise HTTPException(status_code=403, detail="User has no tenant assigned") revenue_data = await get_revenue_summary(property_id, tenant_id) diff --git a/backend/app/core/tenant_resolver.py b/backend/app/core/tenant_resolver.py index db09a4629..b1cf81d84 100644 --- a/backend/app/core/tenant_resolver.py +++ b/backend/app/core/tenant_resolver.py @@ -69,7 +69,7 @@ def resolve_tenant_from_user(user_data: dict) -> Optional[str]: return None @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. @@ -78,7 +78,7 @@ async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] user_email: User email Returns: - Tenant ID + Tenant ID, or None if the tenant cannot be resolved """ # Fallback mapping by known user email. if user_email == "sunset@propertyflow.com": @@ -88,8 +88,8 @@ async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] if user_email == "candidate@propertyflow.com": return "tenant-a" - # Default fallback - return "tenant-a" + # Unknown users must never be silently assigned to another client's tenant. + return None @staticmethod async def update_user_tenant_metadata(user_id: str, tenant_id: str) -> None: diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957..672e3f9d6 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -10,7 +10,7 @@ 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 = f"revenue:{tenant_id}:{property_id}" # Try to get from cache cached = await redis_client.get(cache_key) diff --git a/backend/tests/test_cache.py b/backend/tests/test_cache.py new file mode 100644 index 000000000..ec4c9cc12 --- /dev/null +++ b/backend/tests/test_cache.py @@ -0,0 +1,12 @@ +import pytest +from app.services.cache import get_revenue_summary + + +@pytest.mark.asyncio +async def test_cached_revenue_is_isolated_per_tenant_for_same_property_id(fake_redis): + tenant_a = await get_revenue_summary("prop-001", "tenant-a") + tenant_b = await get_revenue_summary("prop-001", "tenant-b") + + assert tenant_a["tenant_id"] == "tenant-a" + assert tenant_b["tenant_id"] == "tenant-b" + assert tenant_b["count"] == 0 diff --git a/backend/tests/test_dashboard_api.py b/backend/tests/test_dashboard_api.py index 0c8a669e2..ba9c20a58 100644 --- a/backend/tests/test_dashboard_api.py +++ b/backend/tests/test_dashboard_api.py @@ -22,6 +22,15 @@ def make_client(tenant_id): return TestClient(make_app(tenant_id)) +@pytest.mark.parametrize("tenant_id", [None, ""]) +def test_summary_rejects_user_without_tenant(tenant_id): + client = make_client(tenant_id) + + response = client.get("/api/v1/dashboard/summary", params={"property_id": "prop-001"}) + + assert response.status_code == 403 + + @pytest.mark.asyncio async def test_summary_returns_total_revenue_as_number(fake_redis): app = make_app("tenant-a") diff --git a/backend/tests/test_tenant_resolver.py b/backend/tests/test_tenant_resolver.py new file mode 100644 index 000000000..31883c36e --- /dev/null +++ b/backend/tests/test_tenant_resolver.py @@ -0,0 +1,9 @@ +import pytest +from app.core.tenant_resolver import TenantResolver + + +@pytest.mark.asyncio +async def test_unknown_user_does_not_default_to_another_tenant(): + tenant_id = await TenantResolver.resolve_tenant_id(user_id="u9", user_email="stranger@example.com") + + assert tenant_id is None From d9d9814555287bb9379649114349b0228d339e29 Mon Sep 17 00:00:00 2001 From: liberto6 Date: Fri, 9 Oct 2026 18:06:01 +0200 Subject: [PATCH 3/4] fix: compute monthly revenue in the property's timezone calculate_monthly_revenue used naive UTC month bounds and returned a placeholder 0. Query the real data and evaluate month bounds in each property's local timezone, so res-tz-1 (Feb 29 23:30 UTC) counts in March for a Paris property. --- backend/app/services/reservations.py | 47 ++++++++++------------ backend/tests/test_reservations_service.py | 10 +++++ 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index fcc2bf721..abd0043fb 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,35 +1,32 @@ -from datetime import datetime -from decimal import Decimal +from decimal import Decimal, ROUND_HALF_UP from typing import Dict, Any, List -async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: +async def calculate_monthly_revenue(property_id: str, tenant_id: str, month: int, year: int, db_session=None) -> Decimal: """ - Calculates revenue for a specific month. + Calculates revenue for a specific month, using the property's local timezone + to decide which month a check-in belongs to. """ + print(f"DEBUG: Querying revenue for {property_id} for {year}-{month:02d}") - 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}") - - # 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 + SELECT SUM(r.total_amount) as total + FROM reservations r + JOIN properties p ON p.id = r.property_id AND p.tenant_id = r.tenant_id + WHERE r.property_id = $1 + AND r.tenant_id = $2 + AND (r.check_in_date AT TIME ZONE p.timezone) >= make_timestamp($3, $4, 1, 0, 0, 0) + AND (r.check_in_date AT TIME ZONE p.timezone) < make_timestamp($3, $4, 1, 0, 0, 0) + interval '1 month' """ - - # 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 + + from app.core.database_pool import db_pool + + if db_pool.pool is None: + await db_pool.initialize() + + async with db_pool.get_session() as conn: + total = await conn.fetchval(query, property_id, tenant_id, year, month) + + return Decimal(str(total or 0)).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP) async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]: """ diff --git a/backend/tests/test_reservations_service.py b/backend/tests/test_reservations_service.py index 0988e0b24..08d04a987 100644 --- a/backend/tests/test_reservations_service.py +++ b/backend/tests/test_reservations_service.py @@ -15,3 +15,13 @@ async def test_database_failure_surfaces_instead_of_returning_mock_data(monkeypa with pytest.raises(Exception): await reservations.calculate_total_revenue("prop-001", "tenant-a") + + +@pytest.mark.asyncio +async def test_month_bounds_use_property_timezone(): + # res-tz-1 checks in 2024-02-29 23:30 UTC = 2024-03-01 00:30 in Europe/Paris. + march = await reservations.calculate_monthly_revenue("prop-001", "tenant-a", 3, 2024) + february = await reservations.calculate_monthly_revenue("prop-001", "tenant-a", 2, 2024) + + assert march == Decimal("2250.00") + assert february == Decimal("0.00") From 17fee5a0e869082968421e4677048f8b1ad238d2 Mon Sep 17 00:00:00 2001 From: liberto6 Date: Fri, 9 Oct 2026 18:06:04 +0200 Subject: [PATCH 4/4] fix: round revenue totals to cents half-up once Amounts are NUMERIC(10,3) and the total was returned unrounded. Round the Decimal total once with ROUND_HALF_UP so cent-level totals match finance. --- backend/app/services/reservations.py | 2 +- backend/tests/test_reservations_service.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index abd0043fb..a377dd373 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -53,7 +53,7 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, row = await conn.fetchrow(query, property_id, tenant_id) if row: - total_revenue = Decimal(str(row["total_revenue"])) + total_revenue = Decimal(str(row["total_revenue"])).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP) return { "property_id": property_id, "tenant_id": tenant_id, diff --git a/backend/tests/test_reservations_service.py b/backend/tests/test_reservations_service.py index 08d04a987..b61c50a21 100644 --- a/backend/tests/test_reservations_service.py +++ b/backend/tests/test_reservations_service.py @@ -25,3 +25,20 @@ async def test_month_bounds_use_property_timezone(): assert march == Decimal("2250.00") assert february == Decimal("0.00") + + +@pytest.mark.asyncio +async def test_total_is_quantized_to_two_decimals(): + result = await reservations.calculate_total_revenue("prop-001", "tenant-a") + + assert result["total"] == "2250.00" + + +@pytest.mark.asyncio +async def test_total_rounds_half_up_once_at_the_end(extra_reservation): + # prop-005 seed total is 3256.000; adding 0.005 gives exactly 3256.005 -> 3256.01 + await extra_reservation("res-half-up", "prop-005", "tenant-b", "0.005") + + result = await reservations.calculate_total_revenue("prop-005", "tenant-b") + + assert result["total"] == "3256.01"