Repository navigation
Fix revenue dashboard accuracy and tenant isolation - #215
ayushkumar912 wants to merge 13 commits into
Conversation
The pool was built from database settings that do not exist, and it used a sync pool on the async engine, so revenue queries never reached Postgres. Co-authored-by: Cursor <cursoragent@cursor.com>
A failed connection used to return a hardcoded total, so the dashboard showed numbers that were never in the books. Co-authored-by: Cursor <cursoragent@cursor.com>
Both clients share prop-001, and the cache key was only the property id, so a refresh could show the other company's total. Co-authored-by: Cursor <cursoragent@cursor.com>
A check-in at 23:30 UTC on 29 Feb is already 1 March in Paris, so the March total was missing that stay. Co-authored-by: Cursor <cursoragent@cursor.com>
The total was cast to a float and rounded again in the browser, which drifted the sub-cent bookings by a few cents. Co-authored-by: Cursor <cursoragent@cursor.com>
The property list was hardcoded, so Ocean could open Sunset's properties and the other way around. Co-authored-by: Cursor <cursoragent@cursor.com>
Unknown users were treated as Sunset, and a missing tenant id fell back to a shared default. Co-authored-by: Cursor <cursoragent@cursor.com>
The revenue card always showed a green 12% increase that the API never sent. Co-authored-by: Cursor <cursoragent@cursor.com>
Revenue requests were sending a simulated tenant header, so the client could ask for someone else's data. Co-authored-by: Cursor <cursoragent@cursor.com>
The session helper was not reading the tenant claim the login endpoint already puts on the token. Co-authored-by: Cursor <cursoragent@cursor.com>
Added asyncpg and greenlet to the backend dependencies for improved database handling. Updated sqlalchemy to use the asyncio variant. Removed the axios package and its related files from the frontend as it was no longer needed. Co-authored-by: Cursor <cursoragent@cursor.com>
The bytecode under __pycache__ was committed by accident. Ignore it so it stays out of later commits. Co-authored-by: Cursor <cursoragent@cursor.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (options?.month) { | ||
| queryParams.append('month', String(options.month)); | ||
| } |
There was a problem hiding this comment.
Bug: Clearing the month input field causes a falsy check if (options?.month) to fail, omitting the required month parameter from the API call and resulting in an error.
Severity: HIGH
Suggested Fix
Modify the conditional check in getDashboardSummary to correctly handle 0 as a valid value. Instead of if (options?.month), use a check that is not falsy for zero, such as if (options?.month !== undefined && options?.month !== null).
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: frontend/src/lib/secureApi.ts#L1453-L1455
Potential issue: When a user clears the month input field, the state is set to `0`
because `Number("")` evaluates to `0`. In the `getDashboardSummary` function, the
conditional `if (options?.month)` is used to check for the presence of the month. Since
`0` is a falsy value, this check fails, and the `month` query parameter is not appended
to the API request. The backend endpoint requires this parameter, so it responds with a
`422 Unprocessable Entity` error, which causes the UI to show a "Failed to load revenue
data" message.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
This one is real. Clearing the number input sets the month to 0, and if (options?.month) dropped it, so the API got a request with no month and returned 422. The same check was on year.
Month and year are now sent whenever they are finite numbers, including 0. The card also skips the request until the month is 1–12 and the year is in range, so an empty field shows a short prompt instead of a failed load.
| if (options?.timestamp) { | ||
| queryParams.append('_t', options.timestamp.toString()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: A unique timestamp parameter is included in the cache key for dashboard summary requests, which effectively disables the client-side cache and causes unnecessary network requests.
Severity: LOW
Suggested Fix
Exclude the _t cache-busting parameter from the cache key generation logic. Modify generateCacheKey to filter out the _t parameter before creating the sorted parameter string used in the cache key.
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: frontend/src/lib/secureApi.ts#L1462
Potential issue: The `getDashboardSummary` function is called with a `timestamp:
Date.now()` parameter, which is then appended to the request URL as a `_t` query
parameter. The client-side caching logic in `generateCacheKey` uses the full request
URL, including all query parameters, to create the cache key. Because the `_t` value is
unique for every request, a new cache key is generated each time, preventing any cache
hits. This effectively disables the intended 5-second client-side cache for this
endpoint, leading to unnecessary backend requests.
Also affects:
frontend/src/lib/secureApi.ts:235~241
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
The dashboard call was the problem. It passed Date.now() on every load, and that unique _t made the 5-second GET cache miss every time. That timestamp is gone, so this endpoint can use the existing cache.
I left generateCacheKey as it is. Other callers use _t on purpose to bypass that cache. Stripping it for every request would serve stale data to those calls.
Clearing the month field sets it to 0, and a falsy check was dropping that parameter. The summary also no longer adds a fresh timestamp on every load, so the short client cache can be reused. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
Test plan
cd backend && python -m unittest discover -s testsMade with Cursor