mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-02 07:06:48 -06:00
fix(ui): refuse a locked cell's edit before a draft blocks Save (#2945)
This commit is contained in:
@@ -558,6 +558,17 @@ impl Session {
|
||||
pub fn doc_mut(&mut self) -> Result<&mut DocState> {
|
||||
self.active_mut().ok_or(EngineError::NoDocument)
|
||||
}
|
||||
/// The warning an edit of `at` on `sheet` gets from sheet protection, or `None` when the edit
|
||||
/// is allowed. Front ends ask before opening an editor, so typing into a locked cell never
|
||||
/// leaves a draft that can't be entered (and so blocks saving).
|
||||
pub fn locked_cell_message(&self, sheet: usize, at: CellRef) -> Option<String> {
|
||||
let d = self.active()?;
|
||||
let sh = d.wb.sheet(sheet)?;
|
||||
if !sh.is_protected() || !d.wb.styles.get(sh.style_id(at)).protection.locked {
|
||||
return None;
|
||||
}
|
||||
cmd::locked_cell_error(sh, at, &d.unlocked_ranges)
|
||||
}
|
||||
/// Opens a workbook in a new window, which becomes the active one.
|
||||
pub fn add_document(&mut self, d: DocState) -> usize {
|
||||
let uid = d.uid;
|
||||
|
||||
@@ -1400,6 +1400,17 @@ fn protection_refuses_edits_of_locked_cells() {
|
||||
assert_eq!(v(&s, "A1"), Value::Number(1.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn locked_cell_message_matches_what_an_edit_would_be_refused_with() {
|
||||
let s = protected(json!({}));
|
||||
let locked = CellRef::parse("A1").unwrap();
|
||||
let unlocked = CellRef::parse("D1").unwrap();
|
||||
|
||||
assert!(s.locked_cell_message(0, locked).is_some_and(|m| m.contains("protected sheet")));
|
||||
assert_eq!(s.locked_cell_message(0, unlocked), None, "an unlocked cell stays editable");
|
||||
assert_eq!(s.locked_cell_message(9, locked), None, "no such sheet");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn protection_allows_permitted_row_and_column_edits() {
|
||||
let mut s = protected(json!({"insertRows": true, "deleteRows": true, "insertColumns": true}));
|
||||
|
||||
@@ -15,6 +15,13 @@ impl SheetApp {
|
||||
let Some(d) = self.session.active() else { return };
|
||||
let Some(sh) = d.wb.active() else { return };
|
||||
let at = sh.merge_at(d.selection.active).map(|m| m.start).unwrap_or(d.selection.active);
|
||||
// A locked cell on a protected sheet can't be entered (Excel's warning, and the engine
|
||||
// refuses `cell.set` anyway): refuse before a draft exists, rather than leaving one that
|
||||
// can never be committed and blocks Save (#2945).
|
||||
if let Some(m) = self.session.locked_cell_message(d.wb.active_sheet(), at) {
|
||||
self.message = Some(("GridCraft".into(), m));
|
||||
return;
|
||||
}
|
||||
// A picture has no editable scalar text. Selecting it, F2, or clicking
|
||||
// the formula bar must not turn its fallback value into a text edit.
|
||||
// Typing still starts an intentional replacement.
|
||||
@@ -102,6 +109,13 @@ impl SheetApp {
|
||||
}
|
||||
return true;
|
||||
}
|
||||
// A draft on a cell sheet protection now locks (the sheet was protected while it was
|
||||
// open) can't be entered: drop it with the warning instead of keeping it, which would
|
||||
// block Save with a draft that can never be committed (#2945).
|
||||
if let Some(m) = self.session.locked_cell_message(ed.sheet, ed.cell) {
|
||||
self.message = Some(("GridCraft".into(), m));
|
||||
return true;
|
||||
}
|
||||
let text = match self.session.active().map(|d| formula_locale::canonical(&text, ed.locale, &d.wb, ed.sheet, ed.cell)) {
|
||||
Some(Ok(text)) => text,
|
||||
Some(Err(error)) => {
|
||||
@@ -209,3 +223,77 @@ impl SheetApp {
|
||||
self.grid.ensure_visible = true;
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod protected_cell_tests {
|
||||
use super::*;
|
||||
use gridcraft_engine::Session;
|
||||
use gridcraft_engine::core::{CellRef, Value};
|
||||
|
||||
/// A protected sheet whose C2 is unlocked; C2 holds "keep" as the last undo step.
|
||||
fn protected_book() -> SheetApp {
|
||||
let mut session = Session::new();
|
||||
session.new_workbook();
|
||||
let mut app = SheetApp::new(session, Default::default());
|
||||
app.run("cell.set", json!({"cell": "C2", "input": "keep"})).unwrap();
|
||||
app.run("home.formatCells", json!({"range": "C2", "style": {"protection": {"locked": false}}})).unwrap();
|
||||
app.run("review.protectSheet", json!({})).unwrap();
|
||||
app
|
||||
}
|
||||
|
||||
fn value(app: &SheetApp, a1: &str) -> Value {
|
||||
let at = CellRef::parse(a1).unwrap();
|
||||
app.session.active().and_then(|d| d.wb.sheet(0)).map(|sh| sh.value(at)).unwrap_or_default()
|
||||
}
|
||||
|
||||
fn protected_message(app: &SheetApp) -> Option<&str> {
|
||||
app.message.as_ref().map(|(_, m)| m.as_str())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typing_in_a_locked_cell_does_not_leave_a_draft_that_blocks_undo_and_save() {
|
||||
let mut app = protected_book();
|
||||
assert_eq!(app.session.active().unwrap().selection.active.a1(), "A1", "the locked A1 is the active cell");
|
||||
let undo = app.session.active().unwrap().undo.len();
|
||||
let dirty = app.session.active().unwrap().is_dirty();
|
||||
|
||||
app.begin_edit(Some("locked".into()), false);
|
||||
|
||||
assert!(app.editor.is_none(), "a locked cell must not open an editor");
|
||||
assert!(protected_message(&app).is_some_and(|m| m.contains("protected sheet")), "the warning names the protected sheet");
|
||||
assert_eq!(app.session.active().unwrap().undo.len(), undo, "a refused edit adds no undo step");
|
||||
assert_eq!(app.session.active().unwrap().is_dirty(), dirty, "a refused edit leaves the workbook as it was");
|
||||
assert!(app.run("file.saveBytes", json!({"format": "xlsx"})).is_ok(), "Save is not blocked by a draft that can never be entered");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unlocked_cell_on_a_protected_sheet_still_edits_undoes_and_saves() {
|
||||
let mut app = protected_book();
|
||||
app.run("selection.set", json!({"cell": "C2"})).unwrap();
|
||||
app.begin_edit(Some("typed".into()), false);
|
||||
assert!(app.editor.is_some(), "an unlocked cell opens its editor");
|
||||
assert!(app.commit_edit(0, 0, false, false));
|
||||
assert_eq!(value(&app, "C2"), Value::text("typed"));
|
||||
|
||||
assert!(app.run("edit.undo", json!({})).is_ok());
|
||||
assert_eq!(value(&app, "C2"), Value::text("keep"), "Undo of the typed edit still works");
|
||||
assert!(app.run("file.saveBytes", json!({"format": "xlsx"})).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_draft_that_becomes_locked_is_cancelled_instead_of_staying_open() {
|
||||
let mut app = protected_book();
|
||||
app.run("selection.set", json!({"cell": "C2"})).unwrap();
|
||||
app.begin_edit(Some("typed".into()), false);
|
||||
// The sheet is unprotected, C2 locked, and the sheet protected again while the draft is open.
|
||||
app.run("review.unprotectSheet", json!({})).unwrap();
|
||||
app.run("home.formatCells", json!({"range": "C2", "style": {"protection": {"locked": true}}})).unwrap();
|
||||
app.run("review.protectSheet", json!({})).unwrap();
|
||||
|
||||
assert!(app.commit_edit(0, 0, false, false), "the refused draft is handled, not left open");
|
||||
assert!(app.editor.is_none());
|
||||
assert!(protected_message(&app).is_some_and(|m| m.contains("protected sheet")));
|
||||
assert_eq!(value(&app, "C2"), Value::text("keep"));
|
||||
assert!(app.run("file.saveBytes", json!({"format": "xlsx"})).is_ok());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,6 +80,7 @@ mod pivot_show_details;
|
||||
mod pivot_show_details_menu;
|
||||
mod popup_dismissal;
|
||||
mod protect_password;
|
||||
mod protected_cell;
|
||||
mod ribbon_layout;
|
||||
mod right_to_left;
|
||||
mod scroll_position;
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
//! Typing into a locked cell of a protected sheet warns before an editor opens, and the warning
|
||||
//! leaves Undo, Save and typing in unlocked cells working (#2945).
|
||||
|
||||
use egui::{Event, Key, Modifiers};
|
||||
use gridcraft_engine::Session;
|
||||
use gridcraft_engine::core::{CellRef, Value};
|
||||
use gridcraft_ui_egui::SheetApp;
|
||||
use serde_json::json;
|
||||
|
||||
type Harness = egui_kittest::Harness<'static, SheetApp>;
|
||||
|
||||
const PROTECTED: &str = "The cell or chart you're trying to change is on a protected sheet.";
|
||||
|
||||
/// A protected sheet whose C2 is unlocked and holds "keep"; A1 is locked and active.
|
||||
fn harness() -> Harness {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
s.execute("cell.set", json!({"cell": "C2", "input": "keep"})).unwrap();
|
||||
s.execute("home.formatCells", json!({"range": "C2", "style": {"protection": {"locked": false}}})).unwrap();
|
||||
s.execute("review.protectSheet", json!({})).unwrap();
|
||||
s.execute("selection.set", json!({"cell": "A1"})).unwrap();
|
||||
crate::support::harness(s, 4)
|
||||
}
|
||||
|
||||
fn key(h: &mut Harness, k: Key) {
|
||||
h.input_mut().events.push(Event::Key { key: k, physical_key: Some(k), pressed: true, repeat: false, modifiers: Modifiers::NONE });
|
||||
h.input_mut().events.push(Event::Key { key: k, physical_key: Some(k), pressed: false, repeat: false, modifiers: Modifiers::NONE });
|
||||
h.run_steps(2);
|
||||
}
|
||||
|
||||
/// Types text the way a keyboard does (into whatever has focus).
|
||||
fn type_text(h: &mut Harness, text: &str) {
|
||||
h.input_mut().events.push(Event::Text(text.into()));
|
||||
h.run_steps(2);
|
||||
}
|
||||
|
||||
fn value(h: &Harness, a1: &str) -> Value {
|
||||
let at = CellRef::parse(a1).unwrap();
|
||||
h.state().session.active().and_then(|d| d.wb.sheet(0)).map(|sh| sh.value(at)).unwrap_or_default()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typing_in_a_locked_cell_warns_without_a_stuck_draft() {
|
||||
let mut h = harness();
|
||||
let undo = h.state().session.active().unwrap().undo.len();
|
||||
|
||||
type_text(&mut h, "locked");
|
||||
|
||||
assert!(h.state().editor.is_none(), "no editor opened on the locked cell");
|
||||
assert_eq!(h.state().message.as_ref().map(|(_, m)| m.as_str()), Some(PROTECTED));
|
||||
assert_eq!(h.state().session.active().unwrap().undo.len(), undo, "the refused edit added no undo step");
|
||||
assert_eq!(value(&h, "A1"), Value::Empty);
|
||||
// No draft exists, so Save is not blocked while the warning is up.
|
||||
assert!(h.state_mut().run("file.saveBytes", json!({"format": "xlsx"})).is_ok());
|
||||
|
||||
// Enter dismisses the warning and the keyboard goes back to the grid.
|
||||
key(&mut h, Key::Enter);
|
||||
assert!(h.state().message.is_none(), "Enter dismisses the warning");
|
||||
|
||||
// An unlocked cell on the same sheet still edits, undoes and saves.
|
||||
h.state_mut().run("selection.set", json!({"cell": "C2"})).unwrap();
|
||||
h.run_steps(1);
|
||||
type_text(&mut h, "typed");
|
||||
assert_eq!(h.state().editor.as_ref().map(|e| e.text.as_str()), Some("typed"));
|
||||
key(&mut h, Key::Enter);
|
||||
assert_eq!(value(&h, "C2"), Value::text("typed"));
|
||||
assert!(h.state_mut().run("edit.undo", json!({})).is_ok());
|
||||
assert_eq!(value(&h, "C2"), Value::text("keep"), "Undo of the unlocked cell's edit still works");
|
||||
assert!(h.state_mut().run("file.saveBytes", json!({"format": "xlsx"})).is_ok());
|
||||
}
|
||||
Reference in New Issue
Block a user