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

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
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+
+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.
+
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+
+<!-- AC:BEGIN -->
+
+- [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.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+
+## 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.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+
+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.
+
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+
+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.
+
+<!-- SECTION:FINAL_SUMMARY:END -->

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}

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

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]
 

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()

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
 

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

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
 

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

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)

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

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)