diff --git a/backlog/tasks/ml-143 - Cart-style-multi-record-import-in-Add-Record-modal.md b/backlog/tasks/ml-143 - Cart-style-multi-record-import-in-Add-Record-modal.md index aa2edcdf..888d1c54 100644 --- a/backlog/tasks/ml-143 - Cart-style-multi-record-import-in-Add-Record-modal.md +++ b/backlog/tasks/ml-143 - Cart-style-multi-record-import-in-Add-Record-modal.md @@ -1,10 +1,10 @@ --- id: ML-143 title: Cart-style multi-record import in Add Record modal -status: To Do +status: In Progress assignee: [] created_date: '2026-04-20 10:00' -updated_date: '2026-04-20 10:01' +updated_date: '2026-04-20 10:15' labels: - ui - liveview @@ -17,6 +17,7 @@ documentation: - backlog/ml-143/plan.md - backlog/ml-143/mockups.html priority: medium +ordinal: 1000 --- ## Description diff --git a/lib/music_library/worker/import_from_musicbrainz_release_group.ex b/lib/music_library/worker/import_from_musicbrainz_release_group.ex new file mode 100644 index 00000000..71968ba8 --- /dev/null +++ b/lib/music_library/worker/import_from_musicbrainz_release_group.ex @@ -0,0 +1,32 @@ +defmodule MusicLibrary.Worker.ImportFromMusicbrainzReleaseGroup do + @moduledoc """ + Imports a record from a MusicBrainz release group in the background. + + Used by the cart-style multi-record import when there are two or more + records to import at once. + """ + + use Oban.Worker, queue: :music_brainz, max_attempts: 3 + + alias MusicLibrary.Records + + @impl Oban.Worker + def perform(%Oban.Job{args: %{"release_group_id" => release_group_id} = args}) do + opts = [ + format: args["format"], + purchased_at: parse_datetime(args["purchased_at"]) + ] + + case Records.import_from_musicbrainz_release_group(release_group_id, opts) do + {:ok, _record} -> :ok + {:error, reason} -> {:error, reason} + end + end + + defp parse_datetime(nil), do: nil + + defp parse_datetime(str) do + {:ok, datetime, _offset} = DateTime.from_iso8601(str) + datetime + end +end diff --git a/lib/music_library_web/components/add_record.ex b/lib/music_library_web/components/add_record.ex index 2442ea31..a83d4bae 100644 --- a/lib/music_library_web/components/add_record.ex +++ b/lib/music_library_web/components/add_record.ex @@ -1,4 +1,16 @@ defmodule MusicLibraryWeb.Components.AddRecord do + @moduledoc """ + Cart-style MusicBrainz import modal. + + Users search MusicBrainz, stage `{release_group, format}` pairs into an ephemeral + cart, then import them all at once. A single-item cart runs synchronously via + `start_async`; two or more items enqueue one Oban job per item and close the modal. + + The parent LiveView receives two messages and handles navigation/toasts: + - `{__MODULE__, {:imported_single, record}}` + - `{__MODULE__, {:imported_async, count}}` + """ + use MusicLibraryWeb, :live_component import MusicLibraryWeb.RecordComponents, only: [format_label: 1, type_label: 1] @@ -6,61 +18,220 @@ defmodule MusicLibraryWeb.Components.AddRecord do alias MusicBrainz.ReleaseGroupSearchResult alias MusicLibrary.Records + alias MusicLibrary.Worker.ImportFromMusicbrainzReleaseGroup + alias MusicLibraryWeb.ErrorMessages + + require Logger @batch_size 20 + @default_format :cd @impl true def render(assigns) do ~H""" -
- <.simple_form - for={@form} - id={:import_form} - phx-target={@myself} - phx-change="search" - phx-submit="search" - class="px-4" - > - <.input - id={:mb_query} - name={:mb_query} - field={@form[:mb_query]} - type="search" - label={gettext("Search for a record")} - phx-debounce="500" - autocomplete="off" - autofocus - /> - - <.alert :if={@error_message} color="danger" hide_close class="mx-4 mt-4"> - {@error_message} - - -
- {gettext("No results")} -
- <.results_footer total_results={@release_groups_total_count} /> +
+
+ <.simple_form + for={@form} + id={:import_form} + phx-target={@myself} + phx-change="search" + phx-submit="search" + > + <.input + id={:mb_query} + name={:mb_query} + field={@form[:mb_query]} + type="search" + label={gettext("Search for a record")} + phx-debounce="500" + autocomplete="off" + autofocus + /> + + <.alert :if={@error_message} color="danger" hide_close class="mt-4"> + {@error_message} + +
    + <.result + :for={release_group <- @release_groups} + id={"musicbrainz_#{release_group.id}"} + myself={@myself} + in_cart?={in_cart?(@cart_pairs, release_group.id)} + cart_format={cart_format(@cart, release_group.id)} + release_group={release_group} + icon_name={@icon_name} + /> +
