diff --git a/README.md b/README.md index e15a6fa..eb9db00 100644 --- a/README.md +++ b/README.md @@ -239,7 +239,7 @@ Cordiale also covers: `/connect`/`/disconnect`/`/reconnect`/`/quit`, `/ignore`, `/notify`, `/hilight`, `/ame`/`/amsg`, `/alias`/`/unalias` with Cicchetto's alias expansion, `/kb` with a default ban type of nick, host or user@host - chosen in Settings and kept on the device, `/beep`, the services + chosen in Settings and kept in your Grappa account, `/beep`, the services shortcuts), `/np` for the radio, and a bare `/umode` that opens the user-mode toggles; a bare `/topic` or `/mode` shows the channel's topic or modes in the status bar. diff --git a/crates/cordiale-core/src/ban.rs b/crates/cordiale-core/src/ban.rs index 5931f1d..3905f45 100644 --- a/crates/cordiale-core/src/ban.rs +++ b/crates/cordiale-core/src/ban.rs @@ -23,8 +23,8 @@ //! Ban masks for `/kb` and `/kickban`: which form the user prefers and how //! each one is built from what is known about the target. //! -//! The choice is a device-local preference (`Settings::default_ban_type`): -//! Grappa has no account-level field for it yet. A mask is only ever built +//! The choice is an account setting kept by Grappa (`ban_mask_form`, see +//! `GrappaClient::fetch_ban_mask_form`). A mask is only ever built //! in the exact form asked for; when a part it needs is missing there is //! no mask, never a broader or different one. @@ -57,6 +57,25 @@ impl BanType { !matches!(self, BanType::Nick) } + /// The value Grappa stores in `ban_mask_form`. + pub fn wire_name(self) -> &'static str { + match self { + BanType::Nick => "nick", + BanType::Host => "host", + BanType::UserHost => "user_host", + } + } + + /// The form for a `ban_mask_form` value; `None` for anything else. + pub fn from_wire_name(name: &str) -> Option { + match name { + "nick" => Some(BanType::Nick), + "host" => Some(BanType::Host), + "user_host" => Some(BanType::UserHost), + _ => None, + } + } + /// The position in the Settings picker. pub fn index(self) -> i32 { match self { @@ -111,6 +130,17 @@ fn mask_part_is_valid(value: &str) -> bool { mod tests { use super::*; + #[test] + fn wire_names_round_trip() { + for ban_type in [BanType::Nick, BanType::Host, BanType::UserHost] { + assert_eq!( + BanType::from_wire_name(ban_type.wire_name()), + Some(ban_type) + ); + } + assert_eq!(BanType::from_wire_name("mask"), None); + } + #[test] fn builds_each_form() { assert_eq!( diff --git a/crates/cordiale-core/src/client.rs b/crates/cordiale-core/src/client.rs index f30e613..32ce25c 100644 --- a/crates/cordiale-core/src/client.rs +++ b/crates/cordiale-core/src/client.rs @@ -1144,6 +1144,59 @@ impl GrappaClient { .await } + /// `GET /me/settings/ban-mask-form` (protocol v38): the account's + /// default ban form. An absent or unknown value, and a server without + /// the endpoint (404), read as `host`, the server default. + pub async fn fetch_ban_mask_form( + &self, + token: &str, + ) -> Result { + match self + .fetch_setting(token, "ban-mask-form", "ban_mask_form") + .await + { + Ok(value) => Ok(value + .as_str() + .and_then(crate::ban::BanType::from_wire_name) + .unwrap_or_default()), + Err(err) if err.status() == Some(StatusCode::NOT_FOUND) => { + Ok(crate::ban::BanType::default()) + } + Err(err) => Err(err), + } + } + + /// `PUT /me/settings/ban-mask-form`: returns the form the server + /// stored, read from its response (the sent one if the response + /// carries none). + pub async fn set_ban_mask_form( + &self, + token: &str, + ban_type: crate::ban::BanType, + ) -> Result { + let url = format!("{}/me/settings/ban-mask-form", self.base_url); + let body = serde_json::json!({ "ban_mask_form": ban_type.wire_name() }); + let response = self + .http + .put(url) + .bearer_auth(token) + .json(&body) + .send() + .await? + .error_for_status()?; + let stored = response + .json::() + .await + .ok() + .and_then(|body| { + body.get("ban_mask_form") + .and_then(Value::as_str) + .and_then(crate::ban::BanType::from_wire_name) + }) + .unwrap_or(ban_type); + Ok(stored) + } + /// The text sent when the bouncer marks the user away (`None`: the /// server's own). pub async fn fetch_auto_away_reason( @@ -4664,6 +4717,90 @@ mod tests { .expect("dcc"); } + #[tokio::test] + async fn ban_mask_form_reads_and_saves_all_three_forms() { + use crate::ban::BanType; + for (wire, expected) in [ + ("nick", BanType::Nick), + ("host", BanType::Host), + ("user_host", BanType::UserHost), + ] { + let mock_server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/me/settings/ban-mask-form")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({ "ban_mask_form": wire })), + ) + .mount(&mock_server) + .await; + Mock::given(method("PUT")) + .and(path("/me/settings/ban-mask-form")) + .and(body_json(serde_json::json!({ "ban_mask_form": wire }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({ "ban_mask_form": wire })), + ) + .expect(1) + .mount(&mock_server) + .await; + let client = GrappaClient::new(mock_server.uri()); + assert_eq!( + client.fetch_ban_mask_form("t").await.expect("get"), + expected + ); + assert_eq!( + client.set_ban_mask_form("t", expected).await.expect("put"), + expected + ); + } + } + + #[tokio::test] + async fn ban_mask_form_falls_back_to_host_and_reports_failures() { + use crate::ban::BanType; + // Pre-38 server: no endpoint at all. + let old_server = MockServer::start().await; + let client = GrappaClient::new(old_server.uri()); + assert_eq!( + client.fetch_ban_mask_form("t").await.expect("404 is host"), + BanType::Host + ); + assert!(client.set_ban_mask_form("t", BanType::Nick).await.is_err()); + + // Absent or unknown value reads as host. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/me/settings/ban-mask-form")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) + .mount(&server) + .await; + let client = GrappaClient::new(server.uri()); + assert_eq!( + client.fetch_ban_mask_form("t").await.expect("absent"), + BanType::Host + ); + + // Any other failure is an error, not a silent host. + let failing = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/me/settings/ban-mask-form")) + .respond_with(ResponseTemplate::new(500)) + .mount(&failing) + .await; + Mock::given(method("PUT")) + .and(path("/me/settings/ban-mask-form")) + .respond_with(ResponseTemplate::new(422)) + .mount(&failing) + .await; + let client = GrappaClient::new(failing.uri()); + assert!(client.fetch_ban_mask_form("t").await.is_err()); + assert!(client + .set_ban_mask_form("t", BanType::UserHost) + .await + .is_err()); + } + #[tokio::test] async fn admin_account_and_network_writes_use_their_contracts() { // Built at run time: the test only checks the value is passed diff --git a/crates/cordiale-core/src/persistence.rs b/crates/cordiale-core/src/persistence.rs index ed2615a..adc2633 100644 --- a/crates/cordiale-core/src/persistence.rs +++ b/crates/cordiale-core/src/persistence.rs @@ -39,7 +39,6 @@ use std::time::{SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; -use crate::ban::BanType; use crate::domain::{Profile, Server}; use crate::passkey_origin::{passkey_origin, OverrideCheck}; use crate::presence::PresencePref; @@ -133,10 +132,6 @@ pub struct Settings { /// shown above the compose box lives here. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub mute_since: BTreeMap, - /// Ban form used by `/kb` and `/kickban`. Kept on this device only: - /// Grappa has no account-level field for it. - #[serde(default)] - pub default_ban_type: BanType, /// "Shrink videos before sending": the default for video uploads, kept on /// this device only (never sent to Grappa). Off until the user turns it /// on; the upload popup starts from it and never writes it back. @@ -185,7 +180,6 @@ impl Default for Settings { presence_pins: BTreeMap::new(), presence_unsynced: BTreeSet::new(), mute_since: BTreeMap::new(), - default_ban_type: BanType::default(), shrink_videos: false, } } @@ -1079,7 +1073,6 @@ mod tests { presence_pins: BTreeMap::from([("libera #rust".to_string(), PresencePref::Hide)]), presence_unsynced: BTreeSet::from(["libera #rust".to_string()]), mute_since: BTreeMap::from([("libera #rust".to_string(), 1_700_000_000)]), - default_ban_type: BanType::UserHost, shrink_videos: true, }; @@ -1100,13 +1093,13 @@ mod tests { } #[test] - fn settings_without_a_ban_type_default_to_host() { - let decoded: Settings = - serde_json::from_str(r#"{"schema_version":1,"theme":"light"}"#).expect("deserialize"); - assert_eq!(decoded.default_ban_type, BanType::Host); + fn settings_with_the_retired_ban_type_still_load() { let decoded: Settings = - serde_json::from_str(r#"{"default_ban_type":"user_host"}"#).expect("deserialize"); - assert_eq!(decoded.default_ban_type, BanType::UserHost); + serde_json::from_str(r#"{"default_ban_type":"user_host","theme":"dark"}"#) + .expect("deserialize"); + assert_eq!(decoded.theme, Theme::Dark); + let json = serde_json::to_value(&decoded).expect("serialize"); + assert!(json.get("default_ban_type").is_none()); } #[test] diff --git a/crates/cordiale-ui/lang/de/LC_MESSAGES/cordiale-ui.po b/crates/cordiale-ui/lang/de/LC_MESSAGES/cordiale-ui.po index f35c394..8cc6f59 100644 --- a/crates/cordiale-ui/lang/de/LC_MESSAGES/cordiale-ui.po +++ b/crates/cordiale-ui/lang/de/LC_MESSAGES/cordiale-ui.po @@ -1229,8 +1229,8 @@ msgid "User and host (*!user@host)" msgstr "Benutzer und Host (*!benutzer@host)" msgctxt "AppWindow" -msgid "Used by /kb and /kickban. Saved on this device only." -msgstr "Wird von /kb und /kickban verwendet. Nur auf diesem Gerät gespeichert." +msgid "Used by /kb and /kickban. Saved to your account." +msgstr "Wird von /kb und /kickban verwendet. In deinem Konto gespeichert." msgctxt "AppWindow" msgid "Done." diff --git a/crates/cordiale-ui/lang/es/LC_MESSAGES/cordiale-ui.po b/crates/cordiale-ui/lang/es/LC_MESSAGES/cordiale-ui.po index 90ee4c9..8d07a60 100644 --- a/crates/cordiale-ui/lang/es/LC_MESSAGES/cordiale-ui.po +++ b/crates/cordiale-ui/lang/es/LC_MESSAGES/cordiale-ui.po @@ -1229,8 +1229,8 @@ msgid "User and host (*!user@host)" msgstr "Usuario y host (*!usuario@host)" msgctxt "AppWindow" -msgid "Used by /kb and /kickban. Saved on this device only." -msgstr "Lo usan /kb y /kickban. Se guarda solo en este dispositivo." +msgid "Used by /kb and /kickban. Saved to your account." +msgstr "Lo usan /kb y /kickban. Se guarda en tu cuenta." msgctxt "AppWindow" msgid "Done." diff --git a/crates/cordiale-ui/lang/fr/LC_MESSAGES/cordiale-ui.po b/crates/cordiale-ui/lang/fr/LC_MESSAGES/cordiale-ui.po index 8e6087c..3a2d541 100644 --- a/crates/cordiale-ui/lang/fr/LC_MESSAGES/cordiale-ui.po +++ b/crates/cordiale-ui/lang/fr/LC_MESSAGES/cordiale-ui.po @@ -1229,8 +1229,8 @@ msgid "User and host (*!user@host)" msgstr "Utilisateur et hôte (*!utilisateur@hôte)" msgctxt "AppWindow" -msgid "Used by /kb and /kickban. Saved on this device only." -msgstr "Utilisé par /kb et /kickban. Enregistré uniquement sur cet appareil." +msgid "Used by /kb and /kickban. Saved to your account." +msgstr "Utilisé par /kb et /kickban. Enregistré dans votre compte." msgctxt "AppWindow" msgid "Done." diff --git a/crates/cordiale-ui/lang/it/LC_MESSAGES/cordiale-ui.po b/crates/cordiale-ui/lang/it/LC_MESSAGES/cordiale-ui.po index 148ff24..440b629 100644 --- a/crates/cordiale-ui/lang/it/LC_MESSAGES/cordiale-ui.po +++ b/crates/cordiale-ui/lang/it/LC_MESSAGES/cordiale-ui.po @@ -1229,8 +1229,8 @@ msgid "User and host (*!user@host)" msgstr "Utente e host (*!utente@host)" msgctxt "AppWindow" -msgid "Used by /kb and /kickban. Saved on this device only." -msgstr "Usato da /kb e /kickban. Salvato solo su questo dispositivo." +msgid "Used by /kb and /kickban. Saved to your account." +msgstr "Usato da /kb e /kickban. Salvato nel tuo account." msgctxt "AppWindow" msgid "Done." diff --git a/crates/cordiale-ui/src/main.rs b/crates/cordiale-ui/src/main.rs index ad72a23..feca815 100644 --- a/crates/cordiale-ui/src/main.rs +++ b/crates/cordiale-ui/src/main.rs @@ -408,6 +408,8 @@ enum WorkerCommand { text_pattern: Option, }, PerformSave(String), + /// A new account default ban form picked in Settings. + BanTypeSave(cordiale_core::ban::BanType), PersonalPrefsSave { leave_message: String, away_message: String, @@ -517,7 +519,6 @@ fn main() -> Result<(), slint::PlatformError> { ui.set_theme(theme_to_slint(settings.theme)); ui.set_palette_muted(slint_color(classic_muted(settings.theme == Theme::Dark))); ui.set_font_size_percent(i32::from(settings.effective_font_size_percent())); - ui.set_ban_type_index(settings.default_ban_type.index()); ui.invoke_apply_color_scheme(); // A built-in color theme applies from the first screen; a Grappa one // needs the session and is applied after sign-in. @@ -1140,6 +1141,10 @@ struct SettingsState { /// The account's command aliases, read on the first slash command and /// dropped whenever they change so the next one reads them again. aliases: Option>, + /// The account's default ban form (`ban_mask_form`), as the server last + /// reported or accepted it; `None` until read, or when the read failed + /// (the next `/kb` reads it again, and `host` applies if that fails). + ban_type: Option, /// Display copy of the account-wide auto-away delay; `None` until the /// server announces it. Grappa applies the value itself. auto_away_debounce: Option, @@ -1430,6 +1435,7 @@ impl WorkerState { presence_unsynced: settings.presence_unsynced, mute_since: settings.mute_since, aliases: None, + ban_type: None, auto_away_debounce: None, quit_part_reason: None, auto_away_reason: None, @@ -2226,6 +2232,9 @@ async fn run_worker( Some(WorkerCommand::AvatarRemove) => { handle_avatar_remove(&state, &ui).await; } + Some(WorkerCommand::BanTypeSave(ban_type)) => { + run_ban_type_save(&mut state, &ui, ban_type).await; + } Some(WorkerCommand::PersonalPrefsSave { leave_message, away_message, @@ -2810,6 +2819,7 @@ async fn finish_connect( state.panels.reply_view = None; state.panels.whois_card = None; state.prefs.auto_away_debounce = None; + state.prefs.ban_type = None; state.prefs.quit_part_reason = None; state.prefs.auto_away_reason = None; state.prefs.away_nick_suffix = None; @@ -2905,6 +2915,9 @@ async fn finish_connect( sync_presence_pins(state).await; handle_load_notification_prefs(state, ui).await; + // Before the first command can run, so the first Kickban uses + // the stored form. + load_ban_type(state, ui).await; let prefs_client = GrappaClient::new(server_url.clone()); let prefs_token = token.clone(); @@ -4577,6 +4590,66 @@ async fn user_aliases(state: &mut WorkerState) -> HashMap { state.prefs.aliases.clone().unwrap_or_default() } +/// Reads the account's default ban form into the worker state and the +/// Settings picker. A failed read leaves it unknown (the picker shows +/// `host`, the server default). +async fn load_ban_type(state: &mut WorkerState, ui: &slint::Weak) { + let (Some(client), Some(token)) = (state.conn.client.clone(), state.conn.token.clone()) else { + return; + }; + match client.fetch_ban_mask_form(&token).await { + Ok(ban_type) => { + state.prefs.ban_type = Some(ban_type); + let _ = ui.upgrade_in_event_loop(move |ui| ui.set_ban_type_index(ban_type.index())); + } + Err(err) => { + persistence::log_line(&format!("ban mask form load failed: {err:?}")); + let host = cordiale_core::ban::BanType::default().index(); + let _ = ui.upgrade_in_event_loop(move |ui| ui.set_ban_type_index(host)); + } + } +} + +/// The form `/kb` and `/kickban` use: the account's, read again if the +/// sign-in read failed, `host` when it still can't be known. +async fn effective_ban_type( + state: &mut WorkerState, + ui: &slint::Weak, +) -> cordiale_core::ban::BanType { + if state.prefs.ban_type.is_none() { + load_ban_type(state, ui).await; + } + state.prefs.ban_type.unwrap_or_default() +} + +/// Body of the `WorkerCommand::BanTypeSave` arm of `run_worker`: the +/// active form changes only once the server accepted the PUT; otherwise +/// the picker goes back to the previous one and the error is shown. +async fn run_ban_type_save( + state: &mut WorkerState, + ui: &slint::Weak, + ban_type: cordiale_core::ban::BanType, +) { + let previous = state.prefs.ban_type.unwrap_or_default(); + let saved = match (state.conn.client.clone(), state.conn.token.clone()) { + (Some(client), Some(token)) => client.set_ban_mask_form(&token, ban_type).await, + _ => return, + }; + match saved { + Ok(stored) => { + state.prefs.ban_type = Some(stored); + let _ = ui.upgrade_in_event_loop(move |ui| ui.set_ban_type_index(stored.index())); + } + Err(err) => { + persistence::log_line(&format!("ban mask form save failed: {err:?}")); + let _ = ui.upgrade_in_event_loop(move |ui| { + ui.set_ban_type_index(previous.index()); + ui.set_status_kind("personal-prefs-failed".into()); + }); + } + } +} + /// A `/kb`, Kickban or Ban host waiting for the target's host; the kick /// follows the ban only when `kick_reason` is set. The ban form is fixed /// when the request starts, so changing the setting meanwhile doesn't diff --git a/crates/cordiale-ui/src/slash.rs b/crates/cordiale-ui/src/slash.rs index 5a1a7ae..0f8ed73 100644 --- a/crates/cordiale-ui/src/slash.rs +++ b/crates/cordiale-ui/src/slash.rs @@ -268,9 +268,7 @@ pub(crate) async fn run_slash_command( } SlashCommand::KickBan { nick, reason } if in_channel => { // Read once here: the form stays fixed for this request. - let ban_type = persistence::load_settings() - .unwrap_or_default() - .default_ban_type; + let ban_type = effective_ban_type(state, ui).await; start_kickban( state, ui, diff --git a/crates/cordiale-ui/src/ui_callbacks.rs b/crates/cordiale-ui/src/ui_callbacks.rs index 0b73649..7ef8812 100644 --- a/crates/cordiale-ui/src/ui_callbacks.rs +++ b/crates/cordiale-ui/src/ui_callbacks.rs @@ -40,10 +40,11 @@ pub(crate) fn register_radio_callbacks( let _ = persistence::save_settings(&settings); }); + let tx_for_ban_type = worker_tx.clone(); ui.on_ban_type_changed(move |index| { - let mut settings = persistence::load_settings().unwrap_or_default(); - settings.default_ban_type = cordiale_core::ban::BanType::from_index(index); - let _ = persistence::save_settings(&settings); + let _ = tx_for_ban_type.send(WorkerCommand::BanTypeSave( + cordiale_core::ban::BanType::from_index(index), + )); }); let weak_for_radio_save = ui.as_weak(); diff --git a/crates/cordiale-ui/ui/appwindow.slint b/crates/cordiale-ui/ui/appwindow.slint index 3db9b23..da36698 100644 --- a/crates/cordiale-ui/ui/appwindow.slint +++ b/crates/cordiale-ui/ui/appwindow.slint @@ -4722,7 +4722,7 @@ export component AppWindow inherits Window { } } Text { - text: @tr("Used by /kb and /kickban. Saved on this device only."); + text: @tr("Used by /kb and /kickban. Saved to your account."); wrap: word-wrap; color: palette-muted; } diff --git a/docs/feature-matrix.md b/docs/feature-matrix.md index 87fdf11..9ffe170 100644 --- a/docs/feature-matrix.md +++ b/docs/feature-matrix.md @@ -281,9 +281,13 @@ risposta non altera la richiesta. Se manca una parte necessaria alla forma scelta (per esempio `not_cached`) non parte né il ban né il kick, la barra di stato lo dice e non si ripiega mai su una maschera diversa; prima il kick partiva comunque. Ordine ban poi kick invariato, comandi senza conferma del -server. Limiti: la preferenza è **solo locale al dispositivo** -(`default_ban_type` in `settings.json`), perché Grappa non ha ancora un campo -di account per questo; la grafia `*!user@host` è quella IRC usuale, non +server. La preferenza è ora +un'impostazione di account (`GET/PUT /me/settings/ban-mask-form`, protocollo +38, `ban_mask_form`): si legge all'accesso, si salva con PUT e il valore +attivo cambia solo a risposta riuscita; assente o 404 (server pre-38) vale +`host`. Il vecchio campo locale `default_ban_type` è ritirato e ignorato in +`settings.json`, senza caricarlo sul server. Cordiale dichiara ancora +`client_proto=37`: il bump a 38 va riletto sul sorgente di Grappa. Limiti: la grafia `*!user@host` è quella IRC usuale, non riletta sul sorgente upstream (issue upstream 2347), e va riallineata quando Cicchetto fissa dove salva l'impostazione. Il menu Kickban e le voci Ban nick / Ban host (issue 246) non ci sono ancora: potranno usare `BanType::mask`.