DashboardQueryParser: Labels and segments (#5964)

* parse segment filters

* fix parsing visit:city filters + tests

* serialize integer clauses too

* serialize segment/location labels

* default segments argument to emtpy list

Author: RobertJoonas

lib/plausible/stats/dashboard_query_parser.ex

@@ -116,17 +116,12 @@ defmodule Plausible.Stats.DashboardQueryParser do
   end
 
   defp decode_filter(filter_expression) do
-    case String.split(filter_expression, ",") do
-      [operator, dimension | clauses] ->
-        {:ok,
-         [
-           operator,
-           with_prefix(dimension),
-           Enum.map(clauses, &URI.decode_www_form/1)
-         ]}
-
-      _ ->
-        {:error, :invalid_filter}
+    with [operator, dimension | clauses] <- String.split(filter_expression, ","),
+         dimension = with_prefix(dimension),
+         {:ok, clauses} <- decode_clauses(clauses, dimension) do
+      {:ok, [operator, dimension, clauses]}
+    else
+      _ -> {:error, :invalid_filter}
     end
   end
 
@@ -141,6 +136,20 @@ defmodule Plausible.Stats.DashboardQueryParser do
     end
   end
 
+  @dimensions_with_integer_clauses ["segment", "visit:city"]
+  defp decode_clauses(clauses, dimension) when dimension in @dimensions_with_integer_clauses do
+    Enum.reduce_while(clauses, {:ok, []}, fn clause, {:ok, acc} ->
+      case Integer.parse(clause) do
+        {int, ""} -> {:cont, {:ok, acc ++ [int]}}
+        _ -> {:halt, {:error, :invalid_filter}}
+      end
+    end)
+  end
+
+  defp decode_clauses(clauses, _dimension) do
+    {:ok, Enum.map(clauses, &URI.decode_www_form/1)}
+  end
+
   @event_props_prefix "props:"
   @event_dimensions ["name", "page", "goal", "hostname"]
   defp event_dimension?(dimension) do

lib/plausible/stats/dashboard_query_serializer.ex

@@ -6,26 +6,26 @@ defmodule Plausible.Stats.DashboardQuerySerializer do
 
   alias Plausible.Stats.{ParsedQueryParams, DashboardQueryParser, QueryInclude}
 
-  def serialize(%ParsedQueryParams{} = params) do
+  def serialize(%ParsedQueryParams{} = params, segments \\ []) do
     params
     |> Map.to_list()
-    |> Enum.flat_map(&get_serialized_fields/1)
+    |> Enum.flat_map(fn pair -> get_serialized_fields(pair, segments) end)
     |> Enum.sort_by(&elem(&1, 0))
     |> Enum.map_join("&", fn {key, value} -> "#{key}=#{value}" end)
   end
 
-  defp get_serialized_fields({_, nil}), do: []
-  defp get_serialized_fields({_, []}), do: []
+  defp get_serialized_fields({_, nil}, _segments), do: []
+  defp get_serialized_fields({_, []}, _segments), do: []
 
-  defp get_serialized_fields({:input_date_range, {:date_range, from_date, to_date}}) do
+  defp get_serialized_fields({:input_date_range, {:date_range, from_date, to_date}}, _segments) do
     [
       {"period", "custom"},
       {"from", Date.to_iso8601(from_date)},
       {"to", Date.to_iso8601(to_date)}
     ]
   end
 
-  defp get_serialized_fields({:input_date_range, input_date_range}) do
+  defp get_serialized_fields({:input_date_range, input_date_range}, _segments) do
     period =
       case input_date_range do
         :realtime -> "realtime"
@@ -44,29 +44,22 @@ defmodule Plausible.Stats.DashboardQuerySerializer do
     [{"period", period}]
   end
 
-  defp get_serialized_fields({:relative_date, date}) do
+  defp get_serialized_fields({:relative_date, date}, _segments) do
     [{"date", Date.to_iso8601(date)}]
   end
 
-  defp get_serialized_fields({:filters, [_ | _] = filters}) do
-    filters
-    |> Enum.map(fn [operator, dimension, clauses] ->
-      clauses = Enum.map_join(clauses, ",", &uri_encode_permissive/1)
-      dimension = String.split(dimension, ":", parts: 2) |> List.last()
-      {"f", "#{operator},#{dimension},#{clauses}"}
-    end)
+  defp get_serialized_fields({:filters, [_ | _] = filters}, segments) do
+    serialize_filters(filters) ++ serialize_labels(filters, segments)
   end
 
-  defp get_serialized_fields({:include, %QueryInclude{} = include}) do
+  defp get_serialized_fields({:include, %QueryInclude{} = include}, _segments) do
     [:imports, :compare, :compare_match_day_of_week]
     |> Enum.flat_map(fn include_key ->
       get_serialized_fields_from_include(include_key, include)
     end)
   end
 
-  defp get_serialized_fields(_) do
-    []
-  end
+  defp get_serialized_fields(_, _), do: []
 
   defp get_serialized_fields_from_include(:imports, %QueryInclude{} = include) do
     if include.imports == DashboardQueryParser.default_include().imports do
@@ -102,15 +95,64 @@ defmodule Plausible.Stats.DashboardQuerySerializer do
     end
   end
 
+  defp serialize_filters(filters) do
+    filters
+    |> Enum.map(fn [operator, dimension, clauses] ->
+      clauses = Enum.map_join(clauses, ",", &custom_uri_encode/1)
+      dimension = String.split(dimension, ":", parts: 2) |> List.last()
+      {"f", "#{operator},#{dimension},#{clauses}"}
+    end)
+  end
+
+  # This is needed by the React dashboard during the migration phase.
+  # In Elixir, we can do cheap location/segment lookups but React needs
+  # URL labels to be able translate filters into a human-readable form.
+  # E.g. `f=is,city,2950159&l=2950159,Berlin`.
+  defp serialize_labels(filters, segments) do
+    Enum.flat_map(filters, fn filter -> labels_for(filter, segments) end)
+  end
+
+  defp labels_for([_operator, dimension, clauses], segments) do
+    name_lookup_fn =
+      case dimension do
+        "visit:country" ->
+          &Location.get_country/1
+
+        "visit:region" ->
+          &Location.get_subdivision/1
+
+        "visit:city" ->
+          &Location.get_city/1
+
+        "segment" ->
+          fn segment_id ->
+            Enum.find(segments, &(&1.id == segment_id))
+          end
+
+        _ ->
+          fn _ -> nil end
+      end
+
+    Enum.reduce(clauses, [], fn code, acc ->
+      case name_lookup_fn.(code) do
+        %{name: name} -> acc ++ [{"l", "#{code},#{name}"}]
+        nil -> acc
+      end
+    end)
+  end
+
   # These characters are not URL encoded to have more readable URLs.
   # Browsers seem to handle this just fine. `?f=is,page,/my/page/:some_param`
   # vs `?f=is,page,%2Fmy%2Fpage%2F%3Asome_param`
   @do_not_url_encode [":", "/"]
   @do_not_url_encode_map Enum.into(@do_not_url_encode, %{}, fn char ->
                            {URI.encode_www_form(char), char}
                          end)
+  defp custom_uri_encode(input) when is_integer(input) do
+    to_string(input)
+  end
 
-  defp uri_encode_permissive(input) do
+  defp custom_uri_encode(input) do
     input
     |> URI.encode_www_form()
     |> String.replace(Map.keys(@do_not_url_encode_map), &@do_not_url_encode_map[&1])

test/plausible/stats/query/dashboard_query_parser_test.exs

@@ -162,8 +162,35 @@ defmodule Plausible.Stats.DashboardQueryParserTest do
              } = parsed
     end
 
