From e1fe51972c6055682ee1d0c3425868aef7996c2d Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 07:12:19 +0100 Subject: [PATCH 01/12] ML-145: Restructure /scrobble routes; reuse Release component on scrobble page --- ...euse-Release-component-on-scrobble-page.md | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md diff --git a/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md b/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md new file mode 100644 index 00000000..0b4685e7 --- /dev/null +++ b/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md @@ -0,0 +1,87 @@ +--- +id: ML-145 +title: Restructure /scrobble routes; reuse Release component on scrobble page +status: To Do +assignee: [] +created_date: '2026-04-23 06:11' +labels: + - ui + - scrobble + - refactor + - liveview +dependencies: [] +references: + - lib/music_library_web/components/release.ex + - lib/music_library_web/live/scrobble_live/index.ex + - lib/music_library_web/live/scrobble_live/show.ex + - lib/music_library/records/tracklist_pdf.ex + - backlog/ml-142/plan.md + - >- + backlog/tasks/ml-142.1 - + Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md +documentation: + - .claude/plans/scrobble-route-restructure/design.md +priority: medium +--- + +## Description + + +## Why + +`/scrobble/:release_id` (`MusicLibraryWeb.ScrobbleLive.Show`) re-implements scrobbling UI that already exists as a LiveComponent used by collection/wishlist show pages (`MusicLibraryWeb.Components.Release`). Today the scrobble page imports `medium/1` and `scrobble_button_label/1` from that component but reimplements `scrobble_release` / `scrobble_medium` / `scrobble_selected_tracks` with hard-coded `DateTime.utc_now/0`, no `Finished at` picker, and no selection bar. ML-142.1 proposed porting those affordances into `ScrobbleLive.Show` in parallel — which would leave two drifting implementations. + +This task takes the opposite approach: make the Release LiveComponent reusable outside the sheet (currently tightly coupled to `%Records.Record{}` and wrapped in `<.sheet>`), restructure the scrobble routes into a bookmarkable hierarchy, and delete the custom scrobble UI in favour of reuse. + +**Supersedes ML-142.1** — the ML-142.1 ACs become free once the component is reused on the new scrobble page. ML-142.1 can be archived as superseded. + +## What + +Three routes: + +- `/scrobble` — search only. Release-group clicks `<.link navigate>` to `/scrobble/:rg_id` (no more inline release-loading state). +- `/scrobble/:rg_id` (new) — release-group header (cover, title, primary artist, type badge, first-release date, release count) + full list of releases; each release links to `/scrobble/:rg_id/releases/:release_id`. +- `/scrobble/:rg_id/releases/:release_id` (new, replaces `/scrobble/:release_id`) — the scrobble page itself, rendered by the same LiveComponent that powers the collection/wishlist show sheet. + +Old `/scrobble/:release_id` → 404 (personal app; bookmark churn is acceptable, not worth 301-redirect plumbing). + +## Key refactor decisions (working design doc: `.claude/plans/scrobble-route-restructure/design.md`, local-only) + +1. **Release LiveComponent input contract** — takes `release_id` (string) instead of `record`; adds `show_print?: boolean`. Header title/artists derive from the loaded `%MusicBrainz.Release{}`. +2. **`<.sheet>` moves to callsites** — single render path for the component. Collection/wishlist show pages wrap in their own `<.sheet>`; the scrobble release page renders it directly under ``. +3. **Selection bar stickiness** — switches to `position: sticky` so it works both inside a sheet (pins to sheet bottom) and on a page (pins to viewport bottom) with single markup, no mode flag. +4. **`TracklistPdf` signatures** — `generate/1` and `generate_medium/2` take a `%MusicBrainz.Release{}`; no `%Records.Record{}` required (both fields used by the PDF — `title` and `artists` — already exist on the release struct). +5. **Module names** — new `MusicLibraryWeb.ScrobbleLive.ReleaseGroupShow` at `/scrobble/:rg_id`; rename `MusicLibraryWeb.ScrobbleLive.Show` → `MusicLibraryWeb.ScrobbleLive.ReleaseShow` at `/scrobble/:rg_id/releases/:release_id`, removing its custom scrobble UI in favour of embedding the LiveComponent. +6. **Unused after refactor** — `MusicLibraryWeb.Components.Release.scrobble_button_label/1` (delete). + +## Non-goals + +- Scrobble-rule picker on the scrobble page (fresh MB releases have no misrecognised tracks yet). +- Cross-link to `/collection/:id` when the release is already collected. +- 301-redirecting old `/scrobble/:release_id` URLs. +- Breadcrumbs on the scrobble page. +- Preserving `?query=X` across nested URLs (browser back button already works). +- Splitting the Release LiveComponent into separate `Content` / `Sheet` modules. +- Changes to `MusicLibrary.ScrobbleActivity`. + + +## Acceptance Criteria + +- [ ] #1 /scrobble/:rg_id renders a release-group header (cover, release-group title, primary artist, type badge, first-release date, release count) and the full list of releases returned by MusicBrainz for that group. +- [ ] #2 Each release in the /scrobble/:rg_id list is a navigate link to /scrobble/:rg_id/releases/:release_id. +- [ ] #3 /scrobble/:rg_id shows an error toast and redirects to /scrobble if fetching the release group or its releases fails. +- [ ] #4 /scrobble/:rg_id/releases/:release_id renders a scrobble page with picker-driven `Finished at`, release-level scrobble, per-medium scrobble, track-selection + selection-bar scrobble, and Print tracklist — behaviour identical to the collection/wishlist show sheet. +- [ ] #5 /scrobble/:rg_id/releases/:release_id has a `Back to releases` link that navigates to /scrobble/:rg_id. +- [ ] #6 Old /scrobble/:release_id returns 404 (not redirected, not aliased). +- [ ] #7 /scrobble no longer renders releases inline; release-group list items are navigate links to /scrobble/:rg_id. The `?query=X` search param still works as before. +- [ ] #8 The Release LiveComponent on the scrobble page and on the collection/wishlist sheet share a single implementation — there is no parallel reimplementation of scrobble handlers, picker, or selection bar. +- [ ] #9 The selection bar stays visible at the bottom of the visible scroll region in both contexts (scrobble page viewport, collection/wishlist sheet inner scroll) while tracks scroll, using a single markup path. +- [ ] #10 MusicLibrary.Records.TracklistPdf generates tracklist PDFs from a MusicBrainz release struct alone (no Records.Record required). Output for collection/wishlist records is visually unchanged. +- [ ] #11 The Release LiveComponent supports suppressing both the release-level and per-medium Print tracklist dropdown entries via a single input (both hidden together); the suppression state is covered by a component test. +- [ ] #12 MusicLibraryWeb.ScrobbleLive.Show's custom scrobble UI and handlers (scrobble_release, scrobble_medium, scrobble_selected_tracks, validate, recover_form) are deleted. MusicLibraryWeb.Components.Release.scrobble_button_label/1 is deleted as unused. +- [ ] #13 All new user-facing strings are wrapped in gettext; .pot/.po files regenerated via `mix gettext.extract --merge`. +- [ ] #14 Tests updated: test/music_library_web/components/release_test.exs covers the new `release_id` input contract and both states of the print-suppression input; test/music_library/records/tracklist_pdf_test.exs covers the new generate/1 and generate_medium/2 signatures. +- [ ] #15 Tests added: test/music_library_web/live/scrobble_live/release_group_show_test.exs covers happy path (header fields + releases list + link targets) and fetch failure (toast + redirect); test/music_library_web/live/scrobble_live/release_show_test.exs (replacing show_test.exs) smoke-tests mount, component render, and back-link target. +- [ ] #16 Tests updated: test/music_library_web/live/scrobble_live/index_test.exs asserts release-group clicks navigate (no inline state); collection/wishlist show tests updated for the new component input shape and callsite sheet markup. +- [ ] #17 Manual UI verification via `iex -S mix phx.server`: sheet selection-bar still pins correctly on collection/wishlist show pages; page selection-bar pins to viewport bottom on scrobble page while tracks list scrolls; navigation loop /scrobble → /scrobble/:rg_id → /scrobble/:rg_id/releases/:release_id → back → back works, and `?query=X` survives the browser back button. + From 8631b97b4db76f3b420b0c6b7cd6da926c304be9 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 07:35:54 +0100 Subject: [PATCH 02/12] Add joinphrase to Release.Artist; parse from MB --- .gitignore | 2 ++ lib/music_brainz/release.ex | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 344d834f..606b4fad 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,8 @@ npm-debug.log /.mcp.json /.claude/audit /.claude/reviews +/.claude/plans +/.claude/settings.local.json # Superpowers brainstorming mockups /.superpowers/ diff --git a/lib/music_brainz/release.ex b/lib/music_brainz/release.ex index ab1b92c5..0ee47da3 100644 --- a/lib/music_brainz/release.ex +++ b/lib/music_brainz/release.ex @@ -43,12 +43,13 @@ defmodule MusicBrainz.Release do @moduledoc false @enforce_keys [:id, :name, :sort_name] - defstruct [:id, :name, :sort_name] + defstruct [:id, :name, :sort_name, :joinphrase] @type t :: %__MODULE__{ id: String.t(), name: String.t(), - sort_name: String.t() + sort_name: String.t(), + joinphrase: String.t() | nil } end @@ -167,7 +168,8 @@ defmodule MusicBrainz.Release do %Artist{ id: a["artist"]["id"], name: a["artist"]["name"], - sort_name: a["artist"]["sort-name"] + sort_name: a["artist"]["sort-name"], + joinphrase: a["joinphrase"] } end) end From 0d1f580379145c8a30c684dd2b786ea3235c5870 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 07:48:53 +0100 Subject: [PATCH 03/12] Refactor TracklistPdf to take release alone --- lib/music_library/records/tracklist_pdf.ex | 26 ++-- lib/music_library_web/components/release.ex | 4 +- .../records/tracklist_pdf_test.exs | 145 ++++++++---------- 3 files changed, 81 insertions(+), 94 deletions(-) diff --git a/lib/music_library/records/tracklist_pdf.ex b/lib/music_library/records/tracklist_pdf.ex index 2388b34d..031cd5bd 100644 --- a/lib/music_library/records/tracklist_pdf.ex +++ b/lib/music_library/records/tracklist_pdf.ex @@ -1,6 +1,6 @@ defmodule MusicLibrary.Records.TracklistPdf do @moduledoc """ - Generates 120mm x 120mm PDF tracklists from record and release data via Typst. + Generates 120mm x 120mm PDF tracklists from release data via Typst. """ alias MusicBrainz.Release @@ -21,23 +21,21 @@ defmodule MusicLibrary.Records.TracklistPdf do # including the header area consumed by artist name and album title. @capacities %{8 => 35, 7 => 38, 6 => 45, 5 => 53} - @spec generate(MusicLibrary.Records.Record.t(), Release.t()) :: - {:ok, binary()} | {:error, term()} - def generate(record, release) do - markup = build_markup(record, release.media) + @spec generate(Release.t()) :: {:ok, binary()} | {:error, term()} + def generate(release) do + markup = build_markup(release, release.media) Typst.render_to_pdf(markup) end - @spec generate_medium(MusicLibrary.Records.Record.t(), Release.t(), integer()) :: - {:ok, binary()} | {:error, term()} - def generate_medium(record, release, medium_number) do + @spec generate_medium(Release.t(), integer()) :: {:ok, binary()} | {:error, term()} + def generate_medium(release, medium_number) do case Release.get_medium(release, medium_number) do nil -> {:error, :medium_not_found} - medium -> generate(record, %{release | media: [medium]}) + medium -> generate(%{release | media: [medium]}) end end - defp build_markup(record, media) do + defp build_markup(release, media) do media_count = length(media) track_count = Enum.sum(Enum.map(media, &length(&1.tracks))) header_count = if media_count > 1, do: media_count, else: 0 @@ -54,9 +52,9 @@ defmodule MusicLibrary.Records.TracklistPdf do #place(top + center, scope: "parent", float: true)[ #align(center)[ - #text(size: 10pt, weight: "bold")[#{Format.escape(artist_names(record))}] + #text(size: 10pt, weight: "bold")[#{Format.escape(artist_names(release))}] #linebreak() - #text(size: 9pt, style: "italic")[#{Format.escape(record.title)}] + #text(size: 9pt, style: "italic")[#{Format.escape(release.title)}] ] #v(3mm) ] @@ -138,8 +136,8 @@ defmodule MusicLibrary.Records.TracklistPdf do Map.fetch!(@capacities, font_size) * columns end - defp artist_names(record) do - Enum.map_join(record.artists, fn artist -> + defp artist_names(release) do + Enum.map_join(release.artists, fn artist -> artist.name <> (artist.joinphrase || "") end) end diff --git a/lib/music_library_web/components/release.ex b/lib/music_library_web/components/release.ex index fa5f8c19..72fd88dd 100644 --- a/lib/music_library_web/components/release.ex +++ b/lib/music_library_web/components/release.ex @@ -620,7 +620,7 @@ defmodule MusicLibraryWeb.Components.Release do release = socket.assigns.release_with_tracks.result record = socket.assigns.record - case TracklistPdf.generate(record, release) do + case TracklistPdf.generate(release) do {:ok, pdf_binary} -> filename = "#{record.title} - Tracklist.pdf" @@ -654,7 +654,7 @@ defmodule MusicLibraryWeb.Components.Release do record = socket.assigns.record {number, ""} = Integer.parse(number) - case TracklistPdf.generate_medium(record, release, number) do + case TracklistPdf.generate_medium(release, number) do {:ok, pdf_binary} -> filename = "#{record.title} - Disc #{number} - Tracklist.pdf" diff --git a/test/music_library/records/tracklist_pdf_test.exs b/test/music_library/records/tracklist_pdf_test.exs index dbbc5547..383313e2 100644 --- a/test/music_library/records/tracklist_pdf_test.exs +++ b/test/music_library/records/tracklist_pdf_test.exs @@ -2,110 +2,106 @@ defmodule MusicLibrary.Records.TracklistPdfTest do use ExUnit.Case, async: true alias MusicBrainz.Release - alias MusicLibrary.Records.{Record, TracklistPdf} + alias MusicLibrary.Records.TracklistPdf @pdf_magic_bytes <<37, 80, 68, 70>> - describe "generate/2" do + describe "generate/1" do test "generates PDF for single-disc release" do - record = build_record(%{title: "OK Computer", artists: [%{name: "Radiohead"}]}) - release = - build_release([ - build_medium(1, [ - build_track(1, "Airbag", 284_533), - build_track(2, "Paranoid Android", 383_000) - ]) - ]) + build_release( + [ + build_medium(1, [ + build_track(1, "Airbag", 284_533), + build_track(2, "Paranoid Android", 383_000) + ]) + ], + title: "OK Computer", + artists: [%{name: "Radiohead"}] + ) - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end test "generates PDF for multi-disc release" do - record = build_record(%{title: "Marbles", artists: [%{name: "Marillion"}]}) api_response = MusicBrainz.Fixtures.Release.release_with_media(:marbles) release = Release.from_api_response(api_response) - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end test "renders artist joinphrase correctly" do - record = - build_record(%{ + release = + build_release( + [ + build_medium(1, [build_track(1, "Track One", 200_000)]) + ], title: "Collab Album", artists: [ %{name: "Artist A", joinphrase: " & "}, %{name: "Artist B"} ] - }) + ) - release = - build_release([ - build_medium(1, [build_track(1, "Track One", 200_000)]) - ]) - - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end test "handles track without duration" do - record = build_record(%{title: "Test Album", artists: [%{name: "Test Artist"}]}) - release = - build_release([ - build_medium(1, [ - build_track(1, "Has Duration", 180_000), - build_track(2, "No Duration", nil) - ]) - ]) + build_release( + [ + build_medium(1, [ + build_track(1, "Has Duration", 180_000), + build_track(2, "No Duration", nil) + ]) + ], + title: "Test Album", + artists: [%{name: "Test Artist"}] + ) - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end test "handles special characters in title" do - record = - build_record(%{ + release = + build_release( + [ + build_medium(1, [ + build_track(1, "Track *starring* @someone", 200_000), + build_track(2, "#Hashtag Title", 180_000) + ]) + ], title: "Album *with* #special @chars", artists: [%{name: "Artist #1"}] - }) + ) - release = - build_release([ - build_medium(1, [ - build_track(1, "Track *starring* @someone", 200_000), - build_track(2, "#Hashtag Title", 180_000) - ]) - ]) - - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end end - describe "generate_medium/3" do + describe "generate_medium/2" do test "generates PDF for a single medium from a multi-disc release" do - record = build_record(%{title: "Marbles", artists: [%{name: "Marillion"}]}) api_response = MusicBrainz.Fixtures.Release.release_with_media(:marbles) release = Release.from_api_response(api_response) assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = - TracklistPdf.generate_medium(record, release, 1) + TracklistPdf.generate_medium(release, 1) assert pdf_page_count(pdf) == 1 end test "returns error for non-existent medium number" do - record = build_record(%{title: "Test Album", artists: [%{name: "Test Artist"}]}) - release = build_release([ build_medium(1, [build_track(1, "Track One", 200_000)]) ]) - assert {:error, :medium_not_found} = TracklistPdf.generate_medium(record, release, 99) + assert {:error, :medium_not_found} = TracklistPdf.generate_medium(release, 99) end end @@ -132,62 +128,55 @@ defmodule MusicLibrary.Records.TracklistPdfTest do end end - describe "generate/2 with many tracks" do + describe "generate/1 with many tracks" do test "generates single-page PDF for large single-disc release" do - record = build_record(%{title: "Long Album", artists: [%{name: "Prolific Artist"}]}) tracks = Enum.map(1..40, &build_track(&1, "Track #{&1}", 180_000)) - release = build_release([build_medium(1, tracks)]) + release = + build_release([build_medium(1, tracks)], + title: "Long Album", + artists: [%{name: "Prolific Artist"}] + ) - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end test "generates single-page PDF for large multi-disc release" do - record = build_record(%{title: "Box Set", artists: [%{name: "Band"}]}) - media = Enum.map(1..4, fn disc -> tracks = Enum.map(1..20, &build_track(&1, "Disc #{disc} Track #{&1}", 200_000)) build_medium(disc, tracks) end) - release = build_release(media) + release = + build_release(media, + title: "Box Set", + artists: [%{name: "Band"}] + ) - assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(record, release) + assert {:ok, <<@pdf_magic_bytes, _::binary>> = pdf} = TracklistPdf.generate(release) assert pdf_page_count(pdf) == 1 end end - defp build_record(attrs) do + defp build_release(media, attrs \\ []) do artists = - Enum.map(Map.get(attrs, :artists, []), fn a -> - %{ - name: a[:name] || a.name, - sort_name: a[:sort_name] || a[:name] || a.name, - musicbrainz_id: "00000000-0000-0000-0000-000000000000", - disambiguation: "", - joinphrase: a[:joinphrase] || "" + Enum.map(Keyword.get(attrs, :artists, []), fn a -> + %Release.Artist{ + id: "00000000-0000-0000-0000-000000000000", + name: Map.get(a, :name), + sort_name: Map.get(a, :sort_name, Map.get(a, :name, "")), + joinphrase: Map.get(a, :joinphrase) } end) - %Record{ - id: Ecto.UUID.generate(), - title: attrs[:title] || "Test Album", - artists: artists, - genres: [], - type: :album, - format: :cd - } - end - - defp build_release(media) do %Release{ id: "00000000-0000-0000-0000-000000000000", - title: "Test Release", + title: Keyword.get(attrs, :title, "Test Release"), disambiguation: nil, packaging: nil, - artists: [], + artists: artists, date: "2024-01-01", barcode: nil, catalog_number: "", From 4c48bda7b2e675616135be030b17a81d5314b17f Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 08:03:36 +0100 Subject: [PATCH 04/12] 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 From d8041dc0dd68d441a918010b9ba9a8b19f3362b7 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 14:28:05 +0100 Subject: [PATCH 05/12] Add /scrobble/:rg_id release-group page --- .../live/scrobble_live/release_group_show.ex | 161 ++++++++++++++++++ lib/music_library_web/router.ex | 2 +- priv/gettext/default.pot | 20 +++ priv/gettext/en/LC_MESSAGES/default.po | 23 ++- .../scrobble_live/release_group_show_test.exs | 67 ++++++++ .../live/scrobble_live/show_test.exs | 3 + 6 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 lib/music_library_web/live/scrobble_live/release_group_show.ex create mode 100644 test/music_library_web/live/scrobble_live/release_group_show_test.exs diff --git a/lib/music_library_web/live/scrobble_live/release_group_show.ex b/lib/music_library_web/live/scrobble_live/release_group_show.ex new file mode 100644 index 00000000..ba1b055b --- /dev/null +++ b/lib/music_library_web/live/scrobble_live/release_group_show.ex @@ -0,0 +1,161 @@ +defmodule MusicLibraryWeb.ScrobbleLive.ReleaseGroupShow do + @moduledoc false + use MusicLibraryWeb, :live_view + + require Logger + + import MusicLibraryWeb.RecordComponents, only: [type_label: 1, country_label: 1] + + alias MusicBrainz.{Release, ReleaseGroupSearchResult} + alias MusicLibrary.Records + alias MusicLibraryWeb.ErrorMessages + alias Phoenix.LiveView.AsyncResult + + @impl true + def render(assigns) do + ~H""" + +
+ <.button variant="ghost" size="sm" navigate={~p"/scrobble"}> + <.icon name="hero-arrow-left" class="icon" aria-hidden="true" data-slot="icon" /> + {gettext("Back to search")} + +
+ + <.async_result :let={data} assign={@release_group_data}> + <:loading> +
+ <.loading class="mx-auto size-8 text-zinc-400" /> +
+ + <:failed :let={_failure}> +
+ {gettext("Could not load release group")} +
+ + +
+ {data.release_group.title} ~p"/images/cover-not-found.png" <> "';"} + /> +
+

