diff --git a/lib/ret/owned_file.ex b/lib/ret/owned_file.ex index e477c00e4..28cb0bd03 100644 --- a/lib/ret/owned_file.ex +++ b/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 diff --git a/lib/ret_web/channels/hub_channel.ex b/lib/ret_web/channels/hub_channel.ex index 4a743b19e..d85b22199 100644 --- a/lib/ret_web/channels/hub_channel.ex +++ b/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 diff --git a/test/ret_web/channels/entity_test.exs b/test/ret_web/channels/entity_test.exs index ab95917f3..f1bb85647 100644 --- a/test/ret_web/channels/entity_test.exs +++ b/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,