Skip to content

Commit

Permalink
Fix filter transformations (plausible#4727)
Browse files Browse the repository at this point in the history
Previously the code would use the incorrect operator and lead to nesting
  • Loading branch information
macobo authored Oct 23, 2024
1 parent 1234cbf commit a59f127
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 9 deletions.
19 changes: 10 additions & 9 deletions lib/plausible/stats/filters/filters.ex
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ defmodule Plausible.Stats.Filters do

filters
|> traverse()
|> Enum.filter(fn {_filter, _root, depth} -> depth >= min_depth end)
|> Enum.map(fn {[_operator, dimension | _rest], _root, _depth} -> dimension end)
|> Enum.filter(fn {_filter, depth} -> depth >= min_depth end)
|> Enum.map(fn {[_operator, dimension | _rest], _depth} -> dimension end)
end

def filtering_on_dimension?(query, dimension) do
Expand Down Expand Up @@ -145,7 +145,8 @@ defmodule Plausible.Stats.Filters do
case {transformer.(filter), filter} do
# Transformer did not return that value - transform that subtree
{nil, [operation, child_filter]} when operation in [:not, :ignore_in_totals_query] ->
[[:not, transform_tree(child_filter, transformer)]]
[transformed_child] = transform_tree(child_filter, transformer)
[[operation, transformed_child]]

{nil, [operation, filters]} when operation in [:and, :or] ->
[[operation, transform_filters(filters, transformer)]]
Expand All @@ -160,22 +161,22 @@ defmodule Plausible.Stats.Filters do
end
end

defp traverse(filters, root \\ nil, depth \\ -1) do
defp traverse(filters, depth \\ -1) do
filters
|> Enum.flat_map(&traverse_tree(&1, root || &1, depth + 1))
|> Enum.flat_map(&traverse_tree(&1, depth + 1))
end

defp traverse_tree(filter, root, depth) do
defp traverse_tree(filter, depth) do
case filter do
[operation, child_filter] when operation in [:not, :ignore_in_totals_query] ->
traverse_tree(child_filter, root, depth + 1)
traverse_tree(child_filter, depth + 1)

[operation, filters] when operation in [:and, :or] ->
traverse(filters, root || filter, depth + 1)
traverse(filters, depth + 1)

# Leaf node
_ ->
[{filter, root, depth}]
[{filter, depth}]
end
end
end
68 changes: 68 additions & 0 deletions test/plausible_web/controllers/api/stats_controller/pages_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,74 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do
}
]
end

test "can compare with previous period", %{conn: conn, site: site} do
populate_stats(site, [
build(:pageview,
pathname: "/page1",
timestamp: ~N[2021-01-01 00:00:00]
),
build(:pageview,
pathname: "/page1",
timestamp: ~N[2021-01-02 00:00:00]
),
build(:pageview,
pathname: "/page2",
timestamp: ~N[2021-01-02 00:00:00]
),
build(:pageview,
pathname: "/page2",
timestamp: ~N[2021-01-02 00:00:00]
)
])

conn =
get(
conn,
"/api/stats/#{site.domain}/pages?period=day&date=2021-01-02&comparison=previous_period&detailed=true"
)

assert json_response(conn, 200)["results"] == [
%{
"bounce_rate" => 100,
"comparison" => %{
"bounce_rate" => 0,
"pageviews" => 0,
"time_on_page" => 0,
"visitors" => 0,
"change" => %{
"bounce_rate" => nil,
"pageviews" => 100,
"time_on_page" => nil,
"visitors" => 100
}
},
"name" => "/page2",
"pageviews" => 2,
"time_on_page" => nil,
"visitors" => 2
},
%{
"bounce_rate" => 100,
"name" => "/page1",
"pageviews" => 1,
"time_on_page" => nil,
"visitors" => 1,
"comparison" => %{
"bounce_rate" => 100,
"pageviews" => 1,
"time_on_page" => nil,
"visitors" => 1,
"change" => %{
"bounce_rate" => 0,
"pageviews" => 0,
"time_on_page" => nil,
"visitors" => 0
}
}
}
]
end
end

describe "GET /api/stats/:domain/entry-pages" do
Expand Down

0 comments on commit a59f127

Please sign in to comment.