From fee2799d5088c141ef20ae60c579b88b359b4c8d Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Fri, 7 Mar 2025 09:14:27 +0000 Subject: [PATCH] Show barcodes that are not found inside the results UI Includes a refactor to extract/cleanup barcode scan logic from the component to a separate context with a better API. --- lib/music_library/barcode_scan.ex | 48 +++++++++ lib/music_library/barcode_scan/result.ex | 36 +++++++ lib/music_library/records.ex | 6 +- .../components/barcode_scanner_component.ex | 100 ++++++++---------- priv/gettext/default.pot | 28 ++--- .../live/collection_live/index_test.exs | 1 + 6 files changed, 149 insertions(+), 70 deletions(-) create mode 100644 lib/music_library/barcode_scan.ex create mode 100644 lib/music_library/barcode_scan/result.ex diff --git a/lib/music_library/barcode_scan.ex b/lib/music_library/barcode_scan.ex new file mode 100644 index 00000000..31f19b85 --- /dev/null +++ b/lib/music_library/barcode_scan.ex @@ -0,0 +1,48 @@ +defmodule MusicLibrary.BarcodeScan do + alias MusicLibrary.BarcodeScan.Result + alias MusicLibrary.Records + + def scan(number) do + case MusicBrainz.search_release_by_barcode(number) do + {:ok, [best_match_release | _other_releases]} -> + format = MusicBrainz.ReleaseSearchResult.format(best_match_release) + + case Records.get_release_status(best_match_release.id, format) do + :new -> + {:ok, Result.new(number, best_match_release)} + + {:wishlisted, record_id} -> + {:ok, Result.wishlisted(number, record_id, best_match_release)} + + {:collected, record_id} -> + {:ok, Result.collected(number, record_id, best_match_release)} + end + + {:ok, []} -> + {:ok, Result.not_found(number)} + + error -> + error + end + end + + def import(scan_result, current_time) do + case scan_result.status do + :new -> + Records.import_from_musicbrainz_release(scan_result.release.id, + format: MusicBrainz.ReleaseSearchResult.format(scan_result.release), + purchased_at: current_time + ) + + :wishlisted -> + record = Records.get_record!(scan_result.record_id) + Records.update_record(record, %{"purchased_at" => current_time}) + + :collected -> + {:error, :already_collected} + + :not_found -> + {:error, :not_found} + end + end +end diff --git a/lib/music_library/barcode_scan/result.ex b/lib/music_library/barcode_scan/result.ex new file mode 100644 index 00000000..9e2c0ed2 --- /dev/null +++ b/lib/music_library/barcode_scan/result.ex @@ -0,0 +1,36 @@ +defmodule MusicLibrary.BarcodeScan.Result do + defstruct [:status, :number, :record_id, :release] + + def new(number, release) do + %__MODULE__{ + number: number, + status: :new, + release: release + } + end + + def wishlisted(number, record_id, release) do + %__MODULE__{ + number: number, + status: :wishlisted, + record_id: record_id, + release: release + } + end + + def collected(number, record_id, release) do + %__MODULE__{ + number: number, + status: :collected, + record_id: record_id, + release: release + } + end + + def not_found(number) do + %__MODULE__{ + number: number, + status: :not_found + } + end +end diff --git a/lib/music_library/records.ex b/lib/music_library/records.ex index 6ba254a2..02745507 100644 --- a/lib/music_library/records.ex +++ b/lib/music_library/records.ex @@ -107,7 +107,11 @@ defmodule MusicLibrary.Records do purchased_at: fragment("records.purchased_at") } - Repo.one(q) + case Repo.one(q) do + nil -> :new + %{record_id: record_id, purchased_at: nil} -> {:wishlisted, record_id} + %{record_id: record_id} -> {:collected, record_id} + end end def get_artist_records(musicbrainz_id) do diff --git a/lib/music_library_web/components/barcode_scanner_component.ex b/lib/music_library_web/components/barcode_scanner_component.ex index ac402090..e6b14dd8 100644 --- a/lib/music_library_web/components/barcode_scanner_component.ex +++ b/lib/music_library_web/components/barcode_scanner_component.ex @@ -2,6 +2,7 @@ defmodule MusicLibraryWeb.BarcodeScannerComponent do use MusicLibraryWeb, :live_component alias MusicBrainz.ReleaseGroupSearchResult + alias MusicLibrary.BarcodeScan alias MusicLibrary.Records alias MusicLibraryWeb.RecordComponents @@ -12,7 +13,7 @@ defmodule MusicLibraryWeb.BarcodeScannerComponent do {:ok, socket |> assign(:camera, :pending) - |> assign(:releases, [])} + |> assign(:scan_results, [])} end @impl true @@ -31,8 +32,8 @@ defmodule MusicLibraryWeb.BarcodeScannerComponent do
<.button - disabled={length(@releases) == 0} + disabled={length(@scan_results) == 0} phx-disable-with={gettext("Importing...")} phx-click={JS.push("import_releases", target: "#barcode-scanner")} > @@ -94,6 +95,35 @@ defmodule MusicLibraryWeb.BarcodeScannerComponent do """ end + attr :scan_result, BarcodeScan.Result, required: true + + defp scan_result(assigns) do + ~H""" + <.barcode_not_found :if={@scan_result.status == :not_found} number={@scan_result.number} /> + <.release + :if={@scan_result.status != :not_found} + release={@scan_result.release} + record_id={@scan_result.record_id} + status={@scan_result.status} + /> + """ + end + + attr :number, :string, required: true + + defp barcode_not_found(assigns) do + ~H""" +
+

+ {gettext("Barcode not found")} +

+

+ {@number} +

