Add listing sites, goals and custom props to Sites API (plausible#4302)

* Add listing sites, goals and custom props to Sites API

* Rename exmaple props in tests

* Rename `allowed_custom_props` -> `custom_properties`

* Expose goal name in GET endpoints for goals in Sites API

* Bump default pagination limit to 100 and max to 1000

* Introduce Goal.name/display_name and use the first one for name in API

* Extend goal list response but hide currency

* Settle on `display_name` instead of `name`

* Allow viewer members to get site details and list site goals

* Don't include currency in goal's display name

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

@@ -2,12 +2,64 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
   use PlausibleWeb, :controller
   use Plausible.Repo
   use PlausibleWeb.Plugs.ErrorHandler
+
+  import Plausible.Pagination
+
   alias Plausible.Sites
+  alias Plausible.Goal
   alias Plausible.Goals
   alias PlausibleWeb.Api.Helpers, as: H
 
+  @pagination_opts [cursor_fields: [{:id, :desc}], limit: 100, maximum_limit: 1000]
+
+  def index(conn, params) do
+    user = conn.assigns.current_user
+
+    page =
+      user
+      |> Sites.for_user_query()
+      |> paginate(params, @pagination_opts)
+
+    json(conn, %{
+      sites: page.entries,
+      meta: pagination_meta(page.metadata)
+    })
+  end
+
+  def goals_index(conn, params) do
+    user = conn.assigns.current_user
+
+    with {:ok, site_id} <- expect_param_key(params, "site_id"),
+         {:ok, site} <- get_site(user, site_id, [:owner, :admin, :viewer]) do
+      page =
+        site
+        |> Plausible.Goals.for_site_query()
+        |> paginate(params, @pagination_opts)
+
+      json(conn, %{
+        goals:
+          Enum.map(page.entries, fn goal ->
+            %{
+              id: goal.id,
+              display_name: Goal.display_name(goal),
+              goal_type: Goal.type(goal),
+              event_name: goal.event_name,
+              page_path: goal.page_path
+            }
+          end),
+        meta: pagination_meta(page.metadata)
+      })
+    else
+      {:missing, "site_id"} ->
+        H.bad_request(conn, "Parameter `site_id` is required to list goals")
+
+      {:error, :site_not_found} ->
+        H.not_found(conn, "Site could not be found")
+    end
+  end
+
   def create_site(conn, params) do
-    user = conn.assigns[:current_user]
+    user = conn.assigns.current_user
 
     case Sites.create(user, params) do
       {:ok, %{site: site}} ->
@@ -29,57 +81,50 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
   end
 
   def get_site(conn, %{"site_id" => site_id}) do
-    site = Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner, :admin])
+    case get_site(conn.assigns.current_user, site_id, [:owner, :admin, :viewer]) do
+      {:ok, site} ->
+        json(conn, %{
+          domain: site.domain,
+          timezone: site.timezone,
+          custom_properties: site.allowed_event_props || []
+        })
 
-    if site do
-      json(conn, site)
-    else
-      H.not_found(conn, "Site could not be found")
+      {:error, :site_not_found} ->
+        H.not_found(conn, "Site could not be found")
     end
   end
 
   def delete_site(conn, %{"site_id" => site_id}) do
-    site = Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner])
+    case get_site(conn.assigns.current_user, site_id, [:owner]) do
+      {:ok, site} ->
+        {:ok, _} = Plausible.Site.Removal.run(site.domain)
+        json(conn, %{"deleted" => true})
 
-    if site do
-      {:ok, _} = Plausible.Site.Removal.run(site.domain)
-      json(conn, %{"deleted" => true})
-    else
-      H.not_found(conn, "Site could not be found")
+      {:error, :site_not_found} ->
+        H.not_found(conn, "Site could not be found")
     end
   end
 
   def update_site(conn, %{"site_id" => site_id} = params) do
     # for now this only allows to change the domain
-    site = Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner, :admin])
-
-    if site do
-      case Plausible.Site.Domain.change(site, params["domain"]) do
-        {:ok, site} ->
-          json(conn, site)
-
-        {:error, changeset} ->
-          conn
-          |> put_status(400)
-          |> json(serialize_errors(changeset))
-      end
+    with {:ok, site} <- get_site(conn.assigns.current_user, site_id, [:owner, :admin]),
+         {:ok, site} <- Plausible.Site.Domain.change(site, params["domain"]) do
+      json(conn, site)
     else