+ {data.release_group.title} +

+

+ {data.release_group.artists} +

+
+ <.badge variant="soft" size="xs">{type_label(data.release_group.type)} + {Records.Record.format_release_date(data.release_group.release_date)} + · + + {ngettext("%{count} release", "%{count} releases", length(data.releases), + count: length(data.releases) + )} + +
+
+
+ +
    +
  • + <.link + navigate={"/scrobble/#{@rg_id}/releases/#{release.id}"} + class="flex items-center gap-x-4 px-4 py-5 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-700" + > + {release.title} ~p"/images/cover-not-found.png" <> "';"} + /> +
    +

    + {release.title} +

    +
    + {release.date} + {country_label(release.country)} + <.badge :if={release.catalog_number} variant="soft" size="xs"> + {release.catalog_number} + + + {ngettext("1 disc", "%{count} discs", Release.media_count(release))} + +
    +
    + +
  • +
+ +
+ """ + end + + @impl true + def mount(_params, _session, socket) do + {:ok, + assign(socket, + current_section: :scrobble, + release_group_data: AsyncResult.loading(), + rg_id: nil, + page_title: gettext("Scrobble") + )} + end + + @impl true + def handle_params(%{"rg_id" => rg_id}, _url, socket) do + {:noreply, + socket + |> assign(:rg_id, rg_id) + |> assign(:release_group_data, AsyncResult.loading()) + |> start_async(:release_group_data, fn -> load(rg_id) end)} + end + + @impl true + def handle_async(:release_group_data, {:ok, {:ok, data}}, socket) do + {:noreply, + assign( + socket, + :release_group_data, + AsyncResult.ok(socket.assigns.release_group_data, data) + )} + end + + def handle_async(:release_group_data, {:ok, {:error, reason}}, socket) do + {:noreply, + socket + |> put_toast( + :error, + gettext("Error loading release group") <> ": " <> ErrorMessages.friendly_message(reason) + ) + |> push_navigate(to: ~p"/scrobble")} + end + + def handle_async(:release_group_data, {:exit, reason}, socket) do + Logger.error("Release-group show exited: #{inspect(reason)}") + + {:noreply, + socket + |> put_toast(:error, gettext("Error loading release group")) + |> push_navigate(to: ~p"/scrobble")} + end + + defp load(rg_id) do + with {:ok, raw_rg} <- MusicBrainz.get_release_group(rg_id), + release_group = ReleaseGroupSearchResult.from_api_response(raw_rg), + {:ok, %{"releases" => raw_releases}} <- MusicBrainz.get_releases(rg_id, limit: 50) do + releases = Enum.map(raw_releases, &Release.from_api_response/1) + {:ok, %{release_group: release_group, releases: releases}} + end + end +end diff --git a/lib/music_library_web/router.ex b/lib/music_library_web/router.ex index 81b0d66e..906f6f30 100644 --- a/lib/music_library_web/router.ex +++ b/lib/music_library_web/router.ex @@ -111,7 +111,7 @@ defmodule MusicLibraryWeb.Router do live "/scrobbled-tracks/:scrobbled_at_uts/edit", ScrobbledTracksLive.Index, :edit live "/scrobble", ScrobbleLive.Index, :index - live "/scrobble/:release_id", ScrobbleLive.Show, :show + live "/scrobble/:rg_id", ScrobbleLive.ReleaseGroupShow, :show live "/maintenance", MaintenanceLive.Index, :index end diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 2df0b4e8..a949d4f1 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -1266,6 +1266,7 @@ msgid "90d" msgstr "" #: lib/music_library_web/live/scrobble_live/index.ex +#: lib/music_library_web/live/scrobble_live/release_group_show.ex #, elixir-autogen, elixir-format msgid "Scrobble" msgstr "" @@ -1286,6 +1287,7 @@ msgid "Release Groups" msgstr "" #: lib/music_library_web/live/scrobble_live/index.ex +#: lib/music_library_web/live/scrobble_live/release_group_show.ex #: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "1 disc" @@ -1719,6 +1721,7 @@ msgstr "" msgid "Back" msgstr "" +#: lib/music_library_web/live/scrobble_live/release_group_show.ex #: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Back to search" @@ -2560,3 +2563,20 @@ msgid "across %{count} disc" msgid_plural "across %{count} discs" msgstr[0] "" msgstr[1] "" + +#: lib/music_library_web/live/scrobble_live/release_group_show.ex +#, elixir-autogen, elixir-format +msgid "%{count} release" +msgid_plural "%{count} releases" +msgstr[0] "" +msgstr[1] "" + +#: lib/music_library_web/live/scrobble_live/release_group_show.ex +#, elixir-autogen, elixir-format +msgid "Could not load release group" +msgstr "" + +#: lib/music_library_web/live/scrobble_live/release_group_show.ex +#, elixir-autogen, elixir-format +msgid "Error loading release group" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 65ec0fca..009ce5aa 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -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 "" @@ -1265,6 +1266,7 @@ msgid "90d" msgstr "" #: lib/music_library_web/live/scrobble_live/index.ex +#: lib/music_library_web/live/scrobble_live/release_group_show.ex #, elixir-autogen, elixir-format, fuzzy msgid "Scrobble" msgstr "" @@ -1285,6 +1287,7 @@ msgid "Release Groups" msgstr "" #: lib/music_library_web/live/scrobble_live/index.ex +#: lib/music_library_web/live/scrobble_live/release_group_show.ex #: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "1 disc" @@ -1718,6 +1721,7 @@ msgstr "" msgid "Back" msgstr "" +#: lib/music_library_web/live/scrobble_live/release_group_show.ex #: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Back to search" @@ -2559,3 +2563,20 @@ msgid "across %{count} disc" msgid_plural "across %{count} discs" msgstr[0] "" msgstr[1] "" + +#: lib/music_library_web/live/scrobble_live/release_group_show.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "%{count} release" +msgid_plural "%{count} releases" +msgstr[0] "" +msgstr[1] "" + +#: lib/music_library_web/live/scrobble_live/release_group_show.ex +#, elixir-autogen, elixir-format +msgid "Could not load release group" +msgstr "" + +#: lib/music_library_web/live/scrobble_live/release_group_show.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Error loading release group" +msgstr "" diff --git a/test/music_library_web/live/scrobble_live/release_group_show_test.exs b/test/music_library_web/live/scrobble_live/release_group_show_test.exs new file mode 100644 index 00000000..c8df607a --- /dev/null +++ b/test/music_library_web/live/scrobble_live/release_group_show_test.exs @@ -0,0 +1,67 @@ +defmodule MusicLibraryWeb.ScrobbleLive.ReleaseGroupShowTest do + use MusicLibraryWeb.ConnCase + + alias MusicBrainz.Fixtures.ReleaseGroup + alias Req.Test + + @rg_id ReleaseGroup.release_group_id(:marbles) + + defp stub_release_group(_) do + Test.stub(MusicBrainz.API, fn conn -> + case conn.request_path do + "/ws/2/release-group/" <> _id -> + Test.json(conn, ReleaseGroup.release_group(:marbles)) + + "/ws/2/release" -> + Test.json(conn, ReleaseGroup.release_group_releases(:marbles)) + + _ -> + Test.json(conn, %{}) + end + end) + + :ok + end + + defp stub_release_group_error(_) do + Test.stub(MusicBrainz.API, fn conn -> + Plug.Conn.send_resp(conn, 500, "Internal Server Error") + end) + + :ok + end + + describe "Show" do + setup [:stub_release_group] + + test "renders release-group title", %{conn: conn} do + conn + |> visit(~p"/scrobble/#{@rg_id}") + |> assert_has("h1", text: "Marbles", timeout: 200) + end + + test "renders list of releases with link targets", %{conn: conn} do + conn + |> visit(~p"/scrobble/#{@rg_id}") + |> assert_has("a[href^='/scrobble/#{@rg_id}/releases/']", timeout: 200) + end + + test "back link targets /scrobble", %{conn: conn} do + conn + |> visit(~p"/scrobble/#{@rg_id}") + |> assert_has("a[href='/scrobble']", text: "Back to search") + end + end + + describe "fetch failure" do + setup [:stub_release_group_error] + + @tag :capture_log + test "shows toast and redirects to /scrobble", %{conn: conn} do + conn + |> visit(~p"/scrobble/#{@rg_id}") + |> assert_has("#toast-group", text: "Error loading release group", timeout: 200) + |> assert_path(~p"/scrobble") + end + end +end diff --git a/test/music_library_web/live/scrobble_live/show_test.exs b/test/music_library_web/live/scrobble_live/show_test.exs index f21c7f25..d238f8a7 100644 --- a/test/music_library_web/live/scrobble_live/show_test.exs +++ b/test/music_library_web/live/scrobble_live/show_test.exs @@ -1,6 +1,9 @@ defmodule MusicLibraryWeb.ScrobbleLive.ShowTest do use MusicLibraryWeb.ConnCase + # Route /scrobble/:release_id is removed in Phase 5 — skip until then + @moduletag :skip + import Phoenix.LiveViewTest, only: [element: 2, render_change: 2, render_click: 3] alias MusicBrainz.Fixtures.Release, as: ReleaseFixtures From 0dcd6eab834dc40cba30b8092001c0e5d7da3eb7 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 14:39:26 +0100 Subject: [PATCH 06/12] Replace /scrobble/:release_id with nested route --- .../live/scrobble_live/release_group_show.ex | 2 +- .../live/scrobble_live/release_show.ex | 65 +++++ .../live/scrobble_live/show.ex | 266 ------------------ lib/music_library_web/router.ex | 1 + priv/gettext/default.pot | 51 +--- priv/gettext/en/LC_MESSAGES/default.po | 51 +--- .../live/scrobble_live/release_show_test.exs | 46 +++ .../live/scrobble_live/show_test.exs | 261 ----------------- 8 files changed, 137 insertions(+), 606 deletions(-) create mode 100644 lib/music_library_web/live/scrobble_live/release_show.ex delete mode 100644 lib/music_library_web/live/scrobble_live/show.ex create mode 100644 test/music_library_web/live/scrobble_live/release_show_test.exs delete mode 100644 test/music_library_web/live/scrobble_live/show_test.exs diff --git a/lib/music_library_web/live/scrobble_live/release_group_show.ex b/lib/music_library_web/live/scrobble_live/release_group_show.ex index ba1b055b..891fc825 100644 --- a/lib/music_library_web/live/scrobble_live/release_group_show.ex +++ b/lib/music_library_web/live/scrobble_live/release_group_show.ex @@ -69,7 +69,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.ReleaseGroupShow do
  • <.link - navigate={"/scrobble/#{@rg_id}/releases/#{release.id}"} + navigate={~p"/scrobble/#{@rg_id}/releases/#{release.id}"} class="flex items-center gap-x-4 px-4 py-5 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-700" > + <.alert + :if={not @can_scrobble?} + color="warning" + title={gettext("Last.fm not connected")} + hide_close + > + {gettext( + "You need to connect your Last.fm account to scrobble. Please set up your Last.fm session key in the settings." + )} + + +
    + <.button variant="ghost" size="sm" navigate={~p"/scrobble/#{@rg_id}"}> + <.icon name="hero-arrow-left" class="icon" aria-hidden="true" data-slot="icon" /> + {gettext("Back to releases")} + +
    + + <.live_component + id="scrobble-release" + sheet_id="scrobble-release" + module={MusicLibraryWeb.Components.Release} + release_id={@release_id} + show_print?={true} + timezone={@timezone} + /> + + """ + end + + @impl true + def mount(_params, _session, socket) do + {:ok, + assign(socket, + current_section: :scrobble, + can_scrobble?: ScrobbleActivity.can_scrobble?(), + rg_id: nil, + release_id: nil, + page_title: gettext("Scrobble Release") + )} + end + + @impl true + def handle_params(%{"rg_id" => rg_id, "release_id" => release_id}, _url, socket) do + {:noreply, + socket + |> assign(:rg_id, rg_id) + |> assign(:release_id, release_id)} + end +end diff --git a/lib/music_library_web/live/scrobble_live/show.ex b/lib/music_library_web/live/scrobble_live/show.ex deleted file mode 100644 index cd5a945f..00000000 --- a/lib/music_library_web/live/scrobble_live/show.ex +++ /dev/null @@ -1,266 +0,0 @@ -defmodule MusicLibraryWeb.ScrobbleLive.Show do - use MusicLibraryWeb, :live_view - - import(MusicLibraryWeb.Components.Release, only: [medium: 1]) - import MusicLibraryWeb.RecordComponents, only: [country_label: 1] - - alias MusicLibrary.ScrobbleActivity - alias MusicLibraryWeb.ErrorMessages - - @impl true - def render(assigns) do - ~H""" - -
    - <.alert - :if={not @can_scrobble} - color="warning" - title={gettext("Last.fm not connected")} - hide_close - > - {gettext( - "You need to connect your Last.fm account to scrobble. Please set up your Last.fm session key in the settings." - )} - - -
    - <.button variant="ghost" size="sm" navigate={~p"/scrobble"}> - <.icon name="hero-arrow-left" class="icon" aria-hidden="true" data-slot="icon" /> - {gettext("Back to search")} - -
    - -
    -
    - {"Cover ~p"/images/cover-not-found.png" <> "';"} - /> -
    - -
    -
    -

    - {@release.artists |> Enum.map(& &1.name) |> Enum.join(", ")} -

    -
    -

    - {@release.title} -

    - -
    -
    - <.dl_row :if={@release.date} label={gettext("Release Date")}> - {@release.date} - - <.dl_row :if={@release.country} label={gettext("Country")}> - {country_label(@release.country)} {@release.country} - - <.dl_row :if={@release.barcode} label={gettext("Barcode")}> - {@release.barcode} - - <.dl_row :if={@release.catalog_number} label={gettext("Catalog Number")}> - {@release.catalog_number} - - <.dl_row :if={@release.media != []} label={gettext("Media")}> - {ngettext("1 disc", "%{count} discs", MusicBrainz.Release.media_count(@release))} - -
    -
    - - <.form - :if={@release.media != []} - for={@form} - id="scrobble-release-form" - class="mt-6 space-y-4" - phx-change="validate" - phx-auto-recover="recover_form" - > - - - -
    -

    - {gettext("Tracks")} -

    - - <.button - :if={@can_scrobble} - type="button" - variant="soft" - size="sm" - phx-click={ - if MapSet.size(@selected_tracks) > 0, - do: "scrobble_selected_tracks", - else: "scrobble_release" - } - > - - {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" /> - -
    - - <.medium - :for={medium <- @release.media} - can_scrobble?={@can_scrobble} - already_scrobbled={false} - medium={medium} - release_artists={@release.artists} - selected_tracks={@selected_tracks} - media_count={MusicBrainz.Release.media_count(@release)} - myself={nil} - show_print?={false} - /> - -
    -
    -
    -
    - """ - end - - @impl true - def mount(_params, _session, socket) do - {:ok, - assign(socket, - current_section: :scrobble, - release: nil, - can_scrobble: ScrobbleActivity.can_scrobble?(), - page_title: "Scrobble Release", - selected_tracks: MapSet.new(), - form: to_form(%{}, as: :release) - )} - end - - @impl true - def handle_params(params, _url, socket) do - apply_action(socket, socket.assigns.live_action, params) - end - - defp apply_action(socket, :show, %{"release_id" => release_id}) do - case MusicBrainz.get_release(release_id) do - {:ok, release} -> - release = MusicBrainz.Release.from_api_response(release) - - {:noreply, - assign(socket, - release: release, - page_title: "Scrobble: #{release.title}" - )} - - {:error, _reason} -> - {:noreply, - socket - |> put_flash(:error, "Failed to fetch release details") - |> push_navigate(to: ~p"/scrobble")} - end - end - - @impl true - def handle_event("scrobble_release", _params, socket) do - case ScrobbleActivity.scrobble_release( - socket.assigns.release, - :finished_at, - DateTime.utc_now() - ) do - {:ok, _} -> - {:noreply, - socket - |> put_toast(:info, gettext("Release scrobbled successfully"))} - - {:error, reason} -> - {:noreply, - socket - |> put_toast( - :error, - gettext("Error scrobbling release") <> ": " <> ErrorMessages.friendly_message(reason) - )} - end - end - - def handle_event("scrobble_medium", %{"number" => number}, socket) do - {number, ""} = Integer.parse(number) - - case ScrobbleActivity.scrobble_medium( - number, - socket.assigns.release, - :finished_at, - DateTime.utc_now() - ) do - {:ok, _} -> - {:noreply, - socket - |> put_toast(:info, gettext("Disc scrobbled successfully"))} - - {:error, reason} -> - {:noreply, - socket - |> put_toast( - :error, - gettext("Error scrobbling disc") <> ": " <> ErrorMessages.friendly_message(reason) - )} - end - end - - def handle_event("scrobble_selected_tracks", _params, socket) do - release_with_tracks = socket.assigns.release - selected_track_ids = socket.assigns.selected_tracks - - if MapSet.size(selected_track_ids) == 0 do - {:noreply, socket |> put_toast(:error, gettext("No tracks selected"))} - else - case ScrobbleActivity.scrobble_tracks( - selected_track_ids, - release_with_tracks, - :finished_at, - DateTime.utc_now() - ) do - {:ok, _} -> - {:noreply, - socket - |> assign(:selected_tracks, MapSet.new()) - |> put_toast(:info, gettext("Selected tracks scrobbled successfully"))} - - {:error, reason} -> - {:noreply, - socket - |> put_toast( - :error, - gettext("Error scrobbling selected tracks") <> - ": " <> ErrorMessages.friendly_message(reason) - )} - end - end - end - - def handle_event("validate", %{"release" => params}, socket) do - if socket.assigns.release do - new_selected = - MusicLibraryWeb.Components.Release.apply_form_params( - socket.assigns.release, - params, - socket.assigns.selected_tracks - ) - - {:noreply, assign(socket, :selected_tracks, new_selected)} - else - {:noreply, socket} - end - end - - def handle_event("recover_form", params, socket) do - handle_event("validate", params, socket) - end -end diff --git a/lib/music_library_web/router.ex b/lib/music_library_web/router.ex index 906f6f30..16df0195 100644 --- a/lib/music_library_web/router.ex +++ b/lib/music_library_web/router.ex @@ -112,6 +112,7 @@ defmodule MusicLibraryWeb.Router do live "/scrobble", ScrobbleLive.Index, :index live "/scrobble/:rg_id", ScrobbleLive.ReleaseGroupShow, :show + live "/scrobble/:rg_id/releases/:release_id", ScrobbleLive.ReleaseShow, :show live "/maintenance", MaintenanceLive.Index, :index end diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index a949d4f1..249163c6 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -551,7 +551,6 @@ msgstr "" msgid "Albums" msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex #: lib/music_library_web/live/stats_live/index.ex #, elixir-autogen, elixir-format msgid "Tracks" @@ -594,7 +593,6 @@ msgid "Oban" msgstr "" #: lib/music_library_web/components/record_form.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Release Date" msgstr "" @@ -642,14 +640,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 "Error scrobbling release" 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 "Release scrobbled successfully" msgstr "" @@ -666,42 +662,31 @@ 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/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Scrobble selected tracks" -msgstr "" - #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Selected tracks scrobbled successfully" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Error scrobbling selected tracks" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "No tracks selected" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Disc scrobbled successfully" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Error scrobbling disc" msgstr "" @@ -1288,7 +1273,6 @@ msgstr "" #: lib/music_library_web/live/scrobble_live/index.ex #: lib/music_library_web/live/scrobble_live/release_group_show.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "1 disc" msgid_plural "%{count} discs" @@ -1300,7 +1284,7 @@ msgstr[1] "" msgid "Releases for \"%{title}\"" msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex +#: lib/music_library_web/live/scrobble_live/release_show.ex #, elixir-autogen, elixir-format msgid "You need to connect your Last.fm account to scrobble. Please set up your Last.fm session key in the settings." msgstr "" @@ -1722,27 +1706,11 @@ msgid "Back" msgstr "" #: lib/music_library_web/live/scrobble_live/release_group_show.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Back to search" msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Barcode" -msgstr "" - -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Catalog Number" -msgstr "" - -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Country" -msgstr "" - -#: lib/music_library_web/live/scrobble_live/show.ex +#: lib/music_library_web/live/scrobble_live/release_show.ex #, elixir-autogen, elixir-format msgid "Last.fm not connected" msgstr "" @@ -1752,11 +1720,6 @@ msgstr "" msgid "Loading releases..." msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Media" -msgstr "" - #: lib/music_library_web/live/scrobble_live/index.ex #, elixir-autogen, elixir-format msgid "No release groups found for \"%{query}\"" @@ -2580,3 +2543,13 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Error loading release group" msgstr "" + +#: lib/music_library_web/live/scrobble_live/release_show.ex +#, elixir-autogen, elixir-format +msgid "Back to releases" +msgstr "" + +#: lib/music_library_web/live/scrobble_live/release_show.ex +#, elixir-autogen, elixir-format +msgid "Scrobble Release" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 009ce5aa..46f6ba94 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -551,7 +551,6 @@ msgstr "" msgid "Albums" msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex #: lib/music_library_web/live/stats_live/index.ex #, elixir-autogen, elixir-format msgid "Tracks" @@ -594,7 +593,6 @@ msgid "Oban" msgstr "" #: lib/music_library_web/components/record_form.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Release Date" msgstr "" @@ -642,14 +640,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 "Error scrobbling release" 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 "Release scrobbled successfully" msgstr "" @@ -666,42 +662,31 @@ 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/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Scrobble selected tracks" -msgstr "" - #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Selected tracks scrobbled successfully" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Error scrobbling selected tracks" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "No tracks selected" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Disc scrobbled successfully" msgstr "" #: lib/music_library_web/components/release.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format, fuzzy msgid "Error scrobbling disc" msgstr "" @@ -1288,7 +1273,6 @@ msgstr "" #: lib/music_library_web/live/scrobble_live/index.ex #: lib/music_library_web/live/scrobble_live/release_group_show.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "1 disc" msgid_plural "%{count} discs" @@ -1300,7 +1284,7 @@ msgstr[1] "" msgid "Releases for \"%{title}\"" msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex +#: lib/music_library_web/live/scrobble_live/release_show.ex #, elixir-autogen, elixir-format msgid "You need to connect your Last.fm account to scrobble. Please set up your Last.fm session key in the settings." msgstr "" @@ -1722,27 +1706,11 @@ msgid "Back" msgstr "" #: lib/music_library_web/live/scrobble_live/release_group_show.ex -#: lib/music_library_web/live/scrobble_live/show.ex #, elixir-autogen, elixir-format msgid "Back to search" msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format, fuzzy -msgid "Barcode" -msgstr "" - -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format, fuzzy -msgid "Catalog Number" -msgstr "" - -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format, fuzzy -msgid "Country" -msgstr "" - -#: lib/music_library_web/live/scrobble_live/show.ex +#: lib/music_library_web/live/scrobble_live/release_show.ex #, elixir-autogen, elixir-format msgid "Last.fm not connected" msgstr "" @@ -1752,11 +1720,6 @@ msgstr "" msgid "Loading releases..." msgstr "" -#: lib/music_library_web/live/scrobble_live/show.ex -#, elixir-autogen, elixir-format -msgid "Media" -msgstr "" - #: lib/music_library_web/live/scrobble_live/index.ex #, elixir-autogen, elixir-format msgid "No release groups found for \"%{query}\"" @@ -2580,3 +2543,13 @@ msgstr "" #, elixir-autogen, elixir-format, fuzzy msgid "Error loading release group" msgstr "" + +#: lib/music_library_web/live/scrobble_live/release_show.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Back to releases" +msgstr "" + +#: lib/music_library_web/live/scrobble_live/release_show.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Scrobble Release" +msgstr "" diff --git a/test/music_library_web/live/scrobble_live/release_show_test.exs b/test/music_library_web/live/scrobble_live/release_show_test.exs new file mode 100644 index 00000000..7cdc125a --- /dev/null +++ b/test/music_library_web/live/scrobble_live/release_show_test.exs @@ -0,0 +1,46 @@ +defmodule MusicLibraryWeb.ScrobbleLive.ReleaseShowTest do + @moduledoc """ + Smoke tests for the scrobble release page. Full scrobble behaviour + (picker, selection bar, handlers) is covered by the Release + LiveComponent tests in `release_test.exs`. + """ + use MusicLibraryWeb.ConnCase + + alias MusicBrainz.Fixtures.Release, as: ReleaseFixtures + alias MusicBrainz.Fixtures.ReleaseGroup + alias Req.Test + + @rg_id ReleaseGroup.release_group_id(:marbles) + @release_id ReleaseFixtures.release_id(:marbles) + + defp stub_mb(_) 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 "ReleaseShow" do + setup [:stub_mb] + + test "renders the scrobble UI for the release", %{conn: conn} do + conn + |> visit(~p"/scrobble/#{@rg_id}/releases/#{@release_id}") + |> unwrap(&Phoenix.LiveViewTest.render_async/1) + |> assert_has("h2", text: "Marbles") + end + + test "back link targets the release-group page", %{conn: conn} do + conn + |> visit(~p"/scrobble/#{@rg_id}/releases/#{@release_id}") + |> assert_has("a[href='/scrobble/#{@rg_id}']", text: "Back to releases") + end + end +end diff --git a/test/music_library_web/live/scrobble_live/show_test.exs b/test/music_library_web/live/scrobble_live/show_test.exs deleted file mode 100644 index d238f8a7..00000000 --- a/test/music_library_web/live/scrobble_live/show_test.exs +++ /dev/null @@ -1,261 +0,0 @@ -defmodule MusicLibraryWeb.ScrobbleLive.ShowTest do - use MusicLibraryWeb.ConnCase - - # Route /scrobble/:release_id is removed in Phase 5 — skip until then - @moduletag :skip - - import Phoenix.LiveViewTest, only: [element: 2, render_change: 2, render_click: 3] - - alias MusicBrainz.Fixtures.Release, as: ReleaseFixtures - alias MusicLibrary.Secrets - alias Req.Test - - @release_id ReleaseFixtures.release_id(:marbles) - - 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 - - defp stub_musicbrainz_release_error(_) do - Test.stub(MusicBrainz.API, fn conn -> - Plug.Conn.send_resp(conn, 404, "Not Found") - end) - - :ok - end - - defp stub_lastfm_scrobble(_) do - Test.stub(LastFm.API, fn conn -> - Test.json(conn, %{"scrobbles" => %{"@attr" => %{"accepted" => 1}}}) - end) - - :ok - end - - defp stub_lastfm_scrobble_error(_) do - Test.stub(LastFm.API, fn conn -> - Test.json(conn, %{"error" => 11, "message" => "Service temporarily unavailable"}) - end) - - :ok - end - - defp store_lastfm_session_key(_) do - Secrets.store("last_fm_session_key", "test_session_key") - :ok - end - - defp first_track_id do - ReleaseFixtures.release_with_media(:marbles) - |> Map.get("media") - |> List.first() - |> Map.get("tracks") - |> List.first() - |> Map.get("id") - end - - describe "Show" do - setup [:stub_musicbrainz_release] - - test "renders release details", %{conn: conn} do - conn - |> visit(~p"/scrobble/#{@release_id}") - |> assert_has("h2", "Marbles") - |> assert_has("a", "Back to search") - end - - test "shows Last.fm not connected alert when no session key", %{conn: conn} do - conn - |> visit(~p"/scrobble/#{@release_id}") - |> assert_has("div", "You need to connect your Last.fm account") - end - - test "displays tracks", %{conn: conn} do - conn - |> visit(~p"/scrobble/#{@release_id}") - |> assert_has("h3", "Tracks") - end - end - - describe "Show with Last.fm connected" do - setup [:stub_musicbrainz_release, :stub_lastfm_scrobble, :store_lastfm_session_key] - - test "does not show Last.fm not connected alert", %{conn: conn} do - conn - |> visit(~p"/scrobble/#{@release_id}") - |> refute_has("[data-part='title']", "Last.fm not connected") - end - - test "scrobble full release", %{conn: conn} do - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - render_click(view, "scrobble_release", %{}) - end) - |> assert_has("#toast-group", "Release scrobbled successfully") - end - - test "scrobble single medium", %{conn: conn} do - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - render_click(view, "scrobble_medium", %{"number" => "1"}) - end) - |> assert_has("#toast-group", "Disc scrobbled successfully") - end - - test "scrobble single medium still works with tracks selected elsewhere", %{conn: conn} do - track_id = first_track_id() - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - # Select a track first — AC#3 regression: medium scrobble must not be - # blocked just because something is ticked. - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"selected_tracks" => [track_id]}}) - - render_click(view, "scrobble_medium", %{"number" => "1"}) - end) - |> assert_has("#toast-group", "Disc scrobbled successfully") - end - - test "toggle track on and off changes button label", %{conn: conn} do - track_id = first_track_id() - - session = visit(conn, ~p"/scrobble/#{@release_id}") - - # Toggle track on — button label changes - session - |> unwrap(fn view -> - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"selected_tracks" => [track_id]}}) - end) - |> assert_has("button", "Scrobble selected tracks") - - # Toggle track off — button label reverts - session - |> unwrap(fn view -> - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"selected_tracks" => []}}) - end) - |> assert_has("button", "Scrobble release") - end - - test "toggle medium selects and deselects all tracks in that medium", %{conn: conn} do - session = visit(conn, ~p"/scrobble/#{@release_id}") - - # Toggle medium 1 on — button label changes - session - |> unwrap(fn view -> - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"toggle_medium" => ["1"]}}) - end) - |> assert_has("button", "Scrobble selected tracks") - - # Toggle medium 1 off — button label reverts - session - |> unwrap(fn view -> - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"toggle_medium" => []}}) - end) - |> assert_has("button", "Scrobble release") - end - - test "scrobble selected tracks", %{conn: conn} do - track_id = first_track_id() - - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"selected_tracks" => [track_id]}}) - - render_click(view, "scrobble_selected_tracks", %{}) - end) - |> assert_has("#toast-group", "Selected tracks scrobbled successfully") - end - - test "scrobble selected tracks with no selection shows error", %{conn: conn} do - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - render_click(view, "scrobble_selected_tracks", %{}) - end) - |> assert_has("#toast-group", "No tracks selected") - end - end - - describe "Show with Last.fm scrobble error" do - setup [:stub_musicbrainz_release, :stub_lastfm_scrobble_error, :store_lastfm_session_key] - - @tag :capture_log - test "scrobble release shows error toast on failure", %{conn: conn} do - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - render_click(view, "scrobble_release", %{}) - end) - |> assert_has("#toast-group", "Error scrobbling release") - end - - @tag :capture_log - test "scrobble medium shows error toast on failure", %{conn: conn} do - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - render_click(view, "scrobble_medium", %{"number" => "1"}) - end) - |> assert_has("#toast-group", "Error scrobbling disc") - end - - @tag :capture_log - test "scrobble selected tracks shows error toast on failure", %{conn: conn} do - track_id = first_track_id() - - session = visit(conn, ~p"/scrobble/#{@release_id}") - - session - |> unwrap(fn view -> - view - |> element("#scrobble-release-form") - |> render_change(%{"release" => %{"selected_tracks" => [track_id]}}) - - render_click(view, "scrobble_selected_tracks", %{}) - end) - |> assert_has("#toast-group", "Error scrobbling selected tracks") - end - end - - describe "Show with failed release fetch" do - setup [:stub_musicbrainz_release_error] - - test "redirects to scrobble index with error", %{conn: conn} do - conn - |> visit(~p"/scrobble/#{@release_id}") - |> assert_path(~p"/scrobble") - end - end -end From c7c7e1fe4edb58e52af1a72c8edb8816699c4b7a Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 14:47:50 +0100 Subject: [PATCH 07/12] Navigate from scrobble search to release-group page --- .../live/scrobble_live/index.ex | 150 ++++-------------- priv/gettext/default.pot | 21 --- priv/gettext/en/LC_MESSAGES/default.po | 21 --- .../live/scrobble_live/index_test.exs | 55 +------ 4 files changed, 33 insertions(+), 214 deletions(-) diff --git a/lib/music_library_web/live/scrobble_live/index.ex b/lib/music_library_web/live/scrobble_live/index.ex index f25bbdb3..ffabdb9c 100644 --- a/lib/music_library_web/live/scrobble_live/index.ex +++ b/lib/music_library_web/live/scrobble_live/index.ex @@ -1,9 +1,9 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do use MusicLibraryWeb, :live_view - import MusicLibraryWeb.RecordComponents, only: [type_label: 1, country_label: 1] + import MusicLibraryWeb.RecordComponents, only: [type_label: 1] - alias MusicBrainz.{Release, ReleaseGroupSearchResult} + alias MusicBrainz.ReleaseGroupSearchResult alias MusicLibrary.Records alias MusicLibrary.ScrobbleActivity @@ -25,7 +25,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do
