From e258cb85a1f7a16e423b7e798f3bc3ad922e66c4 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Tue, 2 Jul 2024 16:55:03 +0200 Subject: [PATCH] Add listing sites, goals and custom props to Sites API --- .../api/external_sites_controller.ex | 145 +++++++---- lib/plausible/sites.ex | 9 + lib/plausible_web/router.ex | 2 + .../api/external_sites_controller_test.exs | 227 +++++++++++++++++- 4 files changed, 332 insertions(+), 51 deletions(-) diff --git a/extra/lib/plausible_web/controllers/api/external_sites_controller.ex b/extra/lib/plausible_web/controllers/api/external_sites_controller.ex index 78da9e0dd8ca1..fa6f2c5f4eb27 100644 --- a/extra/lib/plausible_web/controllers/api/external_sites_controller.ex +++ b/extra/lib/plausible_web/controllers/api/external_sites_controller.ex @@ -2,12 +2,52 @@ defmodule PlausibleWeb.Api.ExternalSitesController do use PlausibleWeb, :controller use Plausible.Repo use PlausibleWeb.Plugs.ErrorHandler + + import Plausible.Pagination + alias Plausible.Sites alias Plausible.Goals alias PlausibleWeb.Api.Helpers, as: H + def index(conn, params) do + user = conn.assigns.current_user + + page = + user + |> Sites.for_user_query() + |> paginate(params, cursor_fields: [{:id, :desc}]) + + 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]) do + page = + site + |> Plausible.Goals.for_site_query() + |> paginate(params, cursor_fields: [{:id, :desc}]) + + json(conn, %{ + goals: page.entries, + 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 +69,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]) do + {:ok, site} -> + json(conn, %{ + domain: site.domain, + timezone: site.timezone, + allowed_custom_props: 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 +129,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 +146,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 +164,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 +185,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 diff --git a/lib/plausible/sites.ex b/lib/plausible/sites.ex index 4fbf857abb338..deab2af04bc10 100644 --- a/lib/plausible/sites.ex +++ b/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}%")) diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index 7c281e06c54a4..bd286fb281049 100644 --- a/lib/plausible_web/router.ex +++ b/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 diff --git a/test/plausible_web/controllers/api/external_sites_controller_test.exs b/test/plausible_web/controllers/api/external_sites_controller_test.exs index 0a079648ac8bd..d8dd56c9ac7d7 100644 --- a/test/plausible_web/controllers/api/external_sites_controller_test.exs +++ b/test/plausible_web/controllers/api/external_sites_controller_test.exs @@ -489,13 +489,104 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do end end + describe "GET /api/v1/sites" do + test "returns empty when there are no sites for user", %{conn: conn} do + conn = get(conn, "/api/v1/sites") + + assert json_response(conn, 200) == %{ + "sites" => [], + "meta" => %{ + "before" => nil, + "after" => nil, + "limit" => 10 + } + } + end + + test "returns sites when present", %{conn: conn, user: user} do + [site1, site2] = insert_list(2, :site, members: [user]) + + conn = get(conn, "/api/v1/sites") + + assert json_response(conn, 200) == %{ + "sites" => [ + %{"domain" => site2.domain, "timezone" => site2.timezone}, + %{"domain" => site1.domain, "timezone" => site1.timezone} + ], + "meta" => %{ + "before" => nil, + "after" => nil, + "limit" => 10 + } + } + end + + test "handles pagination correctly", %{conn: conn, user: user} do + [ + %{domain: site1_domain}, + %{domain: site2_domain}, + %{domain: site3_domain} + ] = insert_list(3, :site, members: [user]) + + conn1 = get(conn, "/api/v1/sites?limit=2") + + assert %{ + "sites" => [ + %{"domain" => ^site3_domain}, + %{"domain" => ^site2_domain} + ], + "meta" => %{ + "before" => nil, + "after" => after_cursor, + "limit" => 2 + } + } = json_response(conn1, 200) + + conn2 = get(conn, "/api/v1/sites?limit=2&after=" <> after_cursor) + + assert %{ + "sites" => [ + %{"domain" => ^site1_domain} + ], + "meta" => %{ + "before" => before_cursor, + "after" => nil, + "limit" => 2 + } + } = json_response(conn2, 200) + + assert is_binary(before_cursor) + end + + test "lists sites for user with read-only scope", %{conn: conn, user: user} do + %{domain: site_domain} = insert(:site, members: [user]) + api_key = insert(:api_key, user: user, scopes: ["stats:read:*"]) + + conn = + conn + |> Plug.Conn.put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/api/v1/sites") + + assert %{"sites" => [%{"domain" => ^site_domain}]} = json_response(conn, 200) + end + end + describe "GET /api/v1/sites/:site_id" do setup :create_new_site test "get a site by its domain", %{conn: conn, site: site} do + site = + site + |> Ecto.Changeset.change(allowed_event_props: ["goals", "funnels"]) + |> Repo.update!() + conn = get(conn, "/api/v1/sites/" <> site.domain) - assert json_response(conn, 200) == %{"domain" => site.domain, "timezone" => site.timezone} + assert json_response(conn, 200) == %{ + "domain" => site.domain, + "timezone" => site.timezone, + "allowed_custom_props" => ["goals", "funnels"] + } end test "get a site by old site_id after domain change", %{conn: conn, site: site} do @@ -506,10 +597,14 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do conn = get(conn, "/api/v1/sites/" <> old_domain) - assert json_response(conn, 200) == %{"domain" => new_domain, "timezone" => site.timezone} + assert json_response(conn, 200) == %{ + "domain" => new_domain, + "timezone" => site.timezone, + "allowed_custom_props" => [] + } end - test "get a site with basic scope config", %{conn: conn, user: user, site: site} do + test "get a site for user with read-only scope", %{conn: conn, user: user, site: site} do api_key = insert(:api_key, user: user, scopes: ["stats:read:*"]) conn = @@ -517,7 +612,11 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do |> Plug.Conn.put_req_header("authorization", "Bearer #{api_key.key}") |> get("/api/v1/sites/" <> site.domain) - assert json_response(conn, 200) == %{"domain" => site.domain, "timezone" => site.timezone} + assert json_response(conn, 200) == %{ + "domain" => site.domain, + "timezone" => site.timezone, + "allowed_custom_props" => [] + } end test "is 404 when site cannot be found", %{conn: conn} do @@ -527,6 +626,126 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do end end + describe "GET /api/v1/goals" do + setup :create_new_site + + test "returns empty when there are no goals for site", %{conn: conn, site: site} do + conn = get(conn, "/api/v1/sites/goals?site_id=" <> site.domain) + + assert json_response(conn, 200) == %{ + "goals" => [], + "meta" => %{ + "before" => nil, + "after" => nil, + "limit" => 10 + } + } + end + + test "returns goals when present", %{conn: conn, site: site} do + goal1 = insert(:goal, %{site: site, page_path: "/login"}) + goal2 = insert(:goal, %{site: site, event_name: "Signup"}) + goal3 = insert(:goal, %{site: site, event_name: "Purchase", currency: "USD"}) + + conn = get(conn, "/api/v1/sites/goals?site_id=" <> site.domain) + + assert json_response(conn, 200) == %{ + "goals" => [ + %{ + "id" => goal3.id, + "domain" => site.domain, + "event_name" => goal3.event_name, + "page_path" => goal3.page_path, + "goal_type" => "event" + }, + %{ + "id" => goal2.id, + "domain" => site.domain, + "event_name" => goal2.event_name, + "page_path" => goal2.page_path, + "goal_type" => "event" + }, + %{ + "id" => goal1.id, + "domain" => site.domain, + "event_name" => goal1.event_name, + "page_path" => goal1.page_path, + "goal_type" => "page" + } + ], + "meta" => %{ + "before" => nil, + "after" => nil, + "limit" => 10 + } + } + end + + test "handles pagination correctly", %{conn: conn, site: site} do + %{id: goal1_id} = insert(:goal, %{site: site, page_path: "/login"}) + %{id: goal2_id} = insert(:goal, %{site: site, event_name: "Signup"}) + %{id: goal3_id} = insert(:goal, %{site: site, event_name: "Purchase", currency: "USD"}) + + conn1 = get(conn, "/api/v1/sites/goals?limit=2&site_id=" <> site.domain) + + assert %{ + "goals" => [ + %{"id" => ^goal3_id}, + %{"id" => ^goal2_id} + ], + "meta" => %{ + "before" => nil, + "after" => after_cursor, + "limit" => 2 + } + } = json_response(conn1, 200) + + conn2 = + get(conn, "/api/v1/sites/goals?limit=2&after=#{after_cursor}&site_id=" <> site.domain) + + assert %{ + "goals" => [ + %{"id" => ^goal1_id} + ], + "meta" => %{ + "before" => before_cursor, + "after" => nil, + "limit" => 2 + } + } = json_response(conn2, 200) + + assert is_binary(before_cursor) + end + + test "lists goals for user with read-only scope", %{conn: conn, user: user, site: site} do + %{id: goal_id} = insert(:goal, %{site: site, page_path: "/login"}) + api_key = insert(:api_key, user: user, scopes: ["stats:read:*"]) + + conn = + conn + |> Plug.Conn.put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/api/v1/sites/goals?site_id=" <> site.domain) + + assert %{"goals" => [%{"id" => ^goal_id}]} = json_response(conn, 200) + end + + test "returns error when `site_id` parameter is missing", %{conn: conn} do + conn = get(conn, "/api/v1/sites/goals") + + assert json_response(conn, 400) == %{ + "error" => "Parameter `site_id` is required to list goals" + } + end + + test "returns error when `site_id` parameter is invalid", %{conn: conn} do + conn = get(conn, "/api/v1/sites/goals?site_id=does.not.exist") + + assert json_response(conn, 404) == %{ + "error" => "Site could not be found" + } + end + end + describe "PUT /api/v1/sites/:site_id" do setup :create_new_site