From 7d6408882d2b81820114845b2d39235a35cc46f6 Mon Sep 17 00:00:00 2001
From: JannikStreek <Jannik.streek@gmail.com>
Date: Tue, 10 Dec 2024 18:41:29 +0100
Subject: [PATCH] Catch wrong UUID and missing brainstorming (#508)

---
 lib/mindwendel/brainstormings.ex              | 52 ++++++++++++------
 lib/mindwendel/lanes.ex                       |  2 +-
 .../live/brainstorming_live/show.ex           | 54 ++++++++++---------
 .../live/lane_live/index_component.ex         |  6 +--
 priv/gettext/de/LC_MESSAGES/default.po        | 13 +++--
 priv/gettext/default.pot                      | 11 ++--
 priv/gettext/en/LC_MESSAGES/default.po        | 13 +++--
 test/mindwendel/accounts_test.exs             |  4 +-
 test/mindwendel/brainstormings_test.exs       | 32 +++++++++--
 .../admin/brainstorming_live/edit_test.exs    |  4 +-
 .../live/brainstorming_live_test.exs          |  4 +-
 .../live/label_live/captions_test.exs         |  2 +-
 12 files changed, 132 insertions(+), 65 deletions(-)

diff --git a/lib/mindwendel/brainstormings.ex b/lib/mindwendel/brainstormings.ex
index 542939ca..0631a59c 100644
--- a/lib/mindwendel/brainstormings.ex
+++ b/lib/mindwendel/brainstormings.ex
@@ -51,29 +51,46 @@ defmodule Mindwendel.Brainstormings do
   @doc """
   Gets a single brainstorming.
 
-  Raises `Ecto.NoResultsError` if the Brainstorming does not exist.
+  Returns an error tuple instead of raising exceptions to handle invalid UUIDs gracefully,
+  particularly important for initial brainstorming fetches that may receive spam requests.
 
   ## Examples
 
-      iex> get_brainstorming!("0323906b-b496-4778-ae67-1dd779d3de3c")
+      iex> get_brainstorming("0323906b-b496-4778-ae67-1dd779d3de3c")
       %Brainstorming{ ... }
 
-      iex> get_brainstorming!("0323906b-b496-4778-ae67-1dd779d3de3c")
-      ** (Ecto.NoResultsError)
+      iex> get_brainstorming("0323906b-b496-4778-ae67-1dd779d3de3c")
+      {:error, :not_found}
 
-      iex> get_brainstorming!("not_a_valid_uuid_string")
-      ** (Ecto.Query.CastError)
+      iex> get_brainstorming("not_a_valid_uuid_string")
+      {:error, :invalid_uuid}
 
   """
-  # See https://stackoverflow.com/questions/53802091/elixir-uuid-how-to-handle-500-error-when-uuid-doesnt-match
-  def get_brainstorming!(id) do
-    Repo.get!(Brainstorming, id)
-    |> Repo.preload([
-      :users,
-      :moderating_users,
-      labels: from(idea_label in IdeaLabel, order_by: idea_label.position_order)
-    ])
-    |> update_last_accessed_at
+  def get_brainstorming(id) do
+    case Ecto.UUID.cast(id) do
+      {:ok, id} -> get_brainstorming_with_valid_uuid(id)
+      :error -> {:error, :invalid_uuid}
+    end
+  end
+
+  # No uuid check here, has to be done before
+  defp get_brainstorming_with_valid_uuid(id) do
+    case Repo.get(Brainstorming, id) do
+      nil ->
+        {:error, :not_found}
+
+      brainstorming ->
+        preloaded_brainstorming =
+          brainstorming
+          |> Repo.preload([
+            :users,
+            :moderating_users,
+            labels: from(idea_label in IdeaLabel, order_by: idea_label.position_order)
+          ])
+          |> update_last_accessed_at()
+
+        {:ok, preloaded_brainstorming}
+    end
   end
 
   @doc """
@@ -240,7 +257,10 @@ defmodule Mindwendel.Brainstormings do
 
   """
   def validate_admin_secret(brainstorming, admin_secret_id) do
-    brainstorming.admin_url_id == admin_secret_id
+    case brainstorming.admin_url_id do
+      nil -> false
+      admin_url_id -> admin_url_id == admin_secret_id
+    end
   end
 
   @doc """
diff --git a/lib/mindwendel/lanes.ex b/lib/mindwendel/lanes.ex
index 3d50e3ef..b1b35d14 100644
--- a/lib/mindwendel/lanes.ex
+++ b/lib/mindwendel/lanes.ex
@@ -70,7 +70,7 @@ defmodule Mindwendel.Lanes do
 
   """
   def get_lanes_for_brainstorming_with_labels_filtered(id) do
-    brainstorming = Brainstormings.get_brainstorming!(id)
+    {:ok, brainstorming} = Brainstormings.get_brainstorming(id)
 
     filter_label =
       if length(brainstorming.filter_labels_ids) > 0,
diff --git a/lib/mindwendel_web/live/brainstorming_live/show.ex b/lib/mindwendel_web/live/brainstorming_live/show.ex
index a85db629..fcd0e751 100644
--- a/lib/mindwendel_web/live/brainstorming_live/show.ex
+++ b/lib/mindwendel_web/live/brainstorming_live/show.ex
@@ -3,7 +3,6 @@ defmodule MindwendelWeb.BrainstormingLive.Show do
 
   alias Mindwendel.Accounts
   alias Mindwendel.Brainstormings
-  alias Mindwendel.FeatureFlag
   alias Mindwendel.Lanes
   alias Mindwendel.Ideas
   alias Mindwendel.Brainstormings.Idea
@@ -16,27 +15,36 @@ defmodule MindwendelWeb.BrainstormingLive.Show do
     # If the admin secret in the URL after the hash (only available inside the client session) is given, add the user as moderating user to the brainstorming.
     # If not, add the user as normal user.
     current_user_id = Mindwendel.Services.SessionService.get_current_user_id(session)
-    brainstorming = Brainstormings.get_brainstorming!(id)
-    admin_secret = get_connect_params(socket)["adminSecret"]
 
-    if Brainstormings.validate_admin_secret(brainstorming, admin_secret) do
-      Accounts.add_moderating_user(brainstorming, current_user_id)
+    case Brainstormings.get_brainstorming(id) do
+      {:ok, brainstorming} ->
+        admin_secret = get_connect_params(socket)["adminSecret"]
+
+        if Brainstormings.validate_admin_secret(brainstorming, admin_secret) do
+          Accounts.add_moderating_user(brainstorming, current_user_id)
+        end
+
+        Accounts.merge_brainstorming_user(brainstorming, current_user_id)
+
+        lanes = Lanes.get_lanes_for_brainstorming_with_labels_filtered(id)
+        # load the user, also for permissions of brainstormings
+        current_user = Mindwendel.Accounts.get_user(current_user_id)
+
+        {
+          :ok,
+          socket
+          |> assign(:brainstorming, brainstorming)
+          |> assign(:lanes, lanes)
+          |> assign(:current_user, current_user)
+          |> assign(:inspiration, Mindwendel.Help.random_inspiration())
+        }
+
+      {:error, _} ->
+        {:ok,
+         socket
+         |> put_flash(:error, gettext("Brainstorming not found"))
+         |> push_navigate(to: "/")}
     end
-
-    Accounts.merge_brainstorming_user(brainstorming, current_user_id)
-
-    lanes = Lanes.get_lanes_for_brainstorming_with_labels_filtered(id)
-    # load the user, also for permissions of brainstormings
-    current_user = Mindwendel.Accounts.get_user(current_user_id)
-
-    {
-      :ok,
-      socket
-      |> assign(:brainstorming, brainstorming)
-      |> assign(:lanes, lanes)
-      |> assign(:current_user, current_user)
-      |> assign(:inspiration, inspiration())
-    }
   end
 
   def mount(%{"brainstorming_id" => brainstorming_id, "idea_id" => _idea_id}, session, socket) do
@@ -180,10 +188,4 @@ defmodule MindwendelWeb.BrainstormingLive.Show do
     socket
     |> assign(:page_title, socket.assigns.brainstorming.name)
   end
-
-  defp inspiration do
-    if FeatureFlag.enabled?(:feature_brainstorming_teasers) do
-      Mindwendel.Help.random_inspiration()
-    end
-  end
 end
diff --git a/lib/mindwendel_web/live/lane_live/index_component.ex b/lib/mindwendel_web/live/lane_live/index_component.ex
index 47dd7be7..c16504ca 100644
--- a/lib/mindwendel_web/live/lane_live/index_component.ex
+++ b/lib/mindwendel_web/live/lane_live/index_component.ex
@@ -31,7 +31,7 @@ defmodule MindwendelWeb.LaneLive.IndexComponent do
         },
         socket
       ) do
