Refactor building the Query struct

[RobertJoonas]

Changes

This PR is a refactor and doesn't introduce any new behaviour.

In the upcoming dashboard migration from React to LiveView, we want to be able to create Query structs from native data structures. E.g.:

• pass the site struct itself into the Query.build function, not the site_id
• pass DateTime structs instead of iso8601 strings
• pass metrics as atoms instead of strings
• etc...

The biggest refactor in this PR is changing the current Query.build function into Query.parse_and_build which calls two separate modules (QueryParser and QueryBuilder (new) for the respective actions).

It also introduces an intermediate data structure that represents the parsed state between those two phases - ParsedQueryParams.

There was a lot of query "building" logic and validations incorporated into QueryParser. The test file (with more than 3k lines of code) got turned into QueryParseAndBuildTest, now asserting on a Query struct outcome rather than an arbitrary map.

This PR doesn't tackle the date_range input aspect yet. For now, QueryBuilder will expect a clean utc_time_range input (via the ParsedQueryParams struct). It shouldn't block this PR though. I can tackle it later. E.g.: we'll likely want to build queries like:

query1 = QueryBuilder.build(site, %{metrics: [:visitors], date_range: :month, date: ~D[2025-01-01]})
query2 = QueryBuilder.build(site, %{metrics: [:visitors], date_range: [~D[2025-01-01], ~D[2025-01-31]]})

With that, we can also migrate any existing ad-hoc Query construction (e.g. in email reports or query_24h_stats) to use this new version of Query.build.

Tests

• Automated tests have been added

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

[github-actions]

Preview environment👷🏼‍♀️🏗️
PR-5893

[zoldar]

Awesome work! 🙌 Looks like a really good starting point for further changes, like moving date range parsing out of building to parsing stage.

Just for the record - one potentially necessary thing the we have touched on when going over this PR though it's outside of scope - we'll also need a way to turn a Query back into query string.

[zoldar]

For future consideration: make the validation errors more friendly for internal consumption (well formed tuples with atoms/values) and build the human-formatted versions on top of it in the relevant contexts?

[zoldar]

That seems to be only used in a couple tests against Query.from, but yeah, not here.

[zoldar]

Hm, I know we went over it but, do we want to let it pass silently? While APIv2 schema might ensure this is a list, passing invalid value internally getting silently ignored might cause confusion. Maybe we should even let it crash then?

[RobertJoonas]

Hmm, actually, that's another thing I wanted to touch base with you on...

passing invalid value internally getting silently ignored might cause confusion.

The change we're currently looking at is in QueryParser, which will not be used for internal query building. Now, while QueryParser "parses" the filters, it's also validating the format. Kind of tricky to separate parsing and validating filters but yeah, I agree that even the internal QueryBuilder.build should return {:error, :invalid_filters} when you try to construct a query with filters: [%{"event:page" => "/"}, %{"visit:source" => "Google"}] (which is obviously not the correct representation of filters).

[zoldar]

Ah, yeah, indeed, filter parsing is still part of builder. Okay, I guess let's leave it be for now and try to address it in the future follow-ups?

[RobertJoonas]

Just for the record - one potentially necessary thing the we have touched on when going over this PR though it's outside of scope - we'll also need a way to turn a Query back into query string.

Hmm I think there are different "query objects" we need to consider. The one that gets serialized into a URL query string will be what's currently defined in React:

export const queryDefaultValue: DashboardQuery = {
  period: '28d' as QueryPeriod,
  comparison: null,
  match_day_of_week: true,
  date: null,
  from: null,
  to: null,
  compare_from: null,
  compare_to: null,
  filters: [],
  resolvedFilters: [],
  labels: {},
  with_imported: true
}

I think the %Query{} struct as we know it will not need to be serializable.

However, we'll still need to be able to construct a %Query{} from the DashboardQuery params (+ some extra report-dependent params e.g. dimensions). Which is pretty much what Query.from{} is currently doing... I think it makes sense to get to a state where:

• There are two query parsers - DashboardQueryParser & StatsAPIQueryParser
• Both these QueryParsers spit out %ParsedQueryParams{}
• From this point, building and validating the query will be the same for API queries and internal queries