Skip to content

Commit

Permalink
Do not try querying imported data for unsupported props (#4467)
Browse files Browse the repository at this point in the history
* Do not try querying imported data for unsupported props

* Remove unnecessary `unquote`s

Co-authored-by: hq1 <[email protected]>

* Add respective regression test to APIv2 tests

---------

Co-authored-by: hq1 <[email protected]>
  • Loading branch information
zoldar and aerosol authored Aug 30, 2024
1 parent 0513639 commit e9dd895
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 2 deletions.
6 changes: 4 additions & 2 deletions lib/plausible/imported.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ defmodule Plausible.Imported do

@spec imported_custom_props() :: [String.t()]
def imported_custom_props do
Plausible.Props.internal_keys()
|> Enum.map(&("event:props:" <> &1))
# NOTE: Keep up to date with `Plausible.Props.internal_keys/1`,
# but _ignore_ unsupported keys. Currently, `search_query` is
# not supported in imported queries.
Enum.map(~w(url path), &("event:props:" <> &1))
end

@spec goals_with_url() :: [String.t()]
Expand Down
2 changes: 2 additions & 0 deletions lib/plausible/props.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ defmodule Plausible.Props do
@max_prop_value_length 2000
def max_prop_value_length, do: @max_prop_value_length

# NOTE: Keep up to date with `Plausible.Imported.imported_custom_props/0`.
@internal_keys ~w(url path search_query)

@doc """
Lists prop keys used internally.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,44 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryImportedTest do
end
end

test "gracefully ignores unsupported WP Search Queries goal for imported data", %{
conn: conn,
site: site
} do
insert(:goal, event_name: "WP Search Queries", site: site)
site_import = insert(:site_import, site: site)

populate_stats(site, site_import.id, [
build(:event,
name: "WP Search Queries",
"meta.key": ["search_query", "result_count"],
"meta.value": ["some phrase", "12"]
),
build(:imported_custom_events,
name: "view_search_results",
visitors: 100,
events: 200
),
build(:imported_visitors, visitors: 9)
])

conn =
post(conn, "/api/v2/query", %{
"site_id" => site.domain,
"metrics" => ["visitors", "events", "conversion_rate"],
"date_range" => "all",
"dimensions" => ["event:props:search_query"],
"filters" => [
["is", "event:goal", ["WP Search Queries"]]
],
"include" => %{"imports" => true}
})

assert json_response(conn, 200)["results"] == [
%{"dimensions" => ["some phrase"], "metrics" => [1, 1, 100.0]}
]
end

test "adds a warning when query params are not supported for imported data", %{
conn: conn,
site: site
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,99 @@ defmodule PlausibleWeb.Api.StatsController.CustomPropBreakdownTest do
describe "with imported data" do
setup [:create_user, :log_in, :create_new_site]

test "gracefully ignores unsupported WP Search Queries goal for imported data", %{
conn: conn,
site: site
} do
insert(:goal, event_name: "WP Search Queries", site: site)
site_import = insert(:site_import, site: site)

populate_stats(site, site_import.id, [
build(:event,
name: "WP Search Queries",
"meta.key": ["search_query", "result_count"],
"meta.value": ["some phrase", "12"]
),
build(:imported_custom_events,
name: "view_search_results",
visitors: 100,
events: 200
),
build(:imported_visitors, visitors: 9)
])

filters = Jason.encode!(%{goal: "WP Search Queries"})

conn =
get(
conn,
"/api/stats/#{site.domain}/custom-prop-values/search_query?period=day&with_imported=true&filters=#{filters}"
)

assert json_response(conn, 200)["results"] == [
%{
"visitors" => 1,
"name" => "some phrase",
"events" => 1,
"conversion_rate" => 100.0
}
]
end

test "returns path breakdown for 404 goal", %{conn: conn, site: site} do
insert(:goal, event_name: "404", site: site)
site_import = insert(:site_import, site: site)

populate_stats(site, site_import.id, [
build(:event,
name: "404",
"meta.key": ["path"],
"meta.value": ["/some/path/first.bin"]
),
build(:imported_custom_events,
name: "404",
visitors: 2,
events: 5,
path: "/some/path/first.bin"
),
build(:imported_custom_events,
name: "404",
visitors: 5,
events: 10,
path: "/some/path/second.bin"
),
build(:imported_custom_events,
name: "view_search_results",
visitors: 100,
events: 200
),
build(:imported_visitors, visitors: 9)
])

filters = Jason.encode!(%{goal: "404"})

conn =
get(
conn,
"/api/stats/#{site.domain}/custom-prop-values/path?period=day&with_imported=true&filters=#{filters}"
)

assert json_response(conn, 200)["results"] == [
%{
"visitors" => 5,
"name" => "/some/path/second.bin",
"events" => 10,
"conversion_rate" => 50.0
},
%{
"visitors" => 3,
"name" => "/some/path/first.bin",
"events" => 6,
"conversion_rate" => 30.0
}
]
end

for goal_name <- ["Outbound Link: Click", "File Download", "Cloaked Link: Click"] do
test "returns url breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
Expand Down

0 comments on commit e9dd895

Please sign in to comment.