Delete media items (#20)

* Added method for deleting media files and their content

* Adds controllers and methods for deleting media and files

* Improved tmpfile setup and teardown for tests

* Actually got tmpfile cleanup running once per suite run

* Finally fixed flash messages

Author: kieraneglin

config/test.exs

@@ -3,8 +3,8 @@ import Config
 config :pinchflat,
   # Specifying backend data here makes mocking and local testing SUPER easy
   yt_dlp_executable: Path.join([File.cwd!(), "/test/support/scripts/yt-dlp-mocks/repeater.sh"]),
-  media_directory: Path.join([System.tmp_dir!(), "videos"]),
-  metadata_directory: Path.join([System.tmp_dir!(), "metadata"])
+  media_directory: Path.join([System.tmp_dir!(), "test", "videos"]),
+  metadata_directory: Path.join([System.tmp_dir!(), "test", "metadata"])
 
 config :pinchflat, Oban, testing: :manual
 

lib/pinchflat/media.ex

@@ -88,6 +88,22 @@ defmodule Pinchflat.Media do
   """
   def get_media_item!(id), do: Repo.get!(MediaItem, id)
 
+  @doc """
+  Produces a flat list of the filesystem paths for a media_item's downloaded files
+
+  Returns [binary()]
+  """
+  def media_filepaths(media_item) do
+    mapped_struct = Map.from_struct(media_item)
+
+    MediaItem.filepath_attributes()
+    |> Enum.map(fn
+      :subtitle_filepaths = field -> Enum.map(mapped_struct[field], fn [_, filepath] -> filepath end)
+      field -> List.wrap(mapped_struct[field])
+    end)
+    |> List.flatten()
+  end
+
   @doc """
   Creates a media_item. Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}}.
   """
@@ -107,7 +123,7 @@ defmodule Pinchflat.Media do
   end
 
   @doc """
-  Deletes a media_item and its associated tasks.
+  Deletes a media_item and its associated tasks. Will leave files on disk.
 
   Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}}.
   """
@@ -116,6 +132,35 @@ defmodule Pinchflat.Media do
     Repo.delete(media_item)
   end
 
+  @doc """
+  Deletes the media_item's associated files. Will leave the media_item in the database.
+
+  Returns {:ok, %MediaItem{}}
+  """
+  def delete_attachments(media_item) do
+    media_item
+    |> media_filepaths()
+    |> Enum.each(&File.rm/1)
+
+    # Fails if the directory is not empty
+    case File.rmdir(Path.dirname(media_item.media_filepath)) do
+      :ok -> {:ok, media_item}
+      {:error, :eexist} -> {:ok, media_item}
+    end
+  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.
   """

lib/pinchflat/media/media_item.ex

@@ -60,4 +60,9 @@ defmodule Pinchflat.Media.MediaItem do
     |> validate_required(@required_fields)
     |> unique_constraint([:media_id, :source_id])
   end
+
+  @doc false
+  def filepath_attributes do
+    ~w(media_filepath thumbnail_filepath metadata_filepath subtitle_filepaths)a
+  end
 end

lib/pinchflat/media_source.ex

