Merge PR #1765: Sheet protection: obey the Sort and Use AutoFilter boxes

Conflicts:
- crates/engine/src/cmd/data.rs: do_sort keeps #1284's sheet parameter and checks Sorting on it; toggle_filter checks Filtering after #1285's data_target
- crates/engine/src/tests.rs: adjacent tests, both kept

crates/engine/src/cmd/data.rs

@@ -266,6 +266,8 @@ fn sort(s: &mut Session, p: &Json) -> Result<Json> {
 }
 
 fn do_sort(s: &mut Session, sheet: usize, r: RangeRef, header: bool, keys: Vec<SortKey>, by_cols: bool, case: bool) -> Result<Json> {
+    // Protect Sheet ▸ Sort. Both Sort… and the A-to-Z buttons come through here.
+    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 {
@@ -471,6 +473,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<Json> {
     let (sheet, r) = data_target(s, p, "data.filter")?;
+    super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?;
     // The active cell belongs to the active sheet: on another sheet the table is the one at `range`.
     let at = if sheet == s.doc()?.wb.active_sheet { s.doc()?.selection.active } else { r.start };
     let has = s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.autofilter.is_some());
@@ -525,6 +528,7 @@ fn install_filter(s: &mut Session, p: &Json) -> Result<Json> {
 
 fn filter_by(s: &mut Session, p: &Json) -> Result<Json> {
     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(),

crates/engine/src/cmd/mod.rs

@@ -399,6 +399,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<String> {

crates/engine/src/tests.rs

@@ -3625,3 +3625,88 @@ fn cell_filename_follows_the_file_the_workbook_lives_in() {
 
     std::fs::remove_dir_all(dir).unwrap();
 }
+
+/// 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)));
+}