Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions linux/ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion linux/crates/app/src/ui/browse_tab/edit_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions linux/crates/app/src/ui/browse_tab/load_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
15 changes: 8 additions & 7 deletions linux/crates/app/src/ui/browse_tab/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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>)
Expand All @@ -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 {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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.
Expand Down
100 changes: 83 additions & 17 deletions linux/crates/app/src/ui/browse_tab/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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>,
}

Expand Down Expand Up @@ -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),
}
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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(),
})
}

Expand Down Expand Up @@ -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()));
Expand Down
4 changes: 4 additions & 0 deletions linux/crates/app/src/ui/connect_dialog/imp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion linux/crates/app/src/ui/connect_dialog/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading
Loading