@@ -56,6 +56,8 @@ defmodule Pinchflat.MediaSource do
 
   @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.
 
   Returns {:ok, %Source{}} | {:error, %Ecto.Changeset{}}
   """

lib/pinchflat_web/components/core_components.ex

@@ -112,24 +112,29 @@ defmodule PinchflatWeb.CoreComponents do
     <div
       :if={msg = render_slot(@inner_block) || Phoenix.Flash.get(@flash, @kind)}
       id={@id}
-      phx-click={JS.push("lv:clear-flash", value: %{key: @kind}) |> hide("##{@id}")}
+      class="pb-8"
       role="alert"
-      class={[
-        "fixed top-2 right-2 mr-2 w-80 sm:w-96 z-50 rounded-lg p-3 ring-1",
-        @kind == :info && "bg-emerald-50 text-emerald-800 ring-emerald-500 fill-cyan-900",
-        @kind == :error && "bg-rose-50 text-rose-900 shadow-md ring-rose-500 fill-rose-900"
-      ]}
       {@rest}
     >
-      <p :if={@title} class="flex items-center gap-1.5 text-sm font-semibold leading-6">
-        <.icon :if={@kind == :info} name="hero-information-circle-mini" class="h-4 w-4" />
-        <.icon :if={@kind == :error} name="hero-exclamation-circle-mini" class="h-4 w-4" />
-        <%= @title %>
-      </p>
-      <p class="mt-2 text-sm leading-5"><%= msg %></p>
-      <button type="button" class="group absolute top-1 right-1 p-2" aria-label={gettext("close")}>
-        <.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
-      </button>
+      <div class={[
+        "flex justify-between w-full border-l-6 bg-opacity-[50%] p-5 shadow-md dark:bg-opacity-40 dark:text-white",
+        @kind == :info && "border-[#34D399] bg-[#34D399]",
+        @kind == :error && "border-[#F87171] bg-[#F87171]"
+      ]}>
+        <main>
+          <h5 :if={@title} class="mb-2 text-lg font-bold">
+            <%= @title %>
+          </h5>
+          <p class="mt-2 text-md leading-5 opacity-80"><%= msg %></p>
+        </main>
+        <button
+          type="button"
+          aria-label={gettext("close")}
+          phx-click={JS.push("lv:clear-flash", value: %{key: @kind}) |> hide("##{@id}")}
+        >
+          <.icon name="hero-x-mark-solid" class="h-7 w-7 opacity-70 hover:opacity-100" />
+        </button>
+      </div>
     </div>
     """
   end
