From 23960b5b23183bbbc32a2c0dc52f7ff460e32c86 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Wed, 20 May 2026 18:04:29 +0100 Subject: [PATCH] ML-169.10.1: Add delete coverage Cover current delete behavior across LiveView index and show paths, and strengthen existing delete tests with persistence assertions.\n\nValidation: mise run dev:precommit --- ...ete-coverage-for-current-LiveView-paths.md | 99 +++++++++++++++++++ test/music_library/records_test.exs | 7 +- .../live/artist_live/show_test.exs | 21 ++++ .../live/collection_live/index_test.exs | 16 +++ .../live/collection_live/show_test.exs | 21 ++++ .../online_store_template_live/index_test.exs | 6 ++ .../live/record_set_live/index_test.exs | 16 +++ .../live/record_set_live/show_test.exs | 4 + .../live/scrobble_rules_live/index_test.exs | 6 ++ .../live/scrobbled_tracks_live/index_test.exs | 16 +++ .../live/wishlist_live/index_test.exs | 14 +++ .../live/wishlist_live/show_test.exs | 16 +++ 12 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 backlog/tasks/ml-169.10.1 - Add-baseline-delete-coverage-for-current-LiveView-paths.md diff --git a/backlog/tasks/ml-169.10.1 - Add-baseline-delete-coverage-for-current-LiveView-paths.md b/backlog/tasks/ml-169.10.1 - Add-baseline-delete-coverage-for-current-LiveView-paths.md new file mode 100644 index 00000000..0cb3d3e1 --- /dev/null +++ b/backlog/tasks/ml-169.10.1 - Add-baseline-delete-coverage-for-current-LiveView-paths.md @@ -0,0 +1,99 @@ +--- +id: ML-169.10.1 +title: Add baseline delete coverage for current LiveView paths +status: Done +assignee: + - Codex +created_date: "2026-05-20 16:31" +updated_date: "2026-05-20 16:37" +labels: + - testing + - coverage +dependencies: [] +references: + - ML-169.10 +documentation: + - docs/architecture.md + - docs/project-conventions.md +modified_files: + - test/music_library/records_test.exs + - test/music_library_web/live/collection_live/index_test.exs + - test/music_library_web/live/wishlist_live/index_test.exs + - test/music_library_web/live/scrobbled_tracks_live/index_test.exs + - test/music_library_web/live/collection_live/show_test.exs + - test/music_library_web/live/wishlist_live/show_test.exs + - test/music_library_web/live/artist_live/show_test.exs + - test/music_library_web/live/record_set_live/index_test.exs + - test/music_library_web/live/record_set_live/show_test.exs + - test/music_library_web/live/online_store_template_live/index_test.exs + - test/music_library_web/live/scrobble_rules_live/index_test.exs +parent_task_id: ML-169.10 +priority: medium +ordinal: 34000 +--- + +## Description + + + +Add focused test coverage for current deletion behavior without implementing the async optimistic UI changes from ML-169.10. This task covers the three stream-backed index paths called out in ML-169.10 and verifies whether other existing user-facing deletion paths lack tests. Scope is limited to tests for current behavior; production delete handlers should not be changed here unless a test reveals an existing broken path that cannot be tested as-is. + + + +## Acceptance Criteria + + + +- [x] #1 Collection index delete is covered by a LiveView test that removes the record from the page and verifies the record is deleted through the Records context. +- [x] #2 Wishlist index delete is covered by a LiveView test that removes the record from the page and verifies the record is deleted through the Records context. +- [x] #3 Scrobbled tracks index delete is covered by a LiveView test that removes the track from the page and verifies the track is deleted through ListeningStats. +- [x] #4 Existing user-facing delete handlers are audited for missing coverage, and any untested current-behavior paths found in that audit receive focused tests or are documented as intentionally out of scope. +- [x] #5 Existing delete tests that only assert UI removal are strengthened with persistence assertions where practical. +- [x] #6 Focused tests for the touched delete paths pass. + + +## Implementation Plan + + + +## Implementation Plan + +1. Add baseline LiveView delete tests for the three ML-169.10 stream-backed index paths without changing production code: + - `test/music_library_web/live/collection_live/index_test.exs` + - `test/music_library_web/live/wishlist_live/index_test.exs` + - `test/music_library_web/live/scrobbled_tracks_live/index_test.exs` + +2. Audit current user-facing delete handlers for missing deletion tests. Based on the initial scan, add focused current-behavior tests for untested deletion paths in: + - `test/music_library_web/live/collection_live/show_test.exs` + - `test/music_library_web/live/wishlist_live/show_test.exs` + - `test/music_library_web/live/artist_live/show_test.exs` + - `test/music_library_web/live/record_set_live/index_test.exs` + +3. Strengthen existing UI-only deletion tests with persistence assertions where practical: + - `test/music_library_web/live/online_store_template_live/index_test.exs` + - `test/music_library_web/live/scrobble_rules_live/index_test.exs` + +4. Run focused tests for the touched files, then update the task with modified files, audit notes, and checked acceptance criteria. + + +## Implementation Notes + + + +Audited current deletion coverage by scanning user-facing delete handlers and existing delete tests. Added baseline coverage for the three ML-169.10 stream-backed index paths: collection index, wishlist index, and scrobbled tracks index. Added current-behavior coverage for untested delete paths found in collection show, wishlist show, artist record list deletion, and record set index deletion. Strengthened existing UI-only delete tests for online store templates, scrobble rules, and record set show deletion with persistence assertions. Also strengthened Records.delete_record/1 to assert the record is actually gone while preserving the PruneArtistInfo enqueue assertion. + +Did not add async success/error/exit handle_async tests from ML-169.10 because those paths do not exist yet; adding them would require implementing the async optimistic UI issue, which is intentionally out of scope for this task. + + + +## Final Summary + + + +Added baseline test coverage for current delete behavior without changing production delete handlers. + +Covered the three ML-169.10 stream-backed index paths: collection index record deletion, wishlist index record deletion, and scrobbled tracks deletion. Also covered additional untested current user-facing delete paths found during the audit: collection show, wishlist show, artist record list deletion, and record set index deletion. Strengthened existing delete tests for online store templates, scrobble rules, record set show deletion, and Records.delete_record/1 so they assert persisted deletion through context lookups. + +Validation: `mix test --max-failures 5 test/music_library/records_test.exs test/music_library_web/live/collection_live/index_test.exs test/music_library_web/live/wishlist_live/index_test.exs test/music_library_web/live/scrobbled_tracks_live/index_test.exs test/music_library_web/live/collection_live/show_test.exs test/music_library_web/live/wishlist_live/show_test.exs test/music_library_web/live/artist_live/show_test.exs test/music_library_web/live/record_set_live/index_test.exs test/music_library_web/live/record_set_live/show_test.exs test/music_library_web/live/online_store_template_live/index_test.exs test/music_library_web/live/scrobble_rules_live/index_test.exs` passed with 115 tests. + + diff --git a/test/music_library/records_test.exs b/test/music_library/records_test.exs index 25d48a83..850fc837 100644 --- a/test/music_library/records_test.exs +++ b/test/music_library/records_test.exs @@ -69,7 +69,12 @@ defmodule MusicLibrary.RecordsTest do [artist] = record.artists - Records.delete_record(record) + assert {:ok, deleted} = Records.delete_record(record) + assert deleted.id == record.id + + assert_raise Ecto.NoResultsError, fn -> + Records.get_record!(record.id) + end assert_enqueued worker: MusicLibrary.Worker.PruneArtistInfo, args: %{id: artist.musicbrainz_id} diff --git a/test/music_library_web/live/artist_live/show_test.exs b/test/music_library_web/live/artist_live/show_test.exs index ea4319d4..9a83e058 100644 --- a/test/music_library_web/live/artist_live/show_test.exs +++ b/test/music_library_web/live/artist_live/show_test.exs @@ -17,6 +17,7 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do alias MusicLibrary.Artists alias MusicLibrary.Assets alias MusicLibrary.Assets.Asset + alias MusicLibrary.Records alias Req.Test defp fill_collection(_config) do @@ -164,6 +165,26 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do |> refute_has("#collection p", escape(other_collection_record.title)) |> refute_has("#wishlist p", escape(other_collection_record.title)) end + + test "deletes a record from the artist record list", %{ + conn: conn, + collection_record: collection_record, + artist_musicbrainz_id: artist_musicbrainz_id + } do + stub_last_fm(%{}) + + conn + |> visit(~p"/artists/#{artist_musicbrainz_id}") + |> render_async() + |> assert_has("#collection p", escape(collection_record.title)) + |> click_link("a[data-confirm='Are you sure?']", "Delete") + |> refute_has("#collection p", escape(collection_record.title)) + |> assert_has("#toast-group", "Record deleted") + + assert_raise Ecto.NoResultsError, fn -> + Records.get_record!(collection_record.id) + end + end end describe "Edit artist image" do diff --git a/test/music_library_web/live/collection_live/index_test.exs b/test/music_library_web/live/collection_live/index_test.exs index 380b63e3..f52920c7 100644 --- a/test/music_library_web/live/collection_live/index_test.exs +++ b/test/music_library_web/live/collection_live/index_test.exs @@ -127,6 +127,22 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do end end + describe "Delete record" do + test "deletes a record from the listing", %{conn: conn} do + record = record(%{title: "Delete From Collection Index"}) + + conn + |> visit(~p"/collection") + |> assert_has("#records-#{record.id}") + |> click_link("#records-#{record.id} a[data-confirm='Are you sure?']", "Delete") + |> refute_has("#records-#{record.id}") + + assert_raise Ecto.NoResultsError, fn -> + Records.get_record!(record.id) + end + end + end + describe "Search and pagination" do setup [:fill_collection] diff --git a/test/music_library_web/live/collection_live/show_test.exs b/test/music_library_web/live/collection_live/show_test.exs index 6fed2251..66126274 100644 --- a/test/music_library_web/live/collection_live/show_test.exs +++ b/test/music_library_web/live/collection_live/show_test.exs @@ -6,6 +6,7 @@ defmodule MusicLibraryWeb.CollectionLive.ShowTest do alias MusicBrainz.Fixtures alias MusicLibrary.Assets.Transform + alias MusicLibrary.Records alias MusicLibrary.Records.Record alias Phoenix.PubSub @@ -69,6 +70,26 @@ defmodule MusicLibraryWeb.CollectionLive.ShowTest do end end + describe "Delete record" do + test "deletes the record and navigates back to collection", %{conn: conn} do + record = record() + release_response = Fixtures.Release.release(:marbles) + + Req.Test.stub(MusicBrainz.API, fn conn -> + Req.Test.json(conn, release_response) + end) + + conn + |> visit(~p"/collection/#{record.id}") + |> click_link("a[data-confirm='Are you sure?']", "Delete") + |> assert_path(~p"/collection") + + assert_raise Ecto.NoResultsError, fn -> + Records.get_record!(record.id) + end + end + end + describe "handle_info({:update, record}) with live_action guard" do test "updates record when showing (live_action is :show)", %{conn: conn} do record = record() diff --git a/test/music_library_web/live/online_store_template_live/index_test.exs b/test/music_library_web/live/online_store_template_live/index_test.exs index 4f287b37..22fdd872 100644 --- a/test/music_library_web/live/online_store_template_live/index_test.exs +++ b/test/music_library_web/live/online_store_template_live/index_test.exs @@ -3,6 +3,8 @@ defmodule MusicLibraryWeb.OnlineStoreTemplateLive.IndexTest do import MusicLibrary.Fixtures.OnlineStoreTemplates + alias MusicLibrary.OnlineStoreTemplates + describe "Index" do test "lists all templates", %{conn: conn} do online_store_template(%{name: "Amazon UK"}) @@ -63,6 +65,10 @@ defmodule MusicLibraryWeb.OnlineStoreTemplateLive.IndexTest do |> visit(~p"/online-store-templates") |> click_button("button[phx-click='delete'][phx-value-id='#{template.id}']", "Delete") |> refute_has("p", "To Delete") + + assert_raise Ecto.NoResultsError, fn -> + OnlineStoreTemplates.get_template!(template.id) + end end end diff --git a/test/music_library_web/live/record_set_live/index_test.exs b/test/music_library_web/live/record_set_live/index_test.exs index 5493409d..d5520dfc 100644 --- a/test/music_library_web/live/record_set_live/index_test.exs +++ b/test/music_library_web/live/record_set_live/index_test.exs @@ -80,4 +80,20 @@ defmodule MusicLibraryWeb.RecordSetLive.IndexTest do assert updated.name == "Updated Name" end end + + describe "Delete set" do + test "deletes a set from the listing", %{conn: conn} do + set = record_set(%{name: "Delete From Index"}) + + conn + |> visit(~p"/record-sets") + |> assert_has("h2", "Delete From Index") + |> click_button("button[phx-click='delete_set'][phx-value-id='#{set.id}']", "Delete") + |> refute_has("h2", "Delete From Index") + + assert_raise Ecto.NoResultsError, fn -> + RecordSets.get_record_set!(set.id) + end + end + end end diff --git a/test/music_library_web/live/record_set_live/show_test.exs b/test/music_library_web/live/record_set_live/show_test.exs index e27c368f..1f5c959d 100644 --- a/test/music_library_web/live/record_set_live/show_test.exs +++ b/test/music_library_web/live/record_set_live/show_test.exs @@ -97,6 +97,10 @@ defmodule MusicLibraryWeb.RecordSetLive.ShowTest do |> visit(~p"/record-sets/#{set}") |> click_button("button[phx-click='delete_set']", "Delete") |> assert_path(~p"/record-sets") + + assert_raise Ecto.NoResultsError, fn -> + RecordSets.get_record_set!(set.id) + end end end diff --git a/test/music_library_web/live/scrobble_rules_live/index_test.exs b/test/music_library_web/live/scrobble_rules_live/index_test.exs index f5f72273..d9105be2 100644 --- a/test/music_library_web/live/scrobble_rules_live/index_test.exs +++ b/test/music_library_web/live/scrobble_rules_live/index_test.exs @@ -4,6 +4,8 @@ defmodule MusicLibraryWeb.ScrobbleRulesLiveTest do import MusicLibrary.ScrobbleRulesFixtures import Phoenix.LiveViewTest, only: [render_change: 1, form: 3] + alias MusicLibrary.ScrobbleRules + defp create_scrobble_rule(_) do scrobble_rule = scrobble_rule_fixture() %{scrobble_rule: scrobble_rule} @@ -92,6 +94,10 @@ defmodule MusicLibraryWeb.ScrobbleRulesLiveTest do "Delete" ) |> refute_has("#scrobble_rule-#{scrobble_rule.id}") + + assert_raise Ecto.NoResultsError, fn -> + ScrobbleRules.get_scrobble_rule!(scrobble_rule.id) + end end test "toggles rule enabled status", %{conn: conn, scrobble_rule: scrobble_rule} do diff --git a/test/music_library_web/live/scrobbled_tracks_live/index_test.exs b/test/music_library_web/live/scrobbled_tracks_live/index_test.exs index 2740a2eb..f0e98d00 100644 --- a/test/music_library_web/live/scrobbled_tracks_live/index_test.exs +++ b/test/music_library_web/live/scrobbled_tracks_live/index_test.exs @@ -86,6 +86,22 @@ defmodule MusicLibraryWeb.ScrobbledTracksLiveTest do end end + describe "Delete track" do + setup [:create_track] + + test "deletes a scrobbled track from the listing", %{conn: conn, track: track} do + conn + |> visit(~p"/scrobbled-tracks") + |> assert_has("p", track.title) + |> click_button("button[data-confirm='Are you sure?']", "Delete") + |> refute_has("p", track.title) + + assert_raise Ecto.NoResultsError, fn -> + ListeningStats.get_track!(track.scrobbled_at_uts) + end + end + end + describe "Pagination" do test "navigates to next page", %{conn: conn} do create_test_tracks(21) diff --git a/test/music_library_web/live/wishlist_live/index_test.exs b/test/music_library_web/live/wishlist_live/index_test.exs index 8c727e59..b854df9e 100644 --- a/test/music_library_web/live/wishlist_live/index_test.exs +++ b/test/music_library_web/live/wishlist_live/index_test.exs @@ -45,6 +45,20 @@ defmodule MusicLibraryWeb.WishlistLive.IndexTest do describe "Wishlist" do setup [:fill_wishlist] + test "deletes a record from the listing", %{conn: conn} do + record = record(%{title: "Delete From Wishlist Index", purchased_at: nil}) + + conn + |> visit(~p"/wishlist") + |> assert_has("#records-#{record.id}") + |> click_link("#records-#{record.id} a[data-confirm='Are you sure?']", "Delete") + |> refute_has("#records-#{record.id}") + + assert_raise Ecto.NoResultsError, fn -> + Records.get_record!(record.id) + end + end + test "can purchase a record and move it to the collection", %{ conn: conn, wishlist: wishlist_records diff --git a/test/music_library_web/live/wishlist_live/show_test.exs b/test/music_library_web/live/wishlist_live/show_test.exs index c9a87bcc..c5e2d07c 100644 --- a/test/music_library_web/live/wishlist_live/show_test.exs +++ b/test/music_library_web/live/wishlist_live/show_test.exs @@ -5,6 +5,7 @@ defmodule MusicLibraryWeb.WishlistLive.ShowTest do import MusicLibraryWeb.RecordComponents, only: [format_label: 1, type_label: 1] alias MusicLibrary.Assets.Transform + alias MusicLibrary.Records alias MusicLibrary.Records.Record alias Phoenix.PubSub @@ -53,6 +54,21 @@ defmodule MusicLibraryWeb.WishlistLive.ShowTest do end end + describe "Delete record" do + test "deletes the record and navigates back to wishlist", %{conn: conn} do + record = record(purchased_at: nil) + + conn + |> visit(~p"/wishlist/#{record.id}") + |> click_link("a[data-confirm='Are you sure?']", "Delete") + |> assert_path(~p"/wishlist") + + assert_raise Ecto.NoResultsError, fn -> + Records.get_record!(record.id) + end + end + end + describe "handle_info({:update, record}) with live_action guard" do test "updates record when showing (live_action is :show)", %{conn: conn} do record = record(purchased_at: nil)