From 1a06405c296924682a546cee4202d7e77f920671 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 3 Apr 2024 13:12:13 -0700 Subject: [PATCH] Added forcing of downloads for media items --- .../downloading/media_download_worker.ex | 12 ++++--- .../media_items/media_item_controller.ex | 10 ++++++ .../actions_dropdown.html.heex | 32 +++++++++++++++++++ .../media_item_html/media_item_form.html.heex | 2 +- .../media_item_html/show.html.heex | 25 ++------------- .../source_html/actions_dropdown.html.heex | 4 +-- .../sources/source_html/show.html.heex | 4 +-- lib/pinchflat_web/router.ex | 4 ++- .../media_download_worker_test.exs | 25 +++++++++++++++ .../media_item_controller_test.exs | 26 +++++++++++++++ 10 files changed, 111 insertions(+), 33 deletions(-) create mode 100644 lib/pinchflat_web/controllers/media_items/media_item_html/actions_dropdown.html.heex diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index ca69153..a59da41 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -19,27 +19,29 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do Returns {:ok, %Task{}} | {:error, :duplicate_job} | {:error, %Ecto.Changeset{}} """ - def kickoff_with_task(media_item, opts \\ []) do + def kickoff_with_task(media_item, job_args \\ %{}, job_opts \\ []) do %{id: media_item.id} - |> MediaDownloadWorker.new(opts) + |> Map.merge(job_args) + |> MediaDownloadWorker.new(job_opts) |> Tasks.create_job_with_task(media_item) end @doc """ For a given media item, download the media alongside any options. - Does not download media if its source is set to not download media. + Does not download media if its source is set to not download media + (unless forced). Returns :ok | {:ok, %MediaItem{}} | {:error, any, ...any} """ @impl Oban.Worker - def perform(%Oban.Job{args: %{"id" => media_item_id}}) do + def perform(%Oban.Job{args: %{"id" => media_item_id} = args}) do media_item = media_item_id |> Media.get_media_item!() |> Repo.preload(:source) # If the source is set to not download media, perform a no-op - if media_item.source.download_media do + if media_item.source.download_media || args["force"] do download_media_and_schedule_jobs(media_item) else :ok 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 bfcb3d2..d348fee 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_controller.ex +++ b/lib/pinchflat_web/controllers/media_items/media_item_controller.ex @@ -6,6 +6,7 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do alias Pinchflat.Repo alias Pinchflat.Media alias Pinchflat.Media.MediaItem + alias Pinchflat.Downloading.MediaDownloadWorker def show(conn, %{"id" => id}) do media_item = @@ -47,6 +48,15 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do |> redirect(to: ~p"/sources/#{media_item.source_id}") end + def force_download(conn, %{"media_item_id" => id}) do + media_item = Media.get_media_item!(id) + {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item, %{force: true}) + + conn + |> put_flash(:info, "Download task enqueued.") + |> redirect(to: ~p"/sources/#{media_item.source_id}/media/#{media_item}") + end + # See here for details on streaming files and range requests: # https://www.zeng.dev/post/2023-http-range-and-play-mp4-in-browser/ # diff --git a/lib/pinchflat_web/controllers/media_items/media_item_html/actions_dropdown.html.heex b/lib/pinchflat_web/controllers/media_items/media_item_html/actions_dropdown.html.heex new file mode 100644 index 0000000..537872e --- /dev/null +++ b/lib/pinchflat_web/controllers/media_items/media_item_html/actions_dropdown.html.heex @@ -0,0 +1,32 @@ +<.button_dropdown text="Actions" class="justify-center w-full sm:w-50"> + <:option> + <.link + href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}/force_download"} + method="post" + data-confirm="Are you sure you force a download of this media?" + > + Force Download + + + <:option> +
+ + <:option> + <.link + href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}"} + method="delete" + data-confirm="Are you sure you want to delete all files for this media item? This cannot be undone." + > + Delete Files + + + <:option> + <.link + href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}?prevent_download=true"} + method="delete" + data-confirm="Are you sure you want to delete all files for this media item and prevent it from re-downloading in the future? This cannot be undone." + > + Delete and Ignore + + + diff --git a/lib/pinchflat_web/controllers/media_items/media_item_html/media_item_form.html.heex b/lib/pinchflat_web/controllers/media_items/media_item_html/media_item_form.html.heex index 1311346..9e61673 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_html/media_item_form.html.heex +++ b/lib/pinchflat_web/controllers/media_items/media_item_html/media_item_form.html.heex @@ -17,7 +17,7 @@ field={f[:prevent_download]} type="toggle" label="Prevent Download" - help="Checking excludes this media item from being downloaded" + help="Checking excludes this media item from automatic download. Download can still be manually forced" /> <.input 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 6de101f..3c02ba1 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 @@ -20,26 +20,7 @@
<.tabbed_layout> <:tab_append> - <.button_dropdown text="Actions" class="justify-center w-full sm:w-50"> - <:option> - <.link - href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}"} - method="delete" - data-confirm="Are you sure you want to delete all files for this media item? This cannot be undone." - > - Delete Files - - - <:option> - <.link - href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}?prevent_download=true"} - method="delete" - data-confirm="Are you sure you want to delete all files for this media item and prevent it from re-downloading in the future? This cannot be undone." - > - Delete and Ignore - - - + <.actions_dropdown media_item={@media_item} /> <:tab title="Attributes"> @@ -53,9 +34,9 @@

Attributes

Source: - <.inline_link href={~p"/sources/#{@media_item.source_id}"}> + <.subtle_link href={~p"/sources/#{@media_item.source_id}"}> <%= @media_item.source.custom_name %> - +
<.list_items_from_map map={Map.from_struct(@media_item)} /> diff --git a/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex b/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex index b5297f2..16a0829 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex @@ -16,7 +16,7 @@ <.link href={~p"/sources/#{@source}/force_download"} method="post" - data-confirm="Are you sure you force a download of all *pending* media items? This isn't normally needed." + data-confirm="Are you sure you want to force a download of all *pending* media items? This isn't normally needed." > Force Download @@ -25,7 +25,7 @@ <.link href={~p"/sources/#{@source}/force_index"} method="post" - data-confirm="Are you sure you force an index of this source? This isn't normally needed." + data-confirm="Are you sure you want to force an index of this source? This isn't normally needed." > Force Index 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 5c49715..d9687a1 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex @@ -28,9 +28,9 @@

Attributes

Media Profile: - <.inline_link href={~p"/media_profiles/#{@source.media_profile_id}"}> + <.subtle_link href={~p"/media_profiles/#{@source.media_profile_id}"}> <%= @source.media_profile.name %> - +
<.list_items_from_map map={Map.from_struct(@source)} /> diff --git a/lib/pinchflat_web/router.ex b/lib/pinchflat_web/router.ex index 75f9ae0..b83e6d8 100644 --- a/lib/pinchflat_web/router.ex +++ b/lib/pinchflat_web/router.ex @@ -35,7 +35,9 @@ defmodule PinchflatWeb.Router do post "/force_download", Sources.SourceController, :force_download post "/force_index", Sources.SourceController, :force_index - resources "/media", MediaItems.MediaItemController, only: [:show, :edit, :update, :delete] + resources "/media", MediaItems.MediaItemController, only: [:show, :edit, :update, :delete] do + post "/force_download", MediaItems.MediaItemController, :force_download + end end end diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index 1f3ed5c..90a7cd5 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -34,6 +34,23 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do assert {:ok, task} = MediaDownloadWorker.kickoff_with_task(media_item) assert task.media_item_id == media_item.id end + + test "can be called with additional job arguments", %{media_item: media_item} do + job_args = %{"force" => true} + + assert {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item, job_args) + + assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id, "force" => true}) + end + + test "can be called with additional job options", %{media_item: media_item} do + job_opts = [max_attempts: 5] + + assert {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item, %{}, job_opts) + + [job] = all_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id}) + assert job.max_attempts == 5 + end end describe "perform/1" do @@ -88,6 +105,14 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do perform_job(MediaDownloadWorker, %{id: media_item.id}) end + test "downloads anyway if forced", %{media_item: media_item} do + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> :ok end) + + Sources.update_source(media_item.source, %{download_media: false}) + + perform_job(MediaDownloadWorker, %{id: media_item.id, force: true}) + end + test "it saves the file's size to the database", %{media_item: media_item} do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> metadata = render_parsed_metadata(:media_metadata) diff --git a/test/pinchflat_web/controllers/media_item_controller_test.exs b/test/pinchflat_web/controllers/media_item_controller_test.exs index e8e035a..b0c8847 100644 --- a/test/pinchflat_web/controllers/media_item_controller_test.exs +++ b/test/pinchflat_web/controllers/media_item_controller_test.exs @@ -4,6 +4,7 @@ defmodule PinchflatWeb.MediaItemControllerTest do import Pinchflat.MediaFixtures alias Pinchflat.Repo + alias Pinchflat.Downloading.MediaDownloadWorker describe "show media" do setup [:create_media_item] @@ -87,6 +88,31 @@ defmodule PinchflatWeb.MediaItemControllerTest do end end + describe "force_download" do + test "enqueues download task", %{conn: conn} do + media_item = media_item_fixture() + + assert [] = all_enqueued(worker: MediaDownloadWorker) + post(conn, ~p"/sources/#{media_item.source_id}/media/#{media_item.id}/force_download") + assert [_] = all_enqueued(worker: MediaDownloadWorker) + end + + test "forces a download even if one wouldn't normally run", %{conn: conn} do + media_item = media_item_fixture(%{media_filepath: nil}) + + post(conn, ~p"/sources/#{media_item.source_id}/media/#{media_item.id}/force_download") + assert [_] = all_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id, "force" => true}) + end + + test "redirects to the show page", %{conn: conn} do + media_item = media_item_fixture() + + conn = post(conn, ~p"/sources/#{media_item.source_id}/media/#{media_item.id}/force_download") + + assert redirected_to(conn) == ~p"/sources/#{media_item.source_id}/media/#{media_item.id}" + end + end + describe "streaming media" do test "returns 404 if the media isn't found", %{conn: conn} do media_item = media_item_fixture()