Error when not authorized and not owned file, unit tests

lib/ret/owned_file.ex

@@ -44,7 +44,13 @@ defmodule Ret.OwnedFile do
   end
 
   def set_inactive(owned_file_uuid, account_id) do
-    get_by_uuid_and_account(owned_file_uuid, account_id) |> set_state(:inactive)
+    case get_by_uuid_and_account(owned_file_uuid, account_id) do
+      nil ->
+        {:error, :non_existent_file_id}
+
+      owned_file ->
+        set_state(owned_file, :inactive)
+    end
   end
 
   def set_inactive(%OwnedFile{} = owned_file) do

lib/ret_web/channels/hub_channel.ex

@@ -499,30 +499,18 @@ defmodule RetWeb.HubChannel do
   end
 
   def handle_in("delete_entity_state", %{"nid" => nid} = payload, socket) do
-    {:ok, hub, account} = authorize(socket, :write_entity_state)
-
-    case Ret.delete_entity(hub.hub_id, nid) do
-      {:ok, _} ->
-        try do
-          RoomObject.perform_unpin(hub, nid)
-        rescue
-          _ -> :ok
-        end
-
-        if payload["file_id"] do
-          case OwnedFile.set_inactive(payload["file_id"], account.account_id) do
-            {:ok, _} -> :ok
-            {:error, reason} -> reply_error(socket, reason)
-          end
-        end
-
-        broadcast!(socket, "entity_state_deleted", %{
-          "nid" => nid,
-          "creator" => socket.assigns.session_id
-        })
+    with {:ok, hub, account} <- authorize(socket, :write_entity_state),
+         {:ok, _} <- Ret.delete_entity(hub.hub_id, nid),
+         {:ok, _} <- maybe_set_owned_file_inactive(payload, account) do
+      RoomObject.perform_unpin(hub, nid)
 
-        {:reply, :ok, socket}
+      broadcast!(socket, "entity_state_deleted", %{
+        "nid" => nid,
+        "creator" => socket.assigns.session_id
+      })
 
+      {:reply, :ok, socket}
+    else
       {:error, reason} ->
         reply_error(socket, reason)
     end
@@ -1484,6 +1472,14 @@ defmodule RetWeb.HubChannel do
     }
   end
 
+  defp maybe_set_owned_file_inactive(%{"file_id" => file_id}, %Account{account_id: account_id}) do
+    OwnedFile.set_inactive(file_id, account_id)
+  end
+
+  defp maybe_set_owned_file_inactive(_payload, _account) do
+    {:ok, :no_file}
+  end
+
   defp maybe_promote_file(%{file_id: nil} = _params, _account, _socket) do
     :ok
   end

test/ret_web/channels/entity_test.exs

@@ -159,6 +159,37 @@ defmodule RetWeb.EntityTest do
     }
   end
 
+  test "delete_entity_state replies with error if owned file does not exist", %{
+    socket: socket,
+    hub: hub,
+    account: account
+  } do
+    %HubRoleMembership{hub: hub, account: account} |> Repo.insert!()
+
+    {:ok, _, socket} =
+      subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))
+
+    push(socket, "save_entity_state", @payload_save_entity_state)
+
+    non_existent_file_payload =
+      Map.put(@payload_delete_entity_state, "file_id", "non_existent_file_id")
+
+    assert_reply push(socket, "delete_entity_state", non_existent_file_payload), :error, %{
+      reason: :non_existent_file_id
+    }
+  end
+
+  test "delete_entity_state replies with error if not authorized", %{
+    socket: socket,
+    hub: hub
+  } do
+    {:ok, _, socket} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", @default_join_params)
+
+    assert_reply push(socket, "delete_entity_state", @payload_delete_entity_state), :error, %{
+      reason: :not_logged_in
+    }
+  end
+
   test "delete_entity_state deletes the entity and deactivates the owned file if file_id is present",
        %{
          socket: socket,