-      H.not_found(conn, "Site could not be found")
-    end
-  end
+      {:error, :site_not_found} ->
+        H.not_found(conn, "Site could not be found")
 
-  defp expect_param_key(params, key) do
-    case Map.fetch(params, key) do
-      :error -> {:missing, key}
-      res -> res
+      {:error, %Ecto.Changeset{} = changeset} ->
+        conn
+        |> put_status(400)
+        |> json(serialize_errors(changeset))
     end
   end
 
   def find_or_create_shared_link(conn, params) do
     with {:ok, site_id} <- expect_param_key(params, "site_id"),
          {:ok, link_name} <- expect_param_key(params, "name"),
-         site when not is_nil(site) <-
-           Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner, :admin]) do
+         {:ok, site} <- get_site(conn.assigns.current_user, site_id, [:owner, :admin]) do
       shared_link = Repo.get_by(Plausible.Site.SharedLink, site_id: site.id, name: link_name)
 
       shared_link =
@@ -96,7 +141,7 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
           })
       end
     else
-      nil ->
+      {:error, :site_not_found} ->
         H.not_found(conn, "Site could not be found")
 
       {:missing, "site_id"} ->
@@ -113,12 +158,11 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
   def find_or_create_goal(conn, params) do
     with {:ok, site_id} <- expect_param_key(params, "site_id"),
          {:ok, _} <- expect_param_key(params, "goal_type"),
-         site when not is_nil(site) <-
-           Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner, :admin]),
+         {:ok, site} <- get_site(conn.assigns.current_user, site_id, [:owner, :admin]),
          {:ok, goal} <- Goals.find_or_create(site, params) do
       json(conn, goal)
     else
-      nil ->
+      {:error, :site_not_found} ->
         H.not_found(conn, "Site could not be found")
 
       {:missing, param} ->
@@ -132,19 +176,16 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
   def delete_goal(conn, params) do
     with {:ok, site_id} <- expect_param_key(params, "site_id"),
          {:ok, goal_id} <- expect_param_key(params, "goal_id"),
-         site when not is_nil(site) <-
-           Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner, :admin]) do
-      case Goals.delete(goal_id, site) do
-        :ok ->
-          json(conn, %{"deleted" => true})
-
-        {:error, :not_found} ->
-          H.not_found(conn, "Goal could not be found")
-      end
+         {:ok, site} <- get_site(conn.assigns.current_user, site_id, [:owner, :admin]),
+         :ok <- Goals.delete(goal_id, site) do
+      json(conn, %{"deleted" => true})
     else
-      nil ->
+      {:error, :site_not_found} ->
         H.not_found(conn, "Site could not be found")
 
+      {:error, :not_found} ->
+        H.not_found(conn, "Goal could not be found")
+
       {:missing, "site_id"} ->
         H.bad_request(conn, "Parameter `site_id` is required to delete a goal")
 
@@ -156,9 +197,31 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
     end
   end
 
+  defp pagination_meta(meta) do
+    %{
+      after: meta.after,
+      before: meta.before,
+      limit: meta.limit
+    }
+  end
+
+  defp get_site(user, site_id, roles) do
+    case Sites.get_for_user(user.id, site_id, roles) do
+      nil -> {:error, :site_not_found}
+      site -> {:ok, site}
+    end
+  end
+
   defp serialize_errors(changeset) do
     {field, {msg, _opts}} = List.first(changeset.errors)
     error_msg = Atom.to_string(field) <> ": " <> msg
     %{"error" => error_msg}
   end
+
+  defp expect_param_key(params, key) do
+    case Map.fetch(params, key) do
+      :error -> {:missing, key}
+      res -> res
+    end
+  end
 end

lib/plausible/goal/schema.ex

@@ -47,6 +47,19 @@ defmodule Plausible.Goal do
     |> maybe_drop_currency()
   end
 
+  @spec display_name(t()) :: String.t()
+  def display_name(%{page_path: path}) when is_binary(path) do
+    "Visit " <> path
+  end
+
+  def display_name(%{event_name: name}) when is_binary(name) do
+    name
+  end
+
+  @spec type(t()) :: :event | :page
+  def type(%{event_name: event_name}) when is_binary(event_name), do: :event
+  def type(%{page_path: page_path}) when is_binary(page_path), do: :page
+
   defp update_leading_slash(changeset) do
     case get_field(changeset, :page_path) do
       "/" <> _ ->