-    test "errors when filter decoding fails" do
+    test "parses city filter with multiple clauses" do
+      {:ok, parsed} =
+        parse("?f=is,city,2988507,2950159")
+
+      assert %ParsedQueryParams{
+               filters: [[:is, "visit:city", [2_988_507, 2_950_159]]],
+               include: @default_include
+             } = parsed
+    end
+
+    test "parses a segment filter" do
+      {:ok, parsed} = parse("?f=is,segment,123")
+
+      assert %ParsedQueryParams{
+               filters: [[:is, "segment", [123]]],
+               include: @default_include
+             } = parsed
+    end
+
+    test "errors when filter structure is wrong" do
       assert {:error, :invalid_filters} = parse("?f=is,page,/&f=what")
     end
+
+    test "errors when city filter cannot be parsed to integer" do
+      assert {:error, :invalid_filters} = parse("?f=is,city,Berlin")
+    end
+
+    test "errors when segment filter cannot be parsed to integer" do
+      assert {:error, :invalid_filters} = parse("?f=is,segment,MySegment")
+    end
   end
 end

test/plausible/stats/query/dashboard_query_serializer_test.exs

@@ -113,7 +113,7 @@ defmodule Plausible.Stats.DashboardQuerySerializerTest do
   end
 
   describe "filters" do