-    brainstorming = Brainstormings.get_brainstorming!(brainstorming_id)
+    {:ok, brainstorming} = Brainstormings.get_brainstorming(brainstorming_id)
 
     if has_move_permission(brainstorming, socket.assigns.current_user) do
       Ideas.update_ideas_for_brainstorming_by_user_move(
@@ -62,7 +62,7 @@ defmodule MindwendelWeb.LaneLive.IndexComponent do
   end
 
   def handle_event("sort_by_likes", %{"id" => id, "lane-id" => lane_id}, socket) do
-    brainstorming = Brainstormings.get_brainstorming!(id)
+    {:ok, brainstorming} = Brainstormings.get_brainstorming(id)
 
     if has_move_permission(brainstorming, socket.assigns.current_user) do
       Ideas.update_ideas_for_brainstorming_by_likes(id, lane_id)
@@ -72,7 +72,7 @@ defmodule MindwendelWeb.LaneLive.IndexComponent do
   end
 
   def handle_event("sort_by_label", %{"id" => id, "lane-id" => lane_id}, socket) do
-    brainstorming = Brainstormings.get_brainstorming!(id)
+    {:ok, brainstorming} = Brainstormings.get_brainstorming(id)
 
     if has_move_permission(brainstorming, socket.assigns.current_user) do
       Ideas.update_ideas_for_brainstorming_by_labels(id, lane_id)
diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po
index 2f188fc2..79c5f79d 100644
--- a/priv/gettext/de/LC_MESSAGES/default.po
+++ b/priv/gettext/de/LC_MESSAGES/default.po
@@ -26,12 +26,12 @@ msgstr "Wie können wir ..."
 msgid "Ready?"
 msgstr "Fertig?"
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:175
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:184
 #, elixir-autogen, elixir-format
 msgid "%{name} - Edit"
 msgstr "%{name} - Editieren"
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:152
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:161
 #, elixir-autogen, elixir-format
 msgid "%{name} - New Idea"
 msgstr "%{name} - Neue Idee"
@@ -299,7 +299,7 @@ msgstr "Bist du sicher, dass das Brainstorming geleert werden soll?"
 msgid "Empty brainstorming"
 msgstr "Leere das Brainstorming"
 
-#: lib/mindwendel_web/live/live_helpers.ex:29
+#: lib/mindwendel_web/live/live_helpers.ex:30
 #, elixir-autogen, elixir-format, fuzzy
 msgid "Brainstorming will be deleted %{days}"
 msgstr "Brainstorming wird gelöscht %{days}"
@@ -385,7 +385,7 @@ msgstr "Löschen"
 msgid "Type the label name"
 msgstr "Gebe dem Label einen Namen"
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:162
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:171
 #, elixir-autogen, elixir-format, fuzzy
 msgid "%{name} - New Lane"
 msgstr "%{name} - Neue Idee"
@@ -562,3 +562,8 @@ msgstr "Detailansicht"
 #, elixir-autogen, elixir-format
 msgid "Give moderating permissions"
 msgstr "Änderungen erlauben"
+
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:45
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Brainstorming not found"
+msgstr "Brainstorming konnte nicht gefunden werden"
diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot
index 5bbfd7e6..830fd1b7 100644
--- a/priv/gettext/default.pot
+++ b/priv/gettext/default.pot
@@ -25,12 +25,12 @@ msgstr ""
 msgid "Ready?"
 msgstr ""
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:176
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:184
 #, elixir-autogen, elixir-format
 msgid "%{name} - Edit"
 msgstr ""
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:153
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:161
 #, elixir-autogen, elixir-format
 msgid "%{name} - New Idea"
 msgstr ""
@@ -384,7 +384,7 @@ msgstr ""
 msgid "Type the label name"
 msgstr ""
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:163
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:171
 #, elixir-autogen, elixir-format
 msgid "%{name} - New Lane"
 msgstr ""
@@ -561,3 +561,8 @@ msgstr ""
 #, elixir-autogen, elixir-format
 msgid "Give moderating permissions"
 msgstr ""
+
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:45
+#, elixir-autogen, elixir-format
+msgid "Brainstorming not found"
+msgstr ""
diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po
index 4c9c1d75..9312c662 100644
--- a/priv/gettext/en/LC_MESSAGES/default.po
+++ b/priv/gettext/en/LC_MESSAGES/default.po
@@ -26,12 +26,12 @@ msgstr ""
 msgid "Ready?"
 msgstr ""
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:175
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:184
 #, elixir-autogen, elixir-format
 msgid "%{name} - Edit"
 msgstr ""
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:152
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:161
 #, elixir-autogen, elixir-format
 msgid "%{name} - New Idea"
 msgstr ""
@@ -299,7 +299,7 @@ msgstr "Are you sure that you want to delete this brainstorming?"
 msgid "Empty brainstorming"
 msgstr ""
 
-#: lib/mindwendel_web/live/live_helpers.ex:29
+#: lib/mindwendel_web/live/live_helpers.ex:30
 #, elixir-autogen, elixir-format, fuzzy
 msgid "Brainstorming will be deleted %{days}"
 msgstr ""
@@ -385,7 +385,7 @@ msgstr ""
 msgid "Type the label name"
 msgstr ""
 
-#: lib/mindwendel_web/live/brainstorming_live/show.ex:162
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:171
 #, elixir-autogen, elixir-format, fuzzy
 msgid "%{name} - New Lane"
 msgstr ""
@@ -562,3 +562,8 @@ msgstr ""
 #, elixir-autogen, elixir-format
 msgid "Give moderating permissions"
 msgstr ""
+
+#: lib/mindwendel_web/live/brainstorming_live/show.ex:45
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Brainstorming not found"
+msgstr ""
diff --git a/test/mindwendel/accounts_test.exs b/test/mindwendel/accounts_test.exs
index d8e067ef..0574a612 100644
--- a/test/mindwendel/accounts_test.exs
+++ b/test/mindwendel/accounts_test.exs
@@ -32,8 +32,8 @@ defmodule Mindwendel.AccountsTest do
       assert brainstorming_moderatoring_user.user_id == user.id
       assert brainstorming_moderatoring_user.brainstorming_id == brainstorming.id
 
-      assert [%User{id: ^user_id}] =
-               Brainstormings.get_brainstorming!(brainstorming.id).moderating_users
+      {:ok, brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
+      assert [%User{id: ^user_id}] = brainstorming.moderating_users
     end
 
     test "responds with an error when brainstorming already contains the moderating user", %{
diff --git a/test/mindwendel/brainstormings_test.exs b/test/mindwendel/brainstormings_test.exs
index d2186dcb..42e336f8 100644
--- a/test/mindwendel/brainstormings_test.exs
+++ b/test/mindwendel/brainstormings_test.exs
@@ -27,11 +27,37 @@ defmodule Mindwendel.BrainstormingsTest do
     }
   end
 
+  describe "get_brainstorming" do
+    test "returns the brainstorming", %{brainstorming: brainstorming} do
+      {:ok, loaded_brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
+      assert loaded_brainstorming.id == brainstorming.id
+    end
+
+    test "returns an error for the wrong uuid" do
+      assert {:error, :invalid_uuid} == Brainstormings.get_brainstorming("invalid_uuid")
+    end
+
+    test "returns an error for a missing brainstorming" do
+      assert {:error, :not_found} ==
+               Brainstormings.get_brainstorming("8a4f5d37-28c4-424e-ac4a-5637a41486c4")
+    end
+
+    test "returns an error for a nil value" do
+      assert {:error, :invalid_uuid} ==
+               Brainstormings.get_brainstorming(nil)
+    end
+  end
+
   describe "validate_admin_secret" do
     test "returns false if secret is wrong", %{brainstorming: brainstorming} do
       refute Brainstormings.validate_admin_secret(brainstorming, "wrong")
     end
 
+    test "returns false if secret is nil", %{brainstorming: brainstorming} do
+      brainstorming = %{brainstorming | admin_url_id: nil}
+      refute Brainstormings.validate_admin_secret(brainstorming, nil)
+    end
+
     test "returns true if secret is correct", %{brainstorming: brainstorming} do
       assert Brainstormings.validate_admin_secret(brainstorming, brainstorming.admin_url_id)
     end
@@ -253,7 +279,7 @@ defmodule Mindwendel.BrainstormingsTest do
       assert Enum.count(brainstorming.ideas) == 1
       Brainstormings.empty(brainstorming)
       # reload brainstorming:
-      brainstorming = Brainstormings.get_brainstorming!(brainstorming.id)
+      {:ok, brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
       brainstorming = brainstorming |> Repo.preload([:ideas, :lanes])
       assert Enum.empty?(brainstorming.lanes)
     end
@@ -302,8 +328,8 @@ defmodule Mindwendel.BrainstormingsTest do
       assert Enum.count(other_brainstorming.ideas) == 1
       Brainstormings.empty(brainstorming)
       # reload brainstorming:
-      brainstorming = Brainstormings.get_brainstorming!(brainstorming.id) |> Repo.preload(:lanes)
-      brainstorming = brainstorming |> Repo.preload([:ideas])
+      {:ok, brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
+      brainstorming = brainstorming |> Repo.preload([:ideas, :lanes])
       other_brainstorming = other_brainstorming |> Repo.preload([:ideas])
       assert Enum.empty?(brainstorming.lanes)
       assert Enum.count(other_brainstorming.lanes) == 1
diff --git a/test/mindwendel_web/live/admin/brainstorming_live/edit_test.exs b/test/mindwendel_web/live/admin/brainstorming_live/edit_test.exs
index ce89c6e2..6986b9e5 100644
--- a/test/mindwendel_web/live/admin/brainstorming_live/edit_test.exs
+++ b/test/mindwendel_web/live/admin/brainstorming_live/edit_test.exs
@@ -89,7 +89,9 @@ defmodule MindwendelWeb.Admin.BrainstormingLive.EditTest do
            |> form("#form-labels", %{brainstorming: %{labels: %{"0": %{name: "new label"}}}})
            |> render_change()
 
-    assert Brainstormings.get_brainstorming!(brainstorming.id).labels
+    {:ok, brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
+
+    assert brainstorming.labels
            |> Enum.map(fn a -> a.name end)
            |> Enum.member?("new label")
   end
diff --git a/test/mindwendel_web/live/brainstorming_live_test.exs b/test/mindwendel_web/live/brainstorming_live_test.exs
index 6d560c52..c8ee0dfc 100644
--- a/test/mindwendel_web/live/brainstorming_live_test.exs
+++ b/test/mindwendel_web/live/brainstorming_live_test.exs
@@ -302,8 +302,10 @@ defmodule MindwendelWeb.BrainstormingLiveTest do
       |> element(".btn[data-testid=\"#{selected_ideal_label.id}\"]")
       |> render_click()
 
+      {:ok, brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
+
       assert(
-        Brainstormings.get_brainstorming!(brainstorming.id).filter_labels_ids == [
+        brainstorming.filter_labels_ids == [
           selected_ideal_label.id
         ]
       )
diff --git a/test/mindwendel_web/live/label_live/captions_test.exs b/test/mindwendel_web/live/label_live/captions_test.exs
index 21c6e5d3..b1d4a62c 100644
--- a/test/mindwendel_web/live/label_live/captions_test.exs
+++ b/test/mindwendel_web/live/label_live/captions_test.exs
@@ -17,7 +17,7 @@ defmodule MindwendelWeb.LabelLive.CaptionsTest do
     brainstorming: brainstorming,
     user: user
   } do
-    preloaded_brainstorming = Brainstormings.get_brainstorming!(brainstorming.id)
+    {:ok, preloaded_brainstorming} = Brainstormings.get_brainstorming(brainstorming.id)
     preloaded_user = Accounts.get_user(user.id)
 
     captions_component =