From 404d6feb997944f1756f89ed17d655b614bb2d93 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Sun, 10 Mar 2024 20:31:28 -0700 Subject: [PATCH] Updates media item creation to update on conflict --- lib/pinchflat/media.ex | 19 +++++++++++++++---- .../components/core_components.ex | 11 ++++++++--- test/pinchflat/media_test.exs | 19 +++++++++++++++++++ .../tasks/media_items_tasks_test.exs | 5 +++-- test/pinchflat/tasks/source_tasks_test.exs | 8 +++++--- 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/lib/pinchflat/media.ex b/lib/pinchflat/media.ex index a3127f6..f3821ea 100644 --- a/lib/pinchflat/media.ex +++ b/lib/pinchflat/media.ex @@ -184,14 +184,25 @@ defmodule Pinchflat.Media do @doc """ Creates a media item from the attributes returned by the video backend - (read: yt-dlp) + (read: yt-dlp). + + Unlike `create_media_item`, this will attempt an update if the media_item + already exists. This is so that future indexing can pick up attributes that + we may not have asked for in the past (eg: upload_date) Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}} """ def create_media_item_from_backend_attrs(source, media_attrs_struct) do - %{source_id: source.id} - |> Map.merge(Map.from_struct(media_attrs_struct)) - |> create_media_item() + attrs = Map.merge(%{source_id: source.id}, Map.from_struct(media_attrs_struct)) + + %MediaItem{} + |> MediaItem.changeset(attrs) + |> Repo.insert( + on_conflict: [ + set: Map.to_list(attrs) + ], + conflict_target: [:source_id, :media_id] + ) end @doc """ diff --git a/lib/pinchflat_web/components/core_components.ex b/lib/pinchflat_web/components/core_components.ex index 3b81cdb..314dbf9 100644 --- a/lib/pinchflat_web/components/core_components.ex +++ b/lib/pinchflat_web/components/core_components.ex @@ -598,9 +598,14 @@ defmodule PinchflatWeb.CoreComponents do def list_items_from_map(assigns) do attrs = Enum.filter(assigns.map, fn - {_, %{__struct__: _}} -> false - {_, [%{__meta__: _} | _]} -> false - _ -> true + {_, %{__struct__: s}} when s not in [Date, DateTime] -> + false + + {_, [%{__meta__: _} | _]} -> + false + + _ -> + true end) assigns = assign(assigns, iterable_attributes: attrs) diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index 8979108..ffddebf 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -378,6 +378,7 @@ defmodule Pinchflat.MediaTest do } assert {:ok, %MediaItem{} = media_item} = Media.create_media_item(valid_attrs) + assert media_item.title == valid_attrs.title assert media_item.media_id == valid_attrs.media_id assert media_item.media_filepath == valid_attrs.media_filepath @@ -398,12 +399,30 @@ defmodule Pinchflat.MediaTest do |> YtDlpMedia.response_to_struct() assert {:ok, %MediaItem{} = media_item} = Media.create_media_item_from_backend_attrs(source, media_attrs) + assert media_item.source_id == source.id assert media_item.title == media_attrs.title assert media_item.media_id == media_attrs.media_id assert media_item.original_url == media_attrs.original_url assert media_item.description == media_attrs.description end + + test "updates the media item if it already exists" do + source = source_fixture() + + media_attrs = + media_attributes_return_fixture() + |> Phoenix.json_library().decode!() + |> YtDlpMedia.response_to_struct() + + different_attrs = %YtDlpMedia{media_attrs | title: "Different title"} + + assert {:ok, %MediaItem{} = media_item_1} = Media.create_media_item_from_backend_attrs(source, media_attrs) + assert {:ok, %MediaItem{} = media_item_2} = Media.create_media_item_from_backend_attrs(source, different_attrs) + + assert media_item_1.id == media_item_2.id + assert media_item_2.title == different_attrs.title + end end describe "update_media_item/2" do diff --git a/test/pinchflat/tasks/media_items_tasks_test.exs b/test/pinchflat/tasks/media_items_tasks_test.exs index 36bc91f..80fcbbd 100644 --- a/test/pinchflat/tasks/media_items_tasks_test.exs +++ b/test/pinchflat/tasks/media_items_tasks_test.exs @@ -49,10 +49,11 @@ defmodule Pinchflat.Tasks.MediaItemTasksTest do end test "won't duplicate media_items based on media_id and source", %{source: source} do - assert {:ok, _} = MediaItemTasks.index_and_enqueue_download_for_media_item(source, @media_url) - assert {:error, _} = MediaItemTasks.index_and_enqueue_download_for_media_item(source, @media_url) + assert {:ok, mi_1} = MediaItemTasks.index_and_enqueue_download_for_media_item(source, @media_url) + assert {:ok, mi_2} = MediaItemTasks.index_and_enqueue_download_for_media_item(source, @media_url) assert Repo.aggregate(MediaItem, :count) == 1 + assert mi_1.id == mi_2.id end test "enqueues a download job", %{source: source} do diff --git a/test/pinchflat/tasks/source_tasks_test.exs b/test/pinchflat/tasks/source_tasks_test.exs index b3a8274..94bc32a 100644 --- a/test/pinchflat/tasks/source_tasks_test.exs +++ b/test/pinchflat/tasks/source_tasks_test.exs @@ -168,12 +168,14 @@ defmodule Pinchflat.Tasks.SourceTasksTest do Enum.map(media_items_other_source, & &1.media_id) end - test "it returns a list of media_items or changesets", %{source: source} do + test "it returns a list of media_items", %{source: source} do first_run = SourceTasks.index_and_enqueue_download_for_media_items(source) duplicate_run = SourceTasks.index_and_enqueue_download_for_media_items(source) - assert Enum.all?(first_run, fn %MediaItem{} -> true end) - assert Enum.all?(duplicate_run, fn %Ecto.Changeset{} -> true end) + first_ids = Enum.map(first_run, & &1.id) + duplicate_ids = Enum.map(duplicate_run, & &1.id) + + assert first_ids == duplicate_ids end test "it updates the source's last_indexed_at field", %{source: source} do