+
+ """ + end + attr :release, MusicBrainz.ReleaseSearchResult, required: true attr :record_id, :string attr :status, :atom, required: true, values: [:collected, :wishlisted, :new] @@ -133,35 +163,23 @@ defmodule MusicLibraryWeb.BarcodeScannerComponent do @impl true def handle_event("camera_allowed", _params, socket) do - Logger.debug(fn -> "Camera access allowed" end) {:noreply, assign(socket, camera: :allowed)} end def handle_event("camera_denied", _params, socket) do - Logger.debug(fn -> "Camera access denied" end) {:noreply, assign(socket, camera: :denied)} end def handle_event("barcode_scanned", %{"number" => number}, socket) do - Logger.debug(fn -> "Scanned barcode #{number}" end) - socket = - case MusicBrainz.search_release_by_barcode(number) do - {:ok, [best_match_release | _other_releases]} -> - Logger.debug(fn -> "Found release #{best_match_release.id}" end) - assign_release_with_status(best_match_release, socket) + case BarcodeScan.scan(number) do + {:ok, scan_result} -> + assign(socket, :scan_results, [scan_result | socket.assigns.scan_results]) - {:ok, []} -> - Logger.debug(fn -> "No release found for barcode #{number}" end) - - put_flash( - socket, - :error, - gettext("No release found for barcode %{number}", number: number) - ) - - {:error, _reason} -> - Logger.error(fn -> "Failed to search release for barcode #{number}" end) + {:error, reason} -> + Logger.error(fn -> + "Failed to search release for barcode #{number}: #{inspect(reason)}" + end) put_flash( socket, @@ -177,47 +195,19 @@ defmodule MusicLibraryWeb.BarcodeScannerComponent do current_time = DateTime.utc_now() # TODO: error handling when a release fails to import :ok = - Enum.each(socket.assigns.releases, fn {status, record_id, release} -> - if status == :new do - Records.import_from_musicbrainz_release(release.id, - format: MusicBrainz.ReleaseSearchResult.format(release), - purchased_at: current_time - ) - end - - if status == :wishlisted do - record = Records.get_record!(record_id) - Records.update_record(record, %{"purchased_at" => current_time}) - end + Enum.each(socket.assigns.scan_results, fn scan_result -> + BarcodeScan.import(scan_result, current_time) end) qs = %{order: :purchase} {:noreply, socket - |> assign(:releases, []) + |> assign(:scan_results, []) |> put_flash(:info, gettext("Records imported successfully")) |> push_patch(to: ~p"/collection?#{qs}")} end - defp assign_release_with_status(release, socket) do - format = MusicBrainz.ReleaseSearchResult.format(release) - - release_with_status = - case Records.get_release_status(release.id, format) do - nil -> - {:new, nil, release} - - %{record_id: record_id, purchased_at: nil} -> - {:wishlisted, record_id, release} - - %{record_id: record_id} -> - {:collected, record_id, release} - end - - assign(socket, :releases, [release_with_status | socket.assigns.releases]) - end - defp release_format_label(release) do release |> MusicBrainz.ReleaseSearchResult.format() diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 2f0752af..17ed37a5 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -338,13 +338,13 @@ msgstr "" msgid "Choose which format to import" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:128 +#: lib/music_library_web/components/barcode_scanner_component.ex:158 #: lib/music_library_web/live/stats_live/index.html.heex:141 #, elixir-autogen, elixir-format msgid "Collected" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:125 +#: lib/music_library_web/components/barcode_scanner_component.ex:155 #: lib/music_library_web/live/stats_live/index.html.heex:147 #, elixir-autogen, elixir-format msgid "Wishlisted" @@ -616,42 +616,37 @@ msgstr "" msgid "Scan barcodes ยท Collection" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:24 +#: lib/music_library_web/components/barcode_scanner_component.ex:25 #, elixir-autogen, elixir-format msgid "Scan one or more barcodes" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:91 +#: lib/music_library_web/components/barcode_scanner_component.ex:92 #, elixir-autogen, elixir-format msgid "Open camera" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:55 +#: lib/music_library_web/components/barcode_scanner_component.ex:56 #, elixir-autogen, elixir-format msgid "Import releases" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:169 +#: lib/music_library_web/components/barcode_scanner_component.ex:187 #, elixir-autogen, elixir-format msgid "Failed to search release for barcode %{number}" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:160 -#, elixir-autogen, elixir-format -msgid "No release found for barcode %{number}" -msgstr "" - -#: lib/music_library_web/components/barcode_scanner_component.ex:52 +#: lib/music_library_web/components/barcode_scanner_component.ex:53 #, elixir-autogen, elixir-format msgid "Importing..." msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:199 +#: lib/music_library_web/components/barcode_scanner_component.ex:207 #, elixir-autogen, elixir-format msgid "Records imported successfully" msgstr "" -#: lib/music_library_web/components/barcode_scanner_component.ex:123 +#: lib/music_library_web/components/barcode_scanner_component.ex:153 #, elixir-autogen, elixir-format msgid "New" msgstr "" @@ -690,3 +685,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Live Dashboard" msgstr "" + +#: lib/music_library_web/components/barcode_scanner_component.ex:118 +#, elixir-autogen, elixir-format +msgid "Barcode not found" +msgstr "" 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 1685763c..9758cbc0 100644 --- a/test/music_library_web/live/collection_live/index_test.exs +++ b/test/music_library_web/live/collection_live/index_test.exs @@ -385,6 +385,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do |> visit(~p"/collection/scan") |> trigger_hook("#barcode-scanner", "barcode_scanned", %{"number" => barcode}) |> assert_has("h2", text: "Marbles") + |> assert_has("span", text: "New") |> click_button("Import releases") [record] = MusicLibrary.Repo.all(MusicLibrary.Records.Record)