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

Remove dead code under Plausible.Google.HTTP#list_stats #4068

Closed
wants to merge 2 commits into from

Conversation

macobo
Copy link
Contributor

@macobo macobo commented May 5, 2024

I'm lacking proper context to evaluate this code, discovered when refactoring filters.

filters["page"] is a typo for filters["event:page"]. It does not exist currently, meaning all code doesn't get referrer stats for individual pages if filtered so.

The fix seems non-trivial since the event:page filter might be e.g. contains or match one of many, so thought it might be best to remove this. Please do share your thoughts if you disagree!

@macobo macobo force-pushed the drop-dead-referrers-code branch 2 times, most recently from 3b2672c to f19a3e0 Compare May 5, 2024 17:25
filters["page"] does not exist currently, meaning all code doesn't get referrer
stats for individual pages if filtered so.

The fix seems non-trivial since the event:page filter might be e.g. contains or match
one of many, so thought it might be best to remove this.
@macobo macobo force-pushed the drop-dead-referrers-code branch from f19a3e0 to a6c292c Compare May 5, 2024 17:29
@macobo macobo marked this pull request as ready for review May 5, 2024 17:40
@ukutaht
Copy link
Contributor

ukutaht commented May 7, 2024

Ah, this code was an early attempt to satisfy this feature request which probably worked at some point but got broken as filters format changed.

Since this is a fairly highly requested feature I'll play around with the code a bit today to see if I can get it to work again.

@macobo macobo marked this pull request as draft May 7, 2024 12:10
@macobo
Copy link
Contributor Author

macobo commented May 7, 2024

Thanks for the context. Let me know how it goes, will be keeping an eye on this as it blocks small part of the filters refactor!

@ukutaht
Copy link
Contributor

ukutaht commented May 7, 2024

Very much WIP but it's functional and I wanted to share as soon as possible: #4077

Need to finish up:

  • Tests which are currently very bad/incomplete
  • UI messaging for filters that cannot be applied to Search Console data

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