+
+ {gettext("No results")} +
+ <.results_footer total_results={@release_groups_total_count} /> +
+ +
""" end @@ -68,6 +239,9 @@ defmodule MusicLibraryWeb.Components.AddRecord do attr :id, :string, required: true attr :icon_name, :string, required: true attr :release_group, MusicBrainz.ReleaseGroupSearchResult, required: true + attr :myself, :any, required: true + attr :in_cart?, :boolean, required: true + attr :cart_format, :atom, required: true defp result(assigns) do ~H""" @@ -93,9 +267,17 @@ defmodule MusicLibraryWeb.Components.AddRecord do

+ + <.icon name="hero-check" class="size-3" aria-hidden="true" data-slot="icon" /> + {gettext("In cart · %{format}", format: format_label(@cart_format))} + + <.dropdown id={"actions-#{@release_group.id}"} placement="bottom-end"> <:toggle> - {gettext("Choose which format to import")} + {gettext("Choose which format to add")} <.icon name="hero-plus" class="size-5 cursor-pointer text-zinc-500 dark:text-zinc-400" @@ -106,9 +288,19 @@ defmodule MusicLibraryWeb.Components.AddRecord do <.focus_wrap id={"actions-#{@release_group.id}-focus-wrap"}> <.dropdown_link :for={format <- Records.Record.formats()} - id={"actions-#{@release_group.id}-#{format}-import"} + id={"actions-#{@release_group.id}-#{format}-add"} phx-click={ - JS.push("import", value: %{id: @release_group.id, format: format}, page_loading: true) + JS.push("add_to_cart", + value: %{ + id: @release_group.id, + format: format, + title: @release_group.title, + artists: @release_group.artists, + release_date: @release_group.release_date, + thumb_url: ReleaseGroupSearchResult.thumb_url(@release_group) + }, + target: @myself + ) } > {format_label(format)} @@ -124,14 +316,15 @@ defmodule MusicLibraryWeb.Components.AddRecord do def mount(socket) do {:ok, socket - |> stream_configure(:release_groups, - dom_id: fn rg -> "musicbrainz_#{rg.id}" end - ) + |> assign(:release_groups, []) |> assign(:release_groups_count, 0) |> assign(:release_groups_total_count, 0) - |> stream(:release_groups, []) |> assign(:loaded_all_results?, false) - |> assign(:error_message, nil)} + |> assign(:error_message, nil) + |> assign(:cart, []) + |> assign(:cart_pairs, MapSet.new()) + |> assign(:cart_expanded?, true) + |> assign(:importing?, false)} end @impl true @@ -148,7 +341,7 @@ defmodule MusicLibraryWeb.Components.AddRecord do |> assign(:error_message, nil) |> assign(:release_groups_count, Enum.count(result.release_groups)) |> assign(:release_groups_total_count, result.total_count) - |> stream(:release_groups, result.release_groups, reset: true) + |> assign(:release_groups, result.release_groups) {:error, _reason} -> assign( @@ -163,19 +356,19 @@ defmodule MusicLibraryWeb.Components.AddRecord do assign(socket, offset: 0, icon_name: assigns.icon_name, + purchased_at_fn: assigns.purchased_at_fn, form: to_form(%{"mb_query" => mb_query}) )} end @impl true - def handle_event("search", %{"mb_query" => ""}, socket) do {:noreply, socket |> assign(:offset, 0) |> assign(:release_groups_count, 0) |> assign(:release_groups_total_count, 0) - |> stream(:release_groups, [], reset: true) + |> assign(:release_groups, []) |> assign(:form, to_form(%{"mb_query" => ""}))} end @@ -188,7 +381,7 @@ defmodule MusicLibraryWeb.Components.AddRecord do |> assign(:offset, 0) |> assign(:release_groups_count, length(result.release_groups)) |> assign(:release_groups_total_count, result.total_count) - |> stream(:release_groups, result.release_groups, reset: true) + |> assign(:release_groups, result.release_groups) |> assign(:form, to_form(%{"mb_query" => mb_query}))} {:error, _reason} -> @@ -198,7 +391,7 @@ defmodule MusicLibraryWeb.Components.AddRecord do |> assign(:offset, 0) |> assign(:release_groups_count, 0) |> assign(:release_groups_total_count, 0) - |> stream(:release_groups, [], reset: true) + |> assign(:release_groups, []) |> assign(:form, to_form(%{"mb_query" => mb_query}))} end end @@ -215,10 +408,209 @@ defmodule MusicLibraryWeb.Components.AddRecord do |> assign(:loaded_all_results?, length(result.release_groups) < @batch_size) |> assign(:release_groups_count, offset + length(result.release_groups)) |> assign(:release_groups_total_count, result.total_count) - |> stream(:release_groups, result.release_groups)} + |> assign( + :release_groups, + socket.assigns.release_groups ++ result.release_groups + )} {:error, _reason} -> {:noreply, socket} end end + + def handle_event("add_to_cart", params, socket) do + case parse_format(params["format"]) do + {:ok, format} -> + {:noreply, add_to_cart(socket, params, format)} + + :error -> + {:noreply, socket} + end + end + + def handle_event("remove_from_cart", %{"cart_item_id" => id}, socket) do + id = cast_id(id) + {:noreply, remove_cart_item(socket, id)} + end + + def handle_event("change_format", %{"cart_item_id" => id, "format" => format_str}, socket) do + id = cast_id(id) + + case parse_format(format_str) do + {:ok, format} -> {:noreply, change_cart_format(socket, id, format)} + :error -> {:noreply, socket} + end + end + + def handle_event("clear_cart", _params, socket) do + {:noreply, + socket + |> assign(:cart, []) + |> assign(:cart_pairs, MapSet.new())} + end + + def handle_event("toggle_cart", _params, socket) do + {:noreply, assign(socket, :cart_expanded?, not socket.assigns.cart_expanded?)} + end + + def handle_event("import_cart", _params, socket) do + case socket.assigns.cart do + [] -> + {:noreply, socket} + + [single] -> + purchased_at = socket.assigns.purchased_at_fn.() + + {:noreply, + socket + |> assign(:importing?, true) + |> start_async(:import_cart, fn -> + Records.import_from_musicbrainz_release_group(single.release_group_id, + format: single.format, + purchased_at: purchased_at + ) + end)} + + items -> + purchased_at = socket.assigns.purchased_at_fn.() + purchased_at_iso = purchased_at && DateTime.to_iso8601(purchased_at) + + changesets = + Enum.map(items, fn item -> + ImportFromMusicbrainzReleaseGroup.new(%{ + "release_group_id" => item.release_group_id, + "format" => Atom.to_string(item.format), + "purchased_at" => purchased_at_iso + }) + end) + + Oban.insert_all(changesets) + notify_parent({:imported_async, length(items)}) + {:noreply, socket} + end + end + + @impl true + # If the user closes the modal mid-import, this callback lands on a detached + # component and is silently dropped by Phoenix — no user-visible bug. + def handle_async(:import_cart, {:ok, {:ok, record}}, socket) do + notify_parent({:imported_single, record}) + {:noreply, socket} + end + + def handle_async(:import_cart, {:ok, {:error, reason}}, socket) do + put_toast!( + :error, + gettext("Error importing record") <> ": " <> ErrorMessages.friendly_message(reason) + ) + + {:noreply, assign(socket, :importing?, false)} + end + + def handle_async(:import_cart, {:exit, reason}, socket) do + Logger.warning("Cart import crashed: #{inspect(reason)}") + put_toast!(:error, gettext("Error importing record")) + {:noreply, assign(socket, :importing?, false)} + end + + defp add_to_cart(socket, params, format) do + rg_id = params["id"] + pair = {rg_id, format} + + if MapSet.member?(socket.assigns.cart_pairs, pair) do + socket + else + item = %{ + cart_item_id: System.unique_integer([:positive]), + release_group_id: rg_id, + title: params["title"], + artists: params["artists"], + release_date: params["release_date"], + thumb_url: params["thumb_url"], + format: format + } + + socket + |> assign(:cart, [item | socket.assigns.cart]) + |> assign(:cart_pairs, MapSet.put(socket.assigns.cart_pairs, pair)) + end + end + + defp remove_cart_item(socket, id) do + {removed, kept} = Enum.split_with(socket.assigns.cart, &(&1.cart_item_id == id)) + + pairs = + Enum.reduce(removed, socket.assigns.cart_pairs, fn item, acc -> + MapSet.delete(acc, {item.release_group_id, item.format}) + end) + + socket + |> assign(:cart, kept) + |> assign(:cart_pairs, pairs) + end + + defp change_cart_format(socket, id, new_format) do + case Enum.find(socket.assigns.cart, &(&1.cart_item_id == id)) do + nil -> + socket + + %{format: ^new_format} -> + socket + + item -> + new_pair = {item.release_group_id, new_format} + + if MapSet.member?(socket.assigns.cart_pairs, new_pair) do + socket + else + updated = %{item | format: new_format} + + cart = + Enum.map(socket.assigns.cart, fn + ^item -> updated + other -> other + end) + + pairs = + socket.assigns.cart_pairs + |> MapSet.delete({item.release_group_id, item.format}) + |> MapSet.put(new_pair) + + socket + |> assign(:cart, cart) + |> assign(:cart_pairs, pairs) + end + end + end + + defp parse_format(nil), do: {:ok, @default_format} + + defp parse_format(format_str) when is_binary(format_str) do + formats = Records.Record.formats() + + case Enum.find(formats, fn f -> Atom.to_string(f) == format_str end) do + nil -> :error + format -> {:ok, format} + end + end + + defp parse_format(format) when is_atom(format) do + if format in Records.Record.formats(), do: {:ok, format}, else: :error + end + + defp cast_id(id) when is_integer(id), do: id + defp cast_id(id) when is_binary(id), do: String.to_integer(id) + + defp in_cart?(cart_pairs, rg_id) do + Enum.any?(cart_pairs, fn {id, _format} -> id == rg_id end) + end + + defp cart_format(cart, rg_id) do + case Enum.find(cart, &(&1.release_group_id == rg_id)) do + nil -> nil + item -> item.format + end + end + + defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) end diff --git a/lib/music_library_web/components/core_components.ex b/lib/music_library_web/components/core_components.ex index 7d25f479..2137cf54 100644 --- a/lib/music_library_web/components/core_components.ex +++ b/lib/music_library_web/components/core_components.ex @@ -154,6 +154,7 @@ defmodule MusicLibraryWeb.CoreComponents do attr :id, :string, required: true attr :on_close, :any, required: false, default: nil attr :open, :boolean, required: false, default: true + attr :width_class, :string, required: false, default: "md:max-w-3xl" slot :inner_block, required: true @@ -161,7 +162,7 @@ defmodule MusicLibraryWeb.CoreComponents do ~H""" ~p"/collection/#{id}" end, index_path_fn: fn qs -> ~p"/collection?#{qs}" end, base_index_path: ~p"/collection" @@ -186,9 +186,10 @@ defmodule MusicLibraryWeb.CollectionLive.Index do :if={@live_action == :import} id="record-modal" on_close={JS.patch(back_path(@record_list_params))} + width_class="md:max-w-4xl lg:max-w-5xl" > <.live_component - module={MusicLibraryWeb.Components.AddRecord} + module={AddRecord} id={:search} title={@page_title} action={@live_action} @@ -196,6 +197,7 @@ defmodule MusicLibraryWeb.CollectionLive.Index do patch={back_path(@record_list_params)} initial_query={@import_query} icon_name="hero-plus" + purchased_at_fn={@index_config.purchased_at_fn} /> @@ -284,6 +286,14 @@ defmodule MusicLibraryWeb.CollectionLive.Index do IndexActions.handle_record_saved(socket) end + def handle_info({AddRecord, {:imported_single, record}}, socket) do + IndexActions.handle_cart_imported_single(socket, record) + end + + def handle_info({AddRecord, {:imported_async, count}}, socket) do + IndexActions.handle_cart_imported_async(socket, count) + end + def handle_info({MusicLibraryWeb.Components.Chat, :chats_changed}, socket) do chat_count = Chats.count_chats(:collection, Chats.collection_musicbrainz_id()) {:noreply, assign(socket, :chat_count, chat_count)} @@ -307,10 +317,6 @@ defmodule MusicLibraryWeb.CollectionLive.Index do IndexActions.handle_search(socket, query) end - def handle_event("import", %{"id" => musicbrainz_id, "format" => format}, socket) do - IndexActions.handle_import(socket, musicbrainz_id, format) - end - def handle_event("set_display", %{"mode" => mode}, socket) do IndexActions.handle_set_display(socket, mode) end diff --git a/lib/music_library_web/live/wishlist_live/index.ex b/lib/music_library_web/live/wishlist_live/index.ex index 23c68c03..b06156cf 100644 --- a/lib/music_library_web/live/wishlist_live/index.ex +++ b/lib/music_library_web/live/wishlist_live/index.ex @@ -6,6 +6,7 @@ defmodule MusicLibraryWeb.WishlistLive.Index do alias MusicLibrary.Records alias MusicLibrary.Wishlist + alias MusicLibraryWeb.Components.AddRecord alias MusicLibraryWeb.LiveHelpers.IndexActions defp index_config do @@ -18,7 +19,6 @@ defmodule MusicLibraryWeb.WishlistLive.Index do import_page_title: gettext("Add new Record · Wishlist"), section_page_title: gettext("Wishlist"), import_success_toast: gettext("Record wishlisted successfully"), - import_error_toast: gettext("Error wishlisting record"), record_path_fn: fn id -> ~p"/wishlist/#{id}" end, index_path_fn: fn qs -> ~p"/wishlist?#{qs}" end, base_index_path: ~p"/wishlist" @@ -154,9 +154,10 @@ defmodule MusicLibraryWeb.WishlistLive.Index do :if={@live_action == :import} id="record-modal" on_close={JS.patch(back_path(@record_list_params))} + width_class="md:max-w-4xl lg:max-w-5xl" > <.live_component - module={MusicLibraryWeb.Components.AddRecord} + module={AddRecord} id={:search} title={@page_title} action={@live_action} @@ -164,6 +165,7 @@ defmodule MusicLibraryWeb.WishlistLive.Index do patch={back_path(@record_list_params)} initial_query={@import_query} icon_name="hero-plus" + purchased_at_fn={@index_config.purchased_at_fn} /> @@ -207,6 +209,14 @@ defmodule MusicLibraryWeb.WishlistLive.Index do IndexActions.handle_record_saved(socket) end + def handle_info({AddRecord, {:imported_single, record}}, socket) do + IndexActions.handle_cart_imported_single(socket, record) + end + + def handle_info({AddRecord, {:imported_async, count}}, socket) do + IndexActions.handle_cart_imported_async(socket, count) + end + @impl true def handle_event("delete", %{"id" => id}, socket) do IndexActions.handle_delete(socket, id) @@ -216,10 +226,6 @@ defmodule MusicLibraryWeb.WishlistLive.Index do IndexActions.handle_search(socket, query) end - def handle_event("import", %{"id" => musicbrainz_id, "format" => format}, socket) do - IndexActions.handle_import(socket, musicbrainz_id, format) - end - def handle_event("add-to-collection", %{"id" => id}, socket) do record = Records.get_record!(id) current_time = DateTime.utc_now() diff --git a/lib/music_library_web/live_helpers/index_actions.ex b/lib/music_library_web/live_helpers/index_actions.ex index d68d8417..814147ea 100644 --- a/lib/music_library_web/live_helpers/index_actions.ex +++ b/lib/music_library_web/live_helpers/index_actions.ex @@ -10,7 +10,6 @@ defmodule MusicLibraryWeb.LiveHelpers.IndexActions do use Gettext, backend: MusicLibraryWeb.Gettext alias MusicLibrary.Records - alias MusicLibraryWeb.ErrorMessages @doc """ Applies the :index action. Reads config from `socket.assigns.index_config` @@ -108,28 +107,30 @@ defmodule MusicLibraryWeb.LiveHelpers.IndexActions do {:noreply, push_patch(socket, to: config.index_path_fn.(qs))} end - def handle_import(socket, musicbrainz_id, format) do + def handle_cart_imported_single(socket, record) do config = socket.assigns.index_config - case Records.import_from_musicbrainz_release_group(musicbrainz_id, - format: format, - purchased_at: config.purchased_at_fn.() - ) do - {:ok, record} -> - {:noreply, - socket - |> put_toast(:info, config.import_success_toast) - |> push_navigate(to: config.record_path_fn.(record.id))} + {:noreply, + socket + |> put_toast(:info, config.import_success_toast) + |> push_navigate(to: config.record_path_fn.(record.id))} + end - {:error, reason} -> - {:noreply, - socket - |> put_toast( - :error, - config.import_error_toast <> ": " <> ErrorMessages.friendly_message(reason) - ) - |> push_patch(to: config.base_index_path)} - end + def handle_cart_imported_async(socket, count) do + config = socket.assigns.index_config + + msg = + ngettext( + "Importing %{count} record in the background...", + "Importing %{count} records in the background...", + count, + count: count + ) + + {:noreply, + socket + |> put_toast(:info, msg) + |> push_patch(to: config.base_index_path)} end def handle_set_display(socket, mode) do diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 07d92dda..391c7ac5 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -64,12 +64,13 @@ msgstr "" msgid "Edit" msgstr "" +#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/live/artist_live/show.ex -#: lib/music_library_web/live/collection_live/index.ex #, elixir-autogen, elixir-format msgid "Error importing record" msgstr "" +#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/components/record_form.ex #, elixir-autogen, elixir-format msgid "Format" @@ -227,7 +228,6 @@ msgstr "" msgid "Scrobble activity" msgstr "" -#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/components/scrobble_components.ex #, elixir-autogen, elixir-format msgid "Choose which format to import" @@ -569,7 +569,6 @@ msgid "Add more · Artist" msgstr "" #: lib/music_library_web/live/stats_live/index.ex -#: lib/music_library_web/live/wishlist_live/index.ex #, elixir-autogen, elixir-format msgid "Error wishlisting record" msgstr "" @@ -1588,6 +1587,7 @@ msgstr "" msgid "Record set updated successfully" msgstr "" +#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/live/record_set_live/index.ex #: lib/music_library_web/live/record_set_live/show.ex #, elixir-autogen, elixir-format @@ -2401,6 +2401,7 @@ msgid "Record has a selected release" msgstr "" #: lib/music_library_web/components/barcode_scanner.ex +#: lib/music_library_web/live_helpers/index_actions.ex #, elixir-autogen, elixir-format msgid "Importing %{count} record in the background..." msgid_plural "Importing %{count} records in the background..." @@ -2461,3 +2462,52 @@ msgstr "" #, elixir-autogen, elixir-format msgid "A rule for this album already exists" msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "%{count} record" +msgid_plural "%{count} records" +msgstr[0] "" +msgstr[1] "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Add records from the search results to get started." +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Cart" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Choose which format to add" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Clear all" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Import %{count} record" +msgid_plural "Import %{count} records" +msgstr[0] "" +msgstr[1] "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "In cart · %{format}" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Toggle cart" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Your cart is empty" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 9f56052a..064d74ac 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -64,12 +64,13 @@ msgstr "" msgid "Edit" msgstr "" +#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/live/artist_live/show.ex -#: lib/music_library_web/live/collection_live/index.ex #, elixir-autogen, elixir-format msgid "Error importing record" msgstr "" +#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/components/record_form.ex #, elixir-autogen, elixir-format msgid "Format" @@ -227,7 +228,6 @@ msgstr "" msgid "Scrobble activity" msgstr "" -#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/components/scrobble_components.ex #, elixir-autogen, elixir-format msgid "Choose which format to import" @@ -569,7 +569,6 @@ msgid "Add more · Artist" msgstr "" #: lib/music_library_web/live/stats_live/index.ex -#: lib/music_library_web/live/wishlist_live/index.ex #, elixir-autogen, elixir-format msgid "Error wishlisting record" msgstr "" @@ -1588,6 +1587,7 @@ msgstr "" msgid "Record set updated successfully" msgstr "" +#: lib/music_library_web/components/add_record.ex #: lib/music_library_web/live/record_set_live/index.ex #: lib/music_library_web/live/record_set_live/show.ex #, elixir-autogen, elixir-format @@ -2401,6 +2401,7 @@ msgid "Record has a selected release" msgstr "" #: lib/music_library_web/components/barcode_scanner.ex +#: lib/music_library_web/live_helpers/index_actions.ex #, elixir-autogen, elixir-format msgid "Importing %{count} record in the background..." msgid_plural "Importing %{count} records in the background..." @@ -2461,3 +2462,52 @@ msgstr "" #, elixir-autogen, elixir-format msgid "A rule for this album already exists" msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "%{count} record" +msgid_plural "%{count} records" +msgstr[0] "" +msgstr[1] "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Add records from the search results to get started." +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Cart" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Choose which format to add" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Clear all" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Import %{count} record" +msgid_plural "Import %{count} records" +msgstr[0] "" +msgstr[1] "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "In cart · %{format}" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Toggle cart" +msgstr "" + +#: lib/music_library_web/components/add_record.ex +#, elixir-autogen, elixir-format +msgid "Your cart is empty" +msgstr "" diff --git a/test/music_library/worker/import_from_musicbrainz_release_group_test.exs b/test/music_library/worker/import_from_musicbrainz_release_group_test.exs new file mode 100644 index 00000000..37ba8ed4 --- /dev/null +++ b/test/music_library/worker/import_from_musicbrainz_release_group_test.exs @@ -0,0 +1,91 @@ +defmodule MusicLibrary.Worker.ImportFromMusicbrainzReleaseGroupTest do + use MusicLibrary.DataCase + + import MusicBrainz.Fixtures.ReleaseGroup + import MusicLibrary.Fixtures.Records + + alias MusicLibrary.Records.Record + alias MusicLibrary.Worker.ImportFromMusicbrainzReleaseGroup + + describe "perform/1" do + test "imports a record from a MusicBrainz release group" do + release_group_data = release_group(:marbles) + release_group_id = release_group_id(:marbles) + release_group_releases_data = release_group_releases(:marbles) + + cover_data = marbles_cover_data() + + Req.Test.stub(MusicBrainz.API, fn conn -> + case conn.path_info do + [_ws, _version, "release-group", ^release_group_id] -> + Req.Test.json(conn, release_group_data) + + [_ws, _version, "release"] -> + Req.Test.json(conn, release_group_releases_data) + + [_release_group, ^release_group_id, "front"] -> + Plug.Conn.send_resp(conn, 200, cover_data) + end + end) + + purchased_at = DateTime.utc_now() + + assert :ok = + perform_job(ImportFromMusicbrainzReleaseGroup, %{ + "release_group_id" => release_group_id, + "format" => "cd", + "purchased_at" => DateTime.to_iso8601(purchased_at) + }) + + imported_record = Repo.get_by!(Record, musicbrainz_id: release_group_id) + assert imported_record.title == "Marbles" + assert imported_record.format == :cd + assert imported_record.purchased_at == DateTime.truncate(purchased_at, :second) + end + + test "imports a wishlist record when purchased_at is nil" do + release_group_data = release_group(:marbles) + release_group_id = release_group_id(:marbles) + release_group_releases_data = release_group_releases(:marbles) + + cover_data = marbles_cover_data() + + Req.Test.stub(MusicBrainz.API, fn conn -> + case conn.path_info do + [_ws, _version, "release-group", ^release_group_id] -> + Req.Test.json(conn, release_group_data) + + [_ws, _version, "release"] -> + Req.Test.json(conn, release_group_releases_data) + + [_release_group, ^release_group_id, "front"] -> + Plug.Conn.send_resp(conn, 200, cover_data) + end + end) + + assert :ok = + perform_job(ImportFromMusicbrainzReleaseGroup, %{ + "release_group_id" => release_group_id, + "format" => "vinyl", + "purchased_at" => nil + }) + + imported_record = Repo.get_by!(Record, musicbrainz_id: release_group_id) + assert imported_record.purchased_at == nil + assert imported_record.format == :vinyl + end + + test "returns error on transport failure" do + Req.Test.stub(MusicBrainz.API, fn conn -> + Req.Test.transport_error(conn, :timeout) + end) + + assert {:error, %Req.TransportError{reason: :timeout}} = + perform_job(ImportFromMusicbrainzReleaseGroup, %{ + "release_group_id" => "nonexistent-release-group-id", + "format" => "cd", + "purchased_at" => DateTime.to_iso8601(DateTime.utc_now()) + }) + end + end +end 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 6c2329d1..d6f1172b 100644 --- a/test/music_library_web/live/collection_live/index_test.exs +++ b/test/music_library_web/live/collection_live/index_test.exs @@ -1,15 +1,17 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do use MusicLibraryWeb.ConnCase + use Oban.Testing, repo: MusicLibrary.BackgroundRepo import MusicBrainz.Fixtures.Release import MusicBrainz.Fixtures.ReleaseGroup import MusicLibrary.Fixtures.Records import MusicLibraryWeb.RecordComponents, only: [format_label: 1, type_label: 1] + import Phoenix.LiveViewTest, only: [assert_redirect: 2] - alias MusicBrainz.ReleaseGroupSearchResult alias MusicLibrary.Assets alias MusicLibrary.Assets.{Image, Transform} alias MusicLibrary.Records.Record + alias MusicLibrary.Worker.ImportFromMusicbrainzReleaseGroup # make it a multiple of 4 for easier calculations @default_records_page_size 4 @@ -281,93 +283,171 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do |> assert_has("input[value='test query']") end - test "imports a record when selected", %{conn: conn} do - release_group_search_results = Map.get(release_group_search_results(), "release-groups") + test "adds a record to the cart instead of importing immediately", %{conn: conn} do + stub_release_group_search() - first_release_group_search_result = hd(release_group_search_results) - first_release_group_search_result_id = first_release_group_search_result["id"] + [first | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] - release_group = release_group(:marbles) - release_group_releases = release_group_releases(:marbles) + conn + |> visit(~p"/collection/import") + |> fill_in("Search for a record", with: "Marillion Marbles") + |> click_link("#musicbrainz_#{first_id} a", "CD") + |> assert_has("#musicbrainz_#{first_id} span", text: "In cart · CD") + |> assert_has("#cart-items li", count: 1) - cover_data = marbles_cover_data() + assert MusicLibrary.Repo.all(Record) == [] + refute_enqueued(worker: ImportFromMusicbrainzReleaseGroup) + end - Req.Test.stub(MusicBrainz.API, fn conn -> - case conn.path_info do - [_ws, _version, "release-group", ^first_release_group_search_result_id] -> - Req.Test.json(conn, release_group) + test "deduplicates the same {release_group, format} pair", %{conn: conn} do + stub_release_group_search() - [_ws, _version, "release-group"] -> - Req.Test.json(conn, release_group_search_results()) - - [_ws, _version, "release"] -> - Req.Test.json(conn, release_group_releases) - - [_release_group, ^first_release_group_search_result_id, "front"] -> - Plug.Conn.send_resp(conn, 200, cover_data) - end - end) + [first | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] session = conn |> visit(~p"/collection/import") |> fill_in("Search for a record", with: "Marillion Marbles") + |> click_link("#musicbrainz_#{first_id} a", "CD") + |> click_link("#musicbrainz_#{first_id} a", "CD") - for release_group_search_result <- release_group_search_results do - result = ReleaseGroupSearchResult.from_api_response(release_group_search_result) + assert_has(session, "#cart-items li", count: 1) + end - session - |> assert_has("h1", result.artists) - |> assert_has("h2", result.title) - |> assert_has("p", Record.format_release_date(result.release_date)) - end + test "allows same release group with different formats", %{conn: conn} do + stub_release_group_search() + + [first | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] session = - session - |> click_link("#musicbrainz_#{first_release_group_search_result_id} a", "CD") + conn + |> visit(~p"/collection/import") + |> fill_in("Search for a record", with: "Marillion Marbles") + |> click_link("#musicbrainz_#{first_id} a", "CD") + |> click_link("#musicbrainz_#{first_id} a", "Vinyl") - [record] = MusicLibrary.Repo.all(MusicLibrary.Records.Record) + assert_has(session, "#cart-items li", count: 2) + end - assert record.musicbrainz_id == first_release_group_search_result_id + test "removes an item from the cart", %{conn: conn} do + stub_release_group_search() + + [first | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] + + session = + conn + |> visit(~p"/collection/import") + |> fill_in("Search for a record", with: "Marillion Marbles") + |> click_link("#musicbrainz_#{first_id} a", "CD") + |> click_button("#cart-items button", "Remove") + + assert_has(session, "#cart-empty") + end + + test "imports a single cart item synchronously and navigates", %{conn: conn} do + alias Phoenix.LiveViewTest, as: LVT + + stub_full_import() + + [first | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] + + {:ok, view, _html} = LVT.live(conn, ~p"/collection/import") + + view + |> LVT.form("#import_form", %{"mb_query" => "Marillion Marbles"}) + |> LVT.render_change() + + view + |> LVT.element("#musicbrainz_#{first_id} a", "CD") + |> LVT.render_click() + + view + |> LVT.element("button", "Import 1 record") + |> LVT.render_click() + + {path, _flash} = assert_redirect(view, 2_000) + "/collection/" <> record_id = path + + record = MusicLibrary.Records.get_record!(record_id) + + assert record.musicbrainz_id == first_id assert record.title == "Marbles" - assert record.release_date == "2004-05-03" assert record.format == :cd - assert record.musicbrainz_data == release_group - - assert record.genres == [ - "alternative rock", - "art rock", - "baroque pop", - "pop rock", - "progressive rock", - "psychedelic pop", - "rock" - ] - - assert record.cover_hash == - "E7238C742E5B8711FC5BFF01A4A1F727D9E404A4D1420429A6B37ABFFC0B5960" - - {:ok, resized_cover_data} = Image.resize(cover_data) + assert record.purchased_at != nil + {:ok, resized_cover_data} = Image.resize(marbles_cover_data()) assets = Assets.get(record.cover_hash) - assert assets.content == resized_cover_data - assert record.inserted_at !== nil - assert record.updated_at !== nil - assert record.purchased_at !== nil - - [marillion] = record.artists - - assert %MusicLibrary.Artists.Artist{ - name: "Marillion", - sort_name: "Marillion", - disambiguation: "British progressive rock band", - musicbrainz_id: "1932f5b6-0b7b-4050-b1df-833ca89e5f44" - } = marillion - - assert_path(session, ~p"/collection/#{record.id}") + refute_enqueued(worker: ImportFromMusicbrainzReleaseGroup) end + + test "enqueues one job per cart item for 2+ items and closes modal", %{conn: conn} do + stub_release_group_search() + + [first, second | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] + second_id = second["id"] + + conn + |> visit(~p"/collection/import") + |> fill_in("Search for a record", with: "Marillion Marbles") + |> click_link("#musicbrainz_#{first_id} a", "CD") + |> click_link("#musicbrainz_#{second_id} a", "Vinyl") + |> click_button("Import 2 records") + |> assert_has("p", text: "Importing 2 records in the background...") + + assert_enqueued( + worker: ImportFromMusicbrainzReleaseGroup, + args: %{"release_group_id" => first_id, "format" => "cd"} + ) + + assert_enqueued( + worker: ImportFromMusicbrainzReleaseGroup, + args: %{"release_group_id" => second_id, "format" => "vinyl"} + ) + + assert MusicLibrary.Repo.all(Record) == [] + end + end + + defp stub_release_group_search do + Req.Test.stub(MusicBrainz.API, fn conn -> + case conn.path_info do + [_ws, _version, "release-group"] -> + Req.Test.json(conn, release_group_search_results()) + end + end) + end + + defp stub_full_import do + [first | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] + + release_group = release_group(:marbles) + release_group_releases = release_group_releases(:marbles) + cover_data = marbles_cover_data() + + Req.Test.stub(MusicBrainz.API, fn conn -> + case conn.path_info do + [_ws, _version, "release-group", ^first_id] -> + Req.Test.json(conn, release_group) + + [_ws, _version, "release-group"] -> + Req.Test.json(conn, release_group_search_results()) + + [_ws, _version, "release"] -> + Req.Test.json(conn, release_group_releases) + + [_release_group, ^first_id, "front"] -> + Plug.Conn.send_resp(conn, 200, cover_data) + end + end) end describe "Add via barcode scan" do 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 719cad81..71d312c8 100644 --- a/test/music_library_web/live/wishlist_live/index_test.exs +++ b/test/music_library_web/live/wishlist_live/index_test.exs @@ -1,8 +1,13 @@ defmodule MusicLibraryWeb.WishlistLive.IndexTest do use MusicLibraryWeb.ConnCase + use Oban.Testing, repo: MusicLibrary.BackgroundRepo + import MusicBrainz.Fixtures.ReleaseGroup import MusicLibrary.Fixtures.Records + alias MusicLibrary.Records.Record + alias MusicLibrary.Worker.ImportFromMusicbrainzReleaseGroup + defp fill_wishlist(_) do records = Enum.map(1..5, fn _ -> record(%{purchased_at: nil}) end) %{wishlist: records} @@ -27,4 +32,49 @@ defmodule MusicLibraryWeb.WishlistLive.IndexTest do assert purchased_record.purchased_at !== nil end end + + describe "Adding a new record" do + test "enqueues one job per cart item with purchased_at: nil for wishlist", %{conn: conn} do + Req.Test.stub(MusicBrainz.API, fn conn -> + case conn.path_info do + [_ws, _version, "release-group"] -> + Req.Test.json(conn, release_group_search_results()) + end + end) + + [first, second | _] = Map.get(release_group_search_results(), "release-groups") + first_id = first["id"] + second_id = second["id"] + + conn + |> visit(~p"/wishlist/import") + |> fill_in("Search for a record", with: "Marillion Marbles") + |> click_link("#musicbrainz_#{first_id} a", "CD") + |> click_link("#musicbrainz_#{second_id} a", "Vinyl") + |> click_button("Import 2 records") + |> assert_has("p", text: "Importing 2 records in the background...") + + assert_enqueued( + worker: ImportFromMusicbrainzReleaseGroup, + args: %{ + "release_group_id" => first_id, + "format" => "cd", + "purchased_at" => nil + } + ) + + assert_enqueued( + worker: ImportFromMusicbrainzReleaseGroup, + args: %{ + "release_group_id" => second_id, + "format" => "vinyl", + "purchased_at" => nil + } + ) + + import Ecto.Query, only: [from: 2] + query = from(r in Record, where: not is_nil(r.purchased_at)) + assert MusicLibrary.Repo.all(query) == [] + end + end end