Repository navigation
Bug fix/grayjourney - #232
Open
grayjourney wants to merge 5 commits into
Open
grayjourney wants to merge 5 commits into
grayjourney wants to merge 5 commits into
Conversation
- add sqlalchemy[asyncio] (greenlet) - use settings.database_url - use AsyncAdaptedQueuePool - get_session: plain def - reuse one db_pool
- query uses properties.timezone, joins on tenant - add tests: March vs UTC, other tenant, New York
- cache key: revenue:{tenant_id}:{property_id}
- log cache HIT/MISS with tenant key
- add tests: cross-tenant cache, HIT/MISS logs
- add GET /dashboard/properties - dashboard loads properties from API, not hardcoded - add tests: property list per tenant
- round once with Decimal, half-up, before JSON - frontend: drop Math.round float rounding - add tests: half-cent totals via real route
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+19
to
+27
| query = text(""" | ||
| SELECT COALESCE(SUM(r.total_amount), 0) 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 = :property_id | ||
| AND r.tenant_id = :tenant_id | ||
| AND r.check_in_date >= (CAST(:start_date AS timestamp) AT TIME ZONE p.timezone) | ||
| AND r.check_in_date < (CAST(:end_date AS timestamp) AT TIME ZONE p.timezone) | ||
| """) |
There was a problem hiding this comment.
Bug: calculate_monthly_revenue is not wired into /dashboard/summary or get_revenue_summary, leaving the timezone fix unreachable by live production requests.
Severity: LOW
Suggested Fix
Update get_revenue_summary in the dashboard service flow to invoke calculate_monthly_revenue instead of calculate_total_revenue when monthly figures and timezone awareness are required.
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#L19-L27
Potential issue: The timezone-aware calculation implemented in
`calculate_monthly_revenue` is not integrated into production endpoints. The
`/dashboard/summary` endpoint routes through `get_revenue_summary` and calls
`calculate_total_revenue`, which aggregates all reservations without filtering by month
or handling property timezones. As a result, the timezone handling introduced in this PR
is unreachable from the API and has no effect on live dashboard metrics.
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.
video test:
https://www.loom.com/share/e8484537ca624bb48ad95864a5172c9c
https://www.loom.com/share/3664785a2efa449797c16ad168e1efda