From 4c48bda7b2e675616135be030b17a81d5314b17f Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 08:03:36 +0100 Subject: [PATCH] Refactor Release component to take release_id --- lib/music_library_web/components/release.ex | 332 +++++++++--------- .../live/collection_live/show.ex | 22 +- .../live/scrobble_live/show.ex | 9 +- priv/gettext/default.pot | 3 +- .../components/release_test.exs | 89 ++++- 5 files changed, 274 insertions(+), 181 deletions(-) diff --git a/lib/music_library_web/components/release.ex b/lib/music_library_web/components/release.ex index 72fd88dd..b1dfa8a7 100644 --- a/lib/music_library_web/components/release.ex +++ b/lib/music_library_web/components/release.ex @@ -20,16 +20,14 @@ defmodule MusicLibraryWeb.Components.Release do |> assign(:release_with_tracks, AsyncResult.loading()) |> assign(:already_scrobbled, false) |> assign(:selected_tracks, MapSet.new()) - |> assign(:timezone, MusicLibrary.default_timezone()) - |> assign(:finished_at, nil) - |> assign(:form, to_form(%{"finished_at" => nil}, as: :release)) |> assign(:pending_form_params, nil)} end @impl true - def update(%{record: record} = assigns, socket) do + def update(%{release_id: release_id} = assigns, socket) do socket = assign(socket, assigns) - current_time = DateTime.utc_now() |> DateTime.shift_zone!(socket.assigns.timezone) + timezone = socket.assigns.timezone + current_time = DateTime.utc_now() |> DateTime.shift_zone!(timezone) {:ok, socket @@ -37,7 +35,7 @@ defmodule MusicLibraryWeb.Components.Release do |> assign(:form, to_form(%{"finished_at" => DateTime.to_naive(current_time)}, as: :release)) |> assign(:release_with_tracks, AsyncResult.loading()) |> start_async(:release_with_tracks, fn -> - load_release_with_tracks(record.selected_release_id) + load_release_with_tracks(release_id) end)} end @@ -116,156 +114,149 @@ defmodule MusicLibraryWeb.Components.Release do def render(assigns) do ~H"""
- <.sheet - :if={@record.selected_release_id} - id={@sheet_id} - placement="right" - class="flex min-w-xs flex-col overflow-hidden p-0 sm:min-w-sm" + <.form + for={@form} + id={"#{@sheet_id}-form"} + phx-target={@myself} + phx-change="validate" + phx-auto-recover="recover_form" + class="flex min-h-0 flex-1 flex-col" > - <.form - for={@form} - id={"#{@sheet_id}-form"} - phx-target={@myself} - phx-change="validate" - phx-auto-recover="recover_form" - class="flex min-h-0 flex-1 flex-col" - > -
- - +
+ + -
-
-

- {@record.title} -

-

- {artist_names} -

-
-
- <.date_time_picker - :if={@can_scrobble?} - field={@form[:finished_at]} - size="sm" - display_format="%b %-d, %H:%M" - time_format="24" - placeholder={gettext("Now")} - class="flex-1 sm:flex-initial" - > - <:inner_prefix class="pl-2 text-zinc-500 dark:text-zinc-400"> - {gettext("Finished at")} - - <:outer_suffix class="pr-2"> - <.button - size="sm" - type="button" - phx-click="reset_to_now" - phx-target={@myself} - > - {gettext("Now")} - - - - <.button - :if={@can_scrobble? && @release_with_tracks.ok?} - type="button" - variant="solid" - size="sm" - disabled={@already_scrobbled} - phx-click="scrobble_release" - phx-target={@myself} - phx-disable-with={gettext("Scrobbling...")} - > - <.icon name="hero-play" class="icon" aria-hidden="true" data-slot="icon" /> - - {gettext("Release")} - - <.dropdown id={"#{@sheet_id}-release-actions"} placement="bottom-end"> - <:toggle> - <.button type="button" variant="outline" size="sm"> - {gettext("More actions")} - <.icon - name="hero-ellipsis-vertical" - class="icon" - aria-hidden="true" - data-slot="icon" - /> - - - <.focus_wrap id={"#{@sheet_id}-release-actions-focus-wrap"}> - <.dropdown_link - :if={@release_with_tracks.ok?} - phx-click="print_tracklist" - phx-target={@myself} - > - <.icon name="hero-printer" class="icon" aria-hidden="true" data-slot="icon" /> - {gettext("Print tracklist")} - - <.dropdown_link :if={!@can_scrobble?} href={LastFm.auth_url()}> - <.icon name="hero-link" class="icon" aria-hidden="true" data-slot="icon" /> - {gettext("Connect Last.fm")} - - - -
+
+
+

+ {header_title(@release_with_tracks)} +

+

+ {artist_names} +

- -
- <.async_result :let={release_with_tracks} assign={@release_with_tracks}> - <:loading> -
- {gettext("Loading release with tracks")} - <.loading /> -
- - <:failed :let={_failure}> -
+
+ <.date_time_picker + :if={@can_scrobble?} + field={@form[:finished_at]} + size="sm" + display_format="%b %-d, %H:%M" + time_format="24" + placeholder={gettext("Now")} + class="flex-1 sm:flex-initial" + > + <:inner_prefix class="pl-2 text-zinc-500 dark:text-zinc-400"> + {gettext("Finished at")} + + <:outer_suffix class="pr-2"> + <.button + size="sm" + type="button" + phx-click="reset_to_now" + phx-target={@myself} + > + {gettext("Now")} + + + + <.button + :if={@can_scrobble? && @release_with_tracks.ok?} + type="button" + variant="solid" + size="sm" + disabled={@already_scrobbled} + phx-click="scrobble_release" + phx-target={@myself} + phx-disable-with={gettext("Scrobbling...")} + > + <.icon name="hero-play" class="icon" aria-hidden="true" data-slot="icon" /> + + {gettext("Release")} + + <.dropdown id={"#{@sheet_id}-release-actions"} placement="bottom-end"> + <:toggle> + <.button type="button" variant="outline" size="sm"> + {gettext("More actions")} <.icon - name="hero-exclamation-triangle" - class="size-5" + name="hero-ellipsis-vertical" + class="icon" aria-hidden="true" data-slot="icon" /> - {gettext("Error loading tracks")} - <.button - type="button" - variant="ghost" - size="xs" - phx-click={JS.push("load_release_tracks", target: @myself)} - class="ml-2 cursor-pointer" - > - {gettext("Retry")} - -
- - <.medium - :for={medium <- release_with_tracks.media} - can_scrobble?={@can_scrobble?} - already_scrobbled={@already_scrobbled} - medium={medium} - release_artists={release_with_tracks.artists} - media_count={MusicBrainz.Release.media_count(release_with_tracks)} - selected_tracks={@selected_tracks} - myself={@myself} - record={@record} - /> - + + + <.focus_wrap id={"#{@sheet_id}-release-actions-focus-wrap"}> + <.dropdown_link + :if={@show_print? && @release_with_tracks.ok?} + phx-click="print_tracklist" + phx-target={@myself} + > + <.icon name="hero-printer" class="icon" aria-hidden="true" data-slot="icon" /> + {gettext("Print tracklist")} + + <.dropdown_link :if={!@can_scrobble?} href={LastFm.auth_url()}> + <.icon name="hero-link" class="icon" aria-hidden="true" data-slot="icon" /> + {gettext("Connect Last.fm")} + + +
- <.selection_bar - :if={@can_scrobble? && @release_with_tracks.ok? && MapSet.size(@selected_tracks) > 0} - release={@release_with_tracks.result} - selected_tracks={@selected_tracks} - already_scrobbled={@already_scrobbled} - myself={@myself} - /> - - +
+ <.async_result :let={release_with_tracks} assign={@release_with_tracks}> + <:loading> +
+ {gettext("Loading release with tracks")} + <.loading /> +
+ + <:failed :let={_failure}> +
+ <.icon + name="hero-exclamation-triangle" + class="size-5" + aria-hidden="true" + data-slot="icon" + /> + {gettext("Error loading tracks")} + <.button + type="button" + variant="ghost" + size="xs" + phx-click={JS.push("load_release_tracks", target: @myself)} + class="ml-2 cursor-pointer" + > + {gettext("Retry")} + +
+ + <.medium + :for={medium <- release_with_tracks.media} + can_scrobble?={@can_scrobble?} + already_scrobbled={@already_scrobbled} + medium={medium} + release_artists={release_with_tracks.artists} + media_count={MusicBrainz.Release.media_count(release_with_tracks)} + selected_tracks={@selected_tracks} + myself={@myself} + show_print?={@show_print?} + /> + +
+
+ + <.selection_bar + :if={@can_scrobble? && @release_with_tracks.ok? && MapSet.size(@selected_tracks) > 0} + release={@release_with_tracks.result} + selected_tracks={@selected_tracks} + already_scrobbled={@already_scrobbled} + myself={@myself} + /> +
""" end @@ -277,7 +268,7 @@ defmodule MusicLibraryWeb.Components.Release do attr :already_scrobbled, :boolean, required: true attr :selected_tracks, :any, required: true attr :myself, :any, required: true - attr :record, :any, default: nil + attr :show_print?, :boolean, required: true def medium(assigns) do ~H""" @@ -317,7 +308,7 @@ defmodule MusicLibraryWeb.Components.Release do {medium_scrobble_label(@medium.format)} <.dropdown - :if={@record} + :if={@show_print?} id={"medium-actions-#{@medium.number}"} placement="bottom-end" > @@ -435,7 +426,7 @@ defmodule MusicLibraryWeb.Components.Release do ) ~H""" -
+