@@ -91,33 +104,20 @@ end
 
 defimpl Jason.Encoder, for: Plausible.Goal do
   def encode(value, opts) do
-    goal_type =
-      cond do
-        value.event_name -> :event
-        value.page_path -> :page
-      end
-
     domain = value.site.domain
 
     value
-    |> Map.put(:goal_type, goal_type)
+    |> Map.put(:goal_type, Plausible.Goal.type(value))
     |> Map.take([:id, :goal_type, :event_name, :page_path])
     |> Map.put(:domain, domain)
+    |> Map.put(:display_name, Plausible.Goal.display_name(value))
     |> Jason.Encode.map(opts)
   end
 end
 
 defimpl String.Chars, for: Plausible.Goal do
-  def to_string(%{page_path: page_path}) when is_binary(page_path) do
-    "Visit " <> page_path
-  end
-
-  def to_string(%{event_name: name, currency: nil}) when is_binary(name) do
-    name
-  end
-
-  def to_string(%{event_name: name, currency: currency}) when is_binary(name) do
-    name <> " (#{currency})"
+  def to_string(goal) do
+    Plausible.Goal.display_name(goal)
   end
 end
 

lib/plausible/sites.ex

@@ -159,6 +159,15 @@ defmodule Plausible.Sites do
     %{result | entries: entries}
   end
 
+  @spec for_user_query(Auth.User.t()) :: Ecto.Query.t()
+  def for_user_query(user) do
+    from(s in Site,
+      inner_join: sm in assoc(s, :memberships),
+      on: sm.user_id == ^user.id,
+      order_by: [desc: s.id]
+    )
+  end
+
   defp maybe_filter_by_domain(query, domain)
        when byte_size(domain) >= 1 and byte_size(domain) <= 64 do
     where(query, [s], ilike(s.domain, ^"%#{domain}%"))

lib/plausible_web/live/goal_settings/list.ex

@@ -14,7 +14,8 @@ defmodule PlausibleWeb.Live.GoalSettings.List do
 
   def render(assigns) do
     revenue_goals_enabled? = Plausible.Billing.Feature.RevenueGoals.enabled?(assigns.site)
-    assigns = assign(assigns, :revenue_goals_enabled?, revenue_goals_enabled?)
+    goals = Enum.map(assigns.goals, &{goal_label(&1), &1})
+    assigns = assign(assigns, goals: goals, revenue_goals_enabled?: revenue_goals_enabled?)
 
     ~H"""
     <div>
@@ -55,18 +56,18 @@ defmodule PlausibleWeb.Live.GoalSettings.List do
       </div>
       <%= if Enum.count(@goals) > 0 do %>
         <div class="mt-12">
-          <%= for goal <- @goals do %>
+          <%= for {goal_label, goal} <- @goals do %>
             <div class="border-b border-gray-300 dark:border-gray-500 py-3 flex justify-between">
               <span class="text-sm font-medium text-gray-900 dark:text-gray-100 w-3/4">
                 <div class="flex">
                   <span class="truncate">
                     <%= if not @revenue_goals_enabled? && goal.currency do %>
                       <div class="text-gray-600 flex items-center">
                         <Heroicons.lock_closed class="w-4 h-4 mr-1 inline" />
-                        <span><%= goal %></span>
+                        <span><%= goal_label %></span>
                       </div>
                     <% else %>
-                      <%= goal %>
+                      <%= goal_label %>
                     <% end %>
                     <span class="text-sm text-gray-400 block mt-1 font-normal">
                       <span :if={goal.page_path}>Pageview</span>
@@ -116,6 +117,14 @@ defmodule PlausibleWeb.Live.GoalSettings.List do
     """
   end
 
+  defp goal_label(%{currency: currency} = goal) when not is_nil(currency) do
+    to_string(goal) <> " (#{currency})"
+  end
+
+  defp goal_label(goal) do
+    to_string(goal)
+  end
+
   defp delete_confirmation_text(goal) do
     if Enum.empty?(goal.funnels) do
       """

lib/plausible_web/router.ex

@@ -188,6 +188,8 @@ defmodule PlausibleWeb.Router do
       scope assigns: %{api_scope: "sites:read:*"} do
         pipe_through PlausibleWeb.Plugs.AuthorizePublicAPI
 
+        get "/", ExternalSitesController, :index
+        get "/goals", ExternalSitesController, :goals_index
         get "/:site_id", ExternalSitesController, :get_site
       end