Skip to content

Commit

Permalink
Fix breakdown comparisons breaking after pagination (#4951)
Browse files Browse the repository at this point in the history
#4697 introduced some
comparisons fixes which changed how comparisons interacted with main
queries. Sadly this broke under pagination and due to lack of coverage
wasn't caught.

The bug can also be tested manually on localhost:
- Change assets/js/dashboard/hooks/api-client.ts LIMIT to e.g. 3
- Open any modal, click load more
- Check tooltips

Before the fix this would have displayed 0 values, after the fix this
works as expected.
  • Loading branch information
macobo authored Jan 9, 2025
1 parent 937d36f commit 50eef62
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ All notable changes to this project will be documented in this file.
- Fix typo on login screen
- Fix Direct / None details modal not opening
- Fix year over year comparisons being offset by a day for leap years
- Breakdown modals now display correct comparison values instead of 0 after pagination

## v2.1.4 - 2024-10-08

Expand Down
8 changes: 6 additions & 2 deletions lib/plausible/stats/comparisons.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,15 @@ defmodule Plausible.Stats.Comparisons do
defp add_query_filters(query, []), do: query

defp add_query_filters(query, [filter]) do
Query.add_filter(query, [:ignore_in_totals_query, filter])
query
|> Query.add_filter([:ignore_in_totals_query, filter])
|> Query.set(pagination: nil)
end

defp add_query_filters(query, filters) do
Query.add_filter(query, [:ignore_in_totals_query, [:or, filters]])
query
|> Query.add_filter([:ignore_in_totals_query, [:or, filters]])
|> Query.set(pagination: nil)
end

defp build_comparison_filter(%{dimensions: dimension_labels}, query) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryComparisonsTest do
build(:pageview, browser: "Chrome", timestamp: ~N[2021-01-01 00:00:00]),
build(:pageview, browser: "Chrome", timestamp: ~N[2021-01-07 00:00:00]),
build(:pageview, browser: "Chrome", timestamp: ~N[2021-01-07 00:00:00]),
build(:pageview, browser: "Firefox", timestamp: ~N[2021-01-07 00:00:00])
build(:pageview, browser: "Chrome", timestamp: ~N[2021-01-07 00:00:00]),
build(:pageview, browser: "Firefox", timestamp: ~N[2021-01-07 00:00:00]),
build(:pageview, browser: "Firefox", timestamp: ~N[2021-01-07 00:00:00]),
build(:pageview, browser: "Safari", timestamp: ~N[2021-01-08 00:00:00])
])

conn =
Expand All @@ -143,20 +146,44 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryComparisonsTest do
assert json_response(conn, 200)["results"] == [
%{
"dimensions" => ["Chrome"],
"metrics" => [2, 66.7],
"metrics" => [3, 50.0],
"comparison" => %{
"dimensions" => ["Chrome"],
"metrics" => [1, 12.5],
"change" => [100, 434]
"change" => [200, 300]
}
},
%{
"dimensions" => ["Firefox"],
"metrics" => [1, 33.3],
"metrics" => [2, 33.3],
"comparison" => %{
"dimensions" => ["Firefox"],
"metrics" => [4, 50.0],
"change" => [-75, -33]
"change" => [-50, -33]
}
}
]

conn2 =
post(conn, "/api/v2/query-internal-test", %{
"site_id" => site.domain,
"metrics" => ["visitors", "percentage"],
"date_range" => ["2021-01-07", "2021-01-13"],
"dimensions" => ["visit:browser"],
"include" => %{
"comparisons" => %{"mode" => "previous_period"}
},
"pagination" => %{"limit" => 2, "offset" => 2}
})

assert json_response(conn2, 200)["results"] == [
%{
"dimensions" => ["Safari"],
"metrics" => [1, 16.7],
"comparison" => %{
"dimensions" => ["Safari"],
"metrics" => [3, 37.5],
"change" => [-67, -55]
}
}
]
Expand Down

0 comments on commit 50eef62

Please sign in to comment.