From c8ad6a52157cf11977aa5bd75ff6995552399f7b Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:28:41 -0600 Subject: [PATCH] fix(settings): serialize ListenBrainz token validation cx --- .../qml/settings/IntegrationsSettings.qml | 43 +++--- crates/qbz-qt/src/integrations_qt.rs | 129 +++++++++++++++--- 2 files changed, 136 insertions(+), 36 deletions(-) diff --git a/crates/qbz-qt/qml/settings/IntegrationsSettings.qml b/crates/qbz-qt/qml/settings/IntegrationsSettings.qml index ea705c285..a4623be9f 100644 --- a/crates/qbz-qt/qml/settings/IntegrationsSettings.qml +++ b/crates/qbz-qt/qml/settings/IntegrationsSettings.qml @@ -19,6 +19,9 @@ Column { id: root property var doc: ({}) + // Password drafts never enter settingsJson. Keep the live token inside + // this component until the user explicitly submits it. + property string listenbrainzTokenDraft: "" /// The view-level SettingsConfirmHost (SettingsView.qml), for the /// destructive "Clear listening history" row. Null in previews. property var confirmHost: null @@ -27,19 +30,10 @@ Column { spacing: 4 - // The Slint mirrors the in-progress token in - // ScrobbleState.listenbrainz-token-input so its "Connect ListenBrainz" - // button can submit it. QbzLineEdit only publishes the COMMITTED value - // (it commits on Enter / focus loss, and a SettingsButton click does not - // steal focus from the TextInput), so read the live value off its inner - // input — the one child that has an echoMode. - function lbTokenText() { - for (var i = 0; i < lbTokenField.children.length; ++i) { - var c = lbTokenField.children[i] - if (c && c.echoMode !== undefined) - return c.text - } - return "" + function submitListenBrainzToken() { + if (root.doc.listenbrainzBusy === true) + return + QbzBridge.settingsString("listenbrainz-token", root.listenbrainzTokenDraft) } // ======================= RECOMMENDATIONS ============================= @@ -242,7 +236,9 @@ Column { GroupHeader { text: QbzSession.tr("LISTENBRAINZ", QbzSession.trRev) } SettingRow { label: QbzSession.tr("Scrobble to ListenBrainz", QbzSession.trRev) - description: root.doc.listenbrainzAuthed === true + description: root.doc.listenbrainzBusy === true + ? QbzSession.tr("Validating...", QbzSession.trRev) + : root.doc.listenbrainzAuthed === true ? QbzSession.tr("Signed in as {}.", QbzSession.trRev).replace("{}", root.doc.listenbrainzUsername || "") : QbzSession.tr("Paste your ListenBrainz user token to enable scrobbling.", QbzSession.trRev) QbzToggle { @@ -259,11 +255,21 @@ Column { isPassword: true enabled: root.doc.listenbrainzBusy !== true placeholder: QbzSession.tr("ListenBrainz token", QbzSession.trRev) - // Enter (and focus loss) validates, like the Slint `accepted`. - onCommitted: function (v) { QbzBridge.settingsString("listenbrainz-token", v) } + // Typing only updates the local draft. Enter submits once; + // focus loss merely settles the draft, so a button click can + // never race an implicit blur validation. + onEdited: function (v) { root.listenbrainzTokenDraft = v } + onCommitted: function (v) { root.listenbrainzTokenDraft = v } + onAccepted: function (v) { + root.listenbrainzTokenDraft = v + root.submitListenBrainzToken() + } // The row hides once the sign-in lands; drop the typed token // with it (Slint resets listenbrainz-token-input on success). - onVisibleChanged: if (!visible) text = "" + onVisibleChanged: if (!visible) { + text = "" + root.listenbrainzTokenDraft = "" + } } } SettingRow { @@ -275,7 +281,8 @@ Column { ? QbzSession.tr("Validating...", QbzSession.trRev) : QbzSession.tr("Connect ListenBrainz", QbzSession.trRev) enabled: root.doc.listenbrainzBusy !== true - onClicked: QbzBridge.settingsString("listenbrainz-token", root.lbTokenText()) + && root.listenbrainzTokenDraft.trim().length > 0 + onClicked: root.submitListenBrainzToken() } } SettingRow { diff --git a/crates/qbz-qt/src/integrations_qt.rs b/crates/qbz-qt/src/integrations_qt.rs index b68603a35..1e489b7ea 100644 --- a/crates/qbz-qt/src/integrations_qt.rs +++ b/crates/qbz-qt/src/integrations_qt.rs @@ -199,6 +199,44 @@ fn set_status(text: String, kind: i32) { g.status_kind = kind; } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ListenBrainzValidationStart { + Started, + Busy, + MissingToken, +} + +/// Acquire the ListenBrainz validation slot and publish its first honest UI +/// state atomically. The busy check deliberately precedes the empty-token +/// check: QML used to submit the real token on blur and an accidental empty +/// value on click, so the empty request overwrote "Validating..." while the +/// real request was still in flight. +fn begin_listenbrainz_validation( + ui: &mut IntegrationUi, + has_token: bool, + validating_status: String, + missing_status: String, +) -> ListenBrainzValidationStart { + if ui.listenbrainz_busy { + return ListenBrainzValidationStart::Busy; + } + if !has_token { + ui.status_text = missing_status; + ui.status_kind = 3; + return ListenBrainzValidationStart::MissingToken; + } + ui.listenbrainz_busy = true; + ui.status_text = validating_status; + ui.status_kind = 1; + ListenBrainzValidationStart::Started +} + +fn finish_listenbrainz_validation(ui: &mut IntegrationUi, status: String, kind: i32) { + ui.listenbrainz_busy = false; + ui.status_text = status; + ui.status_kind = kind; +} + // --------------------------------------------------------------------------- // Snapshot fields (folded into SettingsDoc by settings_qt::publish_snapshot) // --------------------------------------------------------------------------- @@ -557,24 +595,27 @@ pub fn discord_clear() { /// sign-in), and force-enable on the first connect. pub async fn listenbrainz_set_token(token: &str) { let token = token.trim().to_string(); - if token.is_empty() { - set_status(qbz_i18n::t("Paste your ListenBrainz user token first"), 3); - crate::settings_qt::publish_snapshot().await; - return; - } - // Re-entrancy guard: the field commits on blur AND the button submits, so - // both can land back-to-back; one validation at a time. - { + let start = { let mut g = ui_state().lock().unwrap(); - if g.listenbrainz_busy { + begin_listenbrainz_validation( + &mut g, + !token.is_empty(), + qbz_i18n::t("Validating..."), + qbz_i18n::t("Paste your ListenBrainz user token first"), + ) + }; + match start { + ListenBrainzValidationStart::Busy => return, + ListenBrainzValidationStart::MissingToken => { + crate::settings_qt::publish_snapshot().await; return; } - g.listenbrainz_busy = true; + ListenBrainzValidationStart::Started => {} } crate::settings_qt::publish_snapshot().await; let client = ListenBrainzClient::new(); - match client.set_token(&token).await { + let (status, kind) = match client.set_token(&token).await { Ok(info) => { if let Err(e) = scrobble().set_listenbrainz_token(&token, &info.user_name) { log::error!("[qbz-qt] persist listenbrainz token failed: {e}"); @@ -593,15 +634,13 @@ pub async fn listenbrainz_set_token(token: &str) { }) .await; } - set_status( - qbz_i18n::t_args("Connected as {}", &[info.user_name.as_str()]), - 2, - ); + let status = qbz_i18n::t_args("Connected as {}", &[info.user_name.as_str()]); request_flush(); + (status, 2) } - Err(e) => set_status(qbz_i18n::t_args("Error: {}", &[&e.to_string()]), 3), - } - ui_state().lock().unwrap().listenbrainz_busy = false; + Err(e) => (qbz_i18n::t_args("Error: {}", &[&e.to_string()]), 3), + }; + finish_listenbrainz_validation(&mut ui_state().lock().unwrap(), status, kind); crate::settings_qt::publish_snapshot().await; } @@ -1453,6 +1492,60 @@ mod tests { } } + #[test] + fn listenbrainz_validation_reports_progress_and_ignores_reentrant_empty_submit() { + let mut ui = IntegrationUi::default(); + assert_eq!( + begin_listenbrainz_validation( + &mut ui, + true, + "Validating...".into(), + "Missing token".into(), + ), + ListenBrainzValidationStart::Started + ); + assert!(ui.listenbrainz_busy); + assert_eq!(ui.status_text, "Validating..."); + assert_eq!(ui.status_kind, 1); + + // Regression: blur submitted the real token, then the button's broken + // child lookup submitted "" and painted a false error over progress. + assert_eq!( + begin_listenbrainz_validation( + &mut ui, + false, + "Validating again...".into(), + "Missing token".into(), + ), + ListenBrainzValidationStart::Busy + ); + assert!(ui.listenbrainz_busy); + assert_eq!(ui.status_text, "Validating..."); + assert_eq!(ui.status_kind, 1); + + finish_listenbrainz_validation(&mut ui, "Connected as listener".into(), 2); + assert!(!ui.listenbrainz_busy); + assert_eq!(ui.status_text, "Connected as listener"); + assert_eq!(ui.status_kind, 2); + } + + #[test] + fn listenbrainz_missing_token_is_an_idle_error() { + let mut ui = IntegrationUi::default(); + assert_eq!( + begin_listenbrainz_validation( + &mut ui, + false, + "Validating...".into(), + "Missing token".into(), + ), + ListenBrainzValidationStart::MissingToken + ); + assert!(!ui.listenbrainz_busy); + assert_eq!(ui.status_text, "Missing token"); + assert_eq!(ui.status_kind, 3); + } + #[test] fn manual_offline_respects_immediate_and_accumulated_modes() { let manual = status(OfflineMode::InducedOffline, Connectivity::Up, false);