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

fix(poe-v2): fix funnels with cohort breakdowns #15457

Merged
merged 9 commits into from May 10, 2023

Conversation

yakkomajuri
Copy link
Collaborator

Fixing the funnels-related tests from #15446

@yakkomajuri yakkomajuri marked this pull request as draft May 9, 2023 16:36
@yakkomajuri yakkomajuri marked this pull request as ready for review May 9, 2023 17:52
@yakkomajuri yakkomajuri requested review from neilkakkar and removed request for neilkakkar May 10, 2023 10:33
Copy link
Collaborator

@neilkakkar neilkakkar left a comment

Choose a reason for hiding this comment

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

Curious why these snapshots got deleted.

Also, add the poe_v2 decorator to the failing test?

@yakkomajuri
Copy link
Collaborator Author

@neilkakkar added the decorator. Unsure why they get deleted 🤔

@yakkomajuri
Copy link
Collaborator Author

Sorted now - though seems we have some unrelated snapshot changes too

@yakkomajuri yakkomajuri merged commit a61087a into master May 10, 2023
47 checks passed
@yakkomajuri yakkomajuri deleted the poe-v2-failing-tests branch May 10, 2023 13:45
@@ -269,6 +275,7 @@ def test_funnel_aggregate_by_groups_breakdown_group(self):
@also_test_with_materialized_columns(
group_properties=[(0, "industry")], materialize_only_with_person_on_events=True
)
@also_test_with_person_on_events_v2
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, I think the sort-of unrelated change is because of the ordering here, materialised one became poe v2 test instead. No biggie though

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.

None yet

2 participants