Expose goals w/ custom props through Sites/Plugins API (#5952)

* Improve goal custom props validation

* Implement goals with custom props in Plugins API

* Implement goals with custom props in Sites API

* DRY: Extract CustomProps schemas

* rename test

* Check for Props feature availability on goals with custom props creation

* Guard goals w/ custom props with billing feature checks

* ce

* ce

* Tidy up test

* credo

* Fix error wording for Sites API

* Update test/support/teams/test.ex

Co-authored-by: Artur Pata <[email protected]>

* Use strict map assertion per @apata's suggestion

* Stronger match

* Improve custom props validation

* Keep the hybrid feature encapsulated

* Format

* Revert "Keep the hybrid feature encapsulated"

This reverts commit 3d5d7cf.

* Fixup

---------

Co-authored-by: Artur Pata <[email protected]>

Author: aerosol

extra/lib/plausible_web/controllers/api/external_sites_controller.ex

@@ -98,7 +98,8 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
               display_name: goal.display_name,
               goal_type: Goal.type(goal),
               event_name: goal.event_name,
-              page_path: goal.page_path
+              page_path: goal.page_path,
+              custom_props: goal.custom_props
             }
           end),
         meta: pagination_meta(page.metadata)
@@ -413,6 +414,12 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
 
         H.bad_request(conn, message)
 
+      {:error, :upgrade_required} ->
+        H.payment_required(
+          conn,
+          "Your current subscription plan does not include Custom Properties"
+        )
+
       e ->
         H.bad_request(conn, "Something went wrong: #{inspect(e)}")
     end

lib/plausible/goal.ex

@@ -54,13 +54,7 @@ defmodule Plausible.Goal do
     |> validate_event_name_and_page_path()
     |> validate_page_path_for_scroll_goal()
     |> maybe_put_display_name()
