Repository navigation
Conversation
Both tenants can have a property with the same id (see seed data), so caching by property_id alone let one tenant's revenue get served to another.
The property list was one hardcoded array shown to every user, so tenants saw each other's property names against their own ids. Also fixes the dashboard showing 0 revenue on first load since the initial selected property depended on tenant data that hadn't loaded yet.
DatabasePool was reading settings that don't exist (supabase_db_* instead of database_url), get_session was declared async but never awaited anything so 'async with' on it failed, greenlet was missing so sqlalchemy's async engine couldn't even import, and QueuePool isn't valid for an async engine. All four together meant every revenue query silently fell back to a hardcoded mock table, which is what was actually causing the wrong/leaked numbers.
calculate_monthly_revenue was a dead placeholder that always returned zero. Implemented it properly, converting check_in_date to the property's own timezone before comparing against month boundaries. A booking at 2024-02-29 23:30 UTC is already March 1st in Europe/Paris, so a naive UTC comparison would have silently dropped it from March - which matches the client's complaint about March totals being wrong. Wired month/year as optional query params on /dashboard/summary.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+25
to
+26
| db_pool = DatabasePool() | ||
| await db_pool.initialize() |
There was a problem hiding this comment.
Bug: The calculate_monthly_revenue function creates a new DatabasePool on every call without closing it, which will leak database connections.
Severity: CRITICAL
Suggested Fix
Remove the local instantiation of DatabasePool within calculate_monthly_revenue. Instead, import and use the existing global db_pool instance from backend/app/db/database_pool.py. This will ensure that a single, shared connection pool is used for all database operations, preventing connection leaks.
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-L26
Potential issue: The `calculate_monthly_revenue` function instantiates and initializes a
new `DatabasePool` on every invocation. This creates a new connection pool of up to 50
connections that is never closed or disposed of, as `db_pool.close()` is not called
before the function returns. Under load, each request to the corresponding endpoint will
leak database connections, quickly exhausting the PostgreSQL connection limit. This will
cause all subsequent database operations across the application to fail with connection
errors, leading to a service-wide outage.
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
1 participant
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.
No description provided.