{ngettext("%{count} track selected", "%{count} tracks selected", @count, count: @count)} @@ -618,11 +609,10 @@ defmodule MusicLibraryWeb.Components.Release do def handle_event("print_tracklist", _params, socket) when release_loaded?(socket.assigns) do release = socket.assigns.release_with_tracks.result - record = socket.assigns.record case TracklistPdf.generate(release) do {:ok, pdf_binary} -> - filename = "#{record.title} - Tracklist.pdf" + filename = "#{release.title} - Tracklist.pdf" {:noreply, push_event(socket, "music_library:download", %{ @@ -651,12 +641,11 @@ defmodule MusicLibraryWeb.Components.Release do def handle_event("print_medium_tracklist", %{"medium-number" => number}, socket) when release_loaded?(socket.assigns) do release = socket.assigns.release_with_tracks.result - record = socket.assigns.record {number, ""} = Integer.parse(number) case TracklistPdf.generate_medium(release, number) do {:ok, pdf_binary} -> - filename = "#{record.title} - Disc #{number} - Tracklist.pdf" + filename = "#{release.title} - Disc #{number} - Tracklist.pdf" {:noreply, push_event(socket, "music_library:download", %{ @@ -683,7 +672,7 @@ defmodule MusicLibraryWeb.Components.Release do end def handle_event("load_release_tracks", _params, socket) do - selected_release_id = socket.assigns.record.selected_release_id + selected_release_id = socket.assigns.release_id {:noreply, socket @@ -753,15 +742,6 @@ defmodule MusicLibraryWeb.Components.Release do end) end - @spec scrobble_button_label(MapSet.t()) :: String.t() - def scrobble_button_label(selected_tracks) do - if MapSet.size(selected_tracks) > 0 do - gettext("Scrobble selected tracks") - else - gettext("Scrobble release") - end - end - defp medium_duration(medium) do medium |> MusicBrainz.Release.medium_duration() @@ -776,16 +756,28 @@ defmodule MusicLibraryWeb.Components.Release do end end - defp header_subtitle(%{artists: artists}) when is_list(artists) and artists != [] do - artists - |> Enum.map_join(", ", & &1.name) - |> case do - "" -> nil - names -> names + @spec header_title(AsyncResult.t() | term()) :: String.t() + defp header_title(%AsyncResult{ok?: true, result: release}), do: release.title + defp header_title(_), do: "" + + @spec header_subtitle(AsyncResult.t() | term()) :: String.t() | nil + defp header_subtitle(%AsyncResult{ok?: true, result: release}) do + case release.artists do + [] -> + nil + + artists -> + artists + |> Enum.map_join(fn artist -> artist.name <> (artist.joinphrase || "") end) + |> String.trim() + |> case do + "" -> nil + names -> names + end end end - defp header_subtitle(_record), do: nil + defp header_subtitle(_), do: nil @spec selected_tracks_summary(MusicBrainz.Release.t(), MapSet.t()) :: {non_neg_integer(), non_neg_integer(), non_neg_integer()} diff --git a/lib/music_library_web/live/collection_live/show.ex b/lib/music_library_web/live/collection_live/show.ex index 708fb115..ee0133cd 100644 --- a/lib/music_library_web/live/collection_live/show.ex +++ b/lib/music_library_web/live/collection_live/show.ex @@ -292,13 +292,21 @@ defmodule MusicLibraryWeb.CollectionLive.Show do <.record_debug_sheet record={@record} embedding_text={@embedding_text} /> - <.live_component - id="release-with-tracks" - sheet_id="release-with-tracks-sheet" - module={MusicLibraryWeb.Components.Release} - record={@record} - timezone={@timezone} - /> + <.sheet + :if={@record.selected_release_id} + id="release-with-tracks-sheet" + placement="right" + class="flex min-w-xs flex-col overflow-hidden p-0 sm:min-w-sm" + > + <.live_component + id="release-with-tracks" + sheet_id="release-with-tracks-sheet" + module={MusicLibraryWeb.Components.Release} + release_id={@record.selected_release_id} + show_print?={true} + timezone={@timezone} + /> + <.live_component id="record-notes" diff --git a/lib/music_library_web/live/scrobble_live/show.ex b/lib/music_library_web/live/scrobble_live/show.ex index 4261c342..cd5a945f 100644 --- a/lib/music_library_web/live/scrobble_live/show.ex +++ b/lib/music_library_web/live/scrobble_live/show.ex @@ -1,7 +1,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.Show do use MusicLibraryWeb, :live_view - import(MusicLibraryWeb.Components.Release, only: [medium: 1, scrobble_button_label: 1]) + import(MusicLibraryWeb.Components.Release, only: [medium: 1]) import MusicLibraryWeb.RecordComponents, only: [country_label: 1] alias MusicLibrary.ScrobbleActivity @@ -103,7 +103,11 @@ defmodule MusicLibraryWeb.ScrobbleLive.Show do else: "scrobble_release" } > - {scrobble_button_label(@selected_tracks)} + + {if MapSet.size(@selected_tracks) > 0, + do: gettext("Scrobble selected tracks"), + else: gettext("Scrobble release")} + <.icon name="hero-play" class="icon" aria-hidden="true" data-slot="icon" />

@@ -117,6 +121,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.Show do selected_tracks={@selected_tracks} media_count={MusicBrainz.Release.media_count(@release)} myself={nil} + show_print?={false} />
diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index b63d7796..2df0b4e8 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -666,11 +666,12 @@ msgstr "" #: lib/music_library_web/components/release.ex #: lib/music_library_web/live/collection_live/show.ex +#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Scrobble release" msgstr "" -#: lib/music_library_web/components/release.ex +#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Scrobble selected tracks" msgstr "" diff --git a/test/music_library_web/components/release_test.exs b/test/music_library_web/components/release_test.exs index 184b9011..e89f7e35 100644 --- a/test/music_library_web/components/release_test.exs +++ b/test/music_library_web/components/release_test.exs @@ -12,7 +12,7 @@ defmodule MusicLibraryWeb.Components.ReleaseTest do import MusicLibrary.Fixtures.Records import Phoenix.LiveViewTest, - only: [element: 2, render_async: 1, render_change: 2, render_click: 1] + only: [element: 2, render: 1, render_async: 1, render_change: 2, render_click: 1] alias MusicBrainz.Fixtures.Release, as: ReleaseFixtures alias MusicLibrary.Secrets @@ -288,3 +288,90 @@ defmodule MusicLibraryWeb.Components.ReleaseTest do end end end + +defmodule ReleaseComponentHost do + @moduledoc false + use MusicLibraryWeb, :live_view + + @impl true + def mount(_params, session, socket) do + {:ok, + assign(socket, + release_id: session["release_id"], + show_print?: session["show_print?"], + timezone: "UTC" + )} + end + + @impl true + def render(assigns) do + ~H""" +
+ <.live_component + id="host-release" + module={MusicLibraryWeb.Components.Release} + release_id={@release_id} + show_print?={@show_print?} + sheet_id="host-sheet" + timezone={@timezone} + /> +
+ """ + end +end + +defmodule MusicLibraryWeb.Components.ReleaseTest.ShowPrintTest do + use MusicLibraryWeb.ConnCase, async: false + + import Phoenix.LiveViewTest, + only: [render: 1, render_async: 1] + + alias MusicBrainz.Fixtures.Release, as: ReleaseFixtures + alias Req.Test + + defp stub_musicbrainz_release(_) do + Test.stub(MusicBrainz.API, fn conn -> + case conn.request_path do + "/ws/2/release/" <> _id -> + Test.json(conn, ReleaseFixtures.release_with_media(:marbles)) + + _ -> + Test.json(conn, %{}) + end + end) + + :ok + end + + describe "show_print? assign" do + setup [:stub_musicbrainz_release] + + test "true renders Print tracklist dropdown entries", %{conn: conn} do + {:ok, view, _html} = + Phoenix.LiveViewTest.live_isolated(conn, ReleaseComponentHost, + session: %{ + "release_id" => ReleaseFixtures.release_id(:marbles), + "show_print?" => true + } + ) + + render_async(view) + + assert render(view) =~ "Print tracklist" + end + + test "false hides Print tracklist dropdown entries", %{conn: conn} do + {:ok, view, _html} = + Phoenix.LiveViewTest.live_isolated(conn, ReleaseComponentHost, + session: %{ + "release_id" => ReleaseFixtures.release_id(:marbles), + "show_print?" => false + } + ) + + render_async(view) + + refute render(view) =~ "Print tracklist" + end + end +end