diff --git a/web/src/crate_web/model.gleam b/web/src/crate_web/model.gleam index 9f79015..b572b44 100644 --- a/web/src/crate_web/model.gleam +++ b/web/src/crate_web/model.gleam @@ -319,6 +319,14 @@ pub type Notice { Notice(level: NoticeLevel, message: String) } +/// Why a form is showing an error, which is also whether RETRY means +/// anything: a rejected write can succeed on a second attempt, a rejected +/// input cannot until the visitor changes it. +pub type FormError { + InvalidInput(message: String) + WriteFailed(message: String) +} + pub type Form { Form( title: String, @@ -746,10 +754,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), + // The Add page's inline failure, shown above SAVE TO CRATE without + // touching `form` itself, so a rejection never costs the fields the + // visitor already typed. + form_error: Option(FormError), notice: Option(Notice), busy: Bool, // The pre-optimistic-update copy of an entry mid-Rate/Regrade, restored diff --git a/web/src/crate_web/pages/add.gleam b/web/src/crate_web/pages/add.gleam index 90bb60d..4e5efb7 100644 --- a/web/src/crate_web/pages/add.gleam +++ b/web/src/crate_web/pages/add.gleam @@ -3,7 +3,8 @@ //// controls live on the Settings page. import crate_web/model.{ - type ArtistHit, type DiscogsResult, type Form, type Model, + type ArtistHit, type DiscogsResult, type Form, type FormError, type Model, + InvalidInput, WriteFailed, } import crate_web/msg.{ type Msg, FormArtist, FormCounterparty, FormFolder, FormFormat, @@ -62,11 +63,14 @@ 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) { +/// Rendered above SAVE TO CRATE, never in place of the form fields: a +/// rejection never costs what the visitor already typed. Only the write +/// failure offers RETRY; re-submitting input the client already rejected +/// would just fail the same way. +fn form_error_region(error: Option(FormError)) -> Element(Msg) { case error { - Some(message) -> states.error_sticker_retry(message, SubmitAdd) + Some(InvalidInput(message)) -> states.invalid_input_sticker(message) + Some(WriteFailed(message)) -> states.write_error_sticker(message, SubmitAdd) None -> element.none() } } diff --git a/web/src/crate_web/pages/browse.gleam b/web/src/crate_web/pages/browse.gleam index 668b81e..268131d 100644 --- a/web/src/crate_web/pages/browse.gleam +++ b/web/src/crate_web/pages/browse.gleam @@ -191,7 +191,7 @@ fn row_error( ) -> Element(Msg) { case add_error { Some(#(uri, status, message)) if uri == row.uri -> - states.error_sticker_retry(message, BrowseAdd(row.uri, row.cid, status)) + states.write_error_sticker(message, BrowseAdd(row.uri, row.cid, status)) _ -> element.none() } } diff --git a/web/src/crate_web/pages/record_amend.gleam b/web/src/crate_web/pages/record_amend.gleam index 5e36141..5a64409 100644 --- a/web/src/crate_web/pages/record_amend.gleam +++ b/web/src/crate_web/pages/record_amend.gleam @@ -77,7 +77,7 @@ fn cover_upload_label(activity: AmendActivity) -> String { /// 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) + Some(message) -> states.write_error_sticker(message, SubmitAmend) None -> element.none() } } diff --git a/web/src/crate_web/ui/states.gleam b/web/src/crate_web/ui/states.gleam index aac2791..49e5a83 100644 --- a/web/src/crate_web/ui/states.gleam +++ b/web/src/crate_web/ui/states.gleam @@ -22,21 +22,32 @@ pub fn loading_page() -> Element(msg) { /// failed list/detail state shares, whatever it's wrapped in above. /// `role="alert"` so a screen reader announces the failure unprompted. pub fn error_sticker(body: String) -> Element(msg) { - error_sticker_view(body, None) + sticker("✕ COULDN'T LOAD", body, None) } /// `error_sticker` with a RETRY action wired to `retry`, for states that can /// actually be retried in place instead of forcing the user to navigate away /// and back. pub fn error_sticker_retry(body: String, retry: msg) -> Element(msg) { - error_sticker_view(body, Some(retry)) + sticker("✕ COULDN'T LOAD", body, Some(retry)) } -fn error_sticker_view(body: String, retry: Option(msg)) -> Element(msg) { +/// A failed *write*, which never loaded anything: same sticker, honest +/// headline. Re-firing the same write can succeed, so RETRY is mandatory here. +pub fn write_error_sticker(body: String, retry: msg) -> Element(msg) { + sticker("✕ COULDN'T SAVE", body, Some(retry)) +} + +/// A client-side validation failure. Deliberately has no RETRY: re-running +/// the same check on unchanged input fails identically, so the only way +/// forward is editing the field the body names. +pub fn invalid_input_sticker(body: String) -> Element(msg) { + sticker("✕ CHECK YOUR ENTRIES", body, None) +} + +fn sticker(title: String, body: String, retry: Option(msg)) -> Element(msg) { html.div([attr.class("error-state"), attr.attribute("role", "alert")], [ - html.span([attr.class("error-state__sticker")], [ - text("✕ COULDN'T LOAD"), - ]), + html.span([attr.class("error-state__sticker")], [text(title)]), html.p([attr.class("error-state__body")], [text(body)]), retry_action(retry), ]) diff --git a/web/src/crate_web/update/add.gleam b/web/src/crate_web/update/add.gleam index 7561527..0303fac 100644 --- a/web/src/crate_web/update/add.gleam +++ b/web/src/crate_web/update/add.gleam @@ -5,7 +5,7 @@ import crate_web/effects.{ } import crate_web/model.{ type ArtistHit, type DiscogsResult, type DiscogsSearchPage, type ImportRun, - type Model, Discogs, Form, Model, blank_form, + type Model, Discogs, Form, InvalidInput, Model, WriteFailed, blank_form, } import crate_web/money import crate_web/msg.{ @@ -116,11 +116,17 @@ pub fn submit_add(model: Model) -> #(Model, Effect(Msg)) { valid_price(model.form.price_amount) { "", _, _ | _, "", _ -> #( - Model(..model, form_error: Some("Title and artist are required.")), + Model( + ..model, + form_error: Some(InvalidInput("Title and artist are required.")), + ), effect.none(), ) _, _, False -> #( - Model(..model, form_error: Some("Enter a valid price amount.")), + Model( + ..model, + form_error: Some(InvalidInput("Enter a valid price amount.")), + ), effect.none(), ) _, _, True -> #( @@ -152,7 +158,7 @@ pub fn got_add( 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) + #(Model(..m, form_error: Some(WriteFailed(fallback))), eff) } } } diff --git a/web/test/add_test.gleam b/web/test/add_test.gleam index e91c890..02e2869 100644 --- a/web/test/add_test.gleam +++ b/web/test/add_test.gleam @@ -1,6 +1,7 @@ import crate/gen/discogs/search_artists import crate_web/model.{ - Discogs, Form, Model, Notice, Success, blank_discogs, blank_form, + Discogs, Form, InvalidInput, Model, Notice, Success, WriteFailed, + blank_discogs, blank_form, } import crate_web/msg.{ FormArtist, FormCounterparty, FormFolder, FormPriceAmount, FormPriceCurrency, @@ -22,7 +23,8 @@ 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.form_error == Some("Title and artist are required.") + assert after.form_error + == Some(InvalidInput("Title and artist are required.")) assert after.form.title == " " } @@ -38,7 +40,7 @@ pub fn submit_add_rejects_an_unparseable_price_test() { ), ) let #(after, _) = update(model, SubmitAdd) - assert after.form_error == Some("Enter a valid price amount.") + assert after.form_error == Some(InvalidInput("Enter a valid price amount.")) assert after.busy == False assert after.form.title == "Spiderland" } @@ -157,7 +159,10 @@ pub fn add_submits_through_a_form_test() { pub fn form_error_renders_above_the_submit_button_test() { let html = - Model(..logged_in(), form_error: Some("Could not save the record.")) + Model( + ..logged_in(), + form_error: Some(WriteFailed("Could not save the record.")), + ) |> add.view |> element.to_string assert string.contains(html, "class=\"error-state\"") @@ -173,7 +178,7 @@ pub fn no_form_error_renders_nothing_extra_test() { /// `on_click` control would fire its own message and submit the page too. /// Covers the whole form, including the error sticker's RETRY. pub fn no_button_in_the_add_form_is_a_bare_button_test() { - [logged_in(), Model(..logged_in(), form_error: Some("boom"))] + [logged_in(), Model(..logged_in(), form_error: Some(WriteFailed("boom")))] |> list.each(fn(model) { let tags = button_tags(add.view(model) |> element.to_string) assert tags != [] @@ -216,3 +221,29 @@ pub fn a_second_submit_while_the_first_is_in_flight_writes_nothing_test() { assert second == first assert second_effect == empty_effect() } + +/// A write failure is retryable and never claims a load went wrong; a +/// validation failure offers no RETRY at all, since re-running the same +/// check on unchanged input fails identically. +pub fn each_form_error_gets_its_own_sticker_and_retry_affordance_test() { + [ + #(WriteFailed("boom"), "✕ COULDN'T SAVE", True), + #( + InvalidInput("Title and artist are required."), + "✕ CHECK YOUR ENTRIES", + False, + ), + ] + |> list.each(fn(row) { + let #(error, sticker, retryable) = row + let html = + Model(..logged_in(), form_error: Some(error)) + |> add.view + |> element.to_string + // The stickers carry an apostrophe, which renders as an entity. + |> string.replace("'", "'") + assert string.contains(html, sticker) + assert !string.contains(html, "COULDN'T LOAD") + assert string.contains(html, "RETRY") == retryable + }) +} diff --git a/web/test/browse_test.gleam b/web/test/browse_test.gleam index e6a94e8..c6fa5e9 100644 --- a/web/test/browse_test.gleam +++ b/web/test/browse_test.gleam @@ -148,6 +148,9 @@ pub fn the_failed_cards_error_region_renders_with_a_retry_test() { let html = seeded |> browse.view |> element.to_string assert string.contains(html, "class=\"error-state\"") assert string.contains(html, "Could not save that record.") + assert string.contains(html, "COULDN'T SAVE") + assert !string.contains(html, "COULDN'T LOAD") + assert string.contains(html, "RETRY") } pub fn only_the_failed_card_shows_the_error_region_test() { diff --git a/web/test/record_amend_test.gleam b/web/test/record_amend_test.gleam index 792a912..3f22d81 100644 --- a/web/test/record_amend_test.gleam +++ b/web/test/record_amend_test.gleam @@ -210,4 +210,8 @@ pub fn amend_error_renders_above_the_publish_button_test() { |> element.to_string assert string.contains(html, "class=\"error-state\"") assert string.contains(html, "Could not publish the amendment.") + // A failed publish never loaded anything, and it IS retryable. + assert string.contains(html, "COULDN'T SAVE") + assert !string.contains(html, "COULDN'T LOAD") + assert string.contains(html, "RETRY") }