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

Refactor including imports in stats queries #5011

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

RobertJoonas
Copy link
Contributor

Changes

This PR prepares the codebase with a refactor in order to make SiteImport structs available in stats queries. The plan is to use SiteImports to decide whether or not scroll_depth gets included from imported data or not. This information will be included in meta.metric_warnings.

This PR also aims to move the logic of responding with imported flags (such as imports_included and skip_imported_reason) into a single place - QueryResult. The values for these fields depend on both query and comparison_query which this PR makes available in QueryResult.

Tests

  • This PR does not require tests

Changelog

  • This PR does not make a user-facing change

Documentation

  • This change does not need a documentation update

Dark mode

  • This PR does not change the UI

@RobertJoonas RobertJoonas requested a review from zoldar January 23, 2025 11:41
Copy link
Contributor

@zoldar zoldar left a comment

Choose a reason for hiding this comment

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

LGTM, though I'm currently not super familiar with the query processing 😅

One thing I think would be worth considering is that preloading optimisation on plug levels at least.

lib/plausible_web/live/goal_settings.ex Show resolved Hide resolved
lib/plausible/stats/query_result.ex Outdated Show resolved Hide resolved
lib/plausible/stats/query_result.ex Show resolved Hide resolved
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