fix(linux): make the Confirm Before Deleting Rows switch take effect (#3448)

Author: datlechin

linux/ROADMAP.md

@@ -47,7 +47,7 @@ The app runs on GNOME 50 ([ADR 0015](docs/decisions/0015-gnome-50-platform-floor
 | Sorting | A header click sorts tables on the server and query results in the grid |
 | Cell editing | Editors by column kind: inline text, a multi-line editor, a calendar for dates, a toggle for booleans, JSON checked before saving; values are edited from their exact text, never from what the cell displays |
 | Rows | Insert, duplicate and delete; Set NULL and Empty, and DEFAULT on a new row; undo and redo; save or discard pending changes, and stop a save |
-| Bulk delete | Deleting 10 rows or more asks first |
+| Bulk delete | Deleting 10 rows or more asks first, unless Confirm Before Deleting Rows is off in Preferences |
 | Copy | Copy, and Copy As rows, with headers, JSON, CSV, Markdown, IN clause or INSERT; Show Row as JSON |
 | Column widths | Kept per table |
 | CSV import | Into an existing table, with a column mapping, in one write batch |
@@ -102,7 +102,7 @@ The app runs on GNOME 50 ([ADR 0015](docs/decisions/0015-gnome-50-platform-floor
 | Session restore | The windows open at quit open again at launch, behind a preference |
 | Closing safely | Unsaved edits, a save in flight or a running export ask before a window closes or the app quits; logout waits while edits are unsaved, a save runs or an export writes |
 | Notifications | Work that finishes in a tab you are not looking at sends a notification |
-| Preferences | Restore session, page size, query history, editor font and scheme, query timeout. A Confirm Before Deleting Rows switch is shown but nothing reads it yet (see the backlog) |
+| Preferences | Restore session, page size, the bulk delete confirmation, query history, editor font and scheme, query timeout |
 | Shortcuts and About | A shortcuts dialog, and About from the metainfo with third-party licenses and debug info |
 
 ### Packaging and platform
@@ -132,7 +132,6 @@ Not built yet, in no order.
 - Edit binary columns from and to a file. They are read-only today.
 - Time and timestamp pickers. Dates have a calendar; times are typed.
 - Save the column order per table.
-- Make the Confirm Before Deleting Rows preference take effect. The switch is stored, but deleting rows does not read it.
 - Functions, procedures, sequences and triggers in the sidebar.
 - The CREATE INDEX statement for one index. Show CREATE TABLE lists them after the table on PostgreSQL, SQL Server and SQLite.
 - Jump from a foreign key to the table it references.

linux/crates/app/src/ui/browse_tab/edit_handlers.rs

@@ -294,7 +294,7 @@ impl BrowseModel {
         if deletion.is_empty() {
             return;
         }
-        if deletion.len() < BULK_DELETE_CONFIRM_THRESHOLD {
+        if deletion.len() < BULK_DELETE_CONFIRM_THRESHOLD || !self.settings.confirm_bulk_delete() {
             fx.input(EditInput::MarkRowsDeleted(deletion));
             return;
         }

linux/crates/app/src/ui/browse_tab/load_tests.rs

@@ -497,11 +497,11 @@ fn the_loading_page_names_a_table_with_an_ampersand() {
         outcome: Ok(rows(&["late"])),
     });
     let fake = crate::test_support::FakeSession::start(script, [], runtime.tasks());
-    let root = tempfile::tempdir().expect("tempdir");
+    let stores = super::tests::TabStores::new();
     let tab = super::tests::table_tab(
         &fake.session,
         &runtime,
-        &root,
+        &stores,
         "R&D",
         Default::default(),
         Default::default(),

linux/crates/app/src/ui/browse_tab/mod.rs

@@ -54,6 +54,7 @@ mod total_tests;
 
 use std::cell::Cell;
 use std::collections::{BTreeSet, VecDeque};
+use std::rc::Rc;
 use std::sync::Arc;
 
 use gtk4 as gtk;
@@ -67,6 +68,7 @@ use tablepro_core::meta::{RowIdentity, TableRef};
 use tablepro_session::operation::DeadlinePolicy;
 use tablepro_session::runtime::{Tasks, WatchForwarder};
 use tablepro_session::{LinkGeneration, Session};
+use tablepro_storage::AppSettings;
 
 use self::browse_actions::BrowseActions;
 pub(crate) use self::browse_command::{BrowseCommand, ColumnsReply};
@@ -104,13 +106,8 @@ use crate::ui::export::ExportOffer;
 
 const PAGE_SIZE_OPTIONS: &[u64] = &[100, 500, 1_000, 5_000, 10_000];
 const DEFAULT_PAGE_SIZE: u64 = 1_000;
-/// Bulk-delete safety net: when the user marks at least this many
-/// rows pending-delete in one shot, surface a confirmation dialog
-/// before tracking. The marker is reversible via Discard / Ctrl+Z,
-/// but a 200-row Ctrl+A → Delete sequence is destructive enough at
-/// a glance that an explicit confirmation matches GNOME Files'
-/// "Delete N items?" pattern.
-const BULK_DELETE_CONFIRM_THRESHOLD: usize = 10;
+/// A delete of fewer rows never asks, since Undo and Discard bring them back.
+pub(crate) const BULK_DELETE_CONFIRM_THRESHOLD: usize = 10;
 
 glib::wrapper! {
     pub struct BrowseTab(ObjectSubclass<imp::BrowseTab>)
@@ -135,6 +132,7 @@ pub struct BrowseTabInit {
     pub tasks: Tasks,
     /// The session's deadlines, which a count that ran out names.
     pub deadlines: Arc<dyn DeadlinePolicy>,
+    pub settings: Rc<AppSettings>,
 }
 
 impl BrowseTab {
@@ -183,6 +181,7 @@ impl BrowseTab {
             filter_settings: init.filter_settings,
             pager: BrowsePager::new(init.page_size, init.initial_offset),
             deadlines: init.deadlines,
+            settings: init.settings,
             current_sort: init.initial_sort,
             current_filter: initial_filter,
             current_columns: Vec::new(),
@@ -488,6 +487,8 @@ pub(crate) struct BrowseModel {
     /// Where the page the tab reads starts, and how many rows it holds.
     pager: BrowsePager,
     deadlines: Arc<dyn DeadlinePolicy>,
+    /// Read at each delete, so a change in Preferences reaches open tabs.
+    settings: Rc<AppSettings>,
     current_sort: Option<(usize, bool)>,
     /// The WHERE every page and count reads under, kept per connection,
     /// schema and table.

linux/crates/app/src/ui/browse_tab/tests.rs

@@ -21,7 +21,7 @@ use crate::services::change_tracker::{RowKey, StatementSource};
 use crate::services::tab_tracker_handle::TabTrackerHandle;
 use crate::services::tracker_delta::TrackerDelta;
 use crate::services::tracker_edit::TrackerEdit;
-use crate::test_support::{FakeSession, after_next_frame, drain_main_context, wait_until};
+use crate::test_support::{FakeSession, MemorySettings, after_next_frame, drain_main_context, wait_until};
 use crate::ui::clipboard::InsertRow;
 use crate::ui::grid::{GridCell, GridRowIndex, RowObject};
 
@@ -152,7 +152,7 @@ pub(super) struct Harness {
     pub(super) session: Arc<Session>,
     pub(super) connection: Arc<FakeConnection>,
     pub(super) fake: FakeSession,
-    root: tempfile::TempDir,
+    pub(super) stores: TabStores,
     runtime: Option<crate::runtime::AppRuntime>,
 }
 
@@ -184,17 +184,17 @@ impl Harness {
         crate::register_test_resources();
         let runtime = crate::test_support::test_runtime();
         let fake = FakeSession::start(script, reconnects, runtime.tasks());
-        let root = tempfile::tempdir().expect("tempdir");
+        let stores = TabStores::new();
         let tracker = TabTrackerHandle::default();
-        let tab = filtered_users_tab(&fake.session, &runtime, &root, saved, tracker.clone(), read_only);
+        let tab = filtered_users_tab(&fake.session, &runtime, &stores, saved, tracker.clone(), read_only);
         Self {
             tab: Some(tab),
             tracker,
             others: Vec::new(),
             session: fake.session.clone(),
             connection: fake.connection.clone(),
             fake,
-            root,
+            stores,
             runtime: Some(runtime),
         }
     }
@@ -206,7 +206,7 @@ impl Harness {
     /// Another tab on the same table and session.
     pub(super) fn open_another(&mut self) -> BrowseTab {
         let runtime = self.runtime.as_ref().expect("the runtime is running");
-        let tab = users_tab(&self.session, runtime, &self.root);
+        let tab = users_tab(&self.session, runtime, &self.stores);
         self.others.push(tab.clone());
         tab
     }
@@ -298,15 +298,26 @@ impl Drop for Shown {
     }
 }
 
-pub(super) fn users_tab(
-    session: &Arc<Session>,
-    runtime: &crate::runtime::AppRuntime,
-    root: &tempfile::TempDir,
-) -> BrowseTab {
+/// The files and preferences a test tab reads.
+pub(super) struct TabStores {
+    root: tempfile::TempDir,
+    pub(super) settings: MemorySettings,
+}
+
+impl TabStores {
+    pub(super) fn new() -> Self {
+        Self {
+            root: tempfile::tempdir().expect("tempdir"),
+            settings: MemorySettings::new(),
+        }
+    }
+}
+
+pub(super) fn users_tab(session: &Arc<Session>, runtime: &crate::runtime::AppRuntime, stores: &TabStores) -> BrowseTab {
     filtered_users_tab(
         session,
         runtime,
-        root,
+        stores,
         FilterSet::default(),
         TabTrackerHandle::default(),
         false,
@@ -316,26 +327,27 @@ pub(super) fn users_tab(
 pub(super) fn filtered_users_tab(
     session: &Arc<Session>,
     runtime: &crate::runtime::AppRuntime,
-    root: &tempfile::TempDir,
+    stores: &TabStores,
     saved: FilterSet,
     tracker: TabTrackerHandle,
     read_only: bool,
 ) -> BrowseTab {
-    table_tab(session, runtime, root, "users", saved, tracker, read_only)
+    table_tab(session, runtime, stores, "users", saved, tracker, read_only)
 }
 
 pub(super) fn table_tab(
     session: &Arc<Session>,
     runtime: &crate::runtime::AppRuntime,
-    root: &tempfile::TempDir,
+    stores: &TabStores,
     table: &str,
     saved: FilterSet,
     tracker: TabTrackerHandle,
     read_only: bool,
 ) -> BrowseTab {
     let tasks = runtime.tasks();
     let connection_id = uuid::Uuid::new_v4();
-    let filter_settings = crate::persistence::FilterSettingsStore::load(root.path().join("filters.json"), &tasks);
+    let root = stores.root.path();
+    let filter_settings = crate::persistence::FilterSettingsStore::load(root.join("filters.json"), &tasks);
     filter_settings.set_filters(connection_id, None, table, saved);
     BrowseTab::new(BrowseTabInit {
         tracker,
@@ -346,13 +358,14 @@ pub(super) fn table_tab(
         page_size: 100,
         initial_offset: 0,
         initial_sort: None,
-        column_widths: crate::persistence::ColumnWidthStore::load(root.path().join("column-widths.json"), &tasks),
+        column_widths: crate::persistence::ColumnWidthStore::load(root.join("column-widths.json"), &tasks),
         filter_settings,
         session: session.clone(),
         tasks,
         deadlines: Arc::new(tablepro_session::operation::DefaultDeadlines::new(
             tablepro_core::LivenessPolicy::DESKTOP,
         )),
+        settings: stores.settings.get().clone(),
     })
 }
 
@@ -2868,6 +2881,59 @@ fn a_second_delete_replaces_the_waiting_confirmation() {
     assert_eq!(harness.tracker.pending_count(), DOZEN.len());
 }
 
+#[gtk4::test]
+fn a_bulk_delete_with_the_confirmation_off_marks_its_rows_at_once() {
+    let mut script = serving_table();
+    script.query_bound.always(FakeReply::ok(rows(&DOZEN)));
+    let harness = Harness::open(script, []);
+    let _shown = Shown::new(harness.tab());
+    wait_until(WAIT, || harness.shown().len() == DOZEN.len()).expect("the first page");
+
+    harness
+        .stores
+        .settings
+        .get()
+        .set_confirm_bulk_delete(false)
+        .expect("turn the confirmation off");
+    harness.selection().select_all();
+    delete_rows(harness.tab());
+    drain_main_context();
+
+    assert!(
+        confirming(harness.tab()).is_none(),
+        "the delete asked with the switch off"
+    );
+    assert_eq!(harness.tracker.pending_count(), DOZEN.len());
+}
+
+#[gtk4::test]
+fn turning_the_confirmation_back_on_asks_again_in_an_open_tab() {
+    let mut script = serving_table();
+    script.query_bound.always(FakeReply::ok(rows(&DOZEN)));
+    let harness = Harness::open(script, []);
+    let _shown = Shown::new(harness.tab());
+    let settings = harness.stores.settings.get();
+    settings
+        .set_confirm_bulk_delete(false)
+        .expect("turn the confirmation off");
+    wait_until(WAIT, || harness.shown().len() == DOZEN.len()).expect("the first page");
+    harness.selection().select_all();
+    delete_rows(harness.tab());
+    drain_main_context();
+    assert_eq!(harness.tracker.pending_count(), DOZEN.len());
+    harness.tab().send(EditInput::Undo);
+    assert_eq!(harness.tracker.pending_count(), 0);
+
+    settings
+        .set_confirm_bulk_delete(true)
+        .expect("turn the confirmation on");
+    let dialog = ask_to_delete_every_row(&harness);
+    crate::test_support::respond(&dialog, "delete");
+    drain_main_context();
+
+    assert_eq!(harness.tracker.pending_count(), DOZEN.len());
+}
+
 #[gtk4::test]
 fn insert_row_opens_the_new_rows_first_editable_cell() {
     let (harness, shown) = one_cell(named_col("name", "text", true), Value::Text("Ada".into()));

linux/crates/app/src/ui/connect_dialog/imp.rs

@@ -75,6 +75,9 @@ pub struct ConnectDialog {
     pub mode: RefCell<ConnectDialogMode>,
     /// An Edit's save in flight, which keeps Save from running twice.
     pub saving: Cell<bool>,
+    /// The user can close the dialog while a save runs, and the save
+    /// must not close it again when it lands.
+    pub has_closed: Cell<bool>,
     pub pem: PemValidation,
     /// The TLS settings the last file check read, so a keystroke in an
     /// unrelated row does not read the files again.
@@ -124,6 +127,7 @@ impl WidgetImpl for ConnectDialog {}
 
 impl AdwDialogImpl for ConnectDialog {
     fn closed(&self) {
+        self.has_closed.set(true);
         self.tests.cancel();
         self.testing.set(false);
         self.parent_closed();

linux/crates/app/src/ui/connect_dialog/mod.rs

@@ -673,7 +673,9 @@ impl ConnectDialog {
                 match outcome {
                     Ok(Ok(saved)) => {
                         dialog.emit_by_name::<()>("saved", &[&saved.id.to_string()]);
-                        dialog.close();
+                        if !dialog.imp().has_closed.get() {
+                            dialog.close();
+                        }
                     }
                     Ok(Err(error)) => {
                         tracing::warn!(%error, "could not save the edited connection");