diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 24bd043f7..e003cfffe 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -232,6 +232,9 @@ fn sort(s: &mut Session, p: &Json) -> Result { } fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_cols: bool, case: bool) -> Result { + // Protect Sheet ▸ Sort. Both Sort… and the A-to-Z buttons come through here. + let sheet = s.doc()?.wb.active_sheet; + super::refuse_unless_allowed(s, sheet, "Sorting", |p| p.sort)?; let body = if by_cols { if header && r.width() > 1 { RangeRef::new(CellRef::new(r.start.row, r.start.col + 1), r.end) } else { r } } else if header && r.height() > 1 { @@ -438,6 +441,7 @@ fn sorted_anchor(sh: &Sheet, a: Anchor, bottom: (u32, u32, f32, f32), by_cols: b fn toggle_filter(s: &mut Session, p: &Json) -> Result { let sheet = s.doc()?.wb.active_sheet; + super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?; let has = s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.autofilter.is_some()); let in_table = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.table_at(s.doc().map(|d| d.selection.active).unwrap_or_default()).map(|t| t.name.clone())); @@ -492,6 +496,7 @@ fn install_filter(s: &mut Session, p: &Json) -> Result { fn filter_by(s: &mut Session, p: &Json) -> Result { let sheet = s.doc()?.wb.active_sheet; + super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?; let Some(sh) = s.doc()?.wb.sheet(sheet) else { return Err(EngineError::NoDocument) }; let af = match &sh.autofilter { Some(a) => a.clone(), diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 50865a2b7..9b44e6741 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -394,6 +394,18 @@ fn check_protection(old: &Workbook, new: &Workbook, unlocked: &[(String, String) Ok(()) } +/// Refuses an action a protected sheet does not allow, by the tick box for it in Review ▸ Protect +/// Sheet. `what` names the action in the message. An unprotected sheet allows everything. +/// +/// Cell edits are guarded separately, by whether the cell is locked (`locked_cell_error`): these +/// permissions are the ones that are not about one cell's contents. +pub(crate) fn refuse_unless_allowed(s: &Session, sheet: usize, what: &str, pick: fn(&gridcraft_model::SheetProtection) -> bool) -> Result<()> { + if s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.protection.as_ref().is_some_and(|pr| !pick(pr))) { + return Err(EngineError::Other(format!("{what} isn't allowed on this protected sheet (Review › Unprotect Sheet)."))); + } + Ok(()) +} + /// Whether a locked cell on a protected sheet stays locked: `None` when it's in an Allow Edit /// Range without a password or one unlocked in this session, else the message to show. pub(crate) fn locked_cell_error(sh: &gridcraft_model::Sheet, c: CellRef, unlocked: &[(String, String)]) -> Option { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ac290654..0e648fd22 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3344,3 +3344,88 @@ fn csv_as_text_keeps_identifier_columns_as_written() { let saved = s.execute("file.saveBytes", json!({"format": "csv"})).unwrap(); assert_eq!(crate::io::base64_decode(saved["base64"].as_str().unwrap()).unwrap(), csv.replace('\n', "\r\n").as_bytes()); } + +/// Protect Sheet's Sort and Use AutoFilter tick boxes are obeyed. They were read from the file and +/// written back but never consulted, so a protected sheet could still be sorted and filtered. +/// +/// Sorting locked cells stays refused whichever way the box is set, which is Excel's rule too +/// ("users cannot sort ranges that contain locked cells on a protected worksheet"), so the box +/// decides for a range that has been unlocked. An AutoFilter changes no cell's contents, so there +/// the box is the only thing that decides. +#[test] +fn a_protected_sheet_allows_sorting_and_filtering_only_when_it_says_so() { + let allow = |s: &mut Session, f: fn(&mut gridcraft_model::SheetProtection)| { + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + if let Some(p) = wb.sheet_mut(0).and_then(|sh| sh.protection.as_mut()) { + f(p); + } + d.wb = std::sync::Arc::new(wb); + }; + // Three rows under a header, and the cells unlocked so that the tick box is what decides. + let book = || { + let mut s = s(); + for (c, v) in [("A1", "Head"), ("A2", "2"), ("A3", "1")] { + s.execute("cell.set", json!({"cell": c, "input": v})).unwrap(); + } + s.execute("home.lockCell", json!({"range": "A1:A3", "on": false})).unwrap(); + s + }; + let order = |s: &Session| (v(s, "A2"), v(s, "A3")); + + // Unprotected: both work, so the guard has not made the ordinary case stricter. + let mut s = book(); + s.execute("data.sortAscending", json!({})).unwrap(); + assert_eq!(order(&s), (Value::Number(1.0), Value::Number(2.0))); + s.execute("data.filter", json!({})).unwrap(); + + // Protected with nothing ticked, which is what Excel's dialog opens with: both refused. + let mut s = book(); + s.execute("review.protectSheet", json!({})).unwrap(); + for (cmd, args) in [ + ("data.sortAscending", json!({})), + ("data.sortDescending", json!({})), + // Sort… needs a key to be a sort at all: with none it does nothing either way. + ("data.sort", json!({"keys": [{"column": "A", "order": "asc"}]})), + ("data.filter", json!({})), + ("data.filterBy", json!({})), + ] { + let e = s.execute(cmd, args).expect_err(cmd).to_string(); + assert!(e.contains("protected sheet"), "{cmd}: {e}"); + } + // Refused, not half-applied: the rows are where they were. + assert_eq!(order(&s), (Value::Number(2.0), Value::Number(1.0))); + assert!(s.doc().unwrap().wb.sheet(0).unwrap().autofilter.is_none()); + + // Sort ticked: sorting works on the unlocked range; filtering is still refused. + let mut s = book(); + s.execute("review.protectSheet", json!({})).unwrap(); + allow(&mut s, |p| p.sort = true); + s.execute("data.sortAscending", json!({})).unwrap(); + assert_eq!(order(&s), (Value::Number(1.0), Value::Number(2.0)), "sorted under the header"); + assert!(s.execute("data.filter", json!({})).is_err(), "Use AutoFilter is still off"); + + // Use AutoFilter ticked: the filter goes on, and sorting is refused. + let mut s = book(); + s.execute("review.protectSheet", json!({})).unwrap(); + allow(&mut s, |p| p.autofilter = true); + s.execute("data.filter", json!({})).unwrap(); + assert!(s.doc().unwrap().wb.sheet(0).unwrap().autofilter.is_some(), "the AutoFilter is on"); + assert!(s.execute("data.sortAscending", json!({})).is_err(), "Sort is still off"); + // Unprotecting gives sorting back. + s.execute("review.unprotectSheet", json!({})).unwrap(); + s.execute("data.sortAscending", json!({})).unwrap(); + + // Locked cells: ticking Sort does not let them be reordered, as in Excel. + let mut s = { + let mut t = super::tests::s(); + for (c, v) in [("A1", "Head"), ("A2", "2"), ("A3", "1")] { + t.execute("cell.set", json!({"cell": c, "input": v})).unwrap(); + } + t + }; + s.execute("review.protectSheet", json!({})).unwrap(); + allow(&mut s, |p| p.sort = true); + assert!(s.execute("data.sortAscending", json!({})).is_err(), "locked cells cannot be sorted"); + assert_eq!(order(&s), (Value::Number(2.0), Value::Number(1.0))); +}