cloud/music_library
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ML-145: done

Browse Source 
Archives ML-142.1 as a consequence
This commit is contained in:
Claudio Ortolina
2026-04-23 15:16:17 +01:00
parent 62586c9ae5
commit 43297ecfde
2 changed files with 88 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backlog/tasks/ml-142.1 - Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md
-58
View File
@@ -1,58 +0,0 @@
---
id: ML-142.1
title: Port Finished at picker and selection bar to ScrobbleLive.Show
status: To Do
assignee: []
created_date: '2026-04-22 16:30'
labels:
- ui
- scrobble
dependencies: []
references:
- lib/music_library_web/live/scrobble_live/show.ex
- lib/music_library_web/components/release.ex
- backlog/ml-142/plan.md
parent_task_id: ML-142
priority: medium
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Bring the `/scrobble/:release_id` page (`MusicLibraryWeb.ScrobbleLive.Show`) up to ML-142 parity for the two affordances that affect scrobbling outcomes: the `Finished at` date/time picker and the pinned selection-bar footer.
Today the page imports `medium/1` and `scrobble_button_label/1` from `MusicLibraryWeb.Components.Release` — so the per-medium UI stays visually consistent — but reimplements its own `scrobble_release`, `scrobble_medium`, and `scrobble_selected_tracks` handlers, each hard-coding `DateTime.utc_now/0`. There is no picker (users cannot back-date or forward-date a scrobble) and no selection bar (when tracks are selected, the user sees no count/duration summary and the only scrobble affordance is an icon-only header button whose behaviour depends on hidden selection state via `scrobble_button_label/1`).
`ScrobbleLive.Show` was explicitly called out as out-of-scope in ML-142 — see `backlog/ml-142/plan.md`: "A full sticky-bar + picker port to `ScrobbleLive.Show`. That page uses its own layout (not the sheet) and can be iterated on separately once the LiveComponent pattern is settled." That deferred work is the subject of this task.
## Out of scope
- `⋯` overflow menu on this page (Print tracklist for release + per-medium).
- Connect Last.fm dropdown entry — the page already has a top-of-page alert banner for that state.
- Extracting a shared function component or LiveHelper is an implementation decision, not a requirement. Implementer may duplicate or refactor; the acceptance criteria are outcome-based.
## Reusable pieces the implementer can draw on
- `MusicLibraryWeb.Components.Release.apply_form_params/3` — already public, already used by this LiveView.
- `MusicLibraryWeb.Components.Release.medium/1` — already imported.
- `MusicLibrary.ScrobbleActivity.scrobble_release/3`, `scrobble_medium/4`, `scrobble_tracks/4` — already used.
- Private helpers in `Release` that may need to be promoted (implementer's choice): `parse_finished_at/1`, `selection_bar/1`, `selected_tracks_summary/2`.
- `scrobble_button_label/1` will likely become unused after AC#8 — remove if so.
- `lib/music_library_web/live_helpers/` pattern (e.g. `record_actions.ex`) is a possible refactor target but not required.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 The page renders a `Finished at` date/time picker in the tracks section header when Last.fm is connected, initialised to the current time at mount
- [ ] #2 The picker has a `Now` reset affordance that resets the value back to the current time
- [ ] #3 `Scrobble release` uses the picker value; no longer calls `DateTime.utc_now/0` directly inside the handler
- [ ] #4 Per-medium `Scrobble disc`/`Scrobble side` uses the picker value; no longer calls `DateTime.utc_now/0` directly inside the handler
- [ ] #5 `Scrobble selected tracks` uses the picker value; no longer calls `DateTime.utc_now/0` directly inside the handler
- [ ] #6 A selection bar renders at the bottom of the tracks section whenever `MapSet.size(@selected_tracks) > 0`, showing track count, medium count (when > 1), aggregate duration, and a `Scrobble selected` button
- [ ] #7 The selection bar is not rendered when no tracks are selected
- [ ] #8 The selection bar stays visible while the tracks list is scrolled on narrow viewports (`position: sticky` or equivalent layout — implementer's choice given this page sits inside `Layouts.app`, not the sheet)
- [ ] #9 The icon-only header scrobble button (which currently toggles via `scrobble_button_label/1`) is replaced by a solid `Scrobble release` button; the selection bar owns the scrobble-selected role
- [ ] #10 All new user-facing strings wrapped in `gettext`; `.pot`/`.po` files regenerated via `mix gettext.extract --merge`
- [ ] #11 LiveView tests cover: picker default state initialised to `utc_now`, picker value propagating to all three scrobble handlers, `reset_to_now` event, selection-bar hidden when no tracks selected, selection-bar shown with correct copy when tracks selected
- [ ] #12 The existing AC#14 regression test (per-medium scrobble works with cross-medium tracks selected, at `test/music_library_web/live/scrobble_live/show_test.exs:115`) continues to pass
<!-- AC:END -->
backlog/tasks/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md
+88 -18
View File
@@ -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
<!-- AC:BEGIN -->
- [ ] #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.
<!-- AC:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
## 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-`<div>` 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 `<div>` 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).
<!-- SECTION:FINAL_SUMMARY:END -->