- <%= if @search_results != [] && @selected_release_group == nil do %> + <%= if @search_results != [] do %>

{gettext("Release Groups")} @@ -34,81 +34,30 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do "mt-5 divide-y divide-zinc-100 dark:divide-slate-300/30", "max-h-125 overflow-y-auto" ]}> -
  • -
    - {release_group.title} ~p"/images/cover-not-found.png" <> "';"} - /> -
    -

    - {release_group.artists} -

    -

    - {release_group.title} -

    -

    - {Records.Record.format_release_date(release_group.release_date)} - · - <.badge variant="soft" size="xs">{type_label(release_group.type)} -

    -
    -
    -
  • - -

    - <% end %> - - <%= if @selected_release_group && @releases != [] do %> -
    -
    - <.button - variant="ghost" - size="sm" - phx-click="clear_selection" - > - <.icon name="hero-arrow-left" class="icon" aria-hidden="true" data-slot="icon" /> - {gettext("Back")} - -

    - {gettext("Releases for \"%{title}\"", title: @selected_release_group.title)} -

    -
    - -
      -
    • +
    • <.link - navigate={~p"/scrobble/#{release.id}"} - class="flex items-center gap-x-4 px-4 py-5 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-700" + navigate={~p"/scrobble/#{release_group.id}"} + class="flex cursor-pointer justify-between gap-x-6 py-5 hover:bg-zinc-100 dark:hover:bg-zinc-700" > - {release.title} ~p"/images/cover-not-found.png" <> "';"} - /> -
      -

      - {release.title} -

      -
      - {release.date} - - {country_label(release.country)} - - <.badge :if={release.catalog_number} variant="soft" size="xs"> - {release.catalog_number} - - - {ngettext("1 disc", "%{count} discs", Release.media_count(release))} - +
      + {release_group.title} ~p"/images/cover-not-found.png" <> "';"} + /> +
      +

      + {release_group.artists} +

      +

      + {release_group.title} +

      +

      + {Records.Record.format_release_date(release_group.release_date)} + · + <.badge variant="soft" size="xs">{type_label(release_group.type)} +

      @@ -121,11 +70,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do
      <.loading class="mx-auto size-8 text-zinc-400" />

      - <%= if @selected_release_group do %> - {gettext("Loading releases...")} - <% else %> - {gettext("Searching...")} - <% end %> + {gettext("Searching...")}

      <% end %> @@ -156,8 +101,6 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do current_section: :scrobble, search_query: "", search_results: [], - selected_release_group: nil, - releases: [], loading: false, can_scrobble?: ScrobbleActivity.can_scrobble?() )} @@ -190,9 +133,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do {:noreply, assign(socket, search_query: query, - search_results: [], - selected_release_group: nil, - releases: [] + search_results: [] )} else send(self(), {:perform_search, query}) @@ -200,22 +141,6 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do end end - def handle_event("select_release_group", %{"release_group_id" => release_group_id}, socket) do - selected_release_group = - Enum.find(socket.assigns.search_results, &(&1.id == release_group_id)) - - if selected_release_group do - send(self(), {:fetch_releases, selected_release_group}) - {:noreply, assign(socket, selected_release_group: selected_release_group, loading: true)} - else - {:noreply, socket} - end - end - - def handle_event("clear_selection", _params, socket) do - {:noreply, assign(socket, selected_release_group: nil, releases: [])} - end - @impl true def handle_info({:perform_search, query}, socket) do case MusicBrainz.search_release_group(query, limit: 20) do @@ -223,9 +148,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do {:noreply, assign(socket, search_results: results.release_groups, - loading: false, - selected_release_group: nil, - releases: [] + loading: false )} {:error, _reason} -> @@ -235,21 +158,4 @@ defmodule MusicLibraryWeb.ScrobbleLive.Index do |> assign(loading: false)} end end - - def handle_info({:fetch_releases, release_group}, socket) do - case MusicBrainz.get_releases(release_group.id, limit: 50) do - {:ok, %{"releases" => releases}} -> - releases = - releases - |> Enum.map(&MusicBrainz.Release.from_api_response/1) - - {:noreply, assign(socket, releases: releases, loading: false)} - - {:error, _reason} -> - {:noreply, - socket - |> put_flash(:error, gettext("Failed to fetch releases for this release group")) - |> assign(loading: false)} - end - end end diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 249163c6..6d88c165 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -1256,11 +1256,6 @@ msgstr "" msgid "Scrobble" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format -msgid "Failed to fetch releases for this release group" -msgstr "" - #: lib/music_library_web/live/scrobble_live/index.ex #, elixir-autogen, elixir-format msgid "Failed to search for release groups" @@ -1271,7 +1266,6 @@ msgstr "" msgid "Release Groups" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex #: lib/music_library_web/live/scrobble_live/release_group_show.ex #, elixir-autogen, elixir-format msgid "1 disc" @@ -1279,11 +1273,6 @@ msgid_plural "%{count} discs" msgstr[0] "" msgstr[1] "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format -msgid "Releases for \"%{title}\"" -msgstr "" - #: lib/music_library_web/live/scrobble_live/release_show.ex #, elixir-autogen, elixir-format msgid "You need to connect your Last.fm account to scrobble. Please set up your Last.fm session key in the settings." @@ -1700,11 +1689,6 @@ msgstr "" msgid "Search for artist image online" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format -msgid "Back" -msgstr "" - #: lib/music_library_web/live/scrobble_live/release_group_show.ex #, elixir-autogen, elixir-format msgid "Back to search" @@ -1715,11 +1699,6 @@ msgstr "" msgid "Last.fm not connected" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format -msgid "Loading releases..." -msgstr "" - #: lib/music_library_web/live/scrobble_live/index.ex #, elixir-autogen, elixir-format msgid "No release groups found for \"%{query}\"" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 46f6ba94..89b6a049 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -1256,11 +1256,6 @@ msgstr "" msgid "Scrobble" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format -msgid "Failed to fetch releases for this release group" -msgstr "" - #: lib/music_library_web/live/scrobble_live/index.ex #, elixir-autogen, elixir-format, fuzzy msgid "Failed to search for release groups" @@ -1271,7 +1266,6 @@ msgstr "" msgid "Release Groups" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex #: lib/music_library_web/live/scrobble_live/release_group_show.ex #, elixir-autogen, elixir-format msgid "1 disc" @@ -1279,11 +1273,6 @@ msgid_plural "%{count} discs" msgstr[0] "" msgstr[1] "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format -msgid "Releases for \"%{title}\"" -msgstr "" - #: lib/music_library_web/live/scrobble_live/release_show.ex #, elixir-autogen, elixir-format msgid "You need to connect your Last.fm account to scrobble. Please set up your Last.fm session key in the settings." @@ -1700,11 +1689,6 @@ msgstr "" msgid "Search for artist image online" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format, fuzzy -msgid "Back" -msgstr "" - #: lib/music_library_web/live/scrobble_live/release_group_show.ex #, elixir-autogen, elixir-format msgid "Back to search" @@ -1715,11 +1699,6 @@ msgstr "" msgid "Last.fm not connected" msgstr "" -#: lib/music_library_web/live/scrobble_live/index.ex -#, elixir-autogen, elixir-format, fuzzy -msgid "Loading releases..." -msgstr "" - #: lib/music_library_web/live/scrobble_live/index.ex #, elixir-autogen, elixir-format msgid "No release groups found for \"%{query}\"" diff --git a/test/music_library_web/live/scrobble_live/index_test.exs b/test/music_library_web/live/scrobble_live/index_test.exs index 1d26f4a2..e16962ea 100644 --- a/test/music_library_web/live/scrobble_live/index_test.exs +++ b/test/music_library_web/live/scrobble_live/index_test.exs @@ -1,7 +1,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.IndexTest do use MusicLibraryWeb.ConnCase - import Phoenix.LiveViewTest, only: [render: 1, render_submit: 1, render_click: 3, form: 3] + import Phoenix.LiveViewTest, only: [render: 1, render_submit: 1, form: 3] alias MusicBrainz.Fixtures.ReleaseGroup alias Req.Test @@ -12,9 +12,6 @@ defmodule MusicLibraryWeb.ScrobbleLive.IndexTest do "/ws/2/release-group" -> Test.json(conn, ReleaseGroup.release_group_search_results()) - "/ws/2/release" -> - Test.json(conn, ReleaseGroup.release_group_releases(:marbles)) - _ -> Test.json(conn, %{}) end @@ -91,56 +88,14 @@ defmodule MusicLibraryWeb.ScrobbleLive.IndexTest do |> refute_has("h3", "Release Groups") end - test "select release group shows releases", %{conn: conn} do + test "clicking a release group navigates to /scrobble/:rg_id", %{conn: conn} do release_group_id = ReleaseGroup.release_group_id(:marbles) - session = visit(conn, ~p"/scrobble") + session = visit(conn, ~p"/scrobble?#{[query: "marbles"]}") session - |> unwrap(fn view -> - view - |> form("form[phx-submit='search']", %{query: "marbles"}) - |> render_submit() - - render(view) - - view - |> render_click("select_release_group", %{ - "release_group_id" => release_group_id - }) - - render(view) - end) - |> assert_has("h3", "Releases for") - |> assert_has("button", "Back") - end - - test "clear selection goes back to release groups", %{conn: conn} do - release_group_id = ReleaseGroup.release_group_id(:marbles) - - session = visit(conn, ~p"/scrobble") - - session - |> unwrap(fn view -> - view - |> form("form[phx-submit='search']", %{query: "marbles"}) - |> render_submit() - - render(view) - - view - |> render_click("select_release_group", %{ - "release_group_id" => release_group_id - }) - - render(view) - - # Click back button - view - |> render_click("clear_selection", %{}) - end) - |> assert_has("h3", "Release Groups") - |> refute_has("h3", "Releases for") + |> click_link("a[href='/scrobble/#{release_group_id}']", "Marbles") + |> assert_path(~p"/scrobble/#{release_group_id}") end end From 72818bfaf28ace71e1bacd5e59337ab14f1e7dcb Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 15:00:49 +0100 Subject: [PATCH 08/12] Validate release-group shape before parsing --- .../live/scrobble_live/release_group_show.ex | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/music_library_web/live/scrobble_live/release_group_show.ex b/lib/music_library_web/live/scrobble_live/release_group_show.ex index 891fc825..f7e11064 100644 --- a/lib/music_library_web/live/scrobble_live/release_group_show.ex +++ b/lib/music_library_web/live/scrobble_live/release_group_show.ex @@ -152,10 +152,18 @@ defmodule MusicLibraryWeb.ScrobbleLive.ReleaseGroupShow do defp load(rg_id) do with {:ok, raw_rg} <- MusicBrainz.get_release_group(rg_id), - release_group = ReleaseGroupSearchResult.from_api_response(raw_rg), + {:ok, release_group} <- parse_release_group(raw_rg), {:ok, %{"releases" => raw_releases}} <- MusicBrainz.get_releases(rg_id, limit: 50) do releases = Enum.map(raw_releases, &Release.from_api_response/1) {:ok, %{release_group: release_group, releases: releases}} end end + + defp parse_release_group(%{"id" => _, "artist-credit" => _} = raw) do + {:ok, ReleaseGroupSearchResult.from_api_response(raw)} + end + + defp parse_release_group(_) do + {:error, :invalid_release_group_response} + end end From 779708e62edee2387e6da485e3c5b6af58013da4 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 15:05:05 +0100 Subject: [PATCH 09/12] Hide print tracklist on scrobble release page --- lib/music_library_web/live/scrobble_live/release_show.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/music_library_web/live/scrobble_live/release_show.ex b/lib/music_library_web/live/scrobble_live/release_show.ex index 5458fc93..90a8646f 100644 --- a/lib/music_library_web/live/scrobble_live/release_show.ex +++ b/lib/music_library_web/live/scrobble_live/release_show.ex @@ -36,7 +36,7 @@ defmodule MusicLibraryWeb.ScrobbleLive.ReleaseShow do sheet_id="scrobble-release" module={MusicLibraryWeb.Components.Release} release_id={@release_id} - show_print?={true} + show_print?={false} timezone={@timezone} /> From ca9cc7687cad0004a97921f852ae36a7007dd5a2 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 15:06:45 +0100 Subject: [PATCH 10/12] Hide release-actions dropdown when empty --- lib/music_library_web/components/release.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/music_library_web/components/release.ex b/lib/music_library_web/components/release.ex index b1dfa8a7..f76d7c3f 100644 --- a/lib/music_library_web/components/release.ex +++ b/lib/music_library_web/components/release.ex @@ -176,7 +176,11 @@ defmodule MusicLibraryWeb.Components.Release do {gettext("Release")} - <.dropdown id={"#{@sheet_id}-release-actions"} placement="bottom-end"> + <.dropdown + :if={(@show_print? && @release_with_tracks.ok?) || !@can_scrobble?} + id={"#{@sheet_id}-release-actions"} + placement="bottom-end" + > <:toggle> <.button type="button" variant="outline" size="sm"> {gettext("More actions")} From 62586c9ae50641734ea1e8a12256ca48a3bd2b01 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 15:12:58 +0100 Subject: [PATCH 11/12] Restore flex context for sheet selection bar --- lib/music_library_web/components/release.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/music_library_web/components/release.ex b/lib/music_library_web/components/release.ex index f76d7c3f..0d7a2231 100644 --- a/lib/music_library_web/components/release.ex +++ b/lib/music_library_web/components/release.ex @@ -113,7 +113,7 @@ defmodule MusicLibraryWeb.Components.Release do @impl true def render(assigns) do ~H""" -
      +
      <.form for={@form} id={"#{@sheet_id}-form"} From 43297ecfde846884e78ea70caddde181d132f002 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 23 Apr 2026 15:16:17 +0100 Subject: [PATCH 12/12] ML-145: done Archives ML-142.1 as a consequence --- ...-and-selection-bar-to-ScrobbleLive.Show.md | 0 ...euse-Release-component-on-scrobble-page.md | 106 +++++++++++++++--- 2 files changed, 88 insertions(+), 18 deletions(-) rename backlog/{ => archive}/tasks/ml-142.1 - Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md (100%) diff --git a/backlog/tasks/ml-142.1 - Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md b/backlog/archive/tasks/ml-142.1 - Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md similarity index 100% rename from backlog/tasks/ml-142.1 - Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md rename to backlog/archive/tasks/ml-142.1 - Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md diff --git a/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md b/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md index 0b4685e7..d8ba8cc2 100644 --- a/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md +++ b/backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md @@ -1,9 +1,10 @@ --- id: ML-145 title: Restructure /scrobble routes; reuse Release component on scrobble page -status: To Do +status: Done assignee: [] created_date: '2026-04-23 06:11' +updated_date: '2026-04-23 14:15' labels: - ui - scrobble @@ -67,21 +68,90 @@ Old `/scrobble/:release_id` → 404 (personal app; bookmark churn is acceptable, ## Acceptance Criteria -- [ ] #1 /scrobble/:rg_id renders a release-group header (cover, release-group title, primary artist, type badge, first-release date, release count) and the full list of releases returned by MusicBrainz for that group. -- [ ] #2 Each release in the /scrobble/:rg_id list is a navigate link to /scrobble/:rg_id/releases/:release_id. -- [ ] #3 /scrobble/:rg_id shows an error toast and redirects to /scrobble if fetching the release group or its releases fails. -- [ ] #4 /scrobble/:rg_id/releases/:release_id renders a scrobble page with picker-driven `Finished at`, release-level scrobble, per-medium scrobble, track-selection + selection-bar scrobble, and Print tracklist — behaviour identical to the collection/wishlist show sheet. -- [ ] #5 /scrobble/:rg_id/releases/:release_id has a `Back to releases` link that navigates to /scrobble/:rg_id. -- [ ] #6 Old /scrobble/:release_id returns 404 (not redirected, not aliased). -- [ ] #7 /scrobble no longer renders releases inline; release-group list items are navigate links to /scrobble/:rg_id. The `?query=X` search param still works as before. -- [ ] #8 The Release LiveComponent on the scrobble page and on the collection/wishlist sheet share a single implementation — there is no parallel reimplementation of scrobble handlers, picker, or selection bar. -- [ ] #9 The selection bar stays visible at the bottom of the visible scroll region in both contexts (scrobble page viewport, collection/wishlist sheet inner scroll) while tracks scroll, using a single markup path. -- [ ] #10 MusicLibrary.Records.TracklistPdf generates tracklist PDFs from a MusicBrainz release struct alone (no Records.Record required). Output for collection/wishlist records is visually unchanged. -- [ ] #11 The Release LiveComponent supports suppressing both the release-level and per-medium Print tracklist dropdown entries via a single input (both hidden together); the suppression state is covered by a component test. -- [ ] #12 MusicLibraryWeb.ScrobbleLive.Show's custom scrobble UI and handlers (scrobble_release, scrobble_medium, scrobble_selected_tracks, validate, recover_form) are deleted. MusicLibraryWeb.Components.Release.scrobble_button_label/1 is deleted as unused. -- [ ] #13 All new user-facing strings are wrapped in gettext; .pot/.po files regenerated via `mix gettext.extract --merge`. -- [ ] #14 Tests updated: test/music_library_web/components/release_test.exs covers the new `release_id` input contract and both states of the print-suppression input; test/music_library/records/tracklist_pdf_test.exs covers the new generate/1 and generate_medium/2 signatures. -- [ ] #15 Tests added: test/music_library_web/live/scrobble_live/release_group_show_test.exs covers happy path (header fields + releases list + link targets) and fetch failure (toast + redirect); test/music_library_web/live/scrobble_live/release_show_test.exs (replacing show_test.exs) smoke-tests mount, component render, and back-link target. -- [ ] #16 Tests updated: test/music_library_web/live/scrobble_live/index_test.exs asserts release-group clicks navigate (no inline state); collection/wishlist show tests updated for the new component input shape and callsite sheet markup. -- [ ] #17 Manual UI verification via `iex -S mix phx.server`: sheet selection-bar still pins correctly on collection/wishlist show pages; page selection-bar pins to viewport bottom on scrobble page while tracks list scrolls; navigation loop /scrobble → /scrobble/:rg_id → /scrobble/:rg_id/releases/:release_id → back → back works, and `?query=X` survives the browser back button. +- [x] #1 /scrobble/:rg_id renders a release-group header (cover, release-group title, primary artist, type badge, first-release date, release count) and the full list of releases returned by MusicBrainz for that group. +- [x] #2 Each release in the /scrobble/:rg_id list is a navigate link to /scrobble/:rg_id/releases/:release_id. +- [x] #3 /scrobble/:rg_id shows an error toast and redirects to /scrobble if fetching the release group or its releases fails. +- [x] #4 /scrobble/:rg_id/releases/:release_id renders a scrobble page with picker-driven `Finished at`, release-level scrobble, per-medium scrobble, track-selection + selection-bar scrobble, and Print tracklist — behaviour identical to the collection/wishlist show sheet. +- [x] #5 /scrobble/:rg_id/releases/:release_id has a `Back to releases` link that navigates to /scrobble/:rg_id. +- [x] #6 Old /scrobble/:release_id returns 404 (not redirected, not aliased). +- [x] #7 /scrobble no longer renders releases inline; release-group list items are navigate links to /scrobble/:rg_id. The `?query=X` search param still works as before. +- [x] #8 The Release LiveComponent on the scrobble page and on the collection/wishlist sheet share a single implementation — there is no parallel reimplementation of scrobble handlers, picker, or selection bar. +- [x] #9 The selection bar stays visible at the bottom of the visible scroll region in both contexts (scrobble page viewport, collection/wishlist sheet inner scroll) while tracks scroll, using a single markup path. +- [x] #10 MusicLibrary.Records.TracklistPdf generates tracklist PDFs from a MusicBrainz release struct alone (no Records.Record required). Output for collection/wishlist records is visually unchanged. +- [x] #11 The Release LiveComponent supports suppressing both the release-level and per-medium Print tracklist dropdown entries via a single input (both hidden together); the suppression state is covered by a component test. +- [x] #12 MusicLibraryWeb.ScrobbleLive.Show's custom scrobble UI and handlers (scrobble_release, scrobble_medium, scrobble_selected_tracks, validate, recover_form) are deleted. MusicLibraryWeb.Components.Release.scrobble_button_label/1 is deleted as unused. +- [x] #13 All new user-facing strings are wrapped in gettext; .pot/.po files regenerated via `mix gettext.extract --merge`. +- [x] #14 Tests updated: test/music_library_web/components/release_test.exs covers the new `release_id` input contract and both states of the print-suppression input; test/music_library/records/tracklist_pdf_test.exs covers the new generate/1 and generate_medium/2 signatures. +- [x] #15 Tests added: test/music_library_web/live/scrobble_live/release_group_show_test.exs covers happy path (header fields + releases list + link targets) and fetch failure (toast + redirect); test/music_library_web/live/scrobble_live/release_show_test.exs (replacing show_test.exs) smoke-tests mount, component render, and back-link target. +- [x] #16 Tests updated: test/music_library_web/live/scrobble_live/index_test.exs asserts release-group clicks navigate (no inline state); collection/wishlist show tests updated for the new component input shape and callsite sheet markup. +- [x] #17 Manual UI verification via `iex -S mix phx.server`: sheet selection-bar still pins correctly on collection/wishlist show pages; page selection-bar pins to viewport bottom on scrobble page while tracks list scrolls; navigation loop /scrobble → /scrobble/:rg_id → /scrobble/:rg_id/releases/:release_id → back → back works, and `?query=X` survives the browser back button. + +## Final Summary + + +## Summary + +Restructured `/scrobble` into a three-level URL hierarchy (`/scrobble` → `/scrobble/:rg_id` → `/scrobble/:rg_id/releases/:release_id`) and eliminated the duplicated scrobble UI by making the Release LiveComponent reusable on a standalone page. + +Supersedes ML-142.1 — picker and selection bar now live on the scrobble page via reuse, not via a parallel port. + +## Implementation + +Ten commits on branch `ml-145-scrobble-route-restructure`: + +- `8631b97b` Add joinphrase to Release.Artist; parse from MB +- `0d1f5803` Refactor TracklistPdf to take release alone +- `4c48bda7` Refactor Release component to take release_id +- `d8041dc0` Add /scrobble/:rg_id release-group page +- `0dcd6eab` Replace /scrobble/:release_id with nested route +- `c7c7e1fe` Navigate from scrobble search to release-group page +- `72818bfa` Validate release-group shape before parsing +- `779708e6` Hide print tracklist on scrobble release page +- `ca9cc768` Hide release-actions dropdown when empty +- `62586c9a` Restore flex context for sheet selection bar + +Key changes: + +- `MusicBrainz.Release.Artist` gains `:joinphrase` so `TracklistPdf` can work from a release struct alone. +- `MusicLibraryWeb.Components.Release` takes `release_id` + `show_print?` (instead of `record`); header title/artists derive from the loaded release; `<.sheet>` wrapper moved to callsites; release-actions dropdown auto-hides when both print + connect-last-fm are disabled. +- `CollectionLive.Show` wraps the component in its own `<.sheet>` with `show_print?={true}`. +- New `ScrobbleLive.ReleaseGroupShow` at `/scrobble/:rg_id` — async-fetches the release group + its releases, renders header (cover, title, primary artist, type badge, first-release date, release count) + list of releases. +- New `ScrobbleLive.ReleaseShow` at `/scrobble/:rg_id/releases/:release_id` — thin LiveView embedding the Release LiveComponent directly (no sheet), passes `show_print?={false}`. +- `ScrobbleLive.Index` loses inline release-loading state; release-group clicks navigate to `/scrobble/:rg_id`. +- Old `ScrobbleLive.Show` module and test, plus `Components.Release.scrobble_button_label/1`, deleted. + +## Verification findings + fixes + +Two bugs surfaced during Phase 8 verification: + +1. **Error-path crash in `ReleaseGroupShow.load/1`.** When `MusicBrainz.get_release_group/1` returned an unexpected response shape, `ReleaseGroupSearchResult.from_api_response/1` crashed on `Enum.map_join(nil, …)`. The test passed because `handle_async({:exit, …}, …)` caught it, but the stack trace appeared in logs. Fixed with an explicit `parse_release_group/1` helper that pattern-guards the shape and returns `{:error, :invalid_release_group_response}` for bad input, routing cleanly through the graceful `{:error, reason}` handler. + +2. **Sheet layout regression.** The Phase-3 root-`
      ` replacement for `<.sheet>` did not establish a flex context, so the form's `flex-1` no longer expanded inside the sheet, which pushed the selection bar away from the bottom. Fixed by giving the root `
      ` classes `flex h-full min-h-0 flex-1 flex-col` — single markup that works in both sheet (flex-pinned last child) and page (sticky-pinned to viewport) contexts. + +## Tests + +- Full suite: 815 passed (43 doctests, 772 tests), 0 skipped. +- `mix compile --warnings-as-errors`: clean. +- `mise run dev:precommit`: clean (shellcheck, credo, sobelow, translations, formatting, unused deps, full test run). + +New coverage: +- `test/music_library_web/live/scrobble_live/release_group_show_test.exs` — happy path (header + release links) and fetch failure (toast + redirect to `/scrobble`). +- `test/music_library_web/live/scrobble_live/release_show_test.exs` — smoke (mount + component render + back link target). +- `show_print?={true|false}` verified via `Phoenix.LiveViewTest.live_isolated/3` in `release_test.exs`. + +Deleted: `test/music_library_web/live/scrobble_live/show_test.exs` (replaced by the smoke test above). + +## Manual UI verification (Phase 8 AC#17) + +Confirmed by user in dev server: +- Navigation loop `/scrobble` → `/scrobble/:rg_id` → `/scrobble/:rg_id/releases/:release_id` works; `?query=X` survives browser back. +- Scrobble release page: picker, per-release/medium/track scrobble, and selection bar sticky at viewport bottom. +- Collection show sheet: selection bar pins to sheet bottom while tracks scroll. +- Print tracklist works on collection show; correctly hidden on scrobble page. +- Release-actions dropdown button hides entirely on scrobble page (empty dropdown) when Last.fm is connected. + +## Follow-ups + +- **ML-142.1** is now fully superseded and can be archived (not done here — archive is a user decision). +