-    |> validate_change(:custom_props, fn :custom_props, custom_props ->
-      if map_size(custom_props) > @max_custom_props_per_goal do
-        [custom_props: "use at most #{@max_custom_props_per_goal} properties per goal"]
-      else
-        []
-      end
-    end)
+    |> validate_change(:custom_props, &validate_custom_props/2)
     |> unique_constraint(:display_name, name: :goals_display_name_unique)
     |> unique_constraint(:event_name, name: :goals_event_config_unique)
     |> unique_constraint([:page_path, :scroll_threshold],
@@ -183,6 +177,35 @@ defmodule Plausible.Goal do
     |> update_change(:display_name, &String.trim/1)
     |> validate_required(:display_name)
   end
+
+  defp validate_custom_props(:custom_props, custom_props) when is_map(custom_props) do
+    cond do
+      map_size(custom_props) > @max_custom_props_per_goal ->
+        [custom_props: "use at most #{@max_custom_props_per_goal} properties per goal"]
+
+      not Enum.all?(custom_props, fn {k, v} ->
+        is_binary(k) and is_binary(v)
+      end) ->
+        [custom_props: "must be a map with string keys and string values"]
+
+      Enum.any?(custom_props, fn {k, _v} ->
+        String.length(k) not in 1..Plausible.Props.max_prop_key_length()
+      end) ->
+        [
+          custom_props: "key length is 1..#{Plausible.Props.max_prop_key_length()} characters"
+        ]
+
+      Enum.any?(custom_props, fn {_k, v} ->
+        String.length(v) not in 1..Plausible.Props.max_prop_value_length()
+      end) ->
+        [
+          custom_props: "value length is 1..#{Plausible.Props.max_prop_value_length()} characters"
+        ]
+
+      true ->
+        []
+    end
+  end
 end
 
 defimpl Jason.Encoder, for: Plausible.Goal do
@@ -191,7 +214,7 @@ defimpl Jason.Encoder, for: Plausible.Goal do
 
     value
     |> Map.put(:goal_type, Plausible.Goal.type(value))
-    |> Map.take([:id, :goal_type, :event_name, :page_path])
+    |> Map.take([:id, :goal_type, :event_name, :page_path, :custom_props])
     |> Map.put(:domain, domain)
     |> Map.put(:display_name, value.display_name)
     |> Jason.Encode.map(opts)

lib/plausible/goals/goals.ex

@@ -387,6 +387,12 @@ defmodule Plausible.Goals do
   end
 
   defp maybe_check_feature_access(site, changeset) do
+    with :ok <- revenue_goals_access_check(site, changeset) do
+      custom_props_goals_access_check(site, changeset)
+    end
+  end
+
+  defp revenue_goals_access_check(site, changeset) do
     if Changeset.get_field(changeset, :currency) do
       site = Plausible.Repo.preload(site, :team)
       Plausible.Billing.Feature.RevenueGoals.check_availability(site.team)
@@ -395,6 +401,15 @@ defmodule Plausible.Goals do
     end
   end
 
+  defp custom_props_goals_access_check(site, changeset) do
+    if map_size(Changeset.get_field(changeset, :custom_props)) > 0 do
+      site = Plausible.Repo.preload(site, :team)
+      Plausible.Billing.Feature.Props.check_availability(site.team)
+    else
+      :ok
+    end
+  end
+
   defp check_goals_limit(site, changeset, opts) do
     if upsert?(opts) and goal_exists_for_upsert?(site, changeset) do
       :ok

lib/plausible/plugins/api/goals.ex

@@ -51,16 +51,42 @@ defmodule Plausible.Plugins.API.Goals do
     :ok
   end
 
+  defp convert_to_create_params(%CreateRequest.CustomEvent{
+         goal: %{event_name: event_name, custom_props: custom_props}
+       })
+       when is_map(custom_props) do
+    %{"goal_type" => "event", "event_name" => event_name, "custom_props" => custom_props}
+  end
+
   defp convert_to_create_params(%CreateRequest.CustomEvent{goal: %{event_name: event_name}}) do
     %{"goal_type" => "event", "event_name" => event_name}
   end
 
+  defp convert_to_create_params(%CreateRequest.Revenue{
+         goal: %{event_name: event_name, currency: currency, custom_props: custom_props}
+       })
+       when is_map(custom_props) do
+    %{
+      "goal_type" => "event",
+      "event_name" => event_name,
+      "currency" => currency,
+      "custom_props" => custom_props
+    }
+  end
+
   defp convert_to_create_params(%CreateRequest.Revenue{
          goal: %{event_name: event_name, currency: currency}
        }) do
     %{"goal_type" => "event", "event_name" => event_name, "currency" => currency}
   end
 
+  defp convert_to_create_params(%CreateRequest.Pageview{
+         goal: %{path: page_path, custom_props: custom_props}
+       })
+       when is_map(custom_props) do
+    %{"goal_type" => "page", "page_path" => page_path, "custom_props" => custom_props}
+  end
+
   defp convert_to_create_params(%CreateRequest.Pageview{goal: %{path: page_path}}) do
     %{"goal_type" => "page", "page_path" => page_path}
   end
@@ -73,6 +99,9 @@ defmodule Plausible.Plugins.API.Goals do
         {:ok, goal} ->
           goal
 
+        {:error, :upgrade_required} ->
+          Repo.rollback(:upgrade_required)
+
         {:error, changeset} ->
           Repo.rollback(changeset)
       end

lib/plausible_web/plugins/api/schemas/goal/create_request/custom_event.ex

@@ -5,6 +5,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CreateRequest.CustomEvent do
 
   use PlausibleWeb, :open_api_schema
 
+  alias Schemas.Goal.CustomProps
+
   OpenApiSpex.schema(%{
     title: "Goal.CreateRequest.CustomEvent",
     description: "Custom Event Goal creation params",
@@ -20,7 +22,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CreateRequest.CustomEvent do
         type: :object,
         required: [:event_name],
         properties: %{
-          event_name: %Schema{type: :string}
+          event_name: %Schema{type: :string},
+          custom_props: CustomProps.request_schema()
         }
       }
     },

lib/plausible_web/plugins/api/schemas/goal/create_request/pageview.ex

@@ -5,6 +5,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CreateRequest.Pageview do
 
   use PlausibleWeb, :open_api_schema
 
+  alias Schemas.Goal.CustomProps
+
   OpenApiSpex.schema(%{
     title: "Goal.CreateRequest.Pageview",
     description: "Pageview Goal creation params",
@@ -20,7 +22,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CreateRequest.Pageview do
         type: :object,
         required: [:path],
         properties: %{
-          path: %Schema{type: :string}
+          path: %Schema{type: :string},
+          custom_props: CustomProps.request_schema()
         }
       }
     },

lib/plausible_web/plugins/api/schemas/goal/create_request/revenue.ex

@@ -5,6 +5,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CreateRequest.Revenue do
 
   use PlausibleWeb, :open_api_schema
 
+  alias Schemas.Goal.CustomProps
+
   OpenApiSpex.schema(%{
     title: "Goal.CreateRequest.Revenue",
     description: "Revenue Goal creation params",
@@ -21,7 +23,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CreateRequest.Revenue do
         required: [:event_name, :currency],
         properties: %{
           event_name: %Schema{type: :string},
-          currency: %Schema{type: :string}
+          currency: %Schema{type: :string},
+          custom_props: CustomProps.request_schema()
         }
       }
     },

lib/plausible_web/plugins/api/schemas/goal/custom_event.ex

@@ -4,6 +4,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CustomEvent do
   """
   use PlausibleWeb, :open_api_schema
 
+  alias Schemas.Goal.CustomProps
+
   OpenApiSpex.schema(%{
     description: "Custom Event Goal object",
     title: "Goal.CustomEvent",
@@ -20,7 +22,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CustomEvent do
             properties: %{
               id: %Schema{type: :integer, description: "Goal ID", readOnly: true},
               display_name: %Schema{type: :string, description: "Display name", readOnly: true},
-              event_name: %Schema{type: :string, description: "Event Name"}
+              event_name: %Schema{type: :string, description: "Event Name"},
+              custom_props: CustomProps.response_schema()
             }
           }
         }

lib/plausible_web/plugins/api/schemas/goal/custom_props.ex

@@ -0,0 +1,25 @@
+defmodule PlausibleWeb.Plugins.API.Schemas.Goal.CustomProps do
+  @moduledoc """
+  Reusable OpenAPI schema definitions for Custom Properties in Goals
+  """
+
+  alias OpenApiSpex.Schema
+
+  def response_schema do
+    %Schema{
+      type: :object,
+      description: "Custom properties (string keys and values)",
+      additionalProperties: %Schema{type: :string},
+      readOnly: true
+    }
+  end
+
+  def request_schema do
+    %Schema{
+      type: :object,
+      description: "Custom properties (max 3, string keys and values)",
+      additionalProperties: %Schema{type: :string},
+      maxProperties: 3
+    }
+  end
+end

lib/plausible_web/plugins/api/schemas/goal/pageview.ex

@@ -4,6 +4,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.Pageview do
   """
   use PlausibleWeb, :open_api_schema
 
+  alias Schemas.Goal.CustomProps
+
   OpenApiSpex.schema(%{
     description: "Pageview Goal object",
     title: "Goal.Pageview",
@@ -20,7 +22,8 @@ defmodule PlausibleWeb.Plugins.API.Schemas.Goal.Pageview do
             properties: %{
               id: %Schema{type: :integer, description: "Goal ID", readOnly: true},
               display_name: %Schema{type: :string, description: "Display name", readOnly: true},
-              path: %Schema{type: :string, description: "Page Path"}
+              path: %Schema{type: :string, description: "Page Path"},
+              custom_props: CustomProps.response_schema()
             }
           }
         }