diff --git a/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex b/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex index 8724d8d..89cb1af 100644 --- a/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex +++ b/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex @@ -40,7 +40,6 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollection do end end - # TODO: test defp format_source_details(response) do %{ channel_id: response["channel_id"], diff --git a/lib/pinchflat/media_source.ex b/lib/pinchflat/media_source.ex index 76117e4..12109d6 100644 --- a/lib/pinchflat/media_source.ex +++ b/lib/pinchflat/media_source.ex @@ -116,13 +116,7 @@ defmodule Pinchflat.MediaSource do case SourceDetails.get_source_details(changes.original_url) do {:ok, source_details} -> - change_source( - source, - Map.merge(changes, %{ - name: source_details.name, - collection_id: source_details.id - }) - ) + add_source_details_by_collection_type(source, changeset, source_details) {:error, runner_error, _status_code} -> Ecto.Changeset.add_error( @@ -134,15 +128,35 @@ defmodule Pinchflat.MediaSource do end end + defp add_source_details_by_collection_type(source, changeset, source_details) do + %Ecto.Changeset{changes: changes} = changeset + collection_type = source.collection_type || changes[: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 + } + + _ -> + %{} + end + + change_source(source, Map.merge(changes, collection_changes)) + end + defp commit_and_start_indexing(changeset) do case Repo.insert_or_update(changeset) do - {:ok, %Source{} = source} -> - maybe_run_indexing_task(changeset, source) - - {:ok, source} - - err -> - err + {:ok, %Source{} = source} -> maybe_run_indexing_task(changeset, source) + err -> err end end @@ -159,5 +173,7 @@ defmodule Pinchflat.MediaSource do SourceTasks.kickoff_indexing_task(source) end end + + {:ok, source} end end diff --git a/lib/pinchflat/media_source/source.ex b/lib/pinchflat/media_source/source.ex index b633bf8..7682d6c 100644 --- a/lib/pinchflat/media_source/source.ex +++ b/lib/pinchflat/media_source/source.ex @@ -9,11 +9,12 @@ defmodule Pinchflat.MediaSource.Source do alias Pinchflat.Media.MediaItem alias Pinchflat.Profiles.MediaProfile - @allowed_fields ~w(name collection_id collection_type index_frequency_minutes original_url media_profile_id)a - @required_fields @allowed_fields -- ~w(index_frequency_minutes)a + @allowed_fields ~w(collection_name collection_id collection_type index_frequency_minutes original_url media_profile_id)a + @required_fields @allowed_fields -- ~w(index_frequency_minutes friendly_name)a schema "sources" do - field :name, :string + field :friendly_name, :string + field :collection_name, :string field :collection_id, :string field :collection_type, Ecto.Enum, values: [:channel, :playlist] field :index_frequency_minutes, :integer diff --git a/lib/pinchflat_web/controllers/media_sources/source_html/index.html.heex b/lib/pinchflat_web/controllers/media_sources/source_html/index.html.heex index 348873d..e5cede9 100644 --- a/lib/pinchflat_web/controllers/media_sources/source_html/index.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/source_html/index.html.heex @@ -8,8 +8,8 @@ <.table id="sources" rows={@sources} row_click={&JS.navigate(~p"/media_sources/sources/#{&1}")}> - <:col :let={source} label="Name"><%= source.name %> - <:col :let={source} label="Source"><%= source.collection_id %> + <:col :let={source} label="Collection Name"><%= source.collection_name %> + <:col :let={source} label="Collection ID"><%= source.collection_id %> <:action :let={source}>
<.link navigate={~p"/media_sources/sources/#{source}"}>Show diff --git a/lib/pinchflat_web/controllers/media_sources/source_html/show.html.heex b/lib/pinchflat_web/controllers/media_sources/source_html/show.html.heex index 5ae2176..6cdf095 100644 --- a/lib/pinchflat_web/controllers/media_sources/source_html/show.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/source_html/show.html.heex @@ -9,8 +9,8 @@ <.list> - <:item title="Source Name"><%= @source.name %> - <:item title="Source ID"><%= @source.collection_id %> + <:item title="Collection Name"><%= @source.collection_name %> + <:item title="Collection ID"><%= @source.collection_id %> <:item title="Original URL"><%= @source.original_url %> diff --git a/priv/repo/migrations/20240202190351_rename_source_name_to_collection_name.exs b/priv/repo/migrations/20240202190351_rename_source_name_to_collection_name.exs new file mode 100644 index 0000000..3639cc3 --- /dev/null +++ b/priv/repo/migrations/20240202190351_rename_source_name_to_collection_name.exs @@ -0,0 +1,11 @@ +defmodule Pinchflat.Repo.Migrations.RenameSourceNameToCollectionName do + use Ecto.Migration + + def change do + rename table(:sources), :name, to: :collection_name + + alter table(:sources) do + add :friendly_name, :string + end + end +end diff --git a/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs b/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs index 3c8bfe7..709050a 100644 --- a/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs +++ b/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs @@ -2,7 +2,6 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do use ExUnit.Case, async: true import Mox - alias Pinchflat.MediaClient.SourceDetails alias Pinchflat.MediaClient.Backends.YtDlp.VideoCollection @channel_url "https://www.youtube.com/c/TheUselessTrials" @@ -45,19 +44,30 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do end describe "get_source_details/1" do - test "it returns a %SourceDetails{} with data on success" do + test "it returns a map with data on success" do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> - {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"} + Phoenix.json_library().encode(%{ + channel: "TheUselessTrials", + channel_id: "UCQH2", + playlist_id: "PLQH2", + playlist_title: "TheUselessTrials - Videos" + }) end) assert {:ok, res} = VideoCollection.get_source_details(@channel_url) - assert %SourceDetails{id: "UCQH2", name: "TheUselessTrials"} = res + + assert %{ + channel_id: "UCQH2", + channel_name: "TheUselessTrials", + playlist_id: "PLQH2", + playlist_name: "TheUselessTrials - Videos" + } = res end test "it passes the expected args to the backend runner" do expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot -> - assert opts == [:skip_download, playlist_end: 1] - assert ot == "%(.{channel,channel_id})j" + assert opts == [:simulate, :skip_download, playlist_end: 1] + assert ot == "%(.{channel,channel_id,playlist_id,playlist_title})j" {:ok, "{}"} end) diff --git a/test/pinchflat/media_client/source_details_test.exs b/test/pinchflat/media_client/source_details_test.exs index 244e65d..5d79cc9 100644 --- a/test/pinchflat/media_client/source_details_test.exs +++ b/test/pinchflat/media_client/source_details_test.exs @@ -8,20 +8,13 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do setup :verify_on_exit! - describe "new/2" do - test "it returns a struct with the given values" do - assert %SourceDetails{id: "UCQH2", name: "TheUselessTrials"} = - SourceDetails.new("UCQH2", "TheUselessTrials") - end - end - describe "get_source_details/2" do test "it passes the expected arguments to the backend" do expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot -> - assert opts == [:skip_download, playlist_end: 1] - assert ot == "%(.{channel,channel_id})j" + assert opts == [:simulate, :skip_download, playlist_end: 1] + assert ot == "%(.{channel,channel_id,playlist_id,playlist_title})j" - {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"} + {:ok, "{}"} end) assert {:ok, _} = SourceDetails.get_source_details(@channel_url) @@ -29,11 +22,22 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do test "it returns a struct composed of the returned data" do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> - {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"} + Phoenix.json_library().encode(%{ + channel: "TheUselessTrials", + channel_id: "UCQH2", + playlist_id: "PLQH2", + playlist_title: "TheUselessTrials - Videos" + }) end) assert {:ok, res} = SourceDetails.get_source_details(@channel_url) - assert %SourceDetails{id: "UCQH2", name: "TheUselessTrials"} = res + + assert %{ + channel_id: "UCQH2", + channel_name: "TheUselessTrials", + playlist_id: "PLQH2", + playlist_name: "TheUselessTrials - Videos" + } = res end end diff --git a/test/pinchflat/media_source_test.exs b/test/pinchflat/media_source_test.exs index e113df5..2ba30fb 100644 --- a/test/pinchflat/media_source_test.exs +++ b/test/pinchflat/media_source_test.exs @@ -29,7 +29,7 @@ defmodule Pinchflat.MediaSourceTest do end describe "create_source/1" do - test "creates a source and adds name + ID from runner response" do + test "creates a source and adds name + ID from runner response for channels" do expect(YtDlpRunnerMock, :run, &runner_function_mock/3) valid_attrs = %{ @@ -39,20 +39,34 @@ defmodule Pinchflat.MediaSourceTest do } assert {:ok, %Source{} = source} = MediaSource.create_source(valid_attrs) - assert source.name == "some name" - assert String.starts_with?(source.collection_id, "some_source_id_") + assert source.collection_name == "some channel name" + assert String.starts_with?(source.collection_id, "some_channel_id_") + end + + test "creates a source and adds name + ID for playlists" do + expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + + valid_attrs = %{ + media_profile_id: media_profile_fixture().id, + original_url: "https://www.youtube.com/playlist?list=abc123", + collection_type: "playlist" + } + + assert {:ok, %Source{} = source} = MediaSource.create_source(valid_attrs) + assert source.collection_name == "some playlist name" + assert String.starts_with?(source.collection_id, "some_playlist_id_") end test "creation with invalid data returns error changeset" do assert {:error, %Ecto.Changeset{}} = MediaSource.create_source(@invalid_source_attrs) end - test "creation enforces uniqueness of source_id scoped to the media_profile" do + test "creation enforces uniqueness of collection_id scoped to the media_profile" do expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot -> {:ok, Phoenix.json_library().encode!(%{ - channel: "some name", - channel_id: "some_source_id_12345678" + channel: "some channel name", + channel_id: "some_channel_id_12345678" })} end) @@ -70,8 +84,8 @@ defmodule Pinchflat.MediaSourceTest do expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot -> {:ok, Phoenix.json_library().encode!(%{ - channel: "some name", - channel_id: "some_source_id_12345678" + channel: "some channel name", + channel_id: "some_channel_id_12345678" })} end) @@ -159,21 +173,32 @@ defmodule Pinchflat.MediaSourceTest do describe "update_source/2" do test "updates with valid data updates the source" do source = source_fixture() - update_attrs = %{name: "some updated name"} + update_attrs = %{collection_name: "some updated name"} assert {:ok, %Source{} = source} = MediaSource.update_source(source, update_attrs) - assert source.name == "some updated name" + assert source.collection_name == "some updated name" end - test "updating the original_url will re-fetch the source details" do + test "updating the original_url will re-fetch the source details for channels" do expect(YtDlpRunnerMock, :run, &runner_function_mock/3) source = source_fixture() update_attrs = %{original_url: "https://www.youtube.com/channel/abc123"} assert {:ok, %Source{} = source} = MediaSource.update_source(source, update_attrs) - assert source.name == "some name" - assert String.starts_with?(source.collection_id, "some_source_id_") + assert source.collection_name == "some channel name" + assert String.starts_with?(source.collection_id, "some_channel_id_") + end + + test "updating the original_url will re-fetch the source details for playlists" do + expect(YtDlpRunnerMock, :run, &runner_function_mock/3) + + source = source_fixture(collection_type: "playlist") + update_attrs = %{original_url: "https://www.youtube.com/playlist?list=abc123"} + + assert {:ok, %Source{} = source} = MediaSource.update_source(source, update_attrs) + assert source.collection_name == "some playlist name" + assert String.starts_with?(source.collection_id, "some_playlist_id_") end test "not updating the original_url will not re-fetch the source details" do @@ -275,16 +300,16 @@ defmodule Pinchflat.MediaSourceTest do media_profile_id = media_profile.id changeset = - MediaSource.change_source_from_url(%Source{}, %{ + MediaSource.change_source_from_url(%Source{collection_type: :channel}, %{ original_url: "https://www.youtube.com/channel/abc123", media_profile_id: media_profile.id }) assert %Ecto.Changeset{} = changeset - assert String.starts_with?(changeset.changes.collection_id, "some_source_id_") + assert String.starts_with?(changeset.changes.collection_id, "some_channel_id_") assert %{ - name: "some name", + collection_name: "some channel name", media_profile_id: ^media_profile_id, original_url: "https://www.youtube.com/channel/abc123" } = changeset.changes @@ -309,8 +334,10 @@ defmodule Pinchflat.MediaSourceTest do { :ok, Phoenix.json_library().encode!(%{ - channel: "some name", - channel_id: "some_source_id_#{:rand.uniform(1_000_000)}" + channel: "some channel name", + channel_id: "some_channel_id_#{:rand.uniform(1_000_000)}", + playlist_id: "some_playlist_id_#{:rand.uniform(1_000_000)}", + playlist_title: "some playlist name" }) } end diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index f973e8d..39cb9d8 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/test/pinchflat_web/controllers/source_controller_test.exs @@ -104,16 +104,17 @@ defmodule PinchflatWeb.SourceControllerTest do end defp create_source(_) do - source = source_fixture() - %{source: source} + %{source: source_fixture()} end defp runner_function_mock(_url, _opts, _ot) do { :ok, Phoenix.json_library().encode!(%{ - channel: "some name", - channel_id: "some_source_id_#{:rand.uniform(1_000_000)}" + channel: "some channel name", + channel_id: "some_channel_id_#{:rand.uniform(1_000_000)}", + playlist_id: "some_playlist_id_#{:rand.uniform(1_000_000)}", + playlist_title: "some playlist name" }) } end diff --git a/test/support/fixtures/media_source_fixtures.ex b/test/support/fixtures/media_source_fixtures.ex index 178ccc9..89500ba 100644 --- a/test/support/fixtures/media_source_fixtures.ex +++ b/test/support/fixtures/media_source_fixtures.ex @@ -16,9 +16,10 @@ defmodule Pinchflat.MediaSourceFixtures do %Source{} |> Source.changeset( Enum.into(attrs, %{ - name: "Source ##{:rand.uniform(1_000_000)}", + collection_name: "Source ##{:rand.uniform(1_000_000)}", collection_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")), collection_type: "channel", + friendly_name: "Cool and good internal name!", original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}", media_profile_id: ProfilesFixtures.media_profile_fixture().id, index_frequency_minutes: 60