From b9be74ceeac60024ab3a36bdb48f6182c7bcdc5d Mon Sep 17 00:00:00 2001 From: aplainzetakind Date: Fri, 31 Jul 2026 16:48:57 +0300 Subject: [PATCH 1/3] fix(core): wire database pool via shared settings - database_url was built from non-existent supabase_* settings, causing init to crash and every request to fall back to mock data. Read settings.database_url directly and coerce scheme to asyncpg. - DatabasePool was recreated per request in reservations service. Switch to the shared global singleton. - Initialise pool in app lifespan; align get_session (sync) with async-with usage. - Add backend test for the fixed code path; include pytest and pytest-asyncio in requirements. --- .gitignore | 2 ++ backend/app/core/database_pool.py | 6 ++-- backend/app/main.py | 3 ++ backend/app/services/reservations.py | 7 +--- backend/requirements.txt | 2 ++ backend/tests/__init__.py | 0 backend/tests/test_db_connection.py | 50 ++++++++++++++++++++++++++++ 7 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 .gitignore create mode 100644 backend/tests/__init__.py create mode 100644 backend/tests/test_db_connection.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 000000000..de359330a --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +**/__pycache__/ diff --git a/backend/app/core/database_pool.py b/backend/app/core/database_pool.py index d638dfcfe..c7f8f085b 100644 --- a/backend/app/core/database_pool.py +++ b/backend/app/core/database_pool.py @@ -1,4 +1,5 @@ import asyncio +import re from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker from sqlalchemy.pool import QueuePool import logging @@ -15,11 +16,10 @@ 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}" + database_url = re.sub(r"^postgresql://", "postgresql+asyncpg://", settings.database_url) 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,7 +45,7 @@ async def close(self): if self.engine: await self.engine.dispose() - async def get_session(self) -> AsyncSession: + def get_session(self) -> AsyncSession: """Get database session from pool""" if not self.session_factory: raise Exception("Database pool not initialized") diff --git a/backend/app/main.py b/backend/app/main.py index 00734b2fa..bf581c513 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -100,6 +100,9 @@ async def lifespan(app: FastAPI): logger.error(f"❌ Supabase connection pool initialization failed: {e}") # Continue startup - fallback to direct connections + from .core.database_pool import db_pool + await db_pool.initialize() + # Initialize Redis connection with timeout try: await redis_client.initialize() diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00ab..410655583 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -36,12 +36,7 @@ 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() - await db_pool.initialize() + from app.core.database_pool import db_pool if db_pool.session_factory: async with db_pool.get_session() as session: diff --git a/backend/requirements.txt b/backend/requirements.txt index 6b777d2aa..1ed1911f0 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -27,3 +27,5 @@ loguru redis>=5.0.0 asyncpg>=0.27.0 requests +pytest>=8.0.0 +pytest-asyncio>=0.24.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/test_db_connection.py b/backend/tests/test_db_connection.py new file mode 100644 index 000000000..4ab1ad6e9 --- /dev/null +++ b/backend/tests/test_db_connection.py @@ -0,0 +1,50 @@ +import pytest +from unittest.mock import MagicMock, AsyncMock, patch + + +@pytest.mark.asyncio +async def test_calculate_total_revenue_never_hits_mock_fallback(): + """ + When database pool is properly initialized, calculate_total_revenue + queries the database and does NOT fall back to mock data. + """ + print_calls = [] + + def capture(*args, **kwargs): + msg = args[0] if args else "" + if isinstance(msg, str): + print_calls.append(msg) + + from app.core.database_pool import db_pool + + mock_row = MagicMock() + mock_row.total_revenue = "2250.000" + mock_row.reservation_count = 4 + + mock_result = MagicMock() + mock_result.fetchone.return_value = mock_row + + mock_session = AsyncMock() + mock_session.execute = AsyncMock(return_value=mock_result) + mock_session.__aenter__.return_value = mock_session + + mock_factory = MagicMock(return_value=mock_session) + + try: + with patch( + "app.core.database_pool.async_sessionmaker", return_value=mock_factory + ): + await db_pool.initialize() + + with patch("builtins.print", capture): + from app.services.reservations import calculate_total_revenue + + result = await calculate_total_revenue("prop-001", "tenant-a") + + errors = [c for c in print_calls if "Database error" in c] + assert not errors, f"Encountered database error: {errors[0]}" + assert result["total"] == "2250.000" + assert result["count"] == 4 + finally: + db_pool.session_factory = None + db_pool.engine = None \ No newline at end of file From 00de22c138312483c8308cac471658847aa3990a Mon Sep 17 00:00:00 2001 From: aplainzetakind Date: Fri, 31 Jul 2026 16:48:57 +0300 Subject: [PATCH 2/3] fix(core): scope property listing and cache keys by tenant - Frontend Dashboard rendered a hardcoded PROPERTIES list shared across clients. Properties are not unique across tenants; another tenant's property names and IDs were visible in the dropdown. Replace with API fetch from /api/v1/properties, which filters by authenticated tenant. - Cache key in services/cache was keyed only on property_id. Two tenants with a property sharing the same id would receive each other's cached revenue summary. Include tenant_id in the key. - Stricten selectedProperty state to string | null and drop the hardcoded propertyId default in RevenueSummary (incidental: no falsy-sentinel bugs, and the magic default would leak another tenant's data if the parent guard ever regressed). --- backend/app/api/v1/properties.py | 21 ++++++ backend/app/main.py | 4 + backend/app/services/cache.py | 20 ++++- backend/app/services/properties.py | 29 ++++++++ backend/tests/test_properties_isolation.py | 85 ++++++++++++++++++++++ frontend/src/components/Dashboard.tsx | 74 +++++++++++++------ frontend/src/components/RevenueSummary.tsx | 20 ++--- 7 files changed, 215 insertions(+), 38 deletions(-) create mode 100644 backend/app/api/v1/properties.py create mode 100644 backend/app/services/properties.py create mode 100644 backend/tests/test_properties_isolation.py diff --git a/backend/app/api/v1/properties.py b/backend/app/api/v1/properties.py new file mode 100644 index 000000000..c620dd775 --- /dev/null +++ b/backend/app/api/v1/properties.py @@ -0,0 +1,21 @@ +from fastapi import APIRouter, Depends, HTTPException +from typing import List, Dict, Any +from ...core.auth import authenticate_request +from ...models.auth import AuthenticatedUser +from ...services.cache import get_tenant_properties + +router = APIRouter() + +@router.get("/properties") +async def get_properties( + current_user: AuthenticatedUser = Depends(authenticate_request) +) -> List[Dict[str, Any]]: + tenant_id = current_user.tenant_id + + try: + return await get_tenant_properties(tenant_id) + except Exception as e: + raise HTTPException( + status_code=500, + detail=f"Failed to fetch properties: {str(e)}" + ) \ No newline at end of file diff --git a/backend/app/main.py b/backend/app/main.py index bf581c513..77db66539 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -29,6 +29,7 @@ persistent_auth, dashboard, login, + properties, ) from .monitoring.middleware import PerformanceMonitoringMiddleware @@ -187,6 +188,9 @@ async def lifespan(app: FastAPI): # Dashboard app.include_router(dashboard.router, prefix="/api/v1", tags=["dashboard"]) +# Properties +app.include_router(properties.router, prefix="/api/v1", tags=["properties"]) + # Bootstrap & Settings (for AppContext) app.include_router(company_settings.router, prefix="/api/v1", tags=["company-settings"]) app.include_router(bootstrap.router, prefix="/api/v1", tags=["bootstrap"]) diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957..0fc7c5c8a 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -1,16 +1,32 @@ import json import redis.asyncio as redis -from typing import Dict, Any +from typing import Dict, Any, List import os # Initialize Redis client (typically configured centrally). redis_client = redis.Redis.from_url(os.getenv("REDIS_URL", "redis://localhost:6379/0")) +async def get_tenant_properties(tenant_id: str) -> List[Dict[str, Any]]: + cache_key = f"properties:{tenant_id}" + + cached = await redis_client.get(cache_key) + if cached: + return json.loads(cached) + + from app.services.properties import get_tenant_properties_db + + result = await get_tenant_properties_db(tenant_id) + + await redis_client.setex(cache_key, 300, json.dumps(result)) + + return result + + 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/app/services/properties.py b/backend/app/services/properties.py new file mode 100644 index 000000000..6dbf0a3d7 --- /dev/null +++ b/backend/app/services/properties.py @@ -0,0 +1,29 @@ +from typing import List, Dict, Any + + +async def get_tenant_properties_db(tenant_id: str) -> List[Dict[str, Any]]: + from app.core.database_pool import db_pool + from sqlalchemy import text + + 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, tenant_id " + "FROM properties " + "WHERE tenant_id = :tenant_id" + ), + {"tenant_id": tenant_id}, + ) + rows = result.fetchall() + return [ + { + "id": row.id, + "name": row.name, + "timezone": row.timezone, + "tenant_id": row.tenant_id, + } + for row in rows + ] \ No newline at end of file diff --git a/backend/tests/test_properties_isolation.py b/backend/tests/test_properties_isolation.py new file mode 100644 index 000000000..752359a63 --- /dev/null +++ b/backend/tests/test_properties_isolation.py @@ -0,0 +1,85 @@ +import pytest +from unittest.mock import MagicMock, AsyncMock, patch + + +@pytest.mark.asyncio +async def test_properties_returned_only_for_correct_tenant(): + """ + get_tenant_properties_db must return only the calling tenant's + properties, even when property IDs overlap across tenants. + """ + mock_session = AsyncMock() + mock_session.__aenter__.return_value = mock_session + + def _row(id, name, timezone, tenant_id): + r = MagicMock() + r.id = id + r.name = name + r.timezone = timezone + r.tenant_id = tenant_id + return r + + tenant_a_props = [ + _row("prop-001", "Beach House Alpha", "Europe/Paris", "tenant-a"), + _row("prop-002", "City Apartment Downtown", "Europe/Paris", "tenant-a"), + _row("prop-003", "Country Villa Estate", "Europe/Paris", "tenant-a"), + ] + tenant_b_props = [ + _row("prop-001", "Mountain Lodge Beta", "America/New_York", "tenant-b"), + _row("prop-004", "Lakeside Cottage", "America/New_York", "tenant-b"), + _row("prop-005", "Urban Loft Modern", "America/New_York", "tenant-b"), + ] + + executed_params = [] + + async def mock_execute(query, params): + executed_params.append(dict(params)) + result = MagicMock() + if params["tenant_id"] == "tenant-a": + result.fetchall.return_value = tenant_a_props + else: + result.fetchall.return_value = tenant_b_props + return result + + mock_session.execute = mock_execute + + mock_factory = MagicMock(return_value=mock_session) + + from app.core.database_pool import db_pool + + try: + with patch( + "app.core.database_pool.async_sessionmaker", return_value=mock_factory + ): + await db_pool.initialize() + + from app.services.properties import get_tenant_properties_db + + a_result = await get_tenant_properties_db("tenant-a") + b_result = await get_tenant_properties_db("tenant-b") + + # tenant-a: has prop-001 (Beach House Alpha), not prop-004 or prop-005 + a_ids = [p["id"] for p in a_result] + assert "prop-001" in a_ids + assert "prop-002" in a_ids + assert "prop-003" in a_ids + assert "prop-004" not in a_ids + assert "prop-005" not in a_ids + assert a_result[0]["name"] == "Beach House Alpha" + + # tenant-b: has prop-001 (Mountain Lodge Beta), not prop-002 or prop-003 + b_ids = [p["id"] for p in b_result] + assert "prop-001" in b_ids + assert "prop-004" in b_ids + assert "prop-005" in b_ids + assert "prop-002" not in b_ids + assert "prop-003" not in b_ids + assert b_result[0]["name"] == "Mountain Lodge Beta" + + # Both queries include tenant_id in params + assert executed_params[0]["tenant_id"] == "tenant-a" + assert executed_params[1]["tenant_id"] == "tenant-b" + + finally: + db_pool.session_factory = None + db_pool.engine = None \ No newline at end of file diff --git a/frontend/src/components/Dashboard.tsx b/frontend/src/components/Dashboard.tsx index a21bba404..857c4229b 100644 --- a/frontend/src/components/Dashboard.tsx +++ b/frontend/src/components/Dashboard.tsx @@ -1,16 +1,34 @@ -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'); + const [properties, setProperties] = useState([]); + const [selectedProperty, setSelectedProperty] = useState(null); + const [loadingProps, setLoadingProps] = useState(true); + + useEffect(() => { + const fetchProperties = async () => { + try { + const response = await SecureAPI.getProperties(); + const data: Property[] = response.data || []; + setProperties(data); + if (data.length > 0 && selectedProperty === null) { + setSelectedProperty(data[0].id); + } + } catch (err) { + console.error("Failed to load properties", err); + } finally { + setLoadingProps(false); + } + }; + fetchProperties(); + }, []); return (
@@ -26,27 +44,35 @@ const Dashboard: React.FC = () => { Monthly performance insights for your properties

- - {/* Property Selector */} +
- + {!loadingProps && properties.length === 0 ? ( +

No properties available for your account.

+ ) : ( + + )}
- + {selectedProperty !== null && }
@@ -54,4 +80,4 @@ const Dashboard: React.FC = () => { ); }; -export default Dashboard; +export default Dashboard; \ No newline at end of file diff --git a/frontend/src/components/RevenueSummary.tsx b/frontend/src/components/RevenueSummary.tsx index dbb6d0629..4be554f23 100644 --- a/frontend/src/components/RevenueSummary.tsx +++ b/frontend/src/components/RevenueSummary.tsx @@ -9,28 +9,24 @@ interface RevenueData { } interface RevenueSummaryProps { - propertyId?: string; - debugTenant?: string; + propertyId: string; + debugTenant?: string; showRaw?: boolean; } -export const RevenueSummary: React.FC = ({ propertyId = 'prop-001', debugTenant, showRaw }) => { +export const RevenueSummary: React.FC = ({ propertyId, debugTenant, 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 - const response = await SecureAPI.getDashboardSummary(propertyId, { - simulatedTenant: activeTenant, - timestamp: Date.now() - }); + const options = debugTenant + ? { simulatedTenant: debugTenant, timestamp: Date.now() } + : { timestamp: Date.now() }; + const response = await SecureAPI.getDashboardSummary(propertyId, options); setData(response); } catch (err) { setError('Failed to load revenue data'); @@ -41,7 +37,7 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr }; fetchRevenue(); - }, [propertyId, activeTenant]); + }, [propertyId, debugTenant]); if (loading) { return ( From f447a08edcfda1bf13e4333db12ff60ad04432a8 Mon Sep 17 00:00:00 2001 From: aplainzetakind Date: Fri, 31 Jul 2026 16:48:57 +0300 Subject: [PATCH 3/3] fix(core): ensure consistent rounding of monetary values - Frontend displayed totals via Math.round(x * 100) / 100 on a float parsed from the DB. A halfway third-decimal value like .555 with an internal float representation of .5549... was rounded down by Math.round's halfway-to-even behaviour, producing values a few cents below the true sum. - reservations.py quantises the SUM(total_amount) to 2 decimal places with ROUND_HALF_UP so .5 always rounds up, regardless of how the float is laid out in memory. - dashboard.py drops the float() round-trip; pass the canonical 2dp string straight through. - RevenueSummary types total_revenue as string, parses via parseFloat (preserves precision from a string source), and removes the "Precision Mismatch Detected" warning UI that no longer reflects reality. - Test assertion updated to match the 2dp output. --- backend/app/api/v1/dashboard.py | 4 +--- backend/app/services/reservations.py | 4 ++-- backend/tests/test_db_connection.py | 5 +++-- frontend/src/components/RevenueSummary.tsx | 16 ++-------------- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e..3ca776c82 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -15,11 +15,9 @@ async def get_dashboard_summary( revenue_data = await get_revenue_summary(property_id, tenant_id) - total_revenue_float = float(revenue_data['total']) - return { "property_id": revenue_data['property_id'], - "total_revenue": total_revenue_float, + "total_revenue": revenue_data['total'], "currency": revenue_data['currency'], "reservations_count": revenue_data['count'] } diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 410655583..67bc74026 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,5 +1,5 @@ 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: @@ -60,7 +60,7 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, row = result.fetchone() if row: - total_revenue = Decimal(str(row.total_revenue)) + total_revenue = 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_db_connection.py b/backend/tests/test_db_connection.py index 4ab1ad6e9..858bec3ad 100644 --- a/backend/tests/test_db_connection.py +++ b/backend/tests/test_db_connection.py @@ -1,4 +1,5 @@ import pytest +from decimal import Decimal from unittest.mock import MagicMock, AsyncMock, patch @@ -18,7 +19,7 @@ def capture(*args, **kwargs): from app.core.database_pool import db_pool mock_row = MagicMock() - mock_row.total_revenue = "2250.000" + mock_row.total_revenue = Decimal("2250.000") mock_row.reservation_count = 4 mock_result = MagicMock() @@ -43,7 +44,7 @@ def capture(*args, **kwargs): errors = [c for c in print_calls if "Database error" in c] assert not errors, f"Encountered database error: {errors[0]}" - assert result["total"] == "2250.000" + assert result["total"] == "2250.00" assert result["count"] == 4 finally: db_pool.session_factory = None diff --git a/frontend/src/components/RevenueSummary.tsx b/frontend/src/components/RevenueSummary.tsx index 4be554f23..07a26c146 100644 --- a/frontend/src/components/RevenueSummary.tsx +++ b/frontend/src/components/RevenueSummary.tsx @@ -3,7 +3,7 @@ import { SecureAPI } from '../lib/secureApi'; interface RevenueData { property_id: string; - total_revenue: number; + total_revenue: string; currency: string; reservations_count: number; } @@ -57,7 +57,7 @@ export const RevenueSummary: React.FC = ({ propertyId, debu if (error) return
{error}
; if (!data) return null; - const displayTotal = Math.round(data.total_revenue * 100) / 100; + const displayTotal = parseFloat(data.total_revenue); return (
@@ -97,18 +97,6 @@ export const RevenueSummary: React.FC = ({ propertyId, debu

{data.reservations_count} bookings

- - {/* Precision Warning Area */} -
- {Math.abs(data.total_revenue - displayTotal) > 0.000001 && showRaw && ( -
- - - - Precision Mismatch Detected -
- )} -
);