From 17a6b6a69190e5510b5c7eccd515b909cb3484f9 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 28 Feb 2024 08:56:47 -0800 Subject: [PATCH] Added deletion to the last models --- lib/pinchflat/profiles.ex | 32 ++++++++-- lib/pinchflat/sources.ex | 12 +++- .../media_items/media_item_controller.ex | 22 +++---- .../media_profile_controller.ex | 15 ++++- .../media_profile_html/show.html.heex | 22 +++++++ .../controllers/sources/source_controller.ex | 22 +++---- .../sources/source_html/show.html.heex | 6 +- test/pinchflat/media_test.exs | 4 +- test/pinchflat/profiles_test.exs | 64 ++++++++++++++++++- test/pinchflat/sources_test.exs | 13 +++- .../media_profile_controller_test.exs | 64 +++++++++++++++++-- 11 files changed, 226 insertions(+), 50 deletions(-) diff --git a/lib/pinchflat/profiles.ex b/lib/pinchflat/profiles.ex index 01d78b4..882a1ef 100644 --- a/lib/pinchflat/profiles.ex +++ b/lib/pinchflat/profiles.ex @@ -4,12 +4,15 @@ defmodule Pinchflat.Profiles do """ import Ecto.Query, warn: false - alias Pinchflat.Repo + alias Pinchflat.Repo + alias Pinchflat.Sources alias Pinchflat.Profiles.MediaProfile @doc """ - Returns the list of media_profiles. Returns [%MediaProfile{}, ...] + Returns the list of media_profiles. + + Returns [%MediaProfile{}, ...] """ def list_media_profiles do Repo.all(MediaProfile) @@ -23,7 +26,9 @@ defmodule Pinchflat.Profiles do def get_media_profile!(id), do: Repo.get!(MediaProfile, id) @doc """ - Creates a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}} + Creates a media_profile. + + Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}} """ def create_media_profile(attrs) do %MediaProfile{} @@ -32,7 +37,9 @@ defmodule Pinchflat.Profiles do end @doc """ - Updates a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}} + Updates a media_profile. + + Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}} """ def update_media_profile(%MediaProfile{} = media_profile, attrs) do media_profile @@ -41,14 +48,25 @@ defmodule Pinchflat.Profiles do end @doc """ - Deletes a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}} + Deletes a media_profile, all its sources, and all their media items. + Can optionally delete the media files. + + Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}} """ - def delete_media_profile(%MediaProfile{} = media_profile) do + def delete_media_profile(%MediaProfile{} = media_profile, opts \\ []) do + delete_files = Keyword.get(opts, :delete_files, false) + + media_profile + |> Sources.list_sources_for() + |> Enum.each(fn source -> + Sources.delete_source(source, delete_files: delete_files) + end) + Repo.delete(media_profile) end @doc """ - Returns an `%Ecto.Changeset{}` for tracking media_profile changes. + Returns `%Ecto.Changeset{}` """ def change_media_profile(%MediaProfile{} = media_profile, attrs \\ %{}) do MediaProfile.changeset(media_profile, attrs) diff --git a/lib/pinchflat/sources.ex b/lib/pinchflat/sources.ex index 8e998a2..334576b 100644 --- a/lib/pinchflat/sources.ex +++ b/lib/pinchflat/sources.ex @@ -8,8 +8,9 @@ defmodule Pinchflat.Sources do alias Pinchflat.Media alias Pinchflat.Tasks - alias Pinchflat.Tasks.SourceTasks alias Pinchflat.Sources.Source + alias Pinchflat.Tasks.SourceTasks + alias Pinchflat.Profiles.MediaProfile alias Pinchflat.MediaClient.SourceDetails @doc """ @@ -19,6 +20,15 @@ defmodule Pinchflat.Sources do Repo.all(Source) end + @doc """ + Returns the list of sources for a media_profile. + + Returns [%Source{}, ...] + """ + def list_sources_for(%MediaProfile{} = media_profile) do + Repo.all(from s in Source, where: s.media_profile_id == ^media_profile.id) + end + @doc """ Gets a single source. 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 4de4863..6d705b5 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_controller.ex +++ b/lib/pinchflat_web/controllers/media_items/media_item_controller.ex @@ -12,19 +12,17 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do def delete(conn, %{"id" => id} = params) do delete_files = Map.get(params, "delete_files", false) media_item = Media.get_media_item!(id) + {:ok, _} = Media.delete_media_item(media_item, delete_files: delete_files) - if delete_files do - {:ok, _} = Media.delete_media_item(media_item, delete_files: true) + flash_message = + if delete_files do + "Record and files deleted successfully." + else + "Record deleted successfully. Files were not deleted." + end - conn - |> put_flash(:info, "Record and files deleted successfully.") - |> redirect(to: ~p"/sources/#{media_item.source_id}") - else - {:ok, _} = Media.delete_media_item(media_item) - - conn - |> put_flash(:info, "Record deleted successfully. Files were not deleted.") - |> redirect(to: ~p"/sources/#{media_item.source_id}") - end + conn + |> put_flash(:info, flash_message) + |> redirect(to: ~p"/sources/#{media_item.source_id}") end end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex b/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex index 9e2aaf4..246f6e3 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex @@ -46,6 +46,7 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do def edit(conn, %{"id" => id}) do media_profile = Profiles.get_media_profile!(id) changeset = Profiles.change_media_profile(media_profile) + render(conn, :edit, media_profile: media_profile, changeset: changeset) end @@ -63,12 +64,20 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do end end - def delete(conn, %{"id" => id}) do + def delete(conn, %{"id" => id} = params) do + delete_files = Map.get(params, "delete_files", false) media_profile = Profiles.get_media_profile!(id) - {:ok, _media_profile} = Profiles.delete_media_profile(media_profile) + {:ok, _media_profile} = Profiles.delete_media_profile(media_profile, delete_files: delete_files) + + flash_message = + if delete_files do + "Media profile, its sources, and its files deleted successfully." + else + "Media profile and its sources deleted successfully. Files were not deleted." + end conn - |> put_flash(:info, "Media profile deleted successfully.") + |> put_flash(:info, flash_message) |> redirect(to: ~p"/media_profiles") end end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_html/show.html.heex b/lib/pinchflat_web/controllers/media_profiles/media_profile_html/show.html.heex index bb39eb9..533ea5b 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_html/show.html.heex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_html/show.html.heex @@ -22,5 +22,27 @@

Attributes

<.list_items_from_map map={Map.from_struct(@media_profile)} /> + +
+ <.link + href={~p"/media_profiles/#{@media_profile}"} + method="delete" + data-confirm="Are you sure you want to delete this profile and all its sources (leaving files in place)? This cannot be undone." + > + <.button color="bg-meta-1" rounding="rounded-full"> + Delete Profile and its Sources + + + <.link + href={~p"/media_profiles/#{@media_profile}?delete_files=true"} + method="delete" + data-confirm="Are you sure you want to delete this profile, all its sources, and its files? This cannot be undone." + class="mt-5 md:mt-0" + > + <.button color="bg-meta-1" rounding="rounded-full"> + Delete Profile, Sources and Files + + +
diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index d5cbaf2..5ecfbc7 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/lib/pinchflat_web/controllers/sources/source_controller.ex @@ -90,20 +90,18 @@ defmodule PinchflatWeb.Sources.SourceController 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, delete_files: delete_files) - if delete_files do - {:ok, _source} = Sources.delete_source(source, delete_files: true) + flash_message = + if delete_files do + "Source and files deleted successfully." + else + "Source deleted successfully. Files were not deleted." + end - 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 + conn + |> put_flash(:info, flash_message) + |> redirect(to: ~p"/sources") 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 6ef48ba..4ef4aa1 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex @@ -75,7 +75,7 @@
<.link - href={~p"/sources/#{@source.id}"} + href={~p"/sources/#{@source}"} method="delete" data-confirm="Are you sure you want to delete this source (leaving files in place)? This cannot be undone." > @@ -84,9 +84,9 @@ <.link - href={~p"/sources/#{@source.id}?delete_files=true"} + href={~p"/sources/#{@source}?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." + data-confirm="Are you sure you want to delete this source AND it's files? This cannot be undone." class="mt-5 md:mt-0" > <.button color="bg-meta-1" rounding="rounded-full"> diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index c088132..fdd87d2 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -340,7 +340,7 @@ defmodule Pinchflat.MediaTest do end end - describe "delete_media_item/1" do + describe "delete_media_item/2" do test "deletion deletes the media_item" do media_item = media_item_fixture() assert {:ok, %MediaItem{}} = Media.delete_media_item(media_item) @@ -363,7 +363,7 @@ defmodule Pinchflat.MediaTest do end end - describe "delete_media_item/1 when testing file deletion" do + describe "delete_media_item/2 when testing file deletion" do test "deletes the media item's files" do media_item = media_item_with_attachments() diff --git a/test/pinchflat/profiles_test.exs b/test/pinchflat/profiles_test.exs index 8635750..ea127c6 100644 --- a/test/pinchflat/profiles_test.exs +++ b/test/pinchflat/profiles_test.exs @@ -1,9 +1,12 @@ defmodule Pinchflat.ProfilesTest do use Pinchflat.DataCase + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + import Pinchflat.ProfilesFixtures + alias Pinchflat.Profiles alias Pinchflat.Profiles.MediaProfile - import Pinchflat.ProfilesFixtures @invalid_attrs %{name: nil, output_path_template: nil} @@ -61,11 +64,66 @@ defmodule Pinchflat.ProfilesTest do end end - describe "delete_media_profile/1" do + describe "delete_media_profile/2" do test "deletion deletes the media_profile" do media_profile = media_profile_fixture() + assert {:ok, %MediaProfile{}} = Profiles.delete_media_profile(media_profile) - assert_raise Ecto.NoResultsError, fn -> Profiles.get_media_profile!(media_profile.id) end + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_profile) end + end + + test "deletion deletes all sources" do + media_profile = media_profile_fixture() + source = source_fixture(media_profile_id: media_profile.id) + + assert {:ok, %MediaProfile{}} = Profiles.delete_media_profile(media_profile) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + end + + test "deletion deletes all media items" do + media_profile = media_profile_fixture() + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_fixture(source_id: source.id) + + assert {:ok, %MediaProfile{}} = Profiles.delete_media_profile(media_profile) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "deletion does not delete files by default" do + media_profile = media_profile_fixture() + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_with_attachments(%{source_id: source.id}) + + assert {:ok, %MediaProfile{}} = Profiles.delete_media_profile(media_profile) + + assert File.exists?(media_item.media_filepath) + end + end + + describe "delete_media_profile/2 when deleting files" do + test "still deletes all the needful records" do + media_profile = media_profile_fixture() + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_fixture(source_id: source.id) + + assert {:ok, %MediaProfile{}} = Profiles.delete_media_profile(media_profile, delete_files: true) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_profile) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "deletes files" do + media_profile = media_profile_fixture() + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_with_attachments(%{source_id: source.id}) + + assert {:ok, %MediaProfile{}} = Profiles.delete_media_profile(media_profile, delete_files: true) + + refute File.exists?(media_item.media_filepath) end end diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 89c479b..b4cb650 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -23,6 +23,15 @@ defmodule Pinchflat.SourcesTest do end end + describe "list_sources_for/1" do + test "returns all sources for a given media profile" do + media_profile = media_profile_fixture() + source = source_fixture(media_profile_id: media_profile.id) + + assert Sources.list_sources_for(media_profile) == [source] + end + end + describe "get_source!/1" do test "it returns the source with given id" do source = source_fixture() @@ -285,7 +294,7 @@ defmodule Pinchflat.SourcesTest do end end - describe "delete_source/1" do + describe "delete_source/2" do test "it deletes the source" do source = source_fixture() assert {:ok, %Source{}} = Sources.delete_source(source) @@ -322,7 +331,7 @@ defmodule Pinchflat.SourcesTest do end end - describe "delete_source/1 when deleting files" do + describe "delete_source/2 when deleting files" do test "deletes source and media_items" do source = source_fixture() media_item = media_item_with_attachments(%{source_id: source.id}) diff --git a/test/pinchflat_web/controllers/media_profile_controller_test.exs b/test/pinchflat_web/controllers/media_profile_controller_test.exs index 7ae2298..16e5133 100644 --- a/test/pinchflat_web/controllers/media_profile_controller_test.exs +++ b/test/pinchflat_web/controllers/media_profile_controller_test.exs @@ -1,8 +1,12 @@ defmodule PinchflatWeb.MediaProfileControllerTest do use PinchflatWeb.ConnCase + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures import Pinchflat.ProfilesFixtures + alias Pinchflat.Repo + @create_attrs %{name: "some name", output_path_template: "some output_path_template"} @update_attrs %{ name: "some updated name", @@ -97,21 +101,71 @@ defmodule PinchflatWeb.MediaProfileControllerTest do end end - describe "delete media_profile" do + describe "delete media_profile when just deleting the records" do setup [:create_media_profile] - test "deletes chosen media_profile", %{conn: conn, media_profile: media_profile} do + test "deletes chosen media_profile and its associations", %{conn: conn, media_profile: media_profile} do + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_with_attachments(%{source_id: source.id}) + conn = delete(conn, ~p"/media_profiles/#{media_profile}") assert redirected_to(conn) == ~p"/media_profiles" - assert_error_sent 404, fn -> - get(conn, ~p"/media_profiles/#{media_profile}") - end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_profile) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "redirects to the media_profiles page", %{conn: conn, media_profile: media_profile} do + conn = delete(conn, ~p"/media_profiles/#{media_profile}") + + assert redirected_to(conn) == ~p"/media_profiles" + end + + test "doesn't delete any files", %{conn: conn, media_profile: media_profile} do + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_with_attachments(%{source_id: source.id}) + + delete(conn, ~p"/media_profiles/#{media_profile}") + + assert File.exists?(media_item.media_filepath) + end + end + + describe "delete media_profile when deleting the records and files" do + setup [:create_media_profile] + + test "deletes chosen media_profile and its associations", %{conn: conn, media_profile: media_profile} do + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_with_attachments(%{source_id: source.id}) + + conn = delete(conn, ~p"/media_profiles/#{media_profile}?delete_files=true") + assert redirected_to(conn) == ~p"/media_profiles" + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_profile) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + end + + test "redirects to the media_profiles page", %{conn: conn, media_profile: media_profile} do + conn = delete(conn, ~p"/media_profiles/#{media_profile}?delete_files=true") + + assert redirected_to(conn) == ~p"/media_profiles" + end + + test "deletes the files", %{conn: conn, media_profile: media_profile} do + source = source_fixture(media_profile_id: media_profile.id) + media_item = media_item_with_attachments(%{source_id: source.id}) + + delete(conn, ~p"/media_profiles/#{media_profile}?delete_files=true") + + refute File.exists?(media_item.media_filepath) end end defp create_media_profile(_) do media_profile = media_profile_fixture() + %{media_profile: media_profile} end end