-    test "serializes multiple filters" do
+    test "serializes multiple is filters" do
       serialized =
         serialize(%ParsedQueryParams{
           filters: [
@@ -127,9 +127,88 @@ defmodule Plausible.Stats.DashboardQuerySerializerTest do
       assert serialized == "f=is,exit_page,/:dashboard&f=is,source,Bing&f=is,props:theme,system"
     end
 
+    test "serializes filters with integer clauses" do
+      serialized =
+        serialize(%ParsedQueryParams{
+          filters: [
+            [:is, "segment", [123, 456, 789]],
+            [:is, "visit:city", [2_950_159]]
+          ],
+          include: @default_include
+        })
+
+      assert serialized == "f=is,segment,123,456,789&f=is,city,2950159&l=2950159,Berlin"
+    end
+
     test "serializes empty filters" do
-      serialized = serialize(%ParsedQueryParams{filters: [], include: @default_include})
+      serialized = serialize(%ParsedQueryParams{filters: [], include: @default_include}, [])
       assert serialized == ""
     end
   end
+
+  describe "labels" do
+    test "adds location labels" do
+      serialized =
+        serialize(%ParsedQueryParams{
+          filters: [
+            [:is, "visit:country", ["EE"]],
+            [:is, "visit:region", ["EE-79"]],
+            [:is, "visit:city", [588_335]]
+          ],
+          include: @default_include
+        })
+
+      assert serialized ==
+               "f=is,country,EE&f=is,region,EE-79&f=is,city,588335&l=EE,Estonia&l=EE-79,Tartumaa&l=588335,Tartu"
+    end
+
+    test "adds segment label" do
+      user = new_user()
+      site = new_site(owner: user)
+
+      segment =
+        insert(:segment,
+          type: :personal,
+          owner: user,
+          site: site,
+          name: "personal segment"
+        )
+
+      serialized =
+        serialize(
+          %ParsedQueryParams{
+            filters: [[:is, "segment", [segment.id]]],
+            include: @default_include
+          },
+          [segment]
+        )
+
+      assert serialized ==
+               "f=is,segment,#{segment.id}&l=#{segment.id},#{segment.name}"
+    end
+
+    test "skips location labels when not found" do
+      serialized =
+        serialize(%ParsedQueryParams{
+          filters: [
+            [:is, "visit:country", ["XX"]],
+            [:is, "visit:region", ["XX-00"]],
+            [:is, "visit:city", [999_999_999]]
+          ],
+          include: @default_include
+        })
+
+      assert serialized == "f=is,country,XX&f=is,region,XX-00&f=is,city,999999999"
+    end
+
+    test "skips segment label when not found" do
+      serialized =
+        serialize(%ParsedQueryParams{
+          filters: [[:is, "segment", [1]]],
+          include: @default_include
+        })
+
+      assert serialized == "f=is,segment,1"
+    end
+  end
 end