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/media_source/source.ex b/lib/pinchflat/media_source/source.ex index 374c614..1c94d64 100644 --- a/lib/pinchflat/media_source/source.ex +++ b/lib/pinchflat/media_source/source.ex @@ -27,6 +27,7 @@ defmodule Pinchflat.Sources.Source do collection_id collection_type friendly_name + index_frequency_minutes download_media original_url media_profile_id 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 96f8f61..4f836bc 100644 --- a/lib/pinchflat/sources.ex +++ b/lib/pinchflat/sources.ex @@ -6,9 +6,11 @@ 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 + alias Pinchflat.Tasks.SourceTasks + alias Pinchflat.Profiles.MediaProfile alias Pinchflat.MediaClient.SourceDetails @doc """ @@ -18,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. @@ -55,13 +66,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 @@ -84,10 +102,6 @@ defmodule Pinchflat.Sources do NOTE: When operating in the ideal path, this effectively adds an API call to the source creation/update process. Should be used only when needed. - - IDEA: Maybe I could discern `collection_type` based on the original URL? - It also seems like it's a channel when the returned yt-dlp channel_id is the - same as the playlist_id - maybe could use that? """ def change_source_from_url(%Source{} = source, attrs) do case change_source(source, attrs) do @@ -118,24 +132,20 @@ defmodule Pinchflat.Sources do defp add_source_details_by_collection_type(source, changeset, source_details) do %Ecto.Changeset{changes: changes} = changeset - collection_type = Ecto.Changeset.get_field(changeset, :collection_type) collection_changes = - case collection_type do - :channel -> - %{ - collection_id: source_details.channel_id, - collection_name: source_details.channel_name - } - - :playlist -> - %{ - collection_id: source_details.playlist_id, - collection_name: source_details.playlist_name - } - - _ -> - %{} + if source_details.playlist_id == source_details.channel_id do + %{ + collection_type: :channel, + collection_id: source_details.channel_id, + collection_name: source_details.channel_name + } + else + %{ + collection_type: :playlist, + collection_id: source_details.playlist_id, + collection_name: source_details.playlist_name + } end change_source(source, Map.merge(changes, collection_changes)) @@ -169,21 +179,19 @@ defmodule Pinchflat.Sources do {:ok, source} end - # IDEA: this uses a pattern where `kickoff_indexing_task` controls whether - # it should run based on the source, but `maybe_handle_media_tasks` handles that - # logic itself. Consider updating one or the other to be consistent (once I've - # decided which I like more) defp maybe_run_indexing_task(changeset, source) do case changeset.data do # If the changeset is new (not persisted), attempt indexing no matter what %{__meta__: %{state: :built}} -> SourceTasks.kickoff_indexing_task(source) - # If the record has been persisted, only attempt indexing if the - # indexing frequency has been changed + # If the record has been persisted, only run indexing if the + # indexing frequency has been changed and is now greater than 0 %{__meta__: %{state: :loaded}} -> - if Map.has_key?(changeset.changes, :index_frequency_minutes) do - SourceTasks.kickoff_indexing_task(source) + case changeset.changes do + %{index_frequency_minutes: mins} when mins > 0 -> SourceTasks.kickoff_indexing_task(source) + %{index_frequency_minutes: _} -> Tasks.delete_pending_tasks_for(source, "MediaIndexingWorker") + _ -> :ok end end diff --git a/lib/pinchflat/tasks.ex b/lib/pinchflat/tasks.ex index 0829a79..e7ae431 100644 --- a/lib/pinchflat/tasks.ex +++ b/lib/pinchflat/tasks.ex @@ -19,30 +19,46 @@ defmodule Pinchflat.Tasks do @doc """ Returns the list of tasks for a given record type and ID. Optionally allows you to specify - which job states to include. + which worker or job states to include. Returns [%Task{}, ...] """ - def list_tasks_for(attached_record_type, attached_record_id, job_states \\ Oban.Job.states()) do + def list_tasks_for(attached_record_type, attached_record_id, worker_name \\ nil, job_states \\ Oban.Job.states()) do stringified_states = Enum.map(job_states, &to_string/1) + worker_name_finder = + if worker_name do + # Workers are the full module name - we want to match on the string ENDING with + # the passed worker name and it should be preceeded with a . so we aren't matching + # on a substring. You can pass in more fragments of the worker name if you need + # to disambiguate. eg: "TestWorker" or "FooBar.TestWorker" + worker_finder = "%.#{worker_name}" + + dynamic([_t, j], fragment("? LIKE ?", j.worker, ^worker_finder)) + else + true + end + Repo.all( from t in Task, join: j in assoc(t, :job), where: field(t, ^attached_record_type) == ^attached_record_id, + where: ^worker_name_finder, where: j.state in ^stringified_states ) end @doc """ - Returns the list of pending tasks for a given record type and ID. + Returns the list of pending tasks for a given record type and ID. Optionally allows you to specify + which worker to include. Returns [%Task{}, ...] """ - def list_pending_tasks_for(attached_record_type, attached_record_id) do + def list_pending_tasks_for(attached_record_type, attached_record_id, worker_name \\ nil) do list_tasks_for( attached_record_type, attached_record_id, + worker_name, [:available, :scheduled, :retryable] ) end @@ -107,14 +123,15 @@ defmodule Pinchflat.Tasks do @doc """ Deletes all tasks attached to a given record, cancelling any attached jobs. + Optionally allows you to specify which worker to include. Returns :ok """ - def delete_tasks_for(attached_record) do + def delete_tasks_for(attached_record, worker_name \\ nil) do tasks = case attached_record do - %Source{} = source -> list_tasks_for(:source_id, source.id) - %MediaItem{} = media_item -> list_tasks_for(:media_item_id, media_item.id) + %Source{} = source -> list_tasks_for(:source_id, source.id, worker_name) + %MediaItem{} = media_item -> list_tasks_for(:media_item_id, media_item.id, worker_name) end Enum.each(tasks, &delete_task/1) @@ -122,14 +139,15 @@ defmodule Pinchflat.Tasks do @doc """ Deletes all _pending_ tasks attached to a given record, cancelling any attached jobs. + Optionally allows you to specify which worker to include. Returns :ok """ - def delete_pending_tasks_for(attached_record) do + def delete_pending_tasks_for(attached_record, worker_name \\ nil) do tasks = case attached_record do - %Source{} = source -> list_pending_tasks_for(:source_id, source.id) - %MediaItem{} = media_item -> list_pending_tasks_for(:media_item_id, media_item.id) + %Source{} = source -> list_pending_tasks_for(:source_id, source.id, worker_name) + %MediaItem{} = media_item -> list_pending_tasks_for(:media_item_id, media_item.id, worker_name) end Enum.each(tasks, &delete_task/1) diff --git a/lib/pinchflat/tasks/source_tasks.ex b/lib/pinchflat/tasks/source_tasks.ex index be2d2a0..51f3938 100644 --- a/lib/pinchflat/tasks/source_tasks.ex +++ b/lib/pinchflat/tasks/source_tasks.ex @@ -12,26 +12,23 @@ defmodule Pinchflat.Tasks.SourceTasks do alias Pinchflat.Workers.VideoDownloadWorker @doc """ - Starts tasks for indexing a source's media. + Starts tasks for indexing a source's media regardless of the source's indexing + frequency. It's assumed the caller will check for that. - Returns {:ok, :should_not_index} | {:ok, %Task{}}. + Returns {:ok, %Task{}}. """ def kickoff_indexing_task(%Source{} = source) do - Tasks.delete_pending_tasks_for(source) + Tasks.delete_pending_tasks_for(source, "MediaIndexingWorker") - if source.index_frequency_minutes <= 0 do - {:ok, :should_not_index} - else - source - |> Map.take([:id]) - # Schedule this one immediately, but future ones will be on an interval - |> MediaIndexingWorker.new() - |> Tasks.create_job_with_task(source) - |> case do - # This should never return {:error, :duplicate_job} since we just deleted - # any pending tasks. I'm being assertive about it so it's obvious if I'm wrong - {:ok, task} -> {:ok, task} - end + source + |> Map.take([:id]) + # Schedule this one immediately, but future ones will be on an interval + |> MediaIndexingWorker.new() + |> Tasks.create_job_with_task(source) + |> case do + # This should never return {:error, :duplicate_job} since we just deleted + # any pending tasks. I'm being assertive about it so it's obvious if I'm wrong + {:ok, task} -> {:ok, task} end 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..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_and_attachments(media_item) + 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_items/media_item_html/show.html.heex b/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex index f23de2c..9c5dce2 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 associated files on disk? This cannot be undone." + > + <.button color="bg-meta-1" rounding="rounded-full"> + Delete Files + + +
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..7032a88 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 on disk? 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 b78245e..5ecfbc7 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/lib/pinchflat_web/controllers/sources/source_controller.ex @@ -87,12 +87,20 @@ 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) + {:ok, _source} = Sources.delete_source(source, delete_files: delete_files) + + 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 deleted successfully.") + |> put_flash(:info, flash_message) |> redirect(to: ~p"/sources") end diff --git a/lib/pinchflat_web/controllers/sources/source_html.ex b/lib/pinchflat_web/controllers/sources/source_html.ex index 9302294..044d783 100644 --- a/lib/pinchflat_web/controllers/sources/source_html.ex +++ b/lib/pinchflat_web/controllers/sources/source_html.ex @@ -24,11 +24,4 @@ defmodule PinchflatWeb.Sources.SourceHTML do {"Monthly", 30 * 24 * 60} ] end - - def friendly_collection_types do - [ - {"Channel", "channel"}, - {"Playlist", "playlist"} - ] - end end 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..0f2489b 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}"} + 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}?delete_files=true"} + method="delete" + data-confirm="Are you sure you want to delete this source and it's files on disk? 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/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex b/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex index 88bbba7..8e3bbfe 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex @@ -19,14 +19,12 @@ label="Media Profile" /> - <.input field={f[:collection_type]} options={friendly_collection_types()} type="select" label="Source Type" /> - <.input field={f[:index_frequency_minutes]} options={friendly_index_frequencies()} type="select" label="Index Frequency" - help="The time between one index of this source finishing and the next one starting" + help="Time between one index of this source finishing and the next one starting. Setting to 'Never' will still run an initial index but no subsequent ones" /> <.input diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index a02fe29..fdd87d2 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() @@ -331,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) @@ -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/2 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/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 edbf358..9fccb70 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() @@ -32,12 +41,11 @@ defmodule Pinchflat.SourcesTest do describe "create_source/1" do test "creates a source and adds name + ID from runner response for channels" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) valid_attrs = %{ media_profile_id: media_profile_fixture().id, - original_url: "https://www.youtube.com/channel/abc123", - collection_type: "channel" + original_url: "https://www.youtube.com/channel/abc123" } assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs) @@ -46,12 +54,11 @@ defmodule Pinchflat.SourcesTest do end test "creates a source and adds name + ID for playlists" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &playlist_mock/3) valid_attrs = %{ media_profile_id: media_profile_fixture().id, - original_url: "https://www.youtube.com/playlist?list=abc123", - collection_type: "playlist" + original_url: "https://www.youtube.com/playlist?list=abc123" } assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs) @@ -60,12 +67,11 @@ defmodule Pinchflat.SourcesTest do end test "you can specify a custom friendly_name" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) valid_attrs = %{ media_profile_id: media_profile_fixture().id, original_url: "https://www.youtube.com/channel/abc123", - collection_type: "channel", friendly_name: "some custom name" } @@ -75,12 +81,11 @@ defmodule Pinchflat.SourcesTest do end test "friendly name is pulled from collection_name if not specified" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) valid_attrs = %{ media_profile_id: media_profile_fixture().id, - original_url: "https://www.youtube.com/channel/abc123", - collection_type: "channel" + original_url: "https://www.youtube.com/channel/abc123" } assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs) @@ -88,6 +93,22 @@ defmodule Pinchflat.SourcesTest do assert source.friendly_name == "some channel name" end + test "collection_type is inferred from source details" do + expect(YtDlpRunnerMock, :run, &channel_mock/3) + expect(YtDlpRunnerMock, :run, &playlist_mock/3) + + valid_attrs = %{ + media_profile_id: media_profile_fixture().id, + original_url: "https://www.youtube.com/channel/abc123" + } + + assert {:ok, %Source{} = source_1} = Sources.create_source(valid_attrs) + assert {:ok, %Source{} = source_2} = Sources.create_source(valid_attrs) + + assert source_1.collection_type == :channel + assert source_2.collection_type == :playlist + end + test "creation with invalid data returns error changeset" do assert {:error, %Ecto.Changeset{}} = Sources.create_source(@invalid_source_attrs) end @@ -97,14 +118,15 @@ defmodule Pinchflat.SourcesTest do {:ok, Phoenix.json_library().encode!(%{ channel: "some channel name", - channel_id: "some_channel_id_12345678" + channel_id: "some_channel_id_12345678", + playlist_id: "some_channel_id_12345678", + playlist_title: "some channel name - videos" })} end) valid_once_attrs = %{ media_profile_id: media_profile_fixture().id, - original_url: "https://www.youtube.com/channel/abc123", - collection_type: "channel" + original_url: "https://www.youtube.com/channel/abc123" } assert {:ok, %Source{}} = Sources.create_source(valid_once_attrs) @@ -116,14 +138,15 @@ defmodule Pinchflat.SourcesTest do {:ok, Phoenix.json_library().encode!(%{ channel: "some channel name", - channel_id: "some_channel_id_12345678" + channel_id: "some_channel_id_12345678", + playlist_id: "some_channel_id_12345678", + playlist_title: "some channel name - videos" })} end) valid_attrs = %{ name: "some name", - original_url: "https://www.youtube.com/channel/abc123", - collection_type: "channel" + original_url: "https://www.youtube.com/channel/abc123" } source_1_attrs = Map.merge(valid_attrs, %{media_profile_id: media_profile_fixture().id}) @@ -134,12 +157,25 @@ defmodule Pinchflat.SourcesTest do end test "creation will schedule the indexing task" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) + + valid_attrs = %{ + media_profile_id: media_profile_fixture().id, + original_url: "https://www.youtube.com/channel/abc123" + } + + assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs) + + assert_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id}) + end + + test "creation schedules an index test even if the index frequency is 0" do + expect(YtDlpRunnerMock, :run, &channel_mock/3) valid_attrs = %{ media_profile_id: media_profile_fixture().id, original_url: "https://www.youtube.com/channel/abc123", - collection_type: "channel" + index_frequency_minutes: 0 } assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs) @@ -158,7 +194,7 @@ defmodule Pinchflat.SourcesTest do end test "updating the original_url will re-fetch the source details for channels" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) source = source_fixture() update_attrs = %{original_url: "https://www.youtube.com/channel/abc123"} @@ -169,9 +205,9 @@ defmodule Pinchflat.SourcesTest do end test "updating the original_url will re-fetch the source details for playlists" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &playlist_mock/3) - source = source_fixture(collection_type: "playlist") + source = source_fixture() update_attrs = %{original_url: "https://www.youtube.com/playlist?list=abc123"} assert {:ok, %Source{} = source} = Sources.update_source(source, update_attrs) @@ -180,7 +216,7 @@ defmodule Pinchflat.SourcesTest do end test "not updating the original_url will not re-fetch the source details" do - expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, 0, &channel_mock/3) source = source_fixture() update_attrs = %{name: "some updated name"} @@ -188,7 +224,7 @@ defmodule Pinchflat.SourcesTest do assert {:ok, %Source{}} = Sources.update_source(source, update_attrs) end - test "updating the index frequency will re-schedule the indexing task" do + test "updating the index frequency to >0 will re-schedule the indexing task" do source = source_fixture() update_attrs = %{index_frequency_minutes: 123} @@ -197,11 +233,34 @@ defmodule Pinchflat.SourcesTest do assert_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id}) end - test "not updating the index frequency will not re-schedule the indexing task" do + test "updating the index frequency to 0 will not re-schedule the indexing task" do source = source_fixture() + update_attrs = %{index_frequency_minutes: 0} + + assert {:ok, %Source{}} = Sources.update_source(source, update_attrs) + + refute_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id}) + end + + test "updating the index frequency to 0 will delete any pending tasks" do + source = source_fixture() + {:ok, job} = Oban.insert(MediaIndexingWorker.new(%{"id" => source.id})) + task = task_fixture(source_id: source.id, job_id: job.id) + update_attrs = %{index_frequency_minutes: 0} + + assert {:ok, %Source{}} = Sources.update_source(source, update_attrs) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end + end + + test "not updating the index frequency will not re-schedule the indexing task or delete tasks" do + source = source_fixture() + task = task_fixture(source_id: source.id) update_attrs = %{name: "some updated name"} assert {:ok, %Source{}} = Sources.update_source(source, update_attrs) + + assert Repo.reload!(task) refute_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id}) end @@ -236,7 +295,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) @@ -255,6 +314,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/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}) + + 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 @@ -267,14 +363,14 @@ defmodule Pinchflat.SourcesTest do describe "change_source_from_url/2" do test "it returns a changeset" do - stub(YtDlpRunnerMock, :run, &runner_function_mock/3) + stub(YtDlpRunnerMock, :run, &channel_mock/3) source = source_fixture() assert %Ecto.Changeset{} = Sources.change_source_from_url(source, %{}) end test "it does not fetch source details if the original_url isn't in the changeset" do - expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, 0, &channel_mock/3) changeset = Sources.change_source_from_url(%Source{}, %{name: "some updated name"}) @@ -282,7 +378,7 @@ defmodule Pinchflat.SourcesTest do end test "it fetches source details if the original_url is in the changeset" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) changeset = Sources.change_source_from_url(%Source{}, %{ @@ -293,13 +389,13 @@ defmodule Pinchflat.SourcesTest do end test "it adds source details to the changeset, keeping the orignal details" do - expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + expect(YtDlpRunnerMock, :run, &channel_mock/3) media_profile = media_profile_fixture() media_profile_id = media_profile.id changeset = - Sources.change_source_from_url(%Source{collection_type: :channel}, %{ + Sources.change_source_from_url(%Source{}, %{ original_url: "https://www.youtube.com/channel/abc123", media_profile_id: media_profile.id }) @@ -329,15 +425,29 @@ defmodule Pinchflat.SourcesTest do end end - defp runner_function_mock(_url, _opts, _ot) do + defp playlist_mock(_url, _opts, _ot) do { :ok, Phoenix.json_library().encode!(%{ - channel: "some channel name", - channel_id: "some_channel_id_#{:rand.uniform(1_000_000)}", + channel: nil, + channel_id: nil, playlist_id: "some_playlist_id_#{:rand.uniform(1_000_000)}", playlist_title: "some playlist name" }) } end + + defp channel_mock(_url, _opts, _ot) do + channel_id = "some_channel_id_#{:rand.uniform(1_000_000)}" + + { + :ok, + Phoenix.json_library().encode!(%{ + channel: "some channel name", + channel_id: channel_id, + playlist_id: channel_id, + playlist_title: "some channel name - videos" + }) + } + end end diff --git a/test/pinchflat/tasks/source_tasks_test.exs b/test/pinchflat/tasks/source_tasks_test.exs index 35cc7e0..b0f9039 100644 --- a/test/pinchflat/tasks/source_tasks_test.exs +++ b/test/pinchflat/tasks/source_tasks_test.exs @@ -16,15 +16,7 @@ defmodule Pinchflat.Tasks.SourceTasksTest do setup :verify_on_exit! describe "kickoff_indexing_task/1" do - test "it does not schedule a job if the interval is <= 0" do - source = source_fixture(index_frequency_minutes: -1) - - assert {:ok, :should_not_index} = SourceTasks.kickoff_indexing_task(source) - - refute_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id}) - end - - test "it schedules a job if the interval is > 0" do + test "it schedules a job" do source = source_fixture(index_frequency_minutes: 1) assert {:ok, _} = SourceTasks.kickoff_indexing_task(source) @@ -32,7 +24,7 @@ defmodule Pinchflat.Tasks.SourceTasksTest do assert_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id}) end - test "it creates and attaches a task if the interval is > 0" do + test "it creates and attaches a task" do source = source_fixture(index_frequency_minutes: 1) assert {:ok, %Task{} = task} = SourceTasks.kickoff_indexing_task(source) @@ -42,7 +34,8 @@ defmodule Pinchflat.Tasks.SourceTasksTest do test "it deletes any pending tasks for the source" do source = source_fixture() - task = task_fixture(source_id: source.id) + {:ok, job} = Oban.insert(MediaIndexingWorker.new(%{"id" => source.id})) + task = task_fixture(source_id: source.id, job_id: job.id) assert {:ok, _} = SourceTasks.kickoff_indexing_task(source) diff --git a/test/pinchflat/tasks_test.exs b/test/pinchflat/tasks_test.exs index 7d3495e..d780280 100644 --- a/test/pinchflat/tasks_test.exs +++ b/test/pinchflat/tasks_test.exs @@ -36,7 +36,7 @@ defmodule Pinchflat.TasksTest do end end - describe "list_tasks_for/3" do + describe "list_tasks_for/4" do test "it lets you specify which record type/ID to join on" do task = task_fixture() @@ -46,12 +46,25 @@ defmodule Pinchflat.TasksTest do test "it lets you specify which job states to include" do task = task_fixture() - assert Tasks.list_tasks_for(:source_id, task.source_id, [:available]) == [task] - assert Tasks.list_tasks_for(:source_id, task.source_id, [:cancelled]) == [] + assert Tasks.list_tasks_for(:source_id, task.source_id, nil, [:available]) == [task] + assert Tasks.list_tasks_for(:source_id, task.source_id, nil, [:cancelled]) == [] + end + + test "it lets you specify which worker to include" do + task = task_fixture() + + assert Tasks.list_tasks_for(:source_id, task.source_id, "TestJobWorker") == [task] + assert Tasks.list_tasks_for(:source_id, task.source_id, "FooBarWorker") == [] + end + + test "it includes all workers if no worker is specified" do + task = task_fixture() + + assert Tasks.list_tasks_for(:source_id, task.source_id, nil) == [task] end end - describe "list_pending_tasks_for/2" do + describe "list_pending_tasks_for/3" do test "it lists pending tasks" do task = task_fixture() @@ -64,6 +77,13 @@ defmodule Pinchflat.TasksTest do assert Tasks.list_pending_tasks_for(:source_id, task.source_id) == [] end + + test "it lets you specify which worker to include" do + task = task_fixture() + + assert Tasks.list_pending_tasks_for(:source_id, task.source_id, "TestJobWorker") == [task] + assert Tasks.list_pending_tasks_for(:source_id, task.source_id, "FooBarWorker") == [] + end end describe "get_task!/1" do @@ -154,7 +174,7 @@ defmodule Pinchflat.TasksTest do end end - describe "delete_tasks_for/1" do + describe "delete_tasks_for/2" do test "it deletes tasks attached to a source" do source = source_fixture() task = task_fixture(source_id: source.id) @@ -170,6 +190,28 @@ defmodule Pinchflat.TasksTest do assert :ok = Tasks.delete_tasks_for(media_item) assert_raise Ecto.NoResultsError, fn -> Tasks.get_task!(task.id) end end + + test "deletion can specify which worker to include" do + media_item = media_item_fixture() + task = task_fixture(media_item_id: media_item.id) + + assert :ok = Tasks.delete_tasks_for(media_item, "FooBarWorker") + assert Repo.reload!(task) + + assert :ok = Tasks.delete_tasks_for(media_item, "TestJobWorker") + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end + end + + test "deletion does not impact unintended records" do + source = source_fixture() + task = task_fixture(source_id: source.id) + + assert :ok = Tasks.delete_tasks_for(source_fixture()) + assert :ok = Tasks.delete_tasks_for(source_fixture(), "FooBarWorker") + assert :ok = Tasks.delete_tasks_for(source_fixture(), "TestJobWorker") + + assert Repo.reload!(task) + end end describe "delete_pending_tasks_for/1" do @@ -200,6 +242,17 @@ defmodule Pinchflat.TasksTest do assert Tasks.get_task!(cancelled_task.id) assert_raise Ecto.NoResultsError, fn -> Repo.reload!(pending_task) end end + + test "deletion can specify which worker to include" do + media_item = media_item_fixture() + task = task_fixture(media_item_id: media_item.id) + + assert :ok = Tasks.delete_pending_tasks_for(media_item, "FooBarWorker") + assert Repo.reload!(task) + + assert :ok = Tasks.delete_pending_tasks_for(media_item, "TestJobWorker") + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end + end end describe "change_task/1" 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/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 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