Repository navigation
Fix revenue dashboard bugs (tenant leak, wrong totals, timezone, rounding) - #210
Open
karthikeyan18v wants to merge 4 commits into
Open
karthikeyan18v wants to merge 4 commits into
karthikeyan18v wants to merge 4 commits into
Conversation
Found these while going through the Sunset and Ocean accounts. 1. Ocean seeing Sunset's revenue The revenue cache key was only the property id (revenue:prop-001). Both tenants have a prop-001, so whoever loaded it first got cached and the other tenant saw those numbers for the next 5 minutes. Added tenant_id to the cache key. 2. Dashboard numbers not coming from the database The db pool built its url from settings.supabase_db_*, which don't exist in config. It also used QueuePool, which doesn't work with the async engine, and get_session was async so "async with" failed on it. All of that was caught by a try/except that returned a hardcoded mock dict, the same for every tenant. Now it uses DATABASE_URL with one shared pool and the mock fallback is removed. Added sqlalchemy[asyncio] so greenlet gets installed, the async engine won't load without it. 3. Sunset's March total doesn't match Month boundaries were done in UTC. res-tz-1 checks in at 2024-02-29 23:30 UTC, which is 1 March 00:30 in Paris, so for that property it's a March booking. The month filter now converts check_in_date to the property's timezone first. 4. Totals off by a few cents Amounts are stored with 3 decimals and the total went out as a raw float and got rounded in the browser. It's rounded to cents on the backend now, using Decimal with half up rounding. Also added two endpoints the dashboard uses: the tenant's properties, and the reservations for a property (optionally for one month).
1. Every client saw every property The property dropdown was a hardcoded list of all 5 properties from both tenants, so Ocean saw Sunset's property names. prop-001 is also a different property for each tenant. The list now comes from /dashboard/properties for the logged in tenant. 2. Profile page failing to load getApiBase() uses VITE_BACKEND_URL, but .env is in .dockerignore, so in the docker build it was empty and the profile request went to nginx on port 3000. nginx sent back index.html and parsing it as JSON failed. It now falls back to localhost:8000, same as localAuthClient already does. Also added a month picker and a bookings table under the revenue card, so a month's total can be checked against the actual bookings.
- TenantResolver.resolve_tenant_id returned "tenant-a" for any user it didn't recognise, so they'd see Sunset's data. It reads tenant_id from the token first now, and returns None if it can't find one. The dashboard endpoints return 403 in that case instead of using a made-up "default_tenant". - asking for a property that belongs to another tenant returned 0.00. It's a 404 now. - currency was hardcoded to USD. It uses the reservations' currency now and returns 422 if a property has more than one. - show an error on the dashboard if the property list fails to load.
- if redis was down, get/setex threw and the whole summary request failed with a 500, even though the db was fine. Cache errors are logged and skipped now, it just reads from the db. - if the db was down the endpoints threw a 500 with a stack trace. They return a 503 "try again shortly" now, and recover on their own once the db is back.
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.
Logged in as both clients and went through the dashboard. Here's what I found and fixed:
Ocean seeing Sunset's numbers: the cache key was just
revenue:{property_id}. Both tenants have a prop-001 so they were sharing the cached result. Added tenant_id to the key.Numbers didn't match the DB at all: the db pool was using
settings.supabase_db_*which don't exist, so it errored every time and fell back to the hardcoded mock data. Switched it to DATABASE_URL, fixed get_session, and removed the mock fallback. Also needed sqlalchemy[asyncio] for greenlet.Sunset's March total was off: months were calculated in UTC. res-tz-1 is Feb 29 23:30 UTC but that's March 1 in Paris. Now it uses the property's timezone.
Few cents off: the total was sent as a float and rounded in the browser. Rounding on the backend with Decimal now.
Property dropdown was hardcoded with all 5 properties, so each client could see the other's. Now it loads from the API for the logged in tenant.
Unknown users got tenant-a: the tenant resolver defaulted anyone it didn't recognise to tenant-a, so they'd see Sunset's data. Now they get a 403. Asking for another tenant's property returns 404 instead of 0.00, and the currency comes from the reservations instead of being hardcoded.
Redis or DB going down broke the whole dashboard with a 500. Cache errors are skipped now so it reads from the db, and a db outage returns a 503 until it's back.
Profile page was broken in docker because .env is dockerignored, so the API url was empty. Added a fallback to localhost:8000.
Also added a month filter and a bookings list so you can check totals against the bookings.
To test: Sunset > Beach House Alpha > March 2024 should be 2,250.00 with 4 bookings. Ocean's prop-001 should show 0, and asking for the other tenant's property gives a 404.