diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d47785321fc..3a4b96006323 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. ## Unreleased ### Fixed +- Fix breakdown API pagination when using event metrics plausible/analytics#2562 - Automatically update all visible dashboard reports in the realtime view ### Changed diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index 792b4beca31f..10e4e7385afd 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -327,7 +327,8 @@ defmodule Plausible.Stats.Breakdown do group_by: e.name, where: meta.key == ^prop, group_by: meta.value, - select_merge: %{^prop => meta.value} + select_merge: %{^prop => meta.value}, + order_by: {:asc, meta.value} ) end @@ -340,7 +341,8 @@ defmodule Plausible.Stats.Breakdown do inner_lateral_join: meta in fragment("meta"), where: meta.key == ^prop, group_by: meta.value, - select_merge: %{^prop => meta.value} + select_merge: %{^prop => meta.value}, + order_by: {:asc, meta.value} ) end @@ -351,7 +353,8 @@ defmodule Plausible.Stats.Breakdown do from( e in q, group_by: e.name, - select_merge: %{name: e.name} + select_merge: %{name: e.name}, + order_by: {:asc, e.name} ) end @@ -362,7 +365,8 @@ defmodule Plausible.Stats.Breakdown do from( e in q, group_by: e.pathname, - select_merge: %{page: e.pathname} + select_merge: %{page: e.pathname}, + order_by: {:asc, e.pathname} ) end @@ -378,7 +382,8 @@ defmodule Plausible.Stats.Breakdown do select_merge: %{ index: fragment("arrayJoin(indices) as index"), page_match: fragment("array(?)[index]", ^match_exprs) - } + }, + order_by: {:asc, fragment("index")} ) end end @@ -389,7 +394,8 @@ defmodule Plausible.Stats.Breakdown do group_by: s.referrer_source, select_merge: %{ source: fragment("if(empty(?), ?, ?)", s.referrer_source, @no_ref, s.referrer_source) - } + }, + order_by: {:asc, s.referrer_source} ) end @@ -398,7 +404,8 @@ defmodule Plausible.Stats.Breakdown do s in q, where: s.country_code != "\0\0" and s.country_code != "ZZ", group_by: s.country_code, - select_merge: %{country: s.country_code} + select_merge: %{country: s.country_code}, + order_by: {:asc, s.country_code} ) end @@ -407,7 +414,8 @@ defmodule Plausible.Stats.Breakdown do s in q, where: s.subdivision1_code != "", group_by: s.subdivision1_code, - select_merge: %{region: s.subdivision1_code} + select_merge: %{region: s.subdivision1_code}, + order_by: {:asc, s.subdivision1_code} ) end @@ -416,7 +424,8 @@ defmodule Plausible.Stats.Breakdown do s in q, where: s.city_geoname_id != 0, group_by: s.city_geoname_id, - select_merge: %{city: s.city_geoname_id} + select_merge: %{city: s.city_geoname_id}, + order_by: {:asc, s.city_geoname_id} ) end @@ -424,7 +433,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.entry_page, - select_merge: %{entry_page: s.entry_page} + select_merge: %{entry_page: s.entry_page}, + order_by: {:asc, s.entry_page} ) end @@ -432,7 +442,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.exit_page, - select_merge: %{exit_page: s.exit_page} + select_merge: %{exit_page: s.exit_page}, + order_by: {:asc, s.exit_page} ) end @@ -442,7 +453,8 @@ defmodule Plausible.Stats.Breakdown do group_by: s.referrer, select_merge: %{ referrer: fragment("if(empty(?), ?, ?)", s.referrer, @no_ref, s.referrer) - } + }, + order_by: {:asc, s.referrer} ) end @@ -500,7 +512,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.screen_size, - select_merge: %{device: s.screen_size} + select_merge: %{device: s.screen_size}, + order_by: {:asc, s.screen_size} ) end @@ -508,7 +521,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.operating_system, - select_merge: %{operating_system: s.operating_system} + select_merge: %{operating_system: s.operating_system}, + order_by: {:asc, s.operating_system} ) end @@ -516,7 +530,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.operating_system_version, - select_merge: %{os_version: s.operating_system_version} + select_merge: %{os_version: s.operating_system_version}, + order_by: {:asc, s.operating_system_version} ) end @@ -524,7 +539,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.browser, - select_merge: %{browser: s.browser} + select_merge: %{browser: s.browser}, + order_by: {:asc, s.browser} ) end @@ -532,7 +548,8 @@ defmodule Plausible.Stats.Breakdown do from( s in q, group_by: s.browser_version, - select_merge: %{browser_version: s.browser_version} + select_merge: %{browser_version: s.browser_version}, + order_by: {:asc, s.browser_version} ) end diff --git a/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs b/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs index 2979823d83fb..f0617136bc8b 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs @@ -1588,6 +1588,41 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do assert Enum.count(res["results"]) == 2 end + test "does not repeat results", %{conn: conn, site: site} do + populate_stats([ + build(:pageview, %{domain: site.domain, "meta.key": ["item"], "meta.value": ["apple"]}), + build(:pageview, %{domain: site.domain, "meta.key": ["item"], "meta.value": ["kiwi"]}), + build(:pageview, %{domain: site.domain, "meta.key": ["item"], "meta.value": ["pineapple"]}), + build(:pageview, %{domain: site.domain, "meta.key": ["item"], "meta.value": ["grapes"]}) + ]) + + params = %{ + "site_id" => site.domain, + "metrics" => "visitors", + "property" => "event:props:item", + "limit" => 3, + "page" => nil + } + + first_page = + conn + |> get("/api/v1/stats/breakdown", %{params | "page" => 1}) + |> json_response(200) + |> Map.get("results") + |> Enum.map(& &1["item"]) + |> MapSet.new() + + second_page = + conn + |> get("/api/v1/stats/breakdown", %{params | "page" => 2}) + |> json_response(200) + |> Map.get("results") + |> Enum.map(& &1["item"]) + |> MapSet.new() + + assert first_page |> MapSet.intersection(second_page) |> Enum.empty?() + end + @invalid_limit_message "Please provide limit as a number between 1 and 1000." test "returns error when limit too large", %{conn: conn, site: site} do diff --git a/test/plausible_web/controllers/api/stats_controller/countries_test.exs b/test/plausible_web/controllers/api/stats_controller/countries_test.exs index 12f3f4859680..db33b8f67d1e 100644 --- a/test/plausible_web/controllers/api/stats_controller/countries_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/countries_test.exs @@ -100,15 +100,6 @@ defmodule PlausibleWeb.Api.StatsController.CountriesTest do conn = get(conn, "/api/stats/#{site.domain}/countries?period=day&filters=#{filters}") assert json_response(conn, 200) == [ - %{ - "code" => "GB", - "alpha_3" => "GBR", - "name" => "United Kingdom", - "flag" => "🇬🇧", - "total_visitors" => 1, - "visitors" => 1, - "conversion_rate" => 100.0 - }, %{ "code" => "EE", "alpha_3" => "EST", @@ -117,6 +108,15 @@ defmodule PlausibleWeb.Api.StatsController.CountriesTest do "total_visitors" => 2, "visitors" => 1, "conversion_rate" => 50.0 + }, + %{ + "code" => "GB", + "alpha_3" => "GBR", + "name" => "United Kingdom", + "flag" => "🇬🇧", + "total_visitors" => 1, + "visitors" => 1, + "conversion_rate" => 100.0 } ] end diff --git a/test/plausible_web/controllers/api/stats_controller/pages_test.exs b/test/plausible_web/controllers/api/stats_controller/pages_test.exs index ee04e1c3c471..ab8a95ce4b29 100644 --- a/test/plausible_web/controllers/api/stats_controller/pages_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/pages_test.exs @@ -745,14 +745,6 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do ) assert json_response(conn, 200) == [ - %{ - "total_visitors" => 1, - "unique_entrances" => 1, - "total_entrances" => 1, - "name" => "/page2", - "visit_duration" => 900, - "conversion_rate" => 100.0 - }, %{ "total_visitors" => 2, "unique_entrances" => 1, @@ -760,6 +752,14 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do "name" => "/page1", "visit_duration" => 0, "conversion_rate" => 50.0 + }, + %{ + "total_visitors" => 1, + "unique_entrances" => 1, + "total_entrances" => 1, + "name" => "/page2", + "visit_duration" => 900, + "conversion_rate" => 100.0 } ] end 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 4ed179d0c355..f2a6944796ce 100644 --- a/test/plausible_web/controllers/api/stats_controller/sources_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/sources_test.exs @@ -279,17 +279,17 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) assert json_response(conn, 200) == [ - %{ - "name" => "Google", - "visitors" => 1, - "bounce_rate" => 0, - "visit_duration" => 900 - }, %{ "name" => "DuckDuckGo", "visitors" => 1, "bounce_rate" => 100, "visit_duration" => 0 + }, + %{ + "name" => "Google", + "visitors" => 1, + "bounce_rate" => 0, + "visit_duration" => 900 } ] end @@ -344,17 +344,17 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) assert json_response(conn, 200) == [ - %{ - "name" => "Google", - "visitors" => 1, - "bounce_rate" => 0, - "visit_duration" => 900 - }, %{ "name" => "DuckDuckGo", "visitors" => 1, "bounce_rate" => 100, "visit_duration" => 0 + }, + %{ + "name" => "Google", + "visitors" => 1, + "bounce_rate" => 0, + "visit_duration" => 900 } ] @@ -812,17 +812,17 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) assert json_response(conn, 200) == [ - %{ - "name" => "Google", - "visitors" => 1, - "conversion_rate" => 50.0, - "total_visitors" => 2 - }, %{ "name" => "DuckDuckGo", "visitors" => 1, "conversion_rate" => 100.0, "total_visitors" => 1 + }, + %{ + "name" => "Google", + "visitors" => 1, + "conversion_rate" => 50.0, + "total_visitors" => 2 } ] end