Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Fetch billing historicals and display them on the billing page #731

Merged
merged 4 commits into from
Aug 29, 2023

Conversation

jshearer
Copy link
Contributor

Changes

Transparently pull data from the new billing_historicals table when fetching billing data for previous months. One interesting side-effect is that the data for the graphs of Data Volume by Month and Task Hours by Month will also be frozen and fetched from billing_historicals. I... think this is fine, just observing that it'll become possible to manually edit those values for prior months. We probably shouldn't do that though?

We also decided to not implement the ability to view line-items in the UI yet, as it's entirely possible/likely that until we finish cleaning up this data, line-items from prior months may still be wrong, even if the total cost is correct.

Screenshots

August is "live" data, June and July are billing_historicals

Screen Shot 2023-08-28 at 16 48 10

Screen Shot 2023-08-28 at 16 52 43

Note:
Do not merge before estuary/flow#1163 is live.

@jshearer jshearer requested a review from a team as a code owner August 28, 2023 20:53
@jshearer jshearer force-pushed the feature/billing_historicals branch from 5ad7997 to 011f22a Compare August 28, 2023 20:56
// the promise-like object returned by the Supabase SDK, we lose that
// hidden URL field in the process, breaking SWR.
// So... for the moment, this hacks it back into existence.
(prom as any).url = url;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we trying to make sure we get this in for the end of the month? If so - we can review this solution. If not... I'd say we should take a slightly different route. The fetchers have more than just the url. I do not think this would cause any major issues but is a worry to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we are unfortunately. If you give me a hint I'm sure I can figure out the non-hacky solution tomorrow!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet - this looks a lot safer to me.

So my initial thought on this one was we should really just fetch the two "types" of rows as two different calls. So we'd make a call to fetch the current month and another to fetch the older ones.

That way we can display right away and easily handle the differences without too many if/else blocks.

However, that could take a lot of effort and time for minimal benefit.

Copy link
Member

@travjenkins travjenkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - pushed two small changes myself (a comment and moving the select into a const)

Local Test
My local is old so the table isn't there and everything still works (but does not show up obviously) so feel good in terms of risk

Production Test
After pointing to prod I tested with the two tenants I have access to. Everything looks good and seems to be working just fine.

Only concern is that the screenshot shows a tons of decimal places but we can worry about that later on.

@jshearer jshearer merged commit 107e2b7 into main Aug 29, 2023
3 checks passed
@jshearer jshearer deleted the feature/billing_historicals branch August 29, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants