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

Frontend: Use jsonurl, update filtering #4117

Merged
merged 50 commits into from
May 22, 2024
Merged

Frontend: Use jsonurl, update filtering #4117

merged 50 commits into from
May 22, 2024

Conversation

macobo
Copy link
Contributor

@macobo macobo commented May 21, 2024

Changes

This PR refactors significant parts of the dashboard react code.

Changes include:

  1. Internally representing filters in a similar manner to the backend. Related RFC: https://github.com/plausible/knowledge_base/blob/main/src/planning/rfcs/0005-filters_v2.md
    1. Note that the filters are not prefixed in the URL for the sake shorter and prettier urls.
  2. Prettier URLs for filters, e.g. http://localhost:8000/dummy.site?filters=((is,country,(US)),(is,page,(/)))&labels=(US:United%2BStates)
    1. This uses jsonurl under the hood
    2. Compatibility with bookmarked links is retained. On app load, we update the url parameters in a backwards-compatible way.
  3. Refactoring linking and url param handling in general to move that logic away from specific components
  4. Re-writing filter modal logic to unify 'regular' and 'props' modals

Note that the FE now supports multiple filters, though the request sent to the backend does not, yet.

Testing

I've extensively tested the changes locally, but also planning on recruiting Marco to test out these changes on staging.

@macobo macobo marked this pull request as ready for review May 21, 2024 08:03
@macobo macobo added the deploy-to-staging Special label that triggers a deploy to a staging environment label May 21, 2024
@macobo macobo requested a review from ukutaht May 21, 2024 08:55
Copy link
Contributor

@ukutaht ukutaht left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. I think the filters behaviour in general can be consolidated and encapsulated a bit more on the frontend to make them easier to work with. The props filter is always a snowflake that makes everything else harder.

EDIT: To be clear I think improving the filters code in general should be done in a follow-up. For now the goal is to get the new stuff functional.

I ran some manual tests myself and haven't found any bugs. We can do some more manual testing on staging next.

assets/js/dashboard/stats/modals/filter-modal-group.js Outdated Show resolved Hide resolved
assets/js/dashboard/stats/modals/pages.js Outdated Show resolved Hide resolved
@macobo macobo merged commit 23a6431 into master May 22, 2024
9 checks passed
@macobo macobo deleted the jsonurl branch May 22, 2024 08:20
macobo added a commit that referenced this pull request May 23, 2024
Follow-up to #4117

Special goals (e.g. 404, Outbound link click) always reported (none) as
results due to a function returning the wrong value.
macobo added a commit that referenced this pull request May 23, 2024
Follow-up to #4117

Opening a utm modal crashed if there was an existing page filter. Bug
struck in during a refactoring.
macobo added a commit that referenced this pull request May 23, 2024
Follow-up to #4117

Special goals (e.g. 404, Outbound link click) always reported (none) as
results due to a function returning the wrong value.
macobo added a commit that referenced this pull request May 23, 2024
Follow-up to #4117

Opening a utm modal crashed if there was an existing page filter. Bug
struck in during a refactoring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-to-staging Special label that triggers a deploy to a staging environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants