Repository navigation
Conversation
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) |
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tenant isolation (Ocean Rentals seeing another company's revenue):
async with, initialize onceWrong March totals (Sunset Properties):
Totals off by a few cents:
Also clear the stale error in RevenueSummary when switching property.