diff --git a/web/src/crate_web/pages/add.gleam b/web/src/crate_web/pages/add.gleam index f94aec8..90bb60d 100644 --- a/web/src/crate_web/pages/add.gleam +++ b/web/src/crate_web/pages/add.gleam @@ -54,8 +54,7 @@ fn add_form_view(model: Model) -> Element(Msg) { frm.field("FOLDER", "Rock A-M", form.folder, FormFolder, "text"), acquisition_view(form), form_error_region(model.form_error), - ctl.button(save_label(model.busy), ctl.Primary, [ - attr.type_("submit"), + ctl.submit_button(save_label(model.busy), ctl.Primary, [ attr.disabled(model.busy), attr.class("btn--block"), ]), diff --git a/web/src/crate_web/pages/login.gleam b/web/src/crate_web/pages/login.gleam index 2c3b0c6..cd9543e 100644 --- a/web/src/crate_web/pages/login.gleam +++ b/web/src/crate_web/pages/login.gleam @@ -161,9 +161,8 @@ fn login_action(handle: String) -> Element(Msg) { // FFI. An anchor would be hijacked by modem (same-origin) and never reach // the server. Submission lives on the form so Enter works too. let disabled = string.trim(handle) == "" - ctl.button("LOG IN WITH ATPROTO →", ctl.Primary, [ + ctl.submit_button("LOG IN WITH ATPROTO →", ctl.Primary, [ attr.disabled(disabled), attr.class("btn--block"), - attr.type_("submit"), ]) } diff --git a/web/src/crate_web/ui/controls.gleam b/web/src/crate_web/ui/controls.gleam index 5a00257..90b8e43 100644 --- a/web/src/crate_web/ui/controls.gleam +++ b/web/src/crate_web/ui/controls.gleam @@ -76,12 +76,31 @@ pub fn status_badge(status: String) -> Element(msg) { } } +/// Always `type="button"`: inside a `
` a bare button is a submit +/// button, so an `on_click` sticker would fire its message and submit the +/// form. Form submits go through `submit_button`, which is the only way to +/// get `type="submit"` here; passing one in `attrs` would emit a duplicate +/// `type` attribute rather than override it. pub fn button( label: String, variant: Btn, attrs: List(attr.Attribute(msg)), ) -> Element(msg) { - html.button([attr.class(btn_class(variant)), ..attrs], [text(label)]) + html.button([attr.class(btn_class(variant)), attr.type_("button"), ..attrs], [ + text(label), + ]) +} + +/// The one button that submits its enclosing form; everything else is a +/// `button`. +pub fn submit_button( + label: String, + variant: Btn, + attrs: List(attr.Attribute(msg)), +) -> Element(msg) { + html.button([attr.class(btn_class(variant)), attr.type_("submit"), ..attrs], [ + text(label), + ]) } pub fn link_button(label: String, variant: Btn, href: String) -> Element(msg) { diff --git a/web/src/crate_web/update/add.gleam b/web/src/crate_web/update/add.gleam index f77c236..7561527 100644 --- a/web/src/crate_web/update/add.gleam +++ b/web/src/crate_web/update/add.gleam @@ -12,6 +12,7 @@ import crate_web/msg.{ type Msg, ArtistSearch, DisarmDiscogsDisconnect, DiscogsSearch, } import crate_web/update/common.{failed, succeeded, write_error} +import gleam/bool import gleam/int import gleam/list import gleam/option.{type Option, None, Some} @@ -105,7 +106,10 @@ pub fn form_counterparty(model: Model, value: String) -> #(Model, Effect(Msg)) { ) } +/// Guarded on `busy`, so a double dispatch (a stray form submit racing the +/// button's own click) can never produce two `add_item` writes. pub fn submit_add(model: Model) -> #(Model, Effect(Msg)) { + use <- bool.guard(when: model.busy, return: #(model, effect.none())) case string.trim(model.form.title), string.trim(model.form.artist), diff --git a/web/test/add_test.gleam b/web/test/add_test.gleam index 6a9664e..e91c890 100644 --- a/web/test/add_test.gleam +++ b/web/test/add_test.gleam @@ -8,10 +8,12 @@ import crate_web/msg.{ } import crate_web/pages/add import crate_web/update.{update} +import gleam/list import gleam/option.{None, Some} +import gleam/result import gleam/string import lustre/element -import support.{base, logged_in} +import support.{base, empty_effect, logged_in} // Validation failures are page-local: they never touch the entered fields, // and they show as an inline region above SAVE TO CRATE rather than the @@ -167,11 +169,50 @@ pub fn no_form_error_renders_nothing_extra_test() { assert !string.contains(html, "class=\"error-state\"") } -pub fn segment_buttons_never_submit_the_form_test() { +/// Inside a `` a bare `", - ) + let back = "aria-label=\"Back\"" + assert string.contains(tag_with(html, "button", back), "class=\"icon-btn\"") + assert list.filter(open_tags(html, "a"), string.contains(_, back)) == [] } pub fn every_route_change_bumps_nav_depth_test() { diff --git a/web/test/record_amend_test.gleam b/web/test/record_amend_test.gleam index 3f22d81..9bbfac8 100644 --- a/web/test/record_amend_test.gleam +++ b/web/test/record_amend_test.gleam @@ -140,7 +140,12 @@ pub fn record_amend_page_titles_itself_amend_regardless_of_provenance_test() { ShelfLoaded(Own, [an_entry()]), ) let html = view.view(seeded) |> element.to_string - assert string.contains(html, "AMEND") + // "AMEND" alone is always present: the submit button reads PUBLISH + // AMENDMENT. Assert the app bar's own title instead. + assert string.contains( + html, + "AMEND", + ) }) } diff --git a/web/test/record_test.gleam b/web/test/record_test.gleam index c29127b..ac5b76e 100644 --- a/web/test/record_test.gleam +++ b/web/test/record_test.gleam @@ -14,7 +14,7 @@ import gleam/dict import gleam/option.{type Option, None, Some} import gleam/string import lustre/element -import support.{a_detail, an_entry, logged_in} +import support.{a_detail, an_entry, logged_in, tag_with} /// `a_detail()` with its folded entry swapped for `entry`. fn detail(entry: Entry) -> EntryDetail { @@ -188,10 +188,8 @@ pub fn record_view_shows_a_copy_share_link_button_with_the_public_path_test() { // screen, not a button dispatching a toggle message. pub fn record_view_renders_the_amend_action_as_a_link_to_its_own_route_test() { let html = record.view(logged_in(), a_detail()) |> element.to_string - assert string.contains( - html, - "AMEND", - ) + assert string.contains(amend_link(html), "class=\"btn btn--ghost\"") + assert string.contains(html, ">AMEND") } // AMEND is the one canonical label for the action regardless of who minted @@ -201,13 +199,15 @@ pub fn record_view_labels_the_amend_link_as_amend_for_an_adopted_entry_too_test( let html = record.view(Model(..logged_in(), via_handles:), a_detail()) |> element.to_string - assert string.contains( - html, - "AMEND", - ) + assert string.contains(amend_link(html), "class=\"btn btn--ghost\"") + assert string.contains(html, ">AMEND") assert !string.contains(html, "SUGGEST A FIX") } +fn amend_link(html: String) -> String { + tag_with(html, "a", "href=\"/record/e1/amend\"") +} + pub fn record_view_labels_move_to_history_not_remove_test() { let html = record.view(logged_in(), a_detail()) |> element.to_string assert string.contains(html, "MOVE TO HISTORY") diff --git a/web/test/states_test.gleam b/web/test/states_test.gleam index 03a724d..0fea21c 100644 --- a/web/test/states_test.gleam +++ b/web/test/states_test.gleam @@ -1,38 +1,53 @@ //// Shared async-state primitives: the ARIA live regions on loading/error, -//// and that the retry variants wire RETRY without forcing it on callers -//// that have nothing to retry. +//// which sticker headline each failure earns, and which of them offer a +//// RETRY the visitor can actually act on. import crate_web/ui/states +import gleam/list import gleam/string -import lustre/element +import lustre/element.{type Element} pub type TestMsg { Retry } -pub fn loading_page_announces_as_status_test() { - let html = states.loading_page() |> element.to_string - assert string.contains(html, "role=\"status\"") +fn rendered(el: Element(TestMsg)) -> String { + element.to_string(el) |> string.replace("'", "'") } -pub fn failed_page_announces_as_alert_test() { - let html = states.failed_page("Couldn't load.") |> element.to_string - assert string.contains(html, "role=\"alert\"") +/// A RETRY only belongs on a state where re-running the same action could +/// land differently: a fetch or a write, never client-side validation. +pub fn each_state_announces_itself_and_offers_retry_only_where_it_helps_test() { + [ + #(states.loading_page(), "status", False), + #(states.failed_page("Couldn't load."), "alert", False), + #(states.failed_page_retry("Couldn't load.", Retry), "alert", True), + #(states.error_sticker("Nope."), "alert", False), + #(states.error_sticker_retry("Nope.", Retry), "alert", True), + #(states.write_error_sticker("Nope.", Retry), "alert", True), + #(states.invalid_input_sticker("Nope."), "alert", False), + ] + |> list.each(fn(row) { + let #(el, role, retryable) = row + let html = rendered(el) + assert string.contains(html, "role=\"" <> role <> "\"") + assert string.contains(html, "RETRY") == retryable + }) } -pub fn failed_page_has_no_retry_action_by_default_test() { - let html = states.failed_page("Couldn't load.") |> element.to_string - assert !string.contains(html, "RETRY") -} - -pub fn failed_page_retry_wires_the_retry_message_test() { - let html = - states.failed_page_retry("Couldn't load.", Retry) |> element.to_string - assert string.contains(html, "RETRY") - assert string.contains(html, "role=\"alert\"") -} - -pub fn error_sticker_retry_wires_the_retry_message_test() { - let html = states.error_sticker_retry("Nope.", Retry) |> element.to_string - assert string.contains(html, "RETRY") +/// The headline names what actually went wrong: a failed write never loaded +/// anything, and a rejected input is not a failure of the app at all. +pub fn each_sticker_names_its_own_kind_of_failure_test() { + [ + #(states.error_sticker("x"), "✕ COULDN'T LOAD"), + #(states.error_sticker_retry("x", Retry), "✕ COULDN'T LOAD"), + #(states.write_error_sticker("x", Retry), "✕ COULDN'T SAVE"), + #(states.invalid_input_sticker("x"), "✕ CHECK YOUR ENTRIES"), + ] + |> list.each(fn(row) { + let #(el, sticker) = row + let html = rendered(el) + assert string.contains(html, sticker) + assert string.contains(html, "x") + }) } diff --git a/web/test/support.gleam b/web/test/support.gleam index e62802d..94a8883 100644 --- a/web/test/support.gleam +++ b/web/test/support.gleam @@ -13,7 +13,10 @@ import crate_web/model.{ import crate_web/msg.{type ApiError} import crate_web/pages/crate import gleam/dict +import gleam/list import gleam/option.{None, Some} +import gleam/result +import gleam/string import lustre/effect.{type Effect} import lustre/element @@ -164,3 +167,24 @@ pub fn discogs_result(id: Int) -> DiscogsResult { cover_url: None, ) } + +/// Every `` open tag in `html`, attributes only. Assertions use +/// this instead of a literal tag string so a Lustre release that reorders +/// attribute serialisation can't red the suite over nothing. +pub fn open_tags(html: String, name: String) -> List(String) { + string.split(html, "<" <> name) + |> list.drop(1) + |> list.filter(fn(rest) { + string.starts_with(rest, " ") || string.starts_with(rest, ">") + }) + |> list.map(fn(rest) { + string.split(rest, ">") |> list.first |> result.unwrap("") + }) +} + +/// The first `name` open tag carrying `needle`, or "" when there is none. +pub fn tag_with(html: String, name: String, needle: String) -> String { + open_tags(html, name) + |> list.find(string.contains(_, needle)) + |> result.unwrap("") +} -- 2.51.2 From 8de47ea5fd61077ab1e83c11a76aa9a13b20bb82 Mon Sep 17 00:00:00 2001 From: Niels Mokkenstorm Date: Mon, 10 Aug 2026 13:55:04 +0200 Subject: [PATCH 5/8] fix: light the add slot on the scan flow it links into --- web/src/crate_web/route.gleam | 10 +++++----- web/test/nav_test.gleam | 23 +++++++++++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/web/src/crate_web/route.gleam b/web/src/crate_web/route.gleam index 9343604..4f18f60 100644 --- a/web/src/crate_web/route.gleam +++ b/web/src/crate_web/route.gleam @@ -49,15 +49,15 @@ pub fn to_path(route: Route) -> String { /// Which bottom-tab section a route belongs to, so drill-downs off a tab /// (a record, the scan flow, a pressing) still show that tab active instead -/// of none. `Add` owns its own section rather than folding into `Crate`, so -/// the CRATE tab doesn't light up while the visitor is on the add form; the -/// centre "+" slot lights up instead, see `nav.add_tab`. +/// of none. The centre "+" slot owns the whole add flow, scan included: it +/// links into `Scan`, so anything less would have it go dark on its own +/// destination while CRATE lit up instead (see `nav.add_tab`). /// `PublicCrate`/`PublicRecord` map to themselves since they're reached /// outside the tab bar entirely. pub fn section(route: Route) -> Route { case route { - Crate | Scan | ScanReview | ScanDone | Record(_) | RecordAmend(_) -> Crate - Add -> Add + Crate | Record(_) | RecordAmend(_) -> Crate + Add | Scan | ScanReview | ScanDone -> Add Browse | PressingDetail(_, _) -> Browse Feed -> Feed Settings | EditInbox | EditProposalDetail(_) -> Settings diff --git a/web/test/nav_test.gleam b/web/test/nav_test.gleam index 971bc1a..cc2da9f 100644 --- a/web/test/nav_test.gleam +++ b/web/test/nav_test.gleam @@ -19,7 +19,7 @@ import gleam/string import lustre/element import support.{an_entry, empty_effect, logged_in, open_tags, tag_with} -/// Every drill-down maps to its tab's own section (crate/browse/settings +/// Every drill-down maps to its tab's own section (crate/add/browse/settings /// each own several, feed maps only to itself); public routes are the one /// family left unmapped (each maps to itself instead of a real section). pub fn route_section_maps_every_drilldown_to_its_tab_test() { @@ -28,9 +28,9 @@ pub fn route_section_maps_every_drilldown_to_its_tab_test() { #(Record("e1"), Crate), #(RecordAmend("e1"), Crate), #(Add, Add), - #(Scan, Crate), - #(ScanReview, Crate), - #(ScanDone, Crate), + #(Scan, Add), + #(ScanReview, Add), + #(ScanDone, Add), #(Browse, Browse), #(PressingDetail("did:plc:abc", "3jz"), Browse), #(Feed, Feed), @@ -141,3 +141,18 @@ pub fn every_route_change_clears_the_page_scoped_state_test() { assert after.discogs.confirm_disconnect == False }) } + +/// The "+" slot links into the scan flow, so it has to light up there: a +/// control that goes dark on its own destination (and hands the highlight +/// to CRATE) is telling the visitor they went somewhere else. +pub fn the_add_slot_lights_up_on_its_own_destination_test() { + [Scan, ScanReview, ScanDone, Add] + |> list.each(fn(destination) { + let html = + view.view(Model(..logged_in(), route: destination)) |> element.to_string + let slot = tag_with(html, "a", "aria-label=\"Add a record\"") + assert string.contains(slot, "class=\"tab-add is-active\"") + assert string.contains(slot, "aria-current=\"page\"") + assert !string.contains(tag_with(html, "a", "href=\"/\""), "is-active") + }) +} -- 2.51.2 From 5fd134a4c994a9af9024c84591519e6361c97ef0 Mon Sep 17 00:00:00 2001 From: Niels Mokkenstorm Date: Mon, 10 Aug 2026 14:02:54 +0200 Subject: [PATCH 6/8] refactor: model the discogs import as one kind instead of two flags --- web/src/crate_web/model.gleam | 22 ++++++------ web/src/crate_web/msg.gleam | 7 ++-- web/src/crate_web/pages/settings.gleam | 20 +++++------ web/src/crate_web/update.gleam | 3 +- web/src/crate_web/update/add.gleam | 46 ++++++++++---------------- web/test/settings_test.gleam | 46 +++++++++++++------------- 6 files changed, 63 insertions(+), 81 deletions(-) diff --git a/web/src/crate_web/model.gleam b/web/src/crate_web/model.gleam index b572b44..9c3a5c2 100644 --- a/web/src/crate_web/model.gleam +++ b/web/src/crate_web/model.gleam @@ -824,10 +824,16 @@ pub type Model { ) } +/// Which Discogs import a run is; only one import can be in flight at a time, +/// and the button that started it is the only one labelled IMPORTING…. +pub type ImportKind { + ImportCollection + ImportWantlist +} + /// Discogs seed/import state: the autocomplete query and its paged results, /// the connected account, and which of the two import runs (if any) is in -/// flight. Collection and wantlist imports get their own flag so triggering -/// one never shows IMPORTING… on the other's button too. +/// flight. pub type Discogs { Discogs( query: String, @@ -837,8 +843,7 @@ pub type Discogs { page: Int, pages: Int, username: Option(String), - importing_collection: Bool, - importing_wantlist: Bool, + importing: Option(ImportKind), // Armed like `confirm_remove`: first tap arms, second tap executes. confirm_disconnect: Bool, // The last import/disconnect failure, shown inline on the Discogs card @@ -860,8 +865,7 @@ pub fn blank_discogs() -> Discogs { page: 1, pages: 0, username: None, - importing_collection: False, - importing_wantlist: False, + importing: None, confirm_disconnect: False, error: None, artist_query: "", @@ -869,9 +873,3 @@ pub fn blank_discogs() -> Discogs { artist_id: None, ) } - -/// Whether either Discogs import is currently in flight, for callers that -/// don't need to know which. -pub fn discogs_importing(discogs: Discogs) -> Bool { - discogs.importing_collection || discogs.importing_wantlist -} diff --git a/web/src/crate_web/msg.gleam b/web/src/crate_web/msg.gleam index e49a94a..89318fa 100644 --- a/web/src/crate_web/msg.gleam +++ b/web/src/crate_web/msg.gleam @@ -4,8 +4,8 @@ import crate/gen/shelf/entry.{type ShelfEntry} import crate_web/model.{ type ArtistHit, type BrowseRelease, type DiscogsResult, type DiscogsSearchPage, type Display, type EditProposal, type Entry, type HandleSuggestion, - type ImportRun, type NetworkMatch, type ReleaseInfo, type Route, type ScanMode, - type Suggestion, type Theme, + type ImportKind, type ImportRun, type NetworkMatch, type ReleaseInfo, + type Route, type ScanMode, type Suggestion, type Theme, } import crate_web/photo_scan.{type DecodeError} import gleam/dict @@ -143,8 +143,7 @@ pub type Msg { DiscogsConnect DiscogsDisconnect GotDiscogsDisconnect(Result(Nil, ApiError)) - DiscogsImport - DiscogsImportWantlist + DiscogsImport(kind: ImportKind) GotDiscogsImport(Result(ImportRun, ApiError)) AmendField(field: String, value: String) ToggleAmendCover(Bool) diff --git a/web/src/crate_web/pages/settings.gleam b/web/src/crate_web/pages/settings.gleam index 913018c..ac6e3c6 100644 --- a/web/src/crate_web/pages/settings.gleam +++ b/web/src/crate_web/pages/settings.gleam @@ -3,11 +3,12 @@ //// out. Reached by tapping the app-bar avatar. import crate_web/model.{ - type Discogs, type Model, type Theme, EditInbox, LoggedIn, + type Discogs, type ImportKind, type Model, type Theme, EditInbox, + ImportCollection, ImportWantlist, LoggedIn, } import crate_web/msg.{ type Msg, ArmDiscogsDisconnect, ArmLogout, DiscogsConnect, DiscogsDisconnect, - DiscogsImport, DiscogsImportWantlist, Logout, SetTheme, + DiscogsImport, Logout, SetTheme, } import crate_web/route import crate_web/ui/app_bar as bar @@ -119,16 +120,14 @@ fn discogs_account_view(model: Model) -> Element(Msg) { import_button( "IMPORT COLLECTION", ctl.Dark, - model.discogs.importing_collection, + ImportCollection, model.discogs, - DiscogsImport, ), import_button( "IMPORT WANTLIST", ctl.Ghost, - model.discogs.importing_wantlist, + ImportWantlist, model.discogs, - DiscogsImportWantlist, ), disconnect_button(model.discogs.confirm_disconnect), ]) @@ -143,20 +142,19 @@ fn discogs_account_view(model: Model) -> Element(Msg) { fn import_button( label: String, variant: ctl.Btn, - running: Bool, + kind: ImportKind, discogs: Discogs, - on_click: Msg, ) -> Element(Msg) { ctl.button( - case running { + case discogs.importing == option.Some(kind) { True -> "IMPORTING…" False -> label }, variant, [ attr.class("btn--import"), - attr.disabled(model.discogs_importing(discogs)), - event.on_click(on_click), + attr.disabled(option.is_some(discogs.importing)), + event.on_click(DiscogsImport(kind)), ], ) } diff --git a/web/src/crate_web/update.gleam b/web/src/crate_web/update.gleam index 951368e..f72c478 100644 --- a/web/src/crate_web/update.gleam +++ b/web/src/crate_web/update.gleam @@ -81,8 +81,7 @@ pub fn update(model: Model, msg: Msg) -> #(Model, Effect(Msg)) { msg.DiscogsDisconnect -> add.discogs_disconnect_msg(model) msg.GotDiscogsDisconnect(result) -> add.got_discogs_disconnect(model, result) - msg.DiscogsImport -> add.discogs_import_msg(model) - msg.DiscogsImportWantlist -> add.discogs_import_wantlist_msg(model) + msg.DiscogsImport(kind) -> add.discogs_import_msg(model, kind) msg.GotDiscogsImport(result) -> add.got_discogs_import(model, result) msg.AmendField(field, value) -> amend.amend_field(model, field, value) diff --git a/web/src/crate_web/update/add.gleam b/web/src/crate_web/update/add.gleam index 0303fac..345b9e3 100644 --- a/web/src/crate_web/update/add.gleam +++ b/web/src/crate_web/update/add.gleam @@ -4,8 +4,9 @@ import crate_web/effects.{ discogs_import_wantlist, discogs_search, load_shelf, } import crate_web/model.{ - type ArtistHit, type DiscogsResult, type DiscogsSearchPage, type ImportRun, - type Model, Discogs, Form, InvalidInput, Model, WriteFailed, blank_form, + type ArtistHit, type DiscogsResult, type DiscogsSearchPage, type ImportKind, + type ImportRun, type Model, Discogs, Form, ImportCollection, ImportWantlist, + InvalidInput, Model, WriteFailed, blank_form, } import crate_web/money import crate_web/msg.{ @@ -319,35 +320,27 @@ pub fn got_discogs_disconnect( /// Guards against a double-fire (tapping IMPORT COLLECTION and IMPORT /// WANTLIST back to back): the first import already in flight wins. -pub fn discogs_import_msg(model: Model) -> #(Model, Effect(Msg)) { - case model.discogs_importing(model.discogs) { - True -> #(model, effect.none()) - False -> #( +pub fn discogs_import_msg( + model: Model, + kind: ImportKind, +) -> #(Model, Effect(Msg)) { + case model.discogs.importing { + Some(_) -> #(model, effect.none()) + None -> #( Model( ..model, - discogs: Discogs( - ..model.discogs, - importing_collection: True, - error: None, - ), + discogs: Discogs(..model.discogs, importing: Some(kind), error: None), notice: None, ), - discogs_import(), + import_effect(kind), ) } } -pub fn discogs_import_wantlist_msg(model: Model) -> #(Model, Effect(Msg)) { - case model.discogs_importing(model.discogs) { - True -> #(model, effect.none()) - False -> #( - Model( - ..model, - discogs: Discogs(..model.discogs, importing_wantlist: True, error: None), - notice: None, - ), - discogs_import_wantlist(), - ) +fn import_effect(kind: ImportKind) -> Effect(Msg) { + case kind { + ImportCollection -> discogs_import() + ImportWantlist -> discogs_import_wantlist() } } @@ -355,12 +348,7 @@ pub fn got_discogs_import( model: Model, result: Result(ImportRun, msg.ApiError), ) -> #(Model, Effect(Msg)) { - let idle = - Discogs( - ..model.discogs, - importing_collection: False, - importing_wantlist: False, - ) + let idle = Discogs(..model.discogs, importing: None) case result { Ok(run) -> #( Model( diff --git a/web/test/settings_test.gleam b/web/test/settings_test.gleam index e9894f2..15fe197 100644 --- a/web/test/settings_test.gleam +++ b/web/test/settings_test.gleam @@ -1,12 +1,12 @@ import crate/gen/discogs/import_collection import crate_web/model.{ - type Model, Dark, Discogs, Failure, Model, Notice, Settings, Success, - blank_discogs, + type Model, Dark, Discogs, Failure, ImportCollection, ImportWantlist, Model, + Notice, Settings, Success, blank_discogs, } import crate_web/msg.{ ArmDiscogsDisconnect, ArmLogout, DisarmDiscogsDisconnect, DisarmLogout, - DiscogsDisconnect, DiscogsImport, DiscogsImportWantlist, GotDiscogsDisconnect, - GotDiscogsImport, GotDiscogsStatus, + DiscogsDisconnect, DiscogsImport, GotDiscogsDisconnect, GotDiscogsImport, + GotDiscogsStatus, } import crate_web/pages/settings import crate_web/route @@ -30,44 +30,45 @@ pub fn discogs_status_sets_username_test() { assert model.discogs.username == Some("crate-digger") } -pub fn discogs_import_wantlist_marks_only_the_wantlist_flag_test() { +pub fn discogs_import_wantlist_marks_the_wantlist_run_test() { let model = Model( ..logged_in(), notice: Some(Notice(Failure, "stale")), discogs: blank_discogs(), ) - let #(after, _) = update(model, DiscogsImportWantlist) - assert after.discogs.importing_wantlist == True - assert after.discogs.importing_collection == False + let #(after, _) = update(model, DiscogsImport(ImportWantlist)) + assert after.discogs.importing == Some(ImportWantlist) assert after.notice == None } -pub fn discogs_import_marks_only_the_collection_flag_test() { +pub fn discogs_import_marks_the_collection_run_test() { let #(after, _) = - update(Model(..logged_in(), discogs: blank_discogs()), DiscogsImport) - assert after.discogs.importing_collection == True - assert after.discogs.importing_wantlist == False + update( + Model(..logged_in(), discogs: blank_discogs()), + DiscogsImport(ImportCollection), + ) + assert after.discogs.importing == Some(ImportCollection) } // A stray double-tap (or the two buttons fired back to back) must not run -// two imports at once, whichever one already claimed the flag. +// two imports at once, whichever one already claimed the slot. pub fn a_second_import_is_a_no_op_while_one_is_running_test() { let running = Model( ..logged_in(), - discogs: Discogs(..blank_discogs(), importing_collection: True), + discogs: Discogs(..blank_discogs(), importing: Some(ImportCollection)), ) - let #(after_wantlist, effect) = update(running, DiscogsImportWantlist) - assert after_wantlist.discogs.importing_wantlist == False + let #(after_wantlist, effect) = update(running, DiscogsImport(ImportWantlist)) + assert after_wantlist.discogs.importing == Some(ImportCollection) assert effect == support.empty_effect() } -pub fn import_result_summarises_partial_run_and_clears_both_flags_test() { +pub fn import_result_summarises_partial_run_and_clears_the_run_test() { let importing = Model( ..logged_in(), - discogs: Discogs(..blank_discogs(), importing_collection: True), + discogs: Discogs(..blank_discogs(), importing: Some(ImportCollection)), ) let #(model, _) = update( @@ -82,8 +83,7 @@ pub fn import_result_summarises_partial_run_and_clears_both_flags_test() { )), ), ) - assert model.discogs.importing_collection == False - assert model.discogs.importing_wantlist == False + assert model.discogs.importing == None assert model.notice == Some(Notice( Success, @@ -112,15 +112,15 @@ pub fn import_result_complete_run_has_no_continue_hint_test() { )) } -pub fn import_error_clears_flags_and_sets_the_discogs_error_region_test() { +pub fn import_error_clears_the_run_and_sets_the_discogs_error_region_test() { let importing = Model( ..logged_in(), - discogs: Discogs(..blank_discogs(), importing_wantlist: True), + discogs: Discogs(..blank_discogs(), importing: Some(ImportWantlist)), ) let #(model, _) = update(importing, GotDiscogsImport(Error(support.network_error()))) - assert model.discogs.importing_wantlist == False + assert model.discogs.importing == None assert model.discogs.error == Some("Import failed. Run it again to resume.") } -- 2.51.2 From 432e4c89937a28fb01768c56d5588d2acbd70e7d Mon Sep 17 00:00:00 2001 From: Niels Mokkenstorm Date: Mon, 10 Aug 2026 14:06:11 +0200 Subject: [PATCH 7/8] fix: carry the quick-add request through the write so a retry keeps its status --- web/src/crate_web/effects.gleam | 12 ++-- web/src/crate_web/model.gleam | 27 ++++--- web/src/crate_web/msg.gleam | 7 +- web/src/crate_web/pages/browse.gleam | 27 +++---- web/src/crate_web/update.gleam | 6 +- web/src/crate_web/update/browse.gleam | 83 +++++++++------------ web/test/browse_test.gleam | 100 +++++++++++++++++++------- web/test/crate_test.gleam | 14 ++-- web/test/nav_test.gleam | 13 ++-- web/test/pressing_test.gleam | 8 ++- 10 files changed, 180 insertions(+), 117 deletions(-) diff --git a/web/src/crate_web/effects.gleam b/web/src/crate_web/effects.gleam index 58e76b0..460245d 100644 --- a/web/src/crate_web/effects.gleam +++ b/web/src/crate_web/effects.gleam @@ -12,8 +12,8 @@ import crate/gen/shelf/list_entries import crate_web/appview import crate_web/browser import crate_web/model.{ - type AmendDraft, type Display, type Form, type ReleaseInfo, type Theme, - DiscogsSearchPage, HandleSuggestion, ReleaseInfo, + type AmendDraft, type BrowseAddRequest, type Display, type Form, + type ReleaseInfo, type Theme, DiscogsSearchPage, HandleSuggestion, ReleaseInfo, } import crate_web/money import crate_web/msg.{ @@ -603,14 +603,14 @@ pub fn load_pressing(release_uri: String) -> Effect(Msg) { /// Want-it / I-have-this quick action from the browse grid: the release is /// already known, so this skips straight to a genesis write. -pub fn browse_add(uri: String, cid: String, status: String) -> Effect(Msg) { +pub fn browse_add(request: BrowseAddRequest) -> Effect(Msg) { let body = json.object([ - #("subject", strong_ref(uri, cid)), - #("status", json.string(status)), + #("subject", strong_ref(request.uri, request.cid)), + #("status", json.string(request.status)), ]) bff_post(xrpc("catalog.adoptRelease", []), body, nil_decoder(), GotBrowseAdd( - uri, + request, _, )) } diff --git a/web/src/crate_web/model.gleam b/web/src/crate_web/model.gleam index 9c3a5c2..c982d5f 100644 --- a/web/src/crate_web/model.gleam +++ b/web/src/crate_web/model.gleam @@ -558,6 +558,19 @@ pub fn blank_amend() -> AmendDraft { pub type BrowseRelease = list_releases.ReleaseRow +/// One browse-card quick add: everything `catalog.adoptRelease` needs, carried +/// through the write so neither the response nor a retry has to re-derive the +/// status from a model slot the page may have cleared meanwhile. +pub type BrowseAddRequest { + BrowseAddRequest(uri: String, cid: String, status: String) +} + +/// A quick add that failed, paired with the message its card shows; the +/// request is the one its RETRY re-fires. +pub type BrowseAddFailure { + BrowseAddFailure(request: BrowseAddRequest, message: String) +} + /// Split a `catalog.release` at:// uri into its did/rkey, the shape the /// `PressingDetail` route needs. Any uri not shaped like /// `at:////` yields `Error`. @@ -781,14 +794,12 @@ pub type Model { amend: AmendDraft, avatar: Option(String), browse: List(BrowseRelease), - // The uri/status of the browse row whose add write is in flight; None - // means idle. Carrying the status lets a successful write flip the right - // flag without re-deriving it from the response. - browse_adding: Option(#(String, String)), - // The uri/status/message of the last quick-add that failed, so its card - // alone shows the inline retry instead of relying on the global toast; - // cleared on the next add attempt (any card) or a fresh grid load. - browse_add_error: Option(#(String, String, String)), + // The quick-add write in flight; None means idle. + browse_adding: Option(BrowseAddRequest), + // The last quick-add that failed, so its card alone shows the inline retry + // instead of relying on the global toast; cleared on the next add attempt + // (any card) or a fresh grid load. + browse_add_error: Option(BrowseAddFailure), // The free-text/genre search box above the browse grid; both empty means // the default live grid from `load_browse`, not a search result. browse_query: String, diff --git a/web/src/crate_web/msg.gleam b/web/src/crate_web/msg.gleam index 89318fa..fa5a37c 100644 --- a/web/src/crate_web/msg.gleam +++ b/web/src/crate_web/msg.gleam @@ -2,7 +2,8 @@ import atproto_core/xrpc import crate/gen/feed/get_feed_skeleton.{type FeedItem} import crate/gen/shelf/entry.{type ShelfEntry} import crate_web/model.{ - type ArtistHit, type BrowseRelease, type DiscogsResult, type DiscogsSearchPage, + type ArtistHit, type BrowseAddRequest, type BrowseRelease, + type DiscogsResult, type DiscogsSearchPage, type Display, type EditProposal, type Entry, type HandleSuggestion, type ImportKind, type ImportRun, type NetworkMatch, type ReleaseInfo, type Route, type ScanMode, type Suggestion, type Theme, @@ -171,8 +172,8 @@ pub type Msg { GotBrowseSearch(Result(List(BrowseRelease), ApiError)) ToggleBrowseFilters BrowseClearFilters - BrowseAdd(uri: String, cid: String, status: String) - GotBrowseAdd(uri: String, result: Result(Nil, ApiError)) + BrowseAdd(request: BrowseAddRequest) + GotBrowseAdd(request: BrowseAddRequest, result: Result(Nil, ApiError)) GotEditInbox(Result(List(EditProposal), ApiError)) ApplyProposal(uri: String, cid: String) GotApplyProposal(uri: String, result: Result(AppliedProposal, ApiError)) diff --git a/web/src/crate_web/pages/browse.gleam b/web/src/crate_web/pages/browse.gleam index 268131d..94e5b88 100644 --- a/web/src/crate_web/pages/browse.gleam +++ b/web/src/crate_web/pages/browse.gleam @@ -4,7 +4,8 @@ //// pressing detail page instead. import crate_web/model.{ - type BrowseRelease, type Model, PressingDetail, PublicCrate, + type BrowseAddFailure, type BrowseAddRequest, type BrowseRelease, type Model, + BrowseAddRequest, PressingDetail, PublicCrate, } import crate_web/msg.{ type Msg, BrowseAdd, BrowseClearFilters, BrowseGenre, BrowseQuery, @@ -149,8 +150,8 @@ pub fn genre_suggestions( fn grid( rows: List(BrowseRelease), - adding: Option(#(String, String)), - add_error: Option(#(String, String, String)), + adding: Option(BrowseAddRequest), + add_error: Option(BrowseAddFailure), busy: Bool, ) -> Element(Msg) { case rows, busy { @@ -166,8 +167,8 @@ fn grid( fn card( row: BrowseRelease, - adding: Option(#(String, String)), - add_error: Option(#(String, String, String)), + adding: Option(BrowseAddRequest), + add_error: Option(BrowseAddFailure), ) -> Element(Msg) { let artist = option.unwrap(row.artist_display, "") let color = cov.cover_color(row.title <> artist) @@ -187,11 +188,11 @@ fn card( /// calls it), so the inline retry lives here instead. fn row_error( row: BrowseRelease, - add_error: Option(#(String, String, String)), + add_error: Option(BrowseAddFailure), ) -> Element(Msg) { case add_error { - Some(#(uri, status, message)) if uri == row.uri -> - states.write_error_sticker(message, BrowseAdd(row.uri, row.cid, status)) + Some(failure) if failure.request.uri == row.uri -> + states.write_error_sticker(failure.message, BrowseAdd(failure.request)) _ -> element.none() } } @@ -243,7 +244,7 @@ fn via_handle(row: BrowseRelease) -> Element(Msg) { pub fn actions( row: BrowseRelease, - adding: Option(#(String, String)), + adding: Option(BrowseAddRequest), ) -> Element(Msg) { case row.owned, row.wanted { True, _ -> ctl.badge("IN YOUR CRATE", ctl.Owned) @@ -254,20 +255,20 @@ pub fn actions( fn quick_actions( row: BrowseRelease, - adding: Option(#(String, String)), + adding: Option(BrowseAddRequest), ) -> Element(Msg) { let busy = option.is_some(adding) let clicked = case adding { - Some(#(uri, status)) if uri == row.uri -> Some(status) + Some(request) if request.uri == row.uri -> Some(request.status) _ -> None } html.div([attr.class("browse-card__actions")], [ ctl.button(action_label("WANT IT", clicked, "wanted"), ctl.Ghost, [ - event.on_click(BrowseAdd(row.uri, row.cid, "wanted")), + event.on_click(BrowseAdd(BrowseAddRequest(row.uri, row.cid, "wanted"))), attr.disabled(busy), ]), ctl.button(action_label("I HAVE THIS", clicked, "owned"), ctl.Primary, [ - event.on_click(BrowseAdd(row.uri, row.cid, "owned")), + event.on_click(BrowseAdd(BrowseAddRequest(row.uri, row.cid, "owned"))), attr.disabled(busy), ]), ]) diff --git a/web/src/crate_web/update.gleam b/web/src/crate_web/update.gleam index f72c478..80c4a3e 100644 --- a/web/src/crate_web/update.gleam +++ b/web/src/crate_web/update.gleam @@ -116,9 +116,9 @@ pub fn update(model: Model, msg: Msg) -> #(Model, Effect(Msg)) { msg.GotBrowseSearch(result) -> browse.got_browse_search(model, result) msg.ToggleBrowseFilters -> browse.toggle_browse_filters(model) msg.BrowseClearFilters -> browse.browse_clear_filters(model) - msg.BrowseAdd(uri, cid, status) -> - browse.browse_add_msg(model, uri, cid, status) - msg.GotBrowseAdd(uri, result) -> browse.got_browse_add(model, uri, result) + msg.BrowseAdd(request) -> browse.browse_add_msg(model, request) + msg.GotBrowseAdd(request, result) -> + browse.got_browse_add(model, request, result) msg.GotPressing(result) -> browse.got_pressing(model, result) msg.GotCrateOverlap(result) -> browse.got_crate_overlap(model, result) msg.ToggleFollow -> browse.toggle_follow(model) diff --git a/web/src/crate_web/update/browse.gleam b/web/src/crate_web/update/browse.gleam index b4f5bcf..0442551 100644 --- a/web/src/crate_web/update/browse.gleam +++ b/web/src/crate_web/update/browse.gleam @@ -5,14 +5,14 @@ import crate_web/effects.{ search_browse, unfollow_user, } import crate_web/model.{ - type BrowseRelease, type Model, CrateOverlap, LoggedIn, LoggedOut, Model, Own, - OwnCrate, PressingDetail, PressingFailed, PressingLoaded, PressingLoading, - ShelfLoaded, + type BrowseAddRequest, type BrowseRelease, type Model, BrowseAddFailure, + CrateOverlap, LoggedIn, LoggedOut, Model, Own, OwnCrate, PressingDetail, + PressingFailed, PressingLoaded, PressingLoading, ShelfLoaded, } import crate_web/msg.{type Msg} import crate_web/update/common.{failed, write_error} import gleam/list -import gleam/option.{type Option, None, Some} +import gleam/option.{None, Some} import gleam/string import lustre/effect.{type Effect} @@ -110,48 +110,41 @@ pub fn browse_clear_filters(model: Model) -> #(Model, Effect(Msg)) { /// Guarded so only one browse add can be in flight at a time. pub fn browse_add_msg( model: Model, - uri: String, - cid: String, - status: String, + request: BrowseAddRequest, ) -> #(Model, Effect(Msg)) { case model.browse_adding { Some(_) -> #(model, effect.none()) None -> #( - Model( - ..model, - browse_adding: Some(#(uri, status)), - browse_add_error: None, - ), - browse_add(uri, cid, status), + Model(..model, browse_adding: Some(request), browse_add_error: None), + browse_add(request), ) } } pub fn got_browse_add( model: Model, - uri: String, + request: BrowseAddRequest, result: Result(Nil, msg.ApiError), ) -> #(Model, Effect(Msg)) { case result { Ok(Nil) -> #( Model( ..model, - browse: mark_browse_row(model.browse, uri, model.browse_adding), - pressing: mark_pressing(model.pressing, uri, model.browse_adding), + browse: mark_browse_row(model.browse, request), + pressing: mark_pressing(model.pressing, request), browse_adding: None, browse_add_error: None, ), effect.none(), ) Error(e) -> { - let status = case model.browse_adding { - Some(#(_, s)) -> s - None -> "" - } let fallback = "Could not save that record. Try again." let #(m, eff) = write_error(Model(..model, browse_adding: None), e, fallback) - #(Model(..m, browse_add_error: Some(#(uri, status, fallback))), eff) + #( + Model(..m, browse_add_error: Some(BrowseAddFailure(request, fallback))), + eff, + ) } } } @@ -284,49 +277,41 @@ pub fn got_unfollow( } } -/// Flip the owned/wanted flag on the browse row that was just written, -/// per the status carried in `browse_adding` (set when the write started). +/// Flip the owned/wanted flag on the browse row that was just written, per +/// the status the write itself carried. fn mark_browse_row( rows: List(BrowseRelease), - uri: String, - adding: Option(#(String, String)), + request: BrowseAddRequest, ) -> List(BrowseRelease) { - case adding { - Some(#(adding_uri, status)) if adding_uri == uri -> - list.map(rows, fn(row) { - case row.uri == uri { - True -> - list_releases.ReleaseRow( - ..row, - owned: row.owned || status == "owned", - wanted: row.wanted || status == "wanted", - ) - False -> row - } - }) - _ -> rows - } + list.map(rows, fn(row) { + case row.uri == request.uri { + True -> + list_releases.ReleaseRow( + ..row, + owned: row.owned || request.status == "owned", + wanted: row.wanted || request.status == "wanted", + ) + False -> row + } + }) } /// Same flip as `mark_browse_row`, for the pressing detail page's own copy /// of the row (it isn't necessarily backed by `model.browse` at all). fn mark_pressing( pressing: model.PressingState, - uri: String, - adding: Option(#(String, String)), + request: BrowseAddRequest, ) -> model.PressingState { - case pressing, adding { - PressingLoaded(row), Some(#(adding_uri, status)) - if adding_uri == uri && row.uri == uri - -> + case pressing { + PressingLoaded(row) if row.uri == request.uri -> PressingLoaded( list_releases.ReleaseRow( ..row, - owned: row.owned || status == "owned", - wanted: row.wanted || status == "wanted", + owned: row.owned || request.status == "owned", + wanted: row.wanted || request.status == "wanted", ), ) - _, _ -> pressing + _ -> pressing } } diff --git a/web/test/browse_test.gleam b/web/test/browse_test.gleam index c6fa5e9..21a4714 100644 --- a/web/test/browse_test.gleam +++ b/web/test/browse_test.gleam @@ -1,7 +1,10 @@ import crate/gen/catalog/list_releases.{ReleaseRow} -import crate_web/model.{type BrowseRelease, Model} +import crate_web/model.{ + type BrowseRelease, Browse, BrowseAddFailure, BrowseAddRequest, Crate, Model, +} import crate_web/msg.{ - BrowseAdd, BrowseClearFilters, GotBrowse, GotBrowseAdd, ToggleBrowseFilters, + BrowseAdd, BrowseClearFilters, GotBrowse, GotBrowseAdd, OnRouteChange, + ToggleBrowseFilters, } import crate_web/pages/browse import crate_web/update.{update} @@ -51,15 +54,20 @@ pub fn got_browse_stores_rows_test() { pub fn browse_add_marks_in_flight_test() { let #(model, _) = - update(logged_in(), BrowseAdd("at://uri", "cid-1", "wanted")) - assert model.browse_adding == Some(#("at://uri", "wanted")) + update( + logged_in(), + BrowseAdd(BrowseAddRequest("at://uri", "cid-1", "wanted")), + ) + assert model.browse_adding + == Some(BrowseAddRequest("at://uri", "cid-1", "wanted")) } pub fn browse_add_is_guarded_while_already_adding_test() { - let seeded = - Model(..logged_in(), browse_adding: Some(#("at://other", "owned"))) - let #(model, _) = update(seeded, BrowseAdd("at://uri", "cid-1", "wanted")) - assert model.browse_adding == Some(#("at://other", "owned")) + let running = BrowseAddRequest("at://other", "cid-0", "owned") + let seeded = Model(..logged_in(), browse_adding: Some(running)) + let #(model, _) = + update(seeded, BrowseAdd(BrowseAddRequest("at://uri", "cid-1", "wanted"))) + assert model.browse_adding == Some(running) } pub fn got_browse_add_success_flips_the_matching_row_and_clears_adding_test() { @@ -68,9 +76,13 @@ pub fn got_browse_add_success_flips_the_matching_row_and_clears_adding_test() { Model( ..logged_in(), browse: [a_browse_release(uri)], - browse_adding: Some(#(uri, "owned")), + browse_adding: Some(BrowseAddRequest(uri, "cid-1", "owned")), + ) + let #(model, _) = + update( + seeded, + GotBrowseAdd(BrowseAddRequest(uri, "cid-1", "owned"), Ok(Nil)), ) - let #(model, _) = update(seeded, GotBrowseAdd(uri, Ok(Nil))) assert model.browse_adding == None assert case model.browse { [row] -> row.owned == True && row.wanted == False @@ -78,16 +90,38 @@ pub fn got_browse_add_success_flips_the_matching_row_and_clears_adding_test() { } } +// A route change clears `browse_adding` mid-flight, so the failure that +// lands afterwards has to carry the status the write was setting; deriving +// it from the model at response time hands the card's RETRY an empty one. +pub fn browse_add_failure_keeps_its_status_across_a_route_change_test() { + let uri = "at://did:plc:abc/catalog.release/1" + let request = BrowseAddRequest(uri, "cid-1", "owned") + let seeded = Model(..logged_in(), browse: [a_browse_release(uri)]) + let #(adding, _) = update(seeded, BrowseAdd(request)) + let #(away, _) = update(adding, OnRouteChange(Crate)) + let #(back, _) = update(away, OnRouteChange(Browse)) + let #(failed, _) = + update(back, GotBrowseAdd(request, Error(support.network_error()))) + assert failed.browse_add_error + == Some(BrowseAddFailure(request, "Could not save that record. Try again.")) +} + pub fn got_browse_add_error_clears_adding_with_notice_test() { let uri = "at://did:plc:abc/catalog.release/1" let seeded = Model( ..logged_in(), browse: [a_browse_release(uri)], - browse_adding: Some(#(uri, "owned")), + browse_adding: Some(BrowseAddRequest(uri, "cid-1", "owned")), ) let #(model, _) = - update(seeded, GotBrowseAdd(uri, Error(support.network_error()))) + update( + seeded, + GotBrowseAdd( + BrowseAddRequest(uri, "cid-1", "owned"), + Error(support.network_error()), + ), + ) assert model.browse_adding == None assert model.notice != None assert case model.browse { @@ -96,20 +130,21 @@ pub fn got_browse_add_error_clears_adding_with_notice_test() { } } -// The card-level retry needs the status alongside the uri, so it can re-fire -// the exact same BrowseAdd; only the card that actually failed carries it. +// The card-level retry re-fires the exact request that failed; only the card +// that actually failed carries it. pub fn got_browse_add_error_records_which_card_and_status_failed_test() { let uri = "at://did:plc:abc/catalog.release/1" + let request = BrowseAddRequest(uri, "cid-1", "owned") let seeded = Model( ..logged_in(), browse: [a_browse_release(uri)], - browse_adding: Some(#(uri, "owned")), + browse_adding: Some(request), ) let #(model, _) = - update(seeded, GotBrowseAdd(uri, Error(support.network_error()))) + update(seeded, GotBrowseAdd(request, Error(support.network_error()))) assert case model.browse_add_error { - Some(#(err_uri, status, _message)) -> err_uri == uri && status == "owned" + Some(failure) -> failure.request == request None -> False } } @@ -120,10 +155,17 @@ pub fn a_successful_add_clears_any_earlier_card_error_test() { Model( ..logged_in(), browse: [a_browse_release(uri)], - browse_adding: Some(#(uri, "owned")), - browse_add_error: Some(#(uri, "owned", "boom")), + browse_adding: Some(BrowseAddRequest(uri, "cid-1", "owned")), + browse_add_error: Some(BrowseAddFailure( + BrowseAddRequest(uri, "cid-1", "owned"), + "boom", + )), + ) + let #(model, _) = + update( + seeded, + GotBrowseAdd(BrowseAddRequest(uri, "cid-1", "owned"), Ok(Nil)), ) - let #(model, _) = update(seeded, GotBrowseAdd(uri, Ok(Nil))) assert model.browse_add_error == None } @@ -131,9 +173,13 @@ pub fn a_new_add_attempt_clears_any_earlier_card_error_test() { let seeded = Model( ..logged_in(), - browse_add_error: Some(#("at://uri/1", "owned", "boom")), + browse_add_error: Some(BrowseAddFailure( + BrowseAddRequest("at://uri/1", "cid-1", "owned"), + "boom", + )), ) - let #(model, _) = update(seeded, BrowseAdd("at://uri/2", "cid-2", "wanted")) + let #(model, _) = + update(seeded, BrowseAdd(BrowseAddRequest("at://uri/2", "cid-2", "wanted"))) assert model.browse_add_error == None } @@ -143,7 +189,10 @@ pub fn the_failed_cards_error_region_renders_with_a_retry_test() { Model( ..logged_in(), browse: [a_browse_release(uri)], - browse_add_error: Some(#(uri, "owned", "Could not save that record.")), + browse_add_error: Some(BrowseAddFailure( + BrowseAddRequest(uri, "cid-1", "owned"), + "Could not save that record.", + )), ) let html = seeded |> browse.view |> element.to_string assert string.contains(html, "class=\"error-state\"") @@ -160,7 +209,10 @@ pub fn only_the_failed_card_shows_the_error_region_test() { Model( ..logged_in(), browse: [a_browse_release(other_uri), a_browse_release(failed_uri)], - browse_add_error: Some(#(failed_uri, "owned", "boom")), + browse_add_error: Some(BrowseAddFailure( + BrowseAddRequest(failed_uri, "cid-1", "owned"), + "boom", + )), ) let html = seeded |> browse.view |> element.to_string assert string.contains(html, "class=\"error-state\"") diff --git a/web/test/crate_test.gleam b/web/test/crate_test.gleam index e40a11f..dd96948 100644 --- a/web/test/crate_test.gleam +++ b/web/test/crate_test.gleam @@ -1,8 +1,8 @@ import crate/gen/shelf/list_entries import crate_web/model.{ - type Entry, Failure, Grid, LoggedIn, LoggedOut, Model, Notice, Own, OwnCrate, - Record, Rows, ShelfFailed, ShelfLoaded, ShelfLoading, Warning, blank_form, - crate_of, set_crate, + type Entry, BrowseAddRequest, Failure, Grid, LoggedIn, LoggedOut, Model, + Notice, Own, OwnCrate, Record, Rows, ShelfFailed, ShelfLoaded, ShelfLoading, + Warning, blank_form, crate_of, set_crate, } import crate_web/msg.{ ClearNotice, FormTitle, GotBrowseAdd, GotShelf, GotShelfMore, HandleChanged, @@ -129,7 +129,13 @@ pub fn set_display_back_to_grid_persists_without_a_shelf_reload_test() { pub fn session_expired_is_a_warning_notice_test() { let #(model, _) = - update(logged_in(), GotBrowseAdd("at://x", Error(unauthorized()))) + update( + logged_in(), + GotBrowseAdd( + BrowseAddRequest("at://x", "cid-1", "owned"), + Error(unauthorized()), + ), + ) assert case model.notice { Some(Notice(Warning, _)) -> True _ -> False diff --git a/web/test/nav_test.gleam b/web/test/nav_test.gleam index cc2da9f..583bb6f 100644 --- a/web/test/nav_test.gleam +++ b/web/test/nav_test.gleam @@ -4,10 +4,10 @@ //// an anchor. import crate_web/model.{ - Add, Browse, Crate, Discogs, EditInbox, EditProposalDetail, Feed, InvalidInput, - Model, Own, OwnCrate, PressingDetail, PressingLoading, PublicCrate, - PublicRecord, Record, RecordAmend, Scan, ScanDone, ScanReview, Settings, - ShelfLoaded, blank_discogs, set_crate, + Add, Browse, BrowseAddFailure, BrowseAddRequest, Crate, Discogs, EditInbox, + EditProposalDetail, Feed, InvalidInput, Model, Own, OwnCrate, PressingDetail, + PressingLoading, PublicCrate, PublicRecord, Record, RecordAmend, Scan, + ScanDone, ScanReview, Settings, ShelfLoaded, blank_discogs, set_crate, } import crate_web/msg.{Back, OnRouteChange} import crate_web/route @@ -124,7 +124,10 @@ pub fn every_route_change_clears_the_page_scoped_state_test() { ..logged_in(), form_error: Some(InvalidInput("Title and artist are required.")), amend_error: Some("Could not publish the amendment."), - browse_add_error: Some(#("at://uri/1", "owned", "boom")), + browse_add_error: Some(BrowseAddFailure( + BrowseAddRequest("at://uri/1", "cid-1", "owned"), + "boom", + )), confirm_apply: Some("at://uri/1"), discogs: Discogs( ..blank_discogs(), diff --git a/web/test/pressing_test.gleam b/web/test/pressing_test.gleam index 3ff4c38..9ce8ec3 100644 --- a/web/test/pressing_test.gleam +++ b/web/test/pressing_test.gleam @@ -92,9 +92,13 @@ pub fn browse_add_success_flips_the_pressing_row_too_test() { Model( ..logged_in(), pressing: PressingLoaded(a_pressing(uri)), - browse_adding: Some(#(uri, "owned")), + browse_adding: Some(model.BrowseAddRequest(uri, "cid-1", "owned")), + ) + let #(model, _) = + update( + seeded, + GotBrowseAdd(model.BrowseAddRequest(uri, "cid-1", "owned"), Ok(Nil)), ) - let #(model, _) = update(seeded, GotBrowseAdd(uri, Ok(Nil))) assert model.pressing == PressingLoaded(ReleaseRow(..a_pressing(uri), owned: True)) } -- 2.51.2 From 0e9573f22f801b4640feb9537d292328e86dd6c9 Mon Sep 17 00:00:00 2001 From: Niels Mokkenstorm Date: Mon, 10 Aug 2026 14:17:08 +0200 Subject: [PATCH 8/8] style: format msg.gleam --- web/src/crate_web/msg.gleam | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/web/src/crate_web/msg.gleam b/web/src/crate_web/msg.gleam index fa5a37c..ab4adba 100644 --- a/web/src/crate_web/msg.gleam +++ b/web/src/crate_web/msg.gleam @@ -2,11 +2,10 @@ import atproto_core/xrpc import crate/gen/feed/get_feed_skeleton.{type FeedItem} import crate/gen/shelf/entry.{type ShelfEntry} import crate_web/model.{ - type ArtistHit, type BrowseAddRequest, type BrowseRelease, - type DiscogsResult, type DiscogsSearchPage, - type Display, type EditProposal, type Entry, type HandleSuggestion, - type ImportKind, type ImportRun, type NetworkMatch, type ReleaseInfo, - type Route, type ScanMode, type Suggestion, type Theme, + type ArtistHit, type BrowseAddRequest, type BrowseRelease, type DiscogsResult, + type DiscogsSearchPage, type Display, type EditProposal, type Entry, + type HandleSuggestion, type ImportKind, type ImportRun, type NetworkMatch, + type ReleaseInfo, type Route, type ScanMode, type Suggestion, type Theme, } import crate_web/photo_scan.{type DecodeError} import gleam/dict