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.

Sign up for GitHub

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

Jump to bottom

Analytic events for time-over-time comparison #43589

Merged
merged 152 commits into from
Jun 6, 2024
Merged

Analytic events for time-over-time comparison #43589

merged 152 commits into from
Jun 6, 2024

Conversation

kamilmielnik
Copy link
Contributor

@kamilmielnik kamilmielnik commented Jun 4, 2024 •
edited
Loading

kamilmielnik added 30 commits May 23, 2024 21:39
@kamilmielnik
Sort functions
ab9ce2b
@kamilmielnik
Add boilerplate for aggregateOffset and tests
edea336
@kamilmielnik
Simplify assertion
03aceaf
@kamilmielnik
Make basic case work
40a4a0d
@kamilmielnik
Merge branch 'master' into 43079-aggregation-helpers
78b746e
@kamilmielnik
Handle name generation
1e59082
@kamilmielnik
Do not require "lib/uuid" attribute to be present since MLv2 will nor…
bfdf5a7 
…malize it under the hood anyway
@kamilmielnik
Fix typing
19d88b3
@kamilmielnik
Group tests
6cd3161
@kamilmielnik
Refactor offsetClause to return a new clause instead of a new query
42bf50e 
- Move offsetClause to expressions.ts
- Add assertion
@kamilmielnik
Revert "Sort functions"
ac5c9e0 
This reverts commit ab9ce2b.
@kamilmielnik
Move tests
e9140c0
@kamilmielnik
Handle names dynamically
85474a1
@kamilmielnik
Shorten a test
37b76aa
@kamilmielnik
Update comment
9a28136
@kamilmielnik
Add TODO
4c47913
@kamilmielnik
Update expression types
caf9fc4
@kamilmielnik
Add diffOffsetClause & percentDiffOffsetClause
d81bb77
@kamilmielnik
Add a test for diffOffsetClause
e7af622
@kamilmielnik
Add TODOs
8a160cb
@kamilmielnik
Add tests for diffOffsetClause and percentDiffOffsetClause
6619d8d
@kamilmielnik
Merge branch 'master' into 43079-aggregation-helpers
a6d9d4a
@kamilmielnik
Merge branch 'master' into 43079-aggregation-helpers
126a8e5
@kamilmielnik
Unwrap tests
b7a586f
@kamilmielnik
Add skeleton for new tests
0fffac3
@kamilmielnik
Refactor first offsetClause tests
2357c15
@kamilmielnik
Add tests for names
2e42403
@kamilmielnik
Fix case of non-datetime columns
0cb5ab4
@kamilmielnik
Handle a case with offset < 1 and no breakouts
f51a49d
@kamilmielnik
Handle a case with offset < 1 and breakouts on non-datetime column
ae25a8f
kamilmielnik
kamilmielnik commented Jun 4, 2024
View reviewed changes
snowplow/iglu-client-embedded/schemas/com.metabase/question/jsonschema/1-0-6
Comment on lines +23 to +25
"column_compare_via_shortcut",
"column_compare_via_column_header",
"column_compare_via_plus_modal",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is the same as 1-0-5 with the exception of these 3 lines

@kamilmielnik kamilmielnik marked this pull request as ready for review June 4, 2024 14:21
@kamilmielnik kamilmielnik requested a review from a team June 4, 2024 14:21
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 4, 2024 •
edited
Loading

Codenotify: Notifying subscribers in CODENOTIFY files for diff 54d9fb2...3d229c1.

Notify File(s)
@alxnddr frontend/src/metabase/visualizations/click-actions/actions/CompareAggregationsAction/CompareAggregationsAction.tsx

@replay-io replay.io
Copy link

replay-io bot commented Jun 4, 2024 •
edited
Loading

Status Complete ↗︎
Commit 3d229c1
Results
⚠️ 2 Flaky
✅ 2585 Passed

ranquild
ranquild reviewed Jun 4, 2024
View reviewed changes
Copy link
Contributor

@ranquild ranquild left a comment

Choose a reason for hiding this comment

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

@kamilmielnik please add e2e tests for each snowplow event

@kamilmielnik kamilmielnik requested review from ranquild and a team June 5, 2024 11:51
@kamilmielnik
Copy link
Contributor Author

@kamilmielnik please add e2e tests for each snowplow event

✔️ @ranquild added e2e/test/scenarios/question/column-compare.cy.spec.ts
This file will grow when #43073 is implemented.

Base automatically changed from 43071-previous-period-column-headers to master June 5, 2024 15:53
@kamilmielnik kamilmielnik requested a review from camsaul as a code owner June 5, 2024 15:53
@kamilmielnik kamilmielnik enabled auto-merge (squash) June 6, 2024 07:26
@kamilmielnik kamilmielnik disabled auto-merge June 6, 2024 07:26
@kamilmielnik kamilmielnik merged commit 49360f6 into master Jun 6, 2024
110 checks passed
@kamilmielnik kamilmielnik deleted the 43072-analytic-events branch June 6, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uladzimirdev uladzimirdev uladzimirdev approved these changes

@ranquild ranquild ranquild approved these changes

@camsaul camsaul Awaiting requested review from camsaul

Assignees

@kamilmielnik kamilmielnik

Labels
no-backport Do not backport this PR to any branch .Team/Querying
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Analytic events
4 participants
@kamilmielnik @ranquild @uladzimirdev @bshepherdson