Skip to content

Fix revenue dashboard: tenant isolation, timezone months, cent rounding - #241

Open
izemhsn wants to merge 1 commit into
Base360-AI:mainfrom
izemhsn:fix/revenue-dashboard-bugs
Open

izemhsn wants to merge 1 commit into
Base360-AI:mainfrom
izemhsn:fix/revenue-dashboard-bugs

Conversation

@izemhsn

@izemhsn izemhsn commented Nov 10, 2026

Copy link
Copy Markdown

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.

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.
Comment on lines +39 to +49
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: calculate_monthly_revenue is implemented but has no callers, leaving /dashboard/summary returning all-time revenue totals instead of monthly figures.
Severity: MEDIUM

Suggested Fix

Add year and month parameters to /dashboard/summary, update the caching layer to include month scope, and update the endpoint to call calculate_monthly_revenue instead of calculate_total_revenue.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: backend/app/services/reservations.py#L25-L49

Potential issue: `calculate_monthly_revenue` is implemented to calculate month-scoped
revenue totals considering property timezones, but it is not invoked anywhere in the
codebase. The live summary endpoint at `/dashboard/summary` continues calling
`calculate_total_revenue`, which sums all-time reservation amounts across all dates.
Consequently, monthly revenue metrics remain incorrect and the new calculation logic has
no effect on API responses.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant