Refactor: Remove `filter_key` terminology from backend (#4994)

* Remove filter_key terminology from the backend

This resurfaced in a recent review, `dimension` or `filter_dimension` is the correct terminology in the backend

* Update table_decider

* Solve new issues

lib/plausible/stats/filters/query_parser.ex

@@ -125,13 +125,13 @@ defmodule Plausible.Stats.Filters.QueryParser do
       when operator in [:not, :has_done, :has_not_done],
       do: parse_filter(filter)
 
-  def parse_filter_second(_operator, filter), do: parse_filter_key(filter)
+  def parse_filter_second(_operator, filter), do: parse_filter_dimension(filter)
 
-  defp parse_filter_key([_operator, filter_key | _rest] = filter) do
-    parse_filter_key_string(filter_key, "Invalid filter '#{i(filter)}")
+  defp parse_filter_dimension([_operator, filter_dimension | _rest] = filter) do
+    parse_filter_dimension_string(filter_dimension, "Invalid filter '#{i(filter)}")
   end
 
-  defp parse_filter_key(filter), do: {:error, "Invalid filter '#{i(filter)}'."}
+  defp parse_filter_dimension(filter), do: {:error, "Invalid filter '#{i(filter)}'."}
 
   defp parse_filter_rest(operator, filter)
        when operator in [
@@ -154,11 +154,11 @@ defmodule Plausible.Stats.Filters.QueryParser do
        when operator in [:not, :and, :or, :has_done, :has_not_done],
        do: {:ok, []}
 
-  defp parse_clauses_list([operator, filter_key, list | _rest] = filter) when is_list(list) do
+  defp parse_clauses_list([operator, dimension, list | _rest] = 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?} do
+    case {dimension, all_strings?} do
       {"visit:city", false} when all_integers? ->
         {:ok, list}
 
@@ -173,7 +173,7 @@ defmodule Plausible.Stats.Filters.QueryParser do
       {"segment", _} when all_integers? ->
         {:ok, list}
 
-      {_, true} when filter_key !== "segment" ->
+      {_, true} when dimension !== "segment" ->
         {:ok, list}
 
       _ ->
@@ -321,10 +321,10 @@ defmodule Plausible.Stats.Filters.QueryParser do
   defp parse_dimension_entry(key, error_message) do
     case {
       parse_time(key),
-      parse_filter_key_string(key)
+      parse_filter_dimension_string(key)
     } do
       {{:ok, time}, _} -> {:ok, time}
-      {_, {:ok, filter_key}} -> {:ok, filter_key}
+      {_, {:ok, dimension}} -> {:ok, dimension}
       _ -> {:error, error_message}
     end
   end
@@ -333,7 +333,7 @@ defmodule Plausible.Stats.Filters.QueryParser do
     case {
       parse_time(value),
       parse_metric(value),
-      parse_filter_key_string(value)
+      parse_filter_dimension_string(value)
     } do
       {{:ok, time}, _, _} -> {:ok, time}
       {_, {:ok, metric}, _} -> {:ok, metric}
@@ -385,31 +385,31 @@ defmodule Plausible.Stats.Filters.QueryParser do
 
   defp atomize_keys(value), do: value
 
-  defp parse_filter_key_string(filter_key, error_message \\ "") do
-    case filter_key do
+  defp parse_filter_dimension_string(dimension, error_message \\ "") do
+    case dimension do
       "event:props:" <> property_name ->
         if String.length(property_name) > 0 do
-          {:ok, filter_key}
+          {:ok, dimension}
         else
           {:error, error_message}
         end
 
       "event:" <> key ->
         if key in Filters.event_props() do
-          {:ok, filter_key}
+          {:ok, dimension}
         else
           {:error, error_message}
         end
 
       "visit:" <> key ->
         if key in Filters.visit_props() do
-          {:ok, filter_key}
+          {:ok, dimension}
         else
           {:error, error_message}
         end
 
       "segment" ->
-        {:ok, filter_key}
+        {:ok, dimension}
 
       _ ->
         {:error, error_message}

lib/plausible/stats/query.ex

@@ -109,8 +109,9 @@ defmodule Plausible.Stats.Query do
   """
   def remove_top_level_filters(query, prefixes) do
     new_filters =
-      Enum.reject(query.filters, fn [_, filter_key | _rest] ->
-        is_binary(filter_key) and Enum.any?(prefixes, &String.starts_with?(filter_key, &1))
+      Enum.reject(query.filters, fn [_, dimension_or_filter_tree | _rest] ->
+        is_binary(dimension_or_filter_tree) and
+          Enum.any?(prefixes, &String.starts_with?(dimension_or_filter_tree, &1))
       end)
 
     query
@@ -179,9 +180,10 @@ defmodule Plausible.Stats.Query do
 
   @spec trace(%__MODULE__{}, [atom()]) :: %__MODULE__{}
   def trace(%__MODULE__{} = query, metrics) do
-    filter_keys =
+    filter_dimensions =
       query.filters
-      |> Enum.map(fn [_op, prop | _rest] -> prop end)
+      |> Enum.map(fn [_op, dimension | _rest] -> dimension end)
+      |> Enum.filter(&is_binary/1)
       |> Enum.sort()
       |> Enum.join(";")
 
@@ -192,7 +194,7 @@ defmodule Plausible.Stats.Query do
       {"plausible.query.period", query.period},
       {"plausible.query.dimensions", query.dimensions |> Enum.join(";")},
       {"plausible.query.include_imported", query.include_imported},
-      {"plausible.query.filter_keys", filter_keys},
+      {"plausible.query.filter_keys", filter_dimensions},
       {"plausible.query.metrics", metrics}
     ])
 

lib/plausible/stats/query_optimizer.ex

@@ -121,7 +121,7 @@ defmodule Plausible.Stats.QueryOptimizer do
     # Note: Only works since event:hostname is only allowed as a top level filter
     hostname_filters =
       query.filters
-      |> Enum.filter(fn [_operation, filter_key | _rest] -> filter_key == "event:hostname" end)
+      |> Enum.filter(fn [_operation, dimension | _rest] -> dimension == "event:hostname" end)
 
     if length(hostname_filters) > 0 do
       extra_filters =
@@ -136,10 +136,10 @@ defmodule Plausible.Stats.QueryOptimizer do
 
   defp hostname_filters_for_dimension(dimension, hostname_filters) do
     if Map.has_key?(@dimensions_hostname_map, dimension) do
-      filter_key = Map.get(@dimensions_hostname_map, dimension)
+      dimension = Map.get(@dimensions_hostname_map, dimension)
 
       hostname_filters
-      |> Enum.map(fn [operation, _filter_key | rest] -> [operation, filter_key | rest] end)
+      |> Enum.map(fn [operation, _dimension | rest] -> [operation, dimension | rest] end)
     else
       []
     end

test/plausible/stats/table_decider_test.exs

@@ -152,9 +152,10 @@ defmodule Plausible.Stats.TableDeciderTest do
     end
   end
 
-  defp make_query(filter_keys, dimensions \\ []) do
+  defp make_query(filter_dimensions, dimensions \\ []) do
     Query.from(build(:site), %{
-      "filters" => Enum.map(filter_keys, fn filter_key -> ["is", filter_key, []] end),
+      "filters" =>
+        Enum.map(filter_dimensions, fn filter_dimension -> ["is", filter_dimension, []] end),
       "dimensions" => dimensions
     })
   end