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 Funnel rendering on dashboards #3641

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Fix Funnel rendering on dashboards #3641

merged 2 commits into from
Mar 12, 2021

Conversation

kpthatsme
Copy link
Contributor

@kpthatsme kpthatsme commented Mar 12, 2021

Changes

Fixes this #3574

Dashboards that had two types of funnel displays were not rendering properly. Only one or the other was rendering.

Did some digging and saw that in the component itself we were using two logics – one keyed by the item and one not. That was causing filters to be cached and shared across since a key wasn't being passed in to the filter query.

Solution:

  • Use the same logic for state ops within a component.

Screen Shot 2021-03-12 at 12 53 33 PM

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl temporarily deployed to posthog-3574-fix-funnel-txqims March 12, 2021 20:57 Inactive
const { results: stepsResult, resultsLoading: funnelLoading } = useValues(logic)
const { loadResults: loadFunnel } = useActions(logic)
const { filters } = useValues(funnelLogic({ filters: defaultFilters }))
const { filters } = useValues(logic)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit but I think you can just add filters to the unpack in line 23

@timgl timgl temporarily deployed to posthog-3574-fix-funnel-txqims March 12, 2021 21:26 Inactive
@timgl timgl merged commit 4210717 into master Mar 12, 2021
@timgl timgl deleted the 3574-fix-funnels-dash branch March 12, 2021 22:15
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