LiveView did not work out but here's a working controller how about

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

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])
+    |> cast(attrs, @allowed_fields)
-    |> validate_required([:name, :channel_id, :media_profile_id])
+    |> validate_required(@required_fields)
     |> unique_constraint([:channel_id, :media_profile_id])
   end
 end

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

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

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.
   </.error>
-  <.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</.button>
   </:actions>

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.</:subtitle>
 </.header>
 
-<.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</.back>

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.</:subtitle>
 </.header>
 
-<.simple_form :let={f} for={@changeset} action={~p"/media_sources/channels"}>
+<.channel_form
-  <.error :if={@changeset.action}>
+  changeset={@changeset}
-    Oops, something went wrong! Please check the errors below.
+  media_profiles={@media_profiles}
-  </.error>
+  action={~p"/media_sources/channels"}
-  <.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</.button>
-  </:actions>
-</.simple_form>
 
 <.back navigate={~p"/media_sources/channels"}>Back to channels</.back>

lib/pinchflat_web/controllers/media_sources/channel_html/show.html.heex

@@ -9,8 +9,9 @@
 </.header>
 
 <.list>
-  <:item title="Name"><%= @channel.name %></:item>
+  <:item title="Channel Name"><%= @channel.name %></:item>
-  <:item title="Channel"><%= @channel.channel_id %></:item>
+  <:item title="Channel ID"><%= @channel.channel_id %></:item>
+  <:item title="Original URL"><%= @channel.original_url %></:item>
 </.list>
 
 <.back navigate={~p"/media_sources/channels"}>Back to channels</.back>

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)

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",
+        media_profile_id: media_profile_fixture().id,
-        channel_id: "some channel_id",
+        original_url: "https://www.youtube.com/channel/abc123"
-        media_profile_id: media_profile_fixture().id
       }
 
       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",
+        media_profile_id: media_profile_fixture().id,
-        channel_id: "abc123",
+        original_url: "https://www.youtube.com/channel/abc123"
-        media_profile_id: media_profile_fixture().id
       }
 
       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

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",
+          media_profile_id: media_profile.id,
-          channel_id: "some channel_id",
+          original_url: "https://www.youtube.com/channel/abc123"
-          media_profile_id: media_profile.id
         },
-        update_attrs: %{name: "some updated name", channel_id: "some updated channel_id"},
+        update_attrs: %{
-        invalid_attrs: %{name: nil, channel_id: nil, media_profile_id: nil}
+          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

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
+      %Channel{}
-      |> Enum.into(%{
+      |> Channel.changeset(
-        channel_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}"), case: :lower),
+        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
         })
-      |> Pinchflat.MediaSource.create_channel()
+      )
+      |> Repo.insert()
 
     channel
   end