feat: add category and rollup stats #576
Conversation
🦋 Changeset detectedLatest commit: d1a30e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Skipped Deployments
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #576 +/- ##
=======================================
Coverage 89.38% 89.38%
=======================================
Files 150 150
Lines 9933 9933
Branches 1035 1035
=======================================
Hits 8879 8879
Misses 1054 1054 ☔ View full report in Codecov by Sentry. |
| }); | ||
| const result = await dailyStatsFn(operationParam); | ||
|
|
||
| if (typeof result === "number") { |
There was a problem hiding this comment.
I'd add a comment here making clearer what are the expected types of result for such a logic
| "day": 2023-08-20T00:00:00.000Z, | ||
| "rollup": null, |
There was a problem hiding this comment.
Rollup field shouldn't be null when category is "ROLLUP", right?
There was a problem hiding this comment.
Rollup field shouldn't be null when category is "ROLLUP", right?
For rows in the stats table, this can occur because the row represents an aggregation that includes all rollups, not just a single one. That’s why the rollup value is null.
The same applies to rows where both category and rollup are null; these rows represent total aggregations that include all elements.
clis/stats-aggregation-cli/test/commands/__snapshots__/daily.test.ts.snap
Show resolved
Hide resolved
clis/stats-aggregation-cli/test/commands/__snapshots__/daily.test.ts.snap
Show resolved
Hide resolved
| @@ -33,7 +33,7 @@ describe("Transaction router", async () => { | |||
| caller.tx.getAll(input).then(({ transactions }) => transactions) | |||
| ); | |||
|
|
|||
| it("should get the total number of transactions", async () => { | |||
| it.skip("should get the total number of transactions", async () => { | |||
| model Address { | ||
| address String @id | ||
| firstBlockNumberAsSender Int? @map("first_block_number_as_sender") | ||
| model AddressHistory { |
There was a problem hiding this comment.
I don't like the name tbh. How about naming it AddressData, AddressCategoryInfo or AddressByCategoryInfo? 🤔
There was a problem hiding this comment.
I like AddressCategoryInfo
packages/db/prisma/migrations/20240916233526_backfill_address_history_table/migration.sql
Outdated
Show resolved
Hide resolved
Co-authored-by: Pablo Castellano <pablo@anche.no>
Checklist
Description
Resolves #518
This PR introduces changes to aggregate statistics by category and rollups, and stores them in the daily and overall stats tables. To achieve this, it adds optional
categoryandrollupcolumns to these tables, allowing for grouped statistics.If either of these columns is
null, it indicates that the row reflects total stats for that grouping. Specifically, when rollup isnull, the row holds the stats for all rollups. When both category and rollup arenull, the row represents the overall total stats.The PR also leverages Prisma’s new TypedSQL feature to implement type-safe raw SQL queries.
Additionally, it updates the unit tests to verify the validity and correctness of each aggregated value by comparing them against the aggregation of the fixture data, rather than checking against a snapshot, as we were doing so far.
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):