diff --git a/lib/pinchflat/media.ex b/lib/pinchflat/media.ex index ccd6e48..e9fb633 100644 --- a/lib/pinchflat/media.ex +++ b/lib/pinchflat/media.ex @@ -12,12 +12,25 @@ defmodule Pinchflat.Media do alias Pinchflat.Media.MediaMetadata @doc """ - Returns the list of media_items. Returns [%MediaItem{}, ...]. + Returns the list of media_items. + + Returns [%MediaItem{}, ...]. """ def list_media_items do Repo.all(MediaItem) end + @doc """ + Returns a list of media_items for a given source. + + Returns [%MediaItem{}, ...]. + """ + def list_media_items_for(%Source{} = source) do + MediaItem + |> where([mi], mi.source_id == ^source.id) + |> Repo.all() + end + @doc """ Returns a list of pending media_items for a given source, where pending means the `media_filepath` is `nil` AND the media_item @@ -138,26 +151,30 @@ defmodule Pinchflat.Media do end @doc """ - Deletes a media_item and its associated tasks. Will leave files on disk. + Deletes a media_item and its associated tasks. + Can optionally delete the media_item's files. Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}}. """ - def delete_media_item(%MediaItem{} = media_item) do + def delete_media_item(%MediaItem{} = media_item, opts \\ []) do + delete_files = Keyword.get(opts, :delete_files, false) + + if delete_files do + {:ok, _} = delete_all_attachments(media_item) + end + Tasks.delete_tasks_for(media_item) Repo.delete(media_item) end @doc """ - Deletes the media_item's associated files. Will leave the media_item in the database. - - NOTE: this deletes the metadata files as well, but maybe it shouldn't? I'm wondering if - the metadata is more a concern of the DB record itself and should be lumped in with those - delete operations. But the metadata does come from the download operation of the file. - Food for thought but not a priority at the moment. - - Returns {:ok, %MediaItem{}} + Returns an `%Ecto.Changeset{}` for tracking media_item changes. """ - def delete_attachments(media_item) do + def change_media_item(%MediaItem{} = media_item, attrs \\ %{}) do + MediaItem.changeset(media_item, attrs) + end + + defp delete_all_attachments(media_item) do media_item = Repo.preload(media_item, :metadata) media_item @@ -177,25 +194,6 @@ defmodule Pinchflat.Media do {:ok, media_item} end - @doc """ - Deletes the media_item and all associated files. Attempts to delete the root directory - but only if it is empty. - - Returns {:ok, %MediaItem{}} - """ - def delete_media_item_and_attachments(media_item) do - {:ok, _} = delete_attachments(media_item) - - delete_media_item(media_item) - end - - @doc """ - Returns an `%Ecto.Changeset{}` for tracking media_item changes. - """ - def change_media_item(%MediaItem{} = media_item, attrs \\ %{}) do - MediaItem.changeset(media_item, attrs) - end - defp build_format_clauses(media_profile) do mapped_struct = Map.from_struct(media_profile) diff --git a/lib/pinchflat/sources.ex b/lib/pinchflat/sources.ex index 4ecc891..3b8d130 100644 --- a/lib/pinchflat/sources.ex +++ b/lib/pinchflat/sources.ex @@ -6,6 +6,7 @@ defmodule Pinchflat.Sources do import Ecto.Query, warn: false alias Pinchflat.Repo + alias Pinchflat.Media alias Pinchflat.Tasks alias Pinchflat.Tasks.SourceTasks alias Pinchflat.Sources.Source @@ -55,13 +56,20 @@ defmodule Pinchflat.Sources do end @doc """ - Deletes a source and it's associated tasks (of any state). - NOTE: will fail if the source has associated media items. Intended - for now, will almost certainly change in the future. + Deletes a source, its media items, and its associated tasks (of any state). + Can optionally delete the source's media files. Returns {:ok, %Source{}} | {:error, %Ecto.Changeset{}} """ - def delete_source(%Source{} = source) do + def delete_source(%Source{} = source, opts \\ []) do + delete_files = Keyword.get(opts, :delete_files, false) + + source + |> Media.list_media_items_for() + |> Enum.each(fn media_item -> + Media.delete_media_item(media_item, delete_files: delete_files) + end) + Tasks.delete_tasks_for(source) Repo.delete(source) end diff --git a/lib/pinchflat_web/controllers/media_items/media_item_controller.ex b/lib/pinchflat_web/controllers/media_items/media_item_controller.ex index 4248cac..4de4863 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_controller.ex +++ b/lib/pinchflat_web/controllers/media_items/media_item_controller.ex @@ -14,7 +14,7 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do media_item = Media.get_media_item!(id) if delete_files do - {:ok, _} = Media.delete_media_item_and_attachments(media_item) + {:ok, _} = Media.delete_media_item(media_item, delete_files: true) conn |> put_flash(:info, "Record and files deleted successfully.") diff --git a/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex b/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex index f23de2c..38a9d07 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex +++ b/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex @@ -7,17 +7,6 @@ Media Item #<%= @media_item.id %> -
@@ -25,5 +14,17 @@

Attributes

<.list_items_from_map map={Map.from_struct(@media_item)} />
+ +
+ <.link + href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}?delete_files=true"} + method="delete" + data-confirm="Are you sure you want to delete this record and ALL files associated with it? This cannot be undone." + > + <.button color="bg-meta-1" rounding="rounded-full"> + Delete Files + + +
diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index b78245e..d5cbaf2 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/lib/pinchflat_web/controllers/sources/source_controller.ex @@ -87,13 +87,23 @@ defmodule PinchflatWeb.Sources.SourceController do end end - def delete(conn, %{"id" => id}) do + def delete(conn, %{"id" => id} = params) do + delete_files = Map.get(params, "delete_files", false) source = Sources.get_source!(id) - {:ok, _source} = Sources.delete_source(source) - conn - |> put_flash(:info, "Source deleted successfully.") - |> redirect(to: ~p"/sources") + if delete_files do + {:ok, _source} = Sources.delete_source(source, delete_files: true) + + conn + |> put_flash(:info, "Source and files deleted successfully.") + |> redirect(to: ~p"/sources") + else + {:ok, _source} = Sources.delete_source(source) + + conn + |> put_flash(:info, "Source deleted successfully. Files were not deleted.") + |> redirect(to: ~p"/sources") + end end defp media_profiles do diff --git a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex index 52c14f6..6ef48ba 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex @@ -72,5 +72,27 @@

Nothing Here!

<% end %> + +
+ <.link + href={~p"/sources/#{@source.id}"} + method="delete" + data-confirm="Are you sure you want to delete this source (leaving files in place)? This cannot be undone." + > + <.button color="bg-meta-1" rounding="rounded-full"> + Delete Source + + + <.link + href={~p"/sources/#{@source.id}?delete_files=true"} + method="delete" + data-confirm="Are you sure you want to delete this source AND it's associated files? This cannot be undone." + class="mt-5 md:mt-0" + > + <.button color="bg-meta-1" rounding="rounded-full"> + Delete Source and Files + + +
diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index a02fe29..c088132 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -36,6 +36,15 @@ defmodule Pinchflat.MediaTest do end end + describe "list_media_items_for/1" do + test "it returns media_items for a given source" do + source = source_fixture() + media_item = media_item_fixture(%{source_id: source.id}) + + assert Media.list_media_items_for(source) == [media_item] + end + end + describe "list_pending_media_items_for/1" do test "it returns pending without a filepath for a given source" do source = source_fixture() @@ -345,13 +354,20 @@ defmodule Pinchflat.MediaTest do assert {:ok, %MediaItem{}} = Media.delete_media_item(media_item) assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end end + + test "does not delete the media_item's files by default" do + media_item = media_item_with_attachments() + + assert {:ok, _} = Media.delete_media_item(media_item) + assert File.exists?(media_item.media_filepath) + end end - describe "delete_attachments/1" do + describe "delete_media_item/1 when testing file deletion" do test "deletes the media item's files" do media_item = media_item_with_attachments() - assert {:ok, _} = Media.delete_attachments(media_item) + assert {:ok, _} = Media.delete_media_item(media_item, delete_files: true) refute File.exists?(media_item.media_filepath) end @@ -371,23 +387,21 @@ defmodule Pinchflat.MediaTest do {:ok, updated_media_item} = Media.update_media_item(media_item, update_attrs) - assert {:ok, _} = Media.delete_attachments(updated_media_item) + assert {:ok, _} = Media.delete_media_item(updated_media_item, delete_files: true) refute File.exists?(updated_media_item.metadata.metadata_filepath) end - test "does not delete the media item" do - media_item = media_item_with_attachments() - - assert {:ok, _} = Media.delete_attachments(media_item) - - assert Repo.reload!(media_item) + test "deletion deletes the media_item" do + media_item = media_item_fixture() + assert {:ok, %MediaItem{}} = Media.delete_media_item(media_item, delete_files: true) + assert_raise Ecto.NoResultsError, fn -> Media.get_media_item!(media_item.id) end end test "deletes the parent folder if it is empty" do media_item = media_item_with_attachments() root_directory = Path.dirname(media_item.media_filepath) - assert {:ok, _} = Media.delete_attachments(media_item) + assert {:ok, _} = Media.delete_media_item(media_item, delete_files: true) refute File.exists?(root_directory) end @@ -396,7 +410,7 @@ defmodule Pinchflat.MediaTest do root_directory = Path.dirname(media_item.media_filepath) File.touch(Path.join([root_directory, "test.txt"])) - assert {:ok, _} = Media.delete_attachments(media_item) + assert {:ok, _} = Media.delete_media_item(media_item, delete_files: true) assert File.exists?(root_directory) :ok = File.rm(Path.join([root_directory, "test.txt"])) @@ -404,24 +418,6 @@ defmodule Pinchflat.MediaTest do end end - describe "delete_media_item_and_attachments/1" do - setup do - media_item = media_item_with_attachments() - {:ok, media_item: media_item} - end - - test "deletes the media item", %{media_item: media_item} do - assert {:ok, _} = Media.delete_media_item_and_attachments(media_item) - assert_raise Ecto.NoResultsError, fn -> Media.get_media_item!(media_item.id) end - end - - test "deletes associated files", %{media_item: media_item} do - assert File.exists?(media_item.media_filepath) - assert {:ok, _} = Media.delete_media_item_and_attachments(media_item) - refute File.exists?(media_item.media_filepath) - end - end - describe "change_media_item/1" do test "change_media_item/1 returns a media_item changeset" do media_item = media_item_fixture() diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 8cf378d..547755b 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -268,6 +268,43 @@ defmodule Pinchflat.SourcesTest do assert {:ok, %Source{}} = Sources.delete_source(source) assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end end + + test "deletion also deletes all associated media items" do + source = source_fixture() + media_item = media_item_fixture(source_id: source.id) + + assert {:ok, %Source{}} = Sources.delete_source(source) + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "deletion does not delete media files by default" do + source = source_fixture() + media_item = media_item_with_attachments(%{source_id: source.id}) + + assert {:ok, %Source{}} = Sources.delete_source(source) + assert File.exists?(media_item.media_filepath) + end + end + + describe "delete_source/1 when deleting files" do + test "deletes source and media_items" do + source = source_fixture() + media_item = media_item_with_attachments(%{source_id: source.id}) + + assert {:ok, %Source{}} = Sources.delete_source(source, delete_files: true) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + end + + test "also deletes media files" do + source = source_fixture() + media_item = media_item_with_attachments(%{source_id: source.id}) + + assert {:ok, %Source{}} = Sources.delete_source(source, delete_files: true) + + refute File.exists?(media_item.media_filepath) + end end describe "change_source/2" do diff --git a/test/pinchflat_web/controllers/media_item_controller_test.exs b/test/pinchflat_web/controllers/media_item_controller_test.exs index 002f3bd..1f50462 100644 --- a/test/pinchflat_web/controllers/media_item_controller_test.exs +++ b/test/pinchflat_web/controllers/media_item_controller_test.exs @@ -4,7 +4,6 @@ defmodule PinchflatWeb.MediaItemControllerTest do import Pinchflat.MediaFixtures alias Pinchflat.Repo - alias Pinchflat.Media describe "show media" do setup [:create_media_item] @@ -19,10 +18,6 @@ defmodule PinchflatWeb.MediaItemControllerTest do setup do media_item = media_item_with_attachments() - on_exit(fn -> - Media.delete_attachments(media_item) - end) - %{media_item: media_item} end diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index b521055..c9be7af 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/test/pinchflat_web/controllers/source_controller_test.exs @@ -2,8 +2,11 @@ defmodule PinchflatWeb.SourceControllerTest do use PinchflatWeb.ConnCase import Mox - import Pinchflat.ProfilesFixtures + import Pinchflat.MediaFixtures import Pinchflat.SourcesFixtures + import Pinchflat.ProfilesFixtures + + alias Pinchflat.Repo setup do media_profile = media_profile_fixture() @@ -119,21 +122,53 @@ defmodule PinchflatWeb.SourceControllerTest do end end - describe "delete source" do + describe "delete source when just deleting the records" do setup [:create_source] - test "deletes chosen source", %{conn: conn, source: source} do + test "deletes chosen source and media_items", %{conn: conn, source: source, media_item: media_item} do + delete(conn, ~p"/sources/#{source}") + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "redirects to the sources page", %{conn: conn, source: source} do conn = delete(conn, ~p"/sources/#{source}") assert redirected_to(conn) == ~p"/sources" + end - assert_error_sent 404, fn -> - get(conn, ~p"/sources/#{source}") - end + test "does not delete the files", %{conn: conn, source: source, media_item: media_item} do + delete(conn, ~p"/sources/#{source}") + assert File.exists?(media_item.media_filepath) + end + end + + describe "delete source when deleting the records and files" do + setup [:create_source] + + test "deletes chosen source and media_items", %{conn: conn, source: source, media_item: media_item} do + delete(conn, ~p"/sources/#{source}?delete_files=true") + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "redirects to the sources page", %{conn: conn, source: source} do + conn = delete(conn, ~p"/sources/#{source}?delete_files=true") + assert redirected_to(conn) == ~p"/sources" + end + + test "deletes the files", %{conn: conn, source: source, media_item: media_item} do + delete(conn, ~p"/sources/#{source}?delete_files=true") + refute File.exists?(media_item.media_filepath) end end defp create_source(_) do - %{source: source_fixture()} + source = source_fixture() + media_item = media_item_with_attachments(%{source_id: source.id}) + + %{source: source, media_item: media_item} end defp runner_function_mock(_url, _opts, _ot) do