From 9b59e95c3334a6b37b199a3efd3ea47406ece8ad Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Mon, 2 Sep 2024 15:00:18 +0300 Subject: [PATCH] Support order_by in URL params of breakdown endpoints (#4502) * Support order_by in URL params of breakdown endpoints --- lib/plausible/stats/breakdown.ex | 8 +- .../stats/legacy/legacy_query_builder.ex | 63 ++++++++++- test/plausible/stats/query_test.exs | 3 + .../api/stats_controller/sources_test.exs | 104 +++++++++++++++++- 4 files changed, 173 insertions(+), 5 deletions(-) diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index 15b8964e4..356b4cabd 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -20,7 +20,10 @@ defmodule Plausible.Stats.Breakdown do Query.set( query, metrics: transformed_metrics, - order_by: infer_order_by(transformed_metrics, dimension), + # Concat client requested order with default order, overriding only if client explicitly requests it + order_by: + Enum.concat(query.order_by || [], infer_order_by(transformed_metrics, dimension)) + |> Enum.uniq_by(&elem(&1, 0)), dimensions: transform_dimensions(dimension), filters: query.filters ++ dimension_filters(dimension), v2: true, @@ -174,7 +177,8 @@ defmodule Plausible.Stats.Breakdown do end) end - defp infer_order_by(metrics, "event:goal"), do: [{metric_to_order_by(metrics), :desc}] + defp infer_order_by(metrics, "event:goal"), + do: [{metric_to_order_by(metrics), :desc}] defp infer_order_by(metrics, dimension), do: [{metric_to_order_by(metrics), :desc}, {dimension, :asc}] diff --git a/lib/plausible/stats/legacy/legacy_query_builder.ex b/lib/plausible/stats/legacy/legacy_query_builder.ex index cd87b8670..aa621384d 100644 --- a/lib/plausible/stats/legacy/legacy_query_builder.ex +++ b/lib/plausible/stats/legacy/legacy_query_builder.ex @@ -1,9 +1,12 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do - @moduledoc false + @moduledoc """ + Module used to parse URL search params to a valid Query, used to power the API for the dashboard. + @deprecated + """ use Plausible - alias Plausible.Stats.{Filters, Interval, Query, DateTimeRange} + alias Plausible.Stats.{Filters, Interval, Query, DateTimeRange, Metrics} def from(site, params, debug_metadata) do now = DateTime.utc_now(:second) @@ -16,6 +19,7 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do |> put_interval(params) |> put_parsed_filters(params) |> put_preloaded_goals(site) + |> put_order_by(params) |> Query.put_experimental_reduced_joins(site, params) |> Query.put_imported_opts(site, params) @@ -160,6 +164,61 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do end end + @doc """ + ### Examples: + iex> QueryBuilder.parse_order_by(nil) + [] + + iex> QueryBuilder.parse_order_by("") + [] + + iex> QueryBuilder.parse_order_by("0") + [] + + iex> QueryBuilder.parse_order_by("[}") + [] + + iex> QueryBuilder.parse_order_by(~s({"any":"object"})) + [] + + iex> QueryBuilder.parse_order_by(~s([["visitors","invalid"]])) + [] + + iex> QueryBuilder.parse_order_by(~s([["visitors","desc"]])) + [{:visitors, :desc}] + + iex> QueryBuilder.parse_order_by(~s([["visitors","asc"],["visit:source","desc"]])) + [{:visitors, :asc}, {"visit:source", :desc}] + """ + def parse_order_by(order_by) when is_binary(order_by) do + case Jason.decode(order_by) do + {:ok, parsed} when is_list(parsed) -> + Enum.flat_map(parsed, &parse_order_by_pair/1) + + _ -> + [] + end + end + + def parse_order_by(_) do + [] + end + + defp parse_order_by_pair([metric_or_dimension, direction]) when direction in ["asc", "desc"] do + case Metrics.from_string(metric_or_dimension) do + {:ok, metric} -> [{metric, String.to_existing_atom(direction)}] + :error -> [{metric_or_dimension, String.to_existing_atom(direction)}] + end + end + + defp parse_order_by_pair(_) do + [] + end + + defp put_order_by(query, %{} = params) do + struct!(query, order_by: parse_order_by(params["order_by"])) + end + defp put_interval(%{:period => "all"} = query, params) do interval = Map.get(params, "interval", Interval.default_for_date_range(query.date_range)) struct!(query, interval: interval) diff --git a/test/plausible/stats/query_test.exs b/test/plausible/stats/query_test.exs index 688c8a14c..ac465e908 100644 --- a/test/plausible/stats/query_test.exs +++ b/test/plausible/stats/query_test.exs @@ -1,6 +1,9 @@ defmodule Plausible.Stats.QueryTest do use Plausible.DataCase, async: true alias Plausible.Stats.{Query, DateTimeRange} + alias Plausible.Stats.Legacy.QueryBuilder + + doctest Plausible.Stats.Legacy.QueryBuilder setup do user = insert(:user) diff --git a/test/plausible_web/controllers/api/stats_controller/sources_test.exs b/test/plausible_web/controllers/api/stats_controller/sources_test.exs index 92ae0d784..b3450ab9c 100644 --- a/test/plausible_web/controllers/api/stats_controller/sources_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/sources_test.exs @@ -474,7 +474,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ] end - test "shows sources for a page", %{conn: conn, site: site} do + test "shows sources for a page (using old page filter)", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, pathname: "/page1", referrer_source: "Google"), build(:pageview, pathname: "/page1", referrer_source: "Google"), @@ -498,6 +498,108 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do %{"name" => "DuckDuckGo", "visitors" => 1} ] end + + test "shows sources for a page (using new filters)", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, pathname: "/page1", referrer_source: "Google"), + build(:pageview, pathname: "/page1", referrer_source: "Google"), + build(:pageview, + user_id: 1, + pathname: "/page2", + referrer_source: "DuckDuckGo" + ), + build(:pageview, + user_id: 1, + pathname: "/page1", + referrer_source: "DuckDuckGo" + ) + ]) + + filters = Jason.encode!([["is", "event:page", ["/page1"]]]) + conn = get(conn, "/api/stats/#{site.domain}/sources?filters=#{filters}") + + assert json_response(conn, 200)["results"] == [ + %{"name" => "Google", "visitors" => 2}, + %{"name" => "DuckDuckGo", "visitors" => 1} + ] + end + + test "order_by [[visit:source, desc]] is respected", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, referrer_source: "C"), + build(:pageview, referrer_source: "A"), + build(:pageview, referrer_source: "B") + ]) + + order_by = Jason.encode!([["visit:source", "desc"]]) + conn = get(conn, "/api/stats/#{site.domain}/sources?order_by=#{order_by}") + + assert json_response(conn, 200)["results"] == [ + %{"name" => "C", "visitors" => 1}, + %{"name" => "B", "visitors" => 1}, + %{"name" => "A", "visitors" => 1} + ] + end + + test "order_by [[visit_duration, asc], [visit:source, desc]]] is respected and flipping the sort orders works", + %{ + conn: conn, + site: site + } do + populate_stats(site, [ + build(:pageview, + pathname: "/in", + user_id: @user_id, + referrer_source: "B", + timestamp: ~N[2024-08-10 09:00:00] + ), + build(:pageview, + pathname: "/out", + user_id: @user_id, + referrer_source: "B", + timestamp: ~N[2024-08-10 09:00:45] + ), + build(:pageview, + pathname: "/in", + user_id: @user_id, + referrer_source: "C", + timestamp: ~N[2024-08-10 10:00:00] + ), + build(:pageview, + pathname: "/out", + user_id: @user_id, + referrer_source: "C", + timestamp: ~N[2024-08-10 10:00:30] + ), + build(:pageview, referrer_source: "A"), + build(:pageview, referrer_source: "A"), + build(:pageview, referrer_source: "Z") + ]) + + order_by_asc = Jason.encode!([["visit_duration", "asc"], ["visit:source", "desc"]]) + + conn1 = + get(conn, "/api/stats/#{site.domain}/sources?detailed=true&order_by=#{order_by_asc}") + + assert json_response(conn1, 200)["results"] == [ + %{"name" => "Z", "visitors" => 1, "bounce_rate" => 100, "visit_duration" => 0}, + %{"name" => "A", "visitors" => 2, "bounce_rate" => 100, "visit_duration" => 0}, + %{"name" => "C", "visitors" => 1, "bounce_rate" => 0, "visit_duration" => 30}, + %{"name" => "B", "visitors" => 1, "bounce_rate" => 0, "visit_duration" => 45} + ] + + order_by_flipped = Jason.encode!([["visit_duration", "desc"], ["visit:source", "asc"]]) + + conn2 = + get(conn, "/api/stats/#{site.domain}/sources?detailed=true&order_by=#{order_by_flipped}") + + assert json_response(conn2, 200)["results"] == [ + %{"name" => "B", "visitors" => 1, "bounce_rate" => 0, "visit_duration" => 45}, + %{"name" => "C", "visitors" => 1, "bounce_rate" => 0, "visit_duration" => 30}, + %{"name" => "A", "visitors" => 2, "bounce_rate" => 100, "visit_duration" => 0}, + %{"name" => "Z", "visitors" => 1, "bounce_rate" => 100, "visit_duration" => 0} + ] + end end describe "UTM parameters with hostname filter" do