From b9895c28961358a1a298a1ddbc36fcd275ca9d2b Mon Sep 17 00:00:00 2001 From: Niels Mokkenstorm Date: Mon, 10 Aug 2026 12:27:14 +0200 Subject: [PATCH] feat: surface inline write-failure state on add, browse, and amend forms --- web/src/crate_web.gleam | 5 +- web/src/crate_web/model.gleam | 30 +++++++- web/src/crate_web/pages/add.gleam | 13 +++- web/src/crate_web/pages/browse.gleam | 34 ++++++++- web/src/crate_web/pages/record_amend.gleam | 80 ++++++++++++++------ web/src/crate_web/update/add.gleam | 14 +++- web/src/crate_web/update/amend.gleam | 50 ++++++++++--- web/src/crate_web/update/browse.gleam | 33 +++++--- web/test/add_test.gleam | 41 ++++++++-- web/test/browse_test.gleam | 70 +++++++++++++++++ web/test/record_amend_test.gleam | 87 +++++++++++++++++++++- web/test/support.gleam | 5 +- 12 files changed, 400 insertions(+), 62 deletions(-) diff --git a/web/src/crate_web.gleam b/web/src/crate_web.gleam index aaf3e02..e341533 100644 --- a/web/src/crate_web.gleam +++ b/web/src/crate_web.gleam @@ -44,6 +44,7 @@ fn init(_flags) -> #(Model, Effect(Msg)) { shelf_loading_more: False, via_handles: dict.new(), form: blank_form(), + form_error: None, notice: None, busy: True, pending_revert: None, @@ -53,11 +54,13 @@ fn init(_flags) -> #(Model, Effect(Msg)) { entry_detail: model.EntryDetailLoading, discogs: model.blank_discogs(), scan: model.blank_scan(), - publishing: False, + amend_activity: model.AmendIdle, + amend_error: None, amend: model.blank_amend(), avatar: None, browse: [], browse_adding: None, + browse_add_error: None, browse_query: "", browse_genre: "", browse_filters_open: False, diff --git a/web/src/crate_web/model.gleam b/web/src/crate_web/model.gleam index ca0f72c..f0dfbdf 100644 --- a/web/src/crate_web/model.gleam +++ b/web/src/crate_web/model.gleam @@ -507,6 +507,16 @@ pub fn scan_importable(rows: List(ScanRow)) -> List(ScanRow) { }) } +/// What the amend page's write path is doing right now: idle, uploading a +/// cover photo (fires immediately on file choice), or publishing the typed +/// draft fields. One field instead of the old `publishing`/`busy` pair, so +/// the submit button's disabled state and its label always agree. +pub type AmendActivity { + AmendIdle + AmendUploadingCover + AmendPublishing +} + /// Draft of a manual catalog amendment (all free text; lists comma-split). pub type AmendDraft { AmendDraft( @@ -726,6 +736,10 @@ pub type Model { // Entry id -> the handle a foreign (adopted) release was minted by. via_handles: dict.Dict(String, String), form: Form, + // The Add page's inline write-failure message, shown above SAVE TO CRATE + // without touching `form` itself, so a failed write never costs the + // fields the visitor already typed. + form_error: Option(String), notice: Option(Notice), busy: Bool, // The pre-optimistic-update copy of an entry mid-Rate/Regrade, restored @@ -738,10 +752,14 @@ pub type Model { entry_detail: EntryDetailState, discogs: Discogs, scan: ScanState, - // True only while the amendment write itself is in flight — narrower - // than `busy`, which also covers unrelated actions (Rate/Regrade, ...) - // that can happen while the amend panel is open. - publishing: Bool, + // What the amend page's own write path is doing right now; drives both + // the PUBLISH AMENDMENT button's disabled state and its label from one + // source instead of two. + amend_activity: AmendActivity, + // The amend page's inline write-failure message (publish or cover + // upload), shown above PUBLISH AMENDMENT; the draft fields themselves + // are never touched by a failure. + amend_error: Option(String), amend: AmendDraft, avatar: Option(String), browse: List(BrowseRelease), @@ -749,6 +767,10 @@ pub type Model { // 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 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/pages/add.gleam b/web/src/crate_web/pages/add.gleam index ecb5207..f94aec8 100644 --- a/web/src/crate_web/pages/add.gleam +++ b/web/src/crate_web/pages/add.gleam @@ -14,9 +14,10 @@ import crate_web/ui/controls as ctl import crate_web/ui/covers as cov import crate_web/ui/forms as frm import crate_web/ui/record_detail as rd +import crate_web/ui/states import gleam/int import gleam/list -import gleam/option.{None, Some} +import gleam/option.{type Option, None, Some} import gleam/string import lustre/attribute as attr import lustre/element.{type Element, text} @@ -52,6 +53,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"), attr.disabled(model.busy), @@ -61,6 +63,15 @@ fn add_form_view(model: Model) -> Element(Msg) { ]) } +/// Rendered above SAVE TO CRATE, never in place of the form fields: a failed +/// write (validation or network) never costs what the visitor already typed. +fn form_error_region(error: Option(String)) -> Element(Msg) { + case error { + Some(message) -> states.error_sticker_retry(message, SubmitAdd) + None -> element.none() + } +} + fn sleeve_grade_select(form: Form) -> Element(Msg) { let options = list.map(["", ..rd.grades], fn(g) { diff --git a/web/src/crate_web/pages/browse.gleam b/web/src/crate_web/pages/browse.gleam index 5fd2954..668b81e 100644 --- a/web/src/crate_web/pages/browse.gleam +++ b/web/src/crate_web/pages/browse.gleam @@ -15,6 +15,7 @@ import crate_web/route import crate_web/ui/controls as ctl import crate_web/ui/covers as cov import crate_web/ui/forms as frm +import crate_web/ui/states import gleam/list import gleam/option.{type Option, None, Some} import gleam/string @@ -28,7 +29,12 @@ pub fn view(model: Model) -> Element(Msg) { html.div([attr.class("body list-body")], [ search_bar(model), html.div([attr.class("list-scroll")], [ - grid(model.browse, model.browse_adding, model.busy), + grid( + model.browse, + model.browse_adding, + model.browse_add_error, + model.busy, + ), ]), ]), ]) @@ -144,6 +150,7 @@ pub fn genre_suggestions( fn grid( rows: List(BrowseRelease), adding: Option(#(String, String)), + add_error: Option(#(String, String, String)), busy: Bool, ) -> Element(Msg) { case rows, busy { @@ -152,22 +159,43 @@ fn grid( text("Nothing in the shared catalog yet."), ]) [], True -> element.none() - _, _ -> html.div([attr.class("grid")], list.map(rows, card(_, adding))) + _, _ -> + html.div([attr.class("grid")], list.map(rows, card(_, adding, add_error))) } } -fn card(row: BrowseRelease, adding: Option(#(String, String))) -> Element(Msg) { +fn card( + row: BrowseRelease, + adding: Option(#(String, String)), + add_error: Option(#(String, String, String)), +) -> Element(Msg) { let artist = option.unwrap(row.artist_display, "") let color = cov.cover_color(row.title <> artist) html.div([attr.class("cover-card browse-card")], [ pressing_link(row, color, artist), html.div([attr.class("cover-card__info")], [ via_handle(row), + row_error(row, add_error), actions(row, adding), ]), ]) } +/// Shown above this card's WANT IT / I HAVE THIS row when its own quick-add +/// just failed; other cards, including one mid-add itself, are untouched. +/// `browse.actions` keeps its existing two-arg shape (`pressing.gleam` also +/// calls it), so the inline retry lives here instead. +fn row_error( + row: BrowseRelease, + add_error: Option(#(String, String, String)), +) -> Element(Msg) { + case add_error { + Some(#(uri, status, message)) if uri == row.uri -> + states.error_sticker_retry(message, BrowseAdd(row.uri, row.cid, status)) + _ -> element.none() + } +} + /// The cover/title area, linking into the shared catalog's pressing detail /// page; falls back to a non-link block on the rare uri that doesn't parse /// (rather than emit a link to nowhere). diff --git a/web/src/crate_web/pages/record_amend.gleam b/web/src/crate_web/pages/record_amend.gleam index 8a17e45..05af429 100644 --- a/web/src/crate_web/pages/record_amend.gleam +++ b/web/src/crate_web/pages/record_amend.gleam @@ -1,17 +1,21 @@ //// The release-amend screen: a full-page form to correct the shared //// catalog pressing behind one crate entry (title/released/country/ //// genres/styles) and optionally refresh or replace its cover art. -//// Routed as `RecordAmend`, reached off the record detail page's -//// AMEND / SUGGEST A FIX action; the draft itself lives in `model.amend`, -//// seeded by `OnRouteChange(RecordAmend(_))` in update.gleam. +//// Routed as `RecordAmend`, reached off the record detail page's AMEND +//// action; the draft itself lives in `model.amend`, seeded by +//// `OnRouteChange(RecordAmend(_))` in update.gleam. -import crate_web/model.{type Model} +import crate_web/model.{ + type AmendActivity, type Model, AmendIdle, AmendPublishing, + AmendUploadingCover, +} import crate_web/msg.{ type Msg, AmendField, CoverFileChosen, SubmitAmend, ToggleAmendCover, } import crate_web/ui/controls as ctl import crate_web/ui/forms as frm -import crate_web/ui/record_detail as rd +import crate_web/ui/states +import gleam/option.{type Option, None, Some} import lustre/attribute as attr import lustre/element.{type Element, text} import lustre/element/html @@ -37,28 +41,50 @@ pub fn view(model: Model) -> Element(Msg) { ]), html.span([], [text("REFRESH FROM DISCOGS ON PUBLISH")]), ]), - html.label([attr.class("field cover-upload")], [ - html.span([], [text("OR SHOOT / UPLOAD A PHOTO (SAVES RIGHT AWAY)")]), - html.input([ - attr.id("cover-file"), - attr.type_("file"), - attr.accept(["image/*"]), - event.on_change(fn(_) { CoverFileChosen }), - ]), - ]), - ctl.button(publish_label(model.publishing), ctl.Dark, [ + cover_upload_field(model.amend_activity), + amend_error_region(model.amend_error), + ctl.button(publish_label(model.amend_activity), ctl.Dark, [ event.on_click(SubmitAmend), - attr.disabled(model.busy), + attr.disabled(model.amend_activity != AmendIdle), attr.class("btn--block"), ]), - rd.actions_footnote(), + amend_footnote(), + ]) +} + +fn cover_upload_field(activity: AmendActivity) -> Element(Msg) { + html.label([attr.class("field cover-upload")], [ + html.span([], [text(cover_upload_label(activity))]), + html.input([ + attr.id("cover-file"), + attr.type_("file"), + attr.accept(["image/*"]), + attr.disabled(activity == AmendUploadingCover), + event.on_change(fn(_) { CoverFileChosen }), + ]), ]) } -fn publish_label(publishing: Bool) -> String { - case publishing { - True -> "PUBLISHING…" - False -> "PUBLISH AMENDMENT" +fn cover_upload_label(activity: AmendActivity) -> String { + case activity { + AmendUploadingCover -> "UPLOADING COVER…" + _ -> "OR SHOOT / UPLOAD A PHOTO (SAVES RIGHT AWAY)" + } +} + +/// Covers both a failed publish and a failed cover upload: whichever write +/// this page just attempted, the draft fields stay exactly as typed either way. +fn amend_error_region(error: Option(String)) -> Element(Msg) { + case error { + Some(message) -> states.error_sticker_retry(message, SubmitAmend) + None -> element.none() + } +} + +fn publish_label(activity: AmendActivity) -> String { + case activity { + AmendPublishing -> "PUBLISHING…" + _ -> "PUBLISH AMENDMENT" } } @@ -72,3 +98,15 @@ fn amend_field(label: String, field: String, value: String) -> Element(Msg) { ]), ]) } + +/// Locally worded rather than `ui/record_detail.amend_footnote` (still on +/// the retired "a fix mints your own version" copy): publishing an amendment +/// saves your own copy of the pressing's catalog fields, it never edits the +/// shared one other crates read from. +fn amend_footnote() -> Element(Msg) { + html.p([attr.class("actions-footnote")], [ + text( + "Publishing an amendment saves your own copy of this pressing's details. It never changes anyone else's.", + ), + ]) +} diff --git a/web/src/crate_web/update/add.gleam b/web/src/crate_web/update/add.gleam index edef3de..0fc0926 100644 --- a/web/src/crate_web/update/add.gleam +++ b/web/src/crate_web/update/add.gleam @@ -109,15 +109,15 @@ pub fn submit_add(model: Model) -> #(Model, Effect(Msg)) { valid_price(model.form.price_amount) { "", _, _ | _, "", _ -> #( - Model(..model, notice: failed("Title and artist are required.")), + Model(..model, form_error: Some("Title and artist are required.")), effect.none(), ) _, _, False -> #( - Model(..model, notice: failed("Enter a valid price amount.")), + Model(..model, form_error: Some("Enter a valid price amount.")), effect.none(), ) _, _, True -> #( - Model(..model, busy: True, notice: None), + Model(..model, busy: True, form_error: None, notice: None), add_item(model.form), ) } @@ -134,13 +134,19 @@ pub fn got_add( Model( ..model, form: blank_form(), + form_error: None, busy: True, notice: succeeded("Added \"" <> title <> "\" to your crate."), ), load_shelf(None, model.view), ) } - Error(e) -> write_error(model, e, "Could not save the record.") + Error(e) -> { + let fallback = + "Could not save the record. Your entries are still here, try again." + let #(m, eff) = write_error(model, e, fallback) + #(Model(..m, form_error: Some(fallback)), eff) + } } } diff --git a/web/src/crate_web/update/amend.gleam b/web/src/crate_web/update/amend.gleam index 943ac3d..e037bde 100644 --- a/web/src/crate_web/update/amend.gleam +++ b/web/src/crate_web/update/amend.gleam @@ -1,8 +1,10 @@ import crate_web/effects -import crate_web/model.{type Model, AmendDraft, Model} +import crate_web/model.{ + type Model, AmendDraft, AmendIdle, AmendPublishing, AmendUploadingCover, Model, +} import crate_web/msg.{type Msg} import crate_web/route -import crate_web/update/common.{after_write, failed, succeeded, write_error} +import crate_web/update/common.{after_write, succeeded, write_error} import gleam/option.{None, Some} import lustre/effect.{type Effect} import modem @@ -33,7 +35,13 @@ pub fn toggle_amend_cover(model: Model, value: Bool) -> #(Model, Effect(Msg)) { pub fn submit_amend(model: Model) -> #(Model, Effect(Msg)) { case model.selected { Some(entry_id) -> #( - Model(..model, busy: True, publishing: True, notice: None), + Model( + ..model, + busy: True, + amend_activity: AmendPublishing, + amend_error: None, + notice: None, + ), effects.amend_entry(entry_id, model.amend), ) None -> #(model, effect.none()) @@ -49,15 +57,24 @@ pub fn got_amend( Model( ..model, busy: True, - publishing: False, + amend_activity: AmendIdle, + amend_error: None, notice: succeeded("Published the amended catalog release."), ), effect.batch([after_write(model), back_to_record(model)]), ) Error(e) -> { - let #(reverted, write_effect) = - write_error(model, e, "Could not publish the amendment.") - #(Model(..reverted, publishing: False), write_effect) + let fallback = + "Could not publish the amendment. Your changes are still here, try again." + let #(reverted, write_effect) = write_error(model, e, fallback) + #( + Model( + ..reverted, + amend_activity: AmendIdle, + amend_error: Some(fallback), + ), + write_effect, + ) } } } @@ -65,7 +82,13 @@ pub fn got_amend( pub fn cover_file_chosen(model: Model) -> #(Model, Effect(Msg)) { case model.selected { Some(entry_id) -> #( - Model(..model, busy: True, notice: None), + Model( + ..model, + busy: True, + amend_activity: AmendUploadingCover, + amend_error: None, + notice: None, + ), effects.upload_cover(entry_id), ) None -> #(model, effect.none()) @@ -78,12 +101,21 @@ pub fn cover_uploaded(model: Model, ok: Bool) -> #(Model, Effect(Msg)) { Model( ..model, busy: True, + amend_activity: AmendIdle, + amend_error: None, notice: succeeded("Cover updated; published an amended release."), ), effect.batch([after_write(model), back_to_record(model)]), ) False -> #( - Model(..model, busy: False, notice: failed("Cover upload failed.")), + Model( + ..model, + busy: False, + amend_activity: AmendIdle, + amend_error: Some( + "Couldn't upload that cover. Choose a photo and try again.", + ), + ), effect.none(), ) } diff --git a/web/src/crate_web/update/browse.gleam b/web/src/crate_web/update/browse.gleam index b1e35e1..7954e6a 100644 --- a/web/src/crate_web/update/browse.gleam +++ b/web/src/crate_web/update/browse.gleam @@ -20,7 +20,10 @@ pub fn got_browse( result: Result(List(BrowseRelease), msg.ApiError), ) -> #(Model, Effect(Msg)) { case result { - Ok(rows) -> #(Model(..model, browse: rows, busy: False), effect.none()) + Ok(rows) -> #( + Model(..model, browse: rows, browse_add_error: None, busy: False), + effect.none(), + ) // Only 401 means logged-out; keep auth on transient errors (network, 5xx). Error(xrpc.BadStatus(status: 401, ..)) -> #( Model( @@ -79,7 +82,10 @@ pub fn got_browse_search( result: Result(List(BrowseRelease), msg.ApiError), ) -> #(Model, Effect(Msg)) { case result { - Ok(rows) -> #(Model(..model, browse: rows), effect.none()) + Ok(rows) -> #( + Model(..model, browse: rows, browse_add_error: None), + effect.none(), + ) // A transient search failure keeps whatever's already on screen instead // of wiping the grid out from under the viewer mid-search. Error(_) -> #( @@ -110,7 +116,11 @@ pub fn browse_add_msg( case model.browse_adding { Some(_) -> #(model, effect.none()) None -> #( - Model(..model, browse_adding: Some(#(uri, status))), + Model( + ..model, + browse_adding: Some(#(uri, status)), + browse_add_error: None, + ), browse_add(uri, cid, status), ) } @@ -128,15 +138,20 @@ pub fn got_browse_add( browse: mark_browse_row(model.browse, uri, model.browse_adding), pressing: mark_pressing(model.pressing, uri, model.browse_adding), browse_adding: None, + browse_add_error: None, ), effect.none(), ) - Error(e) -> - write_error( - Model(..model, browse_adding: None), - e, - "Could not save that record.", - ) + 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) + } } } diff --git a/web/test/add_test.gleam b/web/test/add_test.gleam index 68c98b9..6a9664e 100644 --- a/web/test/add_test.gleam +++ b/web/test/add_test.gleam @@ -1,6 +1,6 @@ import crate/gen/discogs/search_artists import crate_web/model.{ - Discogs, Failure, Form, Model, Notice, Success, blank_discogs, blank_form, + Discogs, Form, Model, Notice, Success, blank_discogs, blank_form, } import crate_web/msg.{ FormArtist, FormCounterparty, FormFolder, FormPriceAmount, FormPriceCurrency, @@ -13,11 +13,15 @@ import gleam/string import lustre/element import support.{base, 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 +// global toast (see `record_amend`/`browse` for the same call). pub fn submit_add_rejects_whitespace_only_title_test() { let model = Model(..base(), form: Form(..blank_form(), title: " ", artist: "x")) let #(after, _) = update(model, SubmitAdd) - assert after.notice == Some(Notice(Failure, "Title and artist are required.")) + assert after.form_error == Some("Title and artist are required.") + assert after.form.title == " " } pub fn submit_add_rejects_an_unparseable_price_test() { @@ -32,8 +36,9 @@ pub fn submit_add_rejects_an_unparseable_price_test() { ), ) let #(after, _) = update(model, SubmitAdd) - assert after.notice == Some(Notice(Failure, "Enter a valid price amount.")) + assert after.form_error == Some("Enter a valid price amount.") assert after.busy == False + assert after.form.title == "Spiderland" } pub fn submit_add_accepts_a_blank_price_test() { @@ -44,7 +49,7 @@ pub fn submit_add_accepts_a_blank_price_test() { ) let #(after, _) = update(model, SubmitAdd) assert after.busy == True - assert after.notice == None + assert after.form_error == None } pub fn submit_add_accepts_a_valid_price_test() { @@ -60,7 +65,19 @@ pub fn submit_add_accepts_a_valid_price_test() { ) let #(after, _) = update(model, SubmitAdd) assert after.busy == True - assert after.notice == None + assert after.form_error == None +} + +pub fn got_add_error_keeps_the_typed_fields_and_sets_the_inline_error_test() { + let model = + Model( + ..logged_in(), + form: Form(..blank_form(), title: "Spiderland", artist: "Slint"), + ) + let #(after, _) = update(model, GotAdd(Error(support.server_error()))) + assert after.form.title == "Spiderland" + assert after.form.artist == "Slint" + assert after.form_error != None } pub fn form_sleeve_grade_updates_form_test() { @@ -136,6 +153,20 @@ pub fn add_submits_through_a_form_test() { assert string.contains(html, "type=\"submit\"") } +pub fn form_error_renders_above_the_submit_button_test() { + let html = + Model(..logged_in(), form_error: Some("Could not save the record.")) + |> add.view + |> element.to_string + assert string.contains(html, "class=\"error-state\"") + assert string.contains(html, "Could not save the record.") +} + +pub fn no_form_error_renders_nothing_extra_test() { + let html = add.view(logged_in()) |> element.to_string + assert !string.contains(html, "class=\"error-state\"") +} + pub fn segment_buttons_never_submit_the_form_test() { let html = add.view(logged_in()) |> element.to_string // Inside a
, a bare