diff --git a/lib/pinchflat/media_source.ex b/lib/pinchflat/media_source.ex index 5082a94..6b98bf7 100644 --- a/lib/pinchflat/media_source.ex +++ b/lib/pinchflat/media_source.ex @@ -28,41 +28,16 @@ defmodule Pinchflat.MediaSource do """ def create_channel(attrs \\ %{}) do %Channel{} - |> Channel.changeset(attrs) + |> change_channel_from_url(attrs) |> Repo.insert() end - @doc """ - Creates a channel from a given Channel URL and additional attrs. - - Returns {:ok, %Channel{}} | {:error, %Ecto.Changeset{}} | {:error, binary()} - - IDEA: maybe instead of creating a channel from a URL, instead the form should - extract details from the URL and automatically update based on that. So the - actual submission would be a normal form object - """ - def create_channel_from_url(channel_url, attrs) do - case ChannelDetails.get_channel_details(channel_url) do - {:ok, %ChannelDetails{} = channel_details} -> - record_attrs = - Map.merge(attrs, %{ - name: channel_details.name, - channel_id: channel_details.id - }) - - create_channel(record_attrs) - - {:error, runner_error, _status_code} -> - {:error, runner_error} - end - end - @doc """ Updates a channel. Returns {:ok, %Channel{}} | {:error, %Ecto.Changeset{}} """ def update_channel(%Channel{} = channel, attrs) do channel - |> Channel.changeset(attrs) + |> change_channel_from_url(attrs) |> Repo.update() end @@ -79,4 +54,46 @@ defmodule Pinchflat.MediaSource do def change_channel(%Channel{} = channel, attrs \\ %{}) do Channel.changeset(channel, attrs) end + + @doc """ + Returns an `%Ecto.Changeset{}` for tracking channel changes and additionally + fetches channel details from the original_url (if provided). If the channel + details cannot be fetched, an error is added to the changeset. + + Note that this fetches channel details as long as the `original_url` is present. + This means that it'll go for it even if a changeset is otherwise invalid. This + is pretty easy to change, but for MVP I'm not concerned. + """ + def change_channel_from_url(%Channel{} = channel, attrs \\ %{}) do + case change_channel(channel, attrs) do + %Ecto.Changeset{changes: %{original_url: _}} = changeset -> + add_channel_details_to_changeset(channel, changeset) + + changeset -> + changeset + end + end + + defp add_channel_details_to_changeset(channel, changeset) do + %Ecto.Changeset{changes: changes} = changeset + + case ChannelDetails.get_channel_details(changes.original_url) do + {:ok, %ChannelDetails{} = channel_details} -> + change_channel( + channel, + Map.merge(changes, %{ + name: channel_details.name, + channel_id: channel_details.id + }) + ) + + {:error, runner_error, _status_code} -> + Ecto.Changeset.add_error( + changeset, + :original_url, + "could not fetch channel details from URL", + error: runner_error + ) + end + end end diff --git a/lib/pinchflat/media_source/channel.ex b/lib/pinchflat/media_source/channel.ex index bbb3b0f..4861f65 100644 --- a/lib/pinchflat/media_source/channel.ex +++ b/lib/pinchflat/media_source/channel.ex @@ -8,9 +8,15 @@ defmodule Pinchflat.MediaSource.Channel do alias Pinchflat.Profiles.MediaProfile + @required_fields ~w(name channel_id original_url media_profile_id)a + @allowed_fields @required_fields + schema "channels" do field :name, :string field :channel_id, :string + # This should only be used for user reference going forward + # as the channel_id should be used for all API calls + field :original_url, :string belongs_to :media_profile, MediaProfile @@ -20,8 +26,8 @@ defmodule Pinchflat.MediaSource.Channel do @doc false def changeset(channel, attrs) do channel - |> cast(attrs, [:name, :channel_id, :media_profile_id]) - |> validate_required([:name, :channel_id, :media_profile_id]) + |> cast(attrs, @allowed_fields) + |> validate_required(@required_fields) |> unique_constraint([:channel_id, :media_profile_id]) end end diff --git a/lib/pinchflat_web/controllers/media_sources/channel_controller.ex b/lib/pinchflat_web/controllers/media_sources/channel_controller.ex index 35bf8f5..08de4f4 100644 --- a/lib/pinchflat_web/controllers/media_sources/channel_controller.ex +++ b/lib/pinchflat_web/controllers/media_sources/channel_controller.ex @@ -7,37 +7,39 @@ defmodule PinchflatWeb.MediaSources.ChannelController do def index(conn, _params) do channels = MediaSource.list_channels() + render(conn, :index, channels: channels) end def new(conn, _params) do - media_profiles = Profiles.list_media_profiles() changeset = MediaSource.change_channel(%Channel{}) - render(conn, :new, changeset: changeset, media_profiles: media_profiles) + render(conn, :new, changeset: changeset, media_profiles: media_profiles()) end def create(conn, %{"channel" => channel_params}) do - case MediaSource.create_channel_from_url(channel_params["channel_url"], channel_params) do + case MediaSource.create_channel(channel_params) do {:ok, channel} -> conn |> put_flash(:info, "Channel created successfully.") |> redirect(to: ~p"/media_sources/channels/#{channel}") {:error, %Ecto.Changeset{} = changeset} -> - render(conn, :new, changeset: changeset) + render(conn, :new, changeset: changeset, media_profiles: media_profiles()) end end def show(conn, %{"id" => id}) do channel = MediaSource.get_channel!(id) + render(conn, :show, channel: channel) end def edit(conn, %{"id" => id}) do channel = MediaSource.get_channel!(id) changeset = MediaSource.change_channel(channel) - render(conn, :edit, channel: channel, changeset: changeset) + + render(conn, :edit, channel: channel, changeset: changeset, media_profiles: media_profiles()) end def update(conn, %{"id" => id, "channel" => channel_params}) do @@ -50,7 +52,11 @@ defmodule PinchflatWeb.MediaSources.ChannelController do |> redirect(to: ~p"/media_sources/channels/#{channel}") {:error, %Ecto.Changeset{} = changeset} -> - render(conn, :edit, channel: channel, changeset: changeset) + render(conn, :edit, + channel: channel, + changeset: changeset, + media_profiles: media_profiles() + ) end end @@ -62,4 +68,8 @@ defmodule PinchflatWeb.MediaSources.ChannelController do |> put_flash(:info, "Channel deleted successfully.") |> redirect(to: ~p"/media_sources/channels") end + + defp media_profiles do + Profiles.list_media_profiles() + end end diff --git a/lib/pinchflat_web/controllers/media_sources/channel_html.ex b/lib/pinchflat_web/controllers/media_sources/channel_html.ex index 8b05cd0..37bc279 100644 --- a/lib/pinchflat_web/controllers/media_sources/channel_html.ex +++ b/lib/pinchflat_web/controllers/media_sources/channel_html.ex @@ -8,6 +8,7 @@ defmodule PinchflatWeb.MediaSources.ChannelHTML do """ attr :changeset, Ecto.Changeset, required: true attr :action, :string, required: true + attr :media_profiles, :list, required: true def channel_form(assigns) end diff --git a/lib/pinchflat_web/controllers/media_sources/channel_html/channel_form.html.heex b/lib/pinchflat_web/controllers/media_sources/channel_html/channel_form.html.heex index 5997de3..00ad253 100644 --- a/lib/pinchflat_web/controllers/media_sources/channel_html/channel_form.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/channel_html/channel_form.html.heex @@ -2,8 +2,16 @@ <.error :if={@changeset.action}> Oops, something went wrong! Please check the errors below. - <.input field={f[:name]} type="text" label="Name" /> - <.input field={f[:channel_id]} type="text" label="Channel" /> + + <.input + field={f[:media_profile_id]} + options={Enum.map(@media_profiles, &{&1.name, &1.id})} + type="select" + label="Media Profile" + /> + + <.input field={f[:original_url]} type="text" label="Channel URL" /> + <:actions> <.button>Save Channel diff --git a/lib/pinchflat_web/controllers/media_sources/channel_html/edit.html.heex b/lib/pinchflat_web/controllers/media_sources/channel_html/edit.html.heex index e0c8928..c65d810 100644 --- a/lib/pinchflat_web/controllers/media_sources/channel_html/edit.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/channel_html/edit.html.heex @@ -3,6 +3,10 @@ <:subtitle>Use this form to manage channel records in your database. -<.channel_form changeset={@changeset} action={~p"/media_sources/channels/#{@channel}"} /> +<.channel_form + changeset={@changeset} + media_profiles={@media_profiles} + action={~p"/media_sources/channels/#{@channel}"} +/> <.back navigate={~p"/media_sources/channels"}>Back to channels diff --git a/lib/pinchflat_web/controllers/media_sources/channel_html/new.html.heex b/lib/pinchflat_web/controllers/media_sources/channel_html/new.html.heex index 51bb517..b09a477 100644 --- a/lib/pinchflat_web/controllers/media_sources/channel_html/new.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/channel_html/new.html.heex @@ -3,20 +3,10 @@ <:subtitle>Use this form to manage channel records in your database. -<.simple_form :let={f} for={@changeset} action={~p"/media_sources/channels"}> - <.error :if={@changeset.action}> - Oops, something went wrong! Please check the errors below. - - <.input - field={f[:media_profile_id]} - options={Enum.map(@media_profiles, &{&1.name, &1.id})} - type="select" - label="Media Profile" - /> - <.input field={f[:channel_url]} type="text" label="Channel URL" /> - <:actions> - <.button>Save Channel - - +<.channel_form + changeset={@changeset} + media_profiles={@media_profiles} + action={~p"/media_sources/channels"} +/> <.back navigate={~p"/media_sources/channels"}>Back to channels diff --git a/lib/pinchflat_web/controllers/media_sources/channel_html/show.html.heex b/lib/pinchflat_web/controllers/media_sources/channel_html/show.html.heex index 1b44381..0f59e3b 100644 --- a/lib/pinchflat_web/controllers/media_sources/channel_html/show.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/channel_html/show.html.heex @@ -9,8 +9,9 @@ <.list> - <:item title="Name"><%= @channel.name %> - <:item title="Channel"><%= @channel.channel_id %> + <:item title="Channel Name"><%= @channel.name %> + <:item title="Channel ID"><%= @channel.channel_id %> + <:item title="Original URL"><%= @channel.original_url %> <.back navigate={~p"/media_sources/channels"}>Back to channels diff --git a/priv/repo/migrations/20240123174417_create_channels.exs b/priv/repo/migrations/20240123174417_create_channels.exs index 30f02cc..4430c3d 100644 --- a/priv/repo/migrations/20240123174417_create_channels.exs +++ b/priv/repo/migrations/20240123174417_create_channels.exs @@ -5,6 +5,7 @@ defmodule Pinchflat.Repo.Migrations.CreateChannels do create table(:channels) do add :name, :string, null: false add :channel_id, :string, null: false + add :original_url, :string, null: false add :media_profile_id, references(:media_profiles, on_delete: :restrict), null: false timestamps(type: :utc_datetime) diff --git a/test/pinchflat/media_source_test.exs b/test/pinchflat/media_source_test.exs index 24c2db9..4fcc8a7 100644 --- a/test/pinchflat/media_source_test.exs +++ b/test/pinchflat/media_source_test.exs @@ -1,5 +1,6 @@ defmodule Pinchflat.MediaSourceTest do use Pinchflat.DataCase + import Mox alias Pinchflat.MediaSource alias Pinchflat.MediaSource.Channel @@ -9,6 +10,8 @@ defmodule Pinchflat.MediaSourceTest do @invalid_channel_attrs %{name: nil, channel_id: nil} + setup :verify_on_exit! + describe "list_channels/0" do test "it returns all channels" do channel = channel_fixture() @@ -24,16 +27,17 @@ defmodule Pinchflat.MediaSourceTest do end describe "create_channel/1" do - test "creates a channel with valid data" do + test "creates a channel and adds name + ID from runner response" do + expect(YtDlpRunnerMock, :run, &runner_function_mock/2) + valid_attrs = %{ - name: "some name", - channel_id: "some channel_id", - media_profile_id: media_profile_fixture().id + media_profile_id: media_profile_fixture().id, + original_url: "https://www.youtube.com/channel/abc123" } assert {:ok, %Channel{} = channel} = MediaSource.create_channel(valid_attrs) assert channel.name == "some name" - assert channel.channel_id == "some channel_id" + assert String.starts_with?(channel.channel_id, "some_channel_id_") end test "creation with invalid data returns error changeset" do @@ -41,10 +45,17 @@ defmodule Pinchflat.MediaSourceTest do end test "creation enforces uniqueness of channel_id scoped to the media_profile" do + expect(YtDlpRunnerMock, :run, 2, fn _url, _opts -> + {:ok, + Phoenix.json_library().encode!(%{ + channel: "some name", + channel_id: "some_channel_id_12345678" + })} + end) + valid_once_attrs = %{ - name: "some name", - channel_id: "abc123", - media_profile_id: media_profile_fixture().id + media_profile_id: media_profile_fixture().id, + original_url: "https://www.youtube.com/channel/abc123" } assert {:ok, %Channel{}} = MediaSource.create_channel(valid_once_attrs) @@ -52,9 +63,17 @@ defmodule Pinchflat.MediaSourceTest do end test "creation lets you duplicate channel_ids as long as the media profile is different" do + expect(YtDlpRunnerMock, :run, 2, fn _url, _opts -> + {:ok, + Phoenix.json_library().encode!(%{ + channel: "some name", + channel_id: "some_channel_id_12345678" + })} + end) + valid_attrs = %{ name: "some name", - channel_id: "abc123" + original_url: "https://www.youtube.com/channel/abc123" } channel_1_attrs = Map.merge(valid_attrs, %{media_profile_id: media_profile_fixture().id}) @@ -65,59 +84,33 @@ defmodule Pinchflat.MediaSourceTest do end end - describe "create_channel_from_url/2" do - import Mox - - setup :verify_on_exit! - - test "it creates a channel with valid data" do - channel_url = "https://www.youtube.com/c/TheUselessTrials" - valid_attrs = %{media_profile_id: media_profile_fixture().id} - - expect(YtDlpRunnerMock, :run, fn ^channel_url, _opts -> - {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"} - end) - - assert {:ok, %Channel{} = channel} = - MediaSource.create_channel_from_url(channel_url, valid_attrs) - - assert channel.name == "TheUselessTrials" - assert channel.channel_id == "UCQH2" - end - - test "it returns an error string if the runner returns an error" do - channel_url = "https://www.youtube.com/c/TheUselessTrials" - valid_attrs = %{media_profile_id: media_profile_fixture().id} - - expect(YtDlpRunnerMock, :run, fn ^channel_url, _opts -> - {:error, "Big issue", 1} - end) - - assert {:error, "Big issue"} = - MediaSource.create_channel_from_url(channel_url, valid_attrs) - end - - test "creation with invalid data returns error changeset" do - channel_url = "https://www.youtube.com/c/TheUselessTrials" - invalid_attrs = %{media_profile_id: nil} - - expect(YtDlpRunnerMock, :run, fn ^channel_url, _opts -> - {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"} - end) - - assert {:error, %Ecto.Changeset{}} = - MediaSource.create_channel_from_url(channel_url, invalid_attrs) - end - end - describe "update_channel/2" do test "updates with valid data updates the channel" do channel = channel_fixture() - update_attrs = %{name: "some updated name", channel_id: "some updated channel_id"} + update_attrs = %{name: "some updated name"} assert {:ok, %Channel{} = channel} = MediaSource.update_channel(channel, update_attrs) assert channel.name == "some updated name" - assert channel.channel_id == "some updated channel_id" + end + + test "updating the original_url will re-fetch the channel details" do + expect(YtDlpRunnerMock, :run, &runner_function_mock/2) + + channel = channel_fixture() + update_attrs = %{original_url: "https://www.youtube.com/channel/abc123"} + + assert {:ok, %Channel{} = channel} = MediaSource.update_channel(channel, update_attrs) + assert channel.name == "some name" + assert String.starts_with?(channel.channel_id, "some_channel_id_") + end + + test "not updating the original_url will not re-fetch the channel details" do + expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/2) + + channel = channel_fixture() + update_attrs = %{name: "some updated name"} + + assert {:ok, %Channel{}} = MediaSource.update_channel(channel, update_attrs) end test "updates with invalid data returns error changeset" do @@ -142,4 +135,86 @@ defmodule Pinchflat.MediaSourceTest do assert %Ecto.Changeset{} = MediaSource.change_channel(channel) end end + + describe "change_channel/2" do + test "it returns a changeset" do + channel = channel_fixture() + + assert %Ecto.Changeset{} = MediaSource.change_channel(channel) + end + end + + describe "change_channel_from_url/2" do + test "it returns a changeset" do + stub(YtDlpRunnerMock, :run, &runner_function_mock/2) + channel = channel_fixture() + + assert %Ecto.Changeset{} = MediaSource.change_channel_from_url(channel) + end + + test "it does not fetch channel details if the original_url isn't in the changeset" do + expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/2) + + changeset = MediaSource.change_channel_from_url(%Channel{}, %{name: "some updated name"}) + + assert %Ecto.Changeset{} = changeset + end + + test "it fetches channel details if the original_url is in the changeset" do + expect(YtDlpRunnerMock, :run, &runner_function_mock/2) + + changeset = + MediaSource.change_channel_from_url(%Channel{}, %{ + original_url: "https://www.youtube.com/channel/abc123" + }) + + assert %Ecto.Changeset{} = changeset + end + + test "it adds channel details to the changeset, keeping the orignal details" do + expect(YtDlpRunnerMock, :run, &runner_function_mock/2) + + media_profile = media_profile_fixture() + media_profile_id = media_profile.id + + changeset = + MediaSource.change_channel_from_url(%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.channel_id, "some_channel_id_") + + assert %{ + name: "some name", + media_profile_id: ^media_profile_id, + original_url: "https://www.youtube.com/channel/abc123" + } = changeset.changes + end + + test "it adds an error to the changeset if the runner fails" do + expect(YtDlpRunnerMock, :run, 1, fn _url, _opts -> + {:error, "some error", 1} + end) + + changeset = + MediaSource.change_channel_from_url(%Channel{}, %{ + original_url: "https://www.youtube.com/channel/abc123" + }) + + assert %Ecto.Changeset{} = changeset + assert errors_on(changeset).original_url == ["could not fetch channel details from URL"] + end + end + + defp runner_function_mock(_url, _opts) do + { + :ok, + Phoenix.json_library().encode!(%{ + channel: "some name", + channel_id: "some_channel_id_#{:rand.uniform(1_000_000)}" + }) + } + end end diff --git a/test/pinchflat_web/controllers/channel_controller_test.exs b/test/pinchflat_web/controllers/channel_controller_test.exs index fad8d93..40215a5 100644 --- a/test/pinchflat_web/controllers/channel_controller_test.exs +++ b/test/pinchflat_web/controllers/channel_controller_test.exs @@ -1,5 +1,6 @@ defmodule PinchflatWeb.ChannelControllerTest do use PinchflatWeb.ConnCase + import Mox import Pinchflat.ProfilesFixtures import Pinchflat.MediaSourceFixtures @@ -11,16 +12,19 @@ defmodule PinchflatWeb.ChannelControllerTest do :ok, %{ create_attrs: %{ - name: "some name", - channel_id: "some channel_id", - media_profile_id: media_profile.id + media_profile_id: media_profile.id, + original_url: "https://www.youtube.com/channel/abc123" }, - update_attrs: %{name: "some updated name", channel_id: "some updated channel_id"}, - invalid_attrs: %{name: nil, channel_id: nil, media_profile_id: nil} + update_attrs: %{ + original_url: "https://www.youtube.com/channel/321xyz" + }, + invalid_attrs: %{original_url: nil, media_profile_id: nil} } } end + setup :verify_on_exit! + describe "index" do test "lists all channels", %{conn: conn} do conn = get(conn, ~p"/media_sources/channels") @@ -37,6 +41,7 @@ defmodule PinchflatWeb.ChannelControllerTest do describe "create channel" do test "redirects to show when data is valid", %{conn: conn, create_attrs: create_attrs} do + expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/2) conn = post(conn, ~p"/media_sources/channels", channel: create_attrs) assert %{id: id} = redirected_params(conn) @@ -65,11 +70,13 @@ defmodule PinchflatWeb.ChannelControllerTest do setup [:create_channel] test "redirects when data is valid", %{conn: conn, channel: channel, update_attrs: update_attrs} do + expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/2) + conn = put(conn, ~p"/media_sources/channels/#{channel}", channel: update_attrs) assert redirected_to(conn) == ~p"/media_sources/channels/#{channel}" conn = get(conn, ~p"/media_sources/channels/#{channel}") - assert html_response(conn, 200) =~ "some updated name" + assert html_response(conn, 200) =~ "https://www.youtube.com/channel/321xyz" end test "renders errors when data is invalid", %{ @@ -99,4 +106,14 @@ defmodule PinchflatWeb.ChannelControllerTest do channel = channel_fixture() %{channel: channel} end + + defp runner_function_mock(_url, _opts) do + { + :ok, + Phoenix.json_library().encode!(%{ + channel: "some name", + channel_id: "some_channel_id_#{:rand.uniform(1_000_000)}" + }) + } + end end diff --git a/test/support/fixtures/media_source_fixtures.ex b/test/support/fixtures/media_source_fixtures.ex index 191bdb0..5be9bd3 100644 --- a/test/support/fixtures/media_source_fixtures.ex +++ b/test/support/fixtures/media_source_fixtures.ex @@ -4,20 +4,25 @@ defmodule Pinchflat.MediaSourceFixtures do entities via the `Pinchflat.MediaSource` context. """ + alias Pinchflat.Repo alias Pinchflat.ProfilesFixtures + alias Pinchflat.MediaSource.Channel @doc """ Generate a channel. """ def channel_fixture(attrs \\ %{}) do {:ok, channel} = - attrs - |> Enum.into(%{ - channel_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}"), case: :lower), - name: "Channel ##{:rand.uniform(1_000_000)}", - media_profile_id: ProfilesFixtures.media_profile_fixture().id - }) - |> Pinchflat.MediaSource.create_channel() + %Channel{} + |> Channel.changeset( + Enum.into(attrs, %{ + name: "Channel ##{:rand.uniform(1_000_000)}", + channel_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")), + original_url: "https://www.youtube.com/channel/#{:rand.uniform(1_000_000)}", + media_profile_id: ProfilesFixtures.media_profile_fixture().id + }) + ) + |> Repo.insert() channel end