diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index a451db665..63d253460 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -90,15 +90,30 @@ defmodule Plausible.Stats.Filters.QueryParser do when operator in [:is, :is_not, :matches, :does_not_match, :contains, :does_not_contain], do: parse_clauses_list(filter) - defp parse_clauses_list([_operation, filter_key, list] = filter) when is_list(list) do + defp parse_clauses_list([operation, filter_key, list] = filter) when is_list(list) do all_strings? = Enum.all?(list, &is_binary/1) all_integers? = Enum.all?(list, &is_integer/1) - case {filter_key, all_strings?, all_integers?} do - {"event:goal", true, _} -> {:ok, [Filters.Utils.wrap_goal_value(list)]} - {"visit:city", _, true} -> {:ok, [list]} - {_, true, _} -> {:ok, [list]} - _ -> {:error, "Invalid filter '#{inspect(filter)}'"} + case {filter_key, all_strings?} do + {"event:goal", true} -> + {:ok, [Filters.Utils.wrap_goal_value(list)]} + + {"visit:city", false} when all_integers? -> + {:ok, [list]} + + {"visit:country", true} when operation in ["is", "is_not"] -> + if Enum.all?(list, &(String.length(&1) == 2)) do + {:ok, [list]} + else + {:error, + "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code"} + end + + {_, true} -> + {:ok, [list]} + + _ -> + {:error, "Invalid filter '#{inspect(filter)}'"} end end diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 5f882e060..9716c5a1e 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -231,20 +231,31 @@ defmodule Plausible.Stats.SQL.WhereBuilder do defp filter_field(db_field, [:matches, _key, glob_exprs]) do page_regexes = Enum.map(glob_exprs, &page_regex/1) - dynamic([x], fragment("multiMatchAny(?, ?)", field(x, ^db_field), ^page_regexes)) + + dynamic( + [x], + fragment("multiMatchAny(?, ?)", type(field(x, ^db_field), :string), ^page_regexes) + ) end defp filter_field(db_field, [:does_not_match, _key, glob_exprs]) do page_regexes = Enum.map(glob_exprs, &page_regex/1) - dynamic([x], fragment("not(multiMatchAny(?, ?))", field(x, ^db_field), ^page_regexes)) + + dynamic( + [x], + fragment("not(multiMatchAny(?, ?))", type(field(x, ^db_field), :string), ^page_regexes) + ) end defp filter_field(db_field, [:contains, _key, values]) do - dynamic([x], fragment("multiSearchAny(?, ?)", field(x, ^db_field), ^values)) + dynamic([x], fragment("multiSearchAny(?, ?)", type(field(x, ^db_field), :string), ^values)) end defp filter_field(db_field, [:does_not_contain, _key, values]) do - dynamic([x], fragment("not(multiSearchAny(?, ?))", field(x, ^db_field), ^values)) + dynamic( + [x], + fragment("not(multiSearchAny(?, ?))", type(field(x, ^db_field), :string), ^values) + ) end defp filter_field(db_field, [:is, _key, list]) do diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index ca160efbc..b97c43986 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -197,14 +197,14 @@ defmodule Plausible.Stats.Filters.QueryParserTest do "metrics" => ["visitors"], "date_range" => "all", "filters" => [ - ["is", "visit:#{unquote(dimension)}", ["foo"]] + ["is", "visit:#{unquote(dimension)}", ["ab"]] ] } |> check_success(site, %{ metrics: [:visitors], date_range: @date_range, filters: [ - [:is, "visit:#{unquote(dimension)}", ["foo"]] + [:is, "visit:#{unquote(dimension)}", ["ab"]] ], dimensions: [], order_by: nil, @@ -292,6 +292,18 @@ defmodule Plausible.Stats.Filters.QueryParserTest do preloaded_goals: [] }) end + + test "invalid visit:country filter", %{site: site} do + %{ + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["is", "visit:country", ["USA"]]] + } + |> check_error( + site, + ~r/Invalid visit:country filter, visit:country needs to be a valid 2-letter country code/ + ) + end end describe "include validation" do diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs index e4f6c6e7a..f9a61acbb 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs @@ -814,7 +814,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [3], "dimensions" => []}] end - test "handles filtering by visit country", %{ + test "handles filtering by visit:country", %{ conn: conn, site: site } do @@ -835,6 +835,28 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [3], "dimensions" => []}] end + test "handles filtering by visit:country with contains", %{ + conn: conn, + site: site + } do + populate_stats(site, [ + build(:pageview, country_code: "EE"), + build(:pageview, country_code: "EE"), + build(:pageview, country_code: "IT"), + build(:pageview, country_code: "DE") + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["pageviews"], + "filters" => [["contains", "visit:country", ["E"]]] + }) + + assert json_response(conn, 200)["results"] == [%{"metrics" => [3], "dimensions" => []}] + end + test "handles filtering by visit:city", %{ conn: conn, site: site diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_validations_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_validations_test.exs index 27c745950..d5bef3115 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_validations_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_validations_test.exs @@ -230,5 +230,30 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryValidationsTest do "error" => "Metrics cannot be queried multiple times" } end + + test "handles filtering by visit:country with invalid country code", %{ + conn: conn, + site: site + } do + populate_stats(site, [ + build(:pageview, country_code: "EE"), + build(:pageview, country_code: "EE"), + build(:pageview, country_code: "IT"), + build(:pageview, country_code: "DE") + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["pageviews"], + "filters" => [["is", "visit:country", ["USA"]]] + }) + + assert json_response(conn, 400) == %{ + "error" => + "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code" + } + end end end