@@ -146,7 +151,7 @@ defmodule PinchflatWeb.CoreComponents do
 
   def flash_group(assigns) do
     ~H"""
-    <div id={@id}>
+    <div class="flex flex-col gap-7.5" id={@id}>
       <.flash kind={:info} title="Success!" flash={@flash} />
       <.flash kind={:error} title="Error!" flash={@flash} />
       <.flash
@@ -632,19 +637,23 @@ defmodule PinchflatWeb.CoreComponents do
   def show(js \\ %JS{}, selector) do
     JS.show(js,
       to: selector,
-      transition:
-        {"transition-all transform ease-out duration-300", "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95",
-         "opacity-100 translate-y-0 sm:scale-100"}
+      transition: {
+        "transition-all transform ease-out duration-300",
+        "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95",
+        "opacity-100 translate-y-0 sm:scale-100"
+      }
     )
   end
 
   def hide(js \\ %JS{}, selector) do
     JS.hide(js,
       to: selector,
       time: 200,
-      transition:
-        {"transition-all transform ease-in duration-200", "opacity-100 translate-y-0 sm:scale-100",
-         "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95"}
+      transition: {
+        "transition-all transform ease-in duration-200",
+        "opacity-100 translate-y-0 sm:scale-100",
+        "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95"
+      }
     )
   end
 

lib/pinchflat_web/controllers/media/media_item_controller.ex

@@ -8,4 +8,23 @@ defmodule PinchflatWeb.Media.MediaItemController do
 
     render(conn, :show, media_item: media_item)
   end
+
+  def delete(conn, %{"id" => id} = params) do
+    delete_files = Map.get(params, "delete_files", false)
+    media_item = Media.get_media_item!(id)
+
+    if delete_files do
+      {:ok, _} = Media.delete_media_item_and_attachments(media_item)
+
+      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
+  end
 end

lib/pinchflat_web/controllers/media/media_item_html/show.html.heex

@@ -1,12 +1,23 @@
 <div class="mb-6 flex gap-3 flex-row items-center justify-between">
   <div class="flex gap-3 items-center">
-    <.link :if={@conn.params["source_id"]} navigate={~p"/sources/#{@media_item.source_id}"}>
+    <.link navigate={~p"/sources/#{@media_item.source_id}"}>
       <.icon name="hero-arrow-left" class="w-10 h-10 hover:dark:text-white" />
     </.link>
     <h2 class="text-title-md2 font-bold text-black dark:text-white ml-4">
       Media Item #<%= @media_item.id %>
     </h2>
   </div>
+  <nav>
+    <.link
+      href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}?delete_files=true"}
+      method="delete"
+      data-confirm="Are you sure?"
+    >
+      <.button color="bg-meta-1" rounding="rounded-full">
+        Delete Record and Files
+      </.button>
+    </.link>
+  </nav>
 </div>
 <div class="rounded-sm border border-stroke bg-white px-5 pb-2.5 pt-6 shadow-default dark:border-strokedark dark:bg-boxdark sm:px-7.5 xl:pb-1">
   <div class="max-w-full overflow-x-auto">

lib/pinchflat_web/controllers/searches/search_html/show.html.heex

@@ -16,7 +16,10 @@
             <.highlight_search_terms text={result.matching_search_term} />
           </:col>
           <:col :let={result} label="" class="flex place-content-evenly">
-            <.link navigate={~p"/media/#{result.id}"} class="hover:text-secondary duration-200 ease-in-out mx-0.5">
+            <.link
+              navigate={~p"/sources/#{result.source_id}/media/#{result.id}"}
+              class="hover:text-secondary duration-200 ease-in-out mx-0.5"
+            >
               <.icon name="hero-eye" />
             </.link>
           </:col>

lib/pinchflat_web/router.ex

@@ -20,11 +20,10 @@ defmodule PinchflatWeb.Router do
     get "/", PageController, :home
 
     resources "/media_profiles", MediaProfiles.MediaProfileController
-    resources "/media", Media.MediaItemController, only: [:show]
     resources "/search", Searches.SearchController, only: [:show], singleton: true
 
     resources "/sources", MediaSources.SourceController do
-      resources "/media", Media.MediaItemController, only: [:show]
+      resources "/media", Media.MediaItemController, only: [:show, :delete]
     end
   end
 

test/pinchflat/media_test.exs

@@ -221,6 +221,24 @@ defmodule Pinchflat.MediaTest do
     end
   end
 
+  describe "media_filepaths/1" do
+    test "returns filepaths in a flat list" do
+      filepaths = %{
+        media_filepath: "/video/test.mp4",
+        thumbnail_filepath: "/video/test.jpg",
+        subtitle_filepaths: [["en", "video/test.srt"]]
+      }
+
+      media_item = media_item_fixture(filepaths)
+
+      assert Media.media_filepaths(media_item) == [
+               "/video/test.mp4",
+               "/video/test.jpg",
+               "video/test.srt"
+             ]
+    end
+  end
+
   describe "create_media_item/1" do
     test "creating with valid data creates a media_item" do
       valid_attrs = %{
@@ -282,6 +300,61 @@ defmodule Pinchflat.MediaTest do
     end
   end
 
+  describe "delete_attachments/1" do
+    test "deletes the media item's files" do
+      media_item = media_item_with_attachments()
+
+      assert {:ok, _} = Media.delete_attachments(media_item)
+      refute File.exists?(media_item.media_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)
+    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)
+      refute File.exists?(root_directory)
+    end
+
+    test "does not delete the parent folder if it is not empty" do
+      media_item = media_item_with_attachments()
+      root_directory = Path.dirname(media_item.media_filepath)
+      File.touch(Path.join([root_directory, "test.txt"]))
+
+      assert {:ok, _} = Media.delete_attachments(media_item)
+      assert File.exists?(root_directory)
+
+      :ok = File.rm(Path.join([root_directory, "test.txt"]))
+      :ok = File.rmdir(root_directory)
+    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()