Add tests for site controller

lib/plausible/goals.ex

@@ -8,7 +8,7 @@ defmodule Plausible.Goals do
       "domain" => site.domain
     })
 
-    changeset = Goal.changeset(%Goal{}, params)
+    Goal.changeset(%Goal{}, params)
                 |> Repo.insert
   end
 
@@ -19,11 +19,8 @@ defmodule Plausible.Goals do
     )
   end
 
-  def delete(site, goal_name) do
-    Repo.one(
-      from g in Goal,
-      where: g.domain == ^site.domain and g.name == ^goal_name
-    ) |> Repo.delete!
+  def delete(id) do
+    Repo.one(from g in Goal, where: g.id == ^id) |> Repo.delete!
   end
 
   defp name_for(%{"event_name" => name}) when name != "" do

lib/plausible_web/controllers/site_controller.ex

@@ -68,7 +68,6 @@ defmodule PlausibleWeb.SiteController do
         |> put_flash(:success, "Goal created succesfully")
         |> redirect(to: "/#{website}/goals")
       {:error, :goal, changeset, _} ->
-        IO.inspect(changeset)
         conn
         |> assign(:skip_plausible_tracking, true)
         |> render("new_goal.html",
@@ -79,9 +78,8 @@ defmodule PlausibleWeb.SiteController do
     end
   end
 
-  def delete_goal(conn, %{"website" => website, "name" => goal_name}) do
-    site = Sites.get_for_user!(conn.assigns[:current_user].id, website)
-    Plausible.Goals.delete(site, goal_name)
+  def delete_goal(conn, %{"website" => website, "id" => goal_id}) do
+    Plausible.Goals.delete(goal_id)
 
     conn
     |> put_flash(:success, "Goal deleted succesfully")

lib/plausible_web/router.ex

@@ -96,7 +96,7 @@ defmodule PlausibleWeb.Router do
     get "/:website/goals", SiteController, :goals
     get "/:website/goals/new", SiteController, :new_goal
     post "/:website/goals", SiteController, :create_goal
-    delete "/:website/goals/:name", SiteController, :delete_goal
+    delete "/:website/goals/:id", SiteController, :delete_goal
     put "/:website/settings", SiteController, :update_settings
     put "/:website/settings/google", SiteController, :update_google_auth
     delete "/:website", SiteController, :delete_site

lib/plausible_web/templates/site/goal_settings.html.eex

@@ -11,7 +11,7 @@
     <%= for goal <- @goals do %>
       <div class="border-b border-grey-light py-3 flex justify-between">
         <small class="font-bold"><%= goal.name %></small>
-        <%= button("❌", to: "/#{@site.domain}/goals/#{URI.encode(goal.name, &URI.char_unreserved?/1)}", method: :delete, class: "text-sm", data: [confirm: "Are you sure? This will permanently delete the goal #{goal.name}"]) %>
+        <%= button("❌", to: "/#{@site.domain}/goals/#{goal.id}", method: :delete, class: "text-sm", data: [confirm: "Are you sure? This will permanently delete the goal #{goal.name}"]) %>
       </div>
     <% end %>
   <% else %>

test/plausible_web/controllers/site_controller_test.exs

@@ -124,6 +124,16 @@ defmodule PlausibleWeb.SiteControllerTest do
     end
   end
 
+  describe "GET /:website/goals/new" do
+    setup [:create_user, :log_in, :create_site]
+
+    test "shows form to create a new goal", %{conn: conn, site: site} do
+      conn = get(conn, "/#{site.domain}/goals/new")
+
+      assert html_response(conn, 200) =~ "Add goal"
+    end
+  end
+
   describe "POST /:website/goals" do
     setup [:create_user, :log_in, :create_site]
 
@@ -155,4 +165,32 @@ defmodule PlausibleWeb.SiteControllerTest do
       assert goal.page_path == nil
     end
   end
+
+  describe "GET /:website/goals" do
+    setup [:create_user, :log_in, :create_site]
+
+    test "lists goals for the site", %{conn: conn, site: site} do
+      insert(:goal, domain: site.domain, event_name: "Custom event", name: "Custom event")
+      insert(:goal, domain: site.domain, page_path: "/register", name: "Visit /register")
+
+      conn = get(conn, "/#{site.domain}/goals")
+
+
+      assert html_response(conn, 200) =~ "Goals for " <> site.domain
+      assert html_response(conn, 200) =~ "Custom event"
+      assert html_response(conn, 200) =~ "Visit /register"
+    end
+  end
+
+  describe "DELETE /:website/goals/:id" do
+    setup [:create_user, :log_in, :create_site]
+
+    test "lists goals for the site", %{conn: conn, site: site} do
+      goal = insert(:goal, domain: site.domain, event_name: "Custom event", name: "Custom event")
+
+      delete(conn, "/#{site.domain}/goals/#{goal.id}")
+
+      assert Repo.aggregate(Plausible.Goal, :count, :id) == 0
+    end
+  end
 end