Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not try querying imported data for unsupported props #4467

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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", %{
zoldar marked this conversation as resolved.
Show resolved Hide resolved
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