Refactor internal Query schema and introduce WhereBuilder #4082
Conversation
lib/plausible/google/api.ex
Outdated
| limit, | ||
| filters["page"] | ||
| # :BUG: This type of filter does not exist. | ||
| Plausible.Stats.Query.get_filter(query, "page") |
macobo
force-pushed
the
api-new-query-schema
branch
from
f7b0d5a
to
89f9cab
Compare
Future-proofing in a tiny way
macobo
force-pushed
the
api-new-query-schema
branch
from
89f9cab
to
ad0e99a
Compare
| @@ -718,9 +734,11 @@ defmodule PlausibleWeb.Api.StatsController do | |||
| site = conn.assigns[:site] | |||
| params = Map.put(params, "property", "visit:referrer") | |||
|
|
|||
| referrer_filter = DashboardFilterParser.filter_value("visit:source", referrer) | |||
There was a problem hiding this comment.
This is guaranteed by the frontend to be a standard is filter so I think the DashboardFilterParser can be bypassed here. We can use ["is", "visit:source", referrer] instead.
There was a problem hiding this comment.
Yes, but the "value" is still serialized. The previous put_filter method had special-case parsing logic for just this route - I decided to move it here instead of having it in the general Query class as it's not needed in any other case currently.
| referrer_filter = DashboardFilterParser.filter_value("visit:source", referrer) | ||
|
|
||
| query = | ||
| Query.from(site, params) | ||
| |> Query.put_filter("visit:source", referrer) | ||
| |> Query.put_filter(referrer_filter) |
There was a problem hiding this comment.
Unrelated comment in context of this PR, but might be relevant later on:
I think referrer as a path parameter in this endpoint doesn't make sense anymore - it did when we only supported :is filters, but we might want to show a referrer URL breakdown when multiple sources are filtered by as well.
In any case, that's up to the React dashboard to decide if we should display referrers or sources when a source :member filter is applied. Currently we're still showing sources in that case.
I believe we should delete this endpoint, and start using PlausibleWeb.Api.StatsController.referrers/2 in the React dashboard instead. The source filter should already be included in the Query automatically in that case.
RobertJoonas
force-pushed
the
api-new-query-schema
branch
from
d7b8677
to
d7f64d6
Compare
Changes
This PR changes:
Motivation is we want to build a saved segments feature and a new version of the API with a more flexible filters system. This work is a prerequisite for both of these.
WhereBuilder specifically makes how we "compile" a query from filters more explicit and makes supporting and/or/not blocks easier in the future.
Note this is a pure refactor - no user-facing changes are occurring here.
Related PRs: #4066, #4071, #4068
Tests
Changelog
Documentation
Dark mode