#2120: clearing links and notes respects sheet protection

Addresses review comment 4455670315:

- Clear Hyperlinks and Clear Comments removed links and notes on
  locked cells of a protected sheet: protection compares stored cells
  only, so their removal went unseen (for stored cells too, and now
  for unstored ones since the whole range is cleared). `clear` now
  refuses a locked cell's note or link with locked_cell_error, which
  keeps Allow Edit Ranges and session-unlocked ranges.
- A removed note or link on a cell with nothing stored wasn't added
  to the changed cells, so a recorded macro's footprint missed it.

Tests: engine tests::clearing_links_and_notes_respects_sheet_protection,
tests::clearing_links_and_notes_records_their_cells.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/edit.rs

@@ -244,6 +244,25 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
             })
         })
     };
+    // Notes and links aren't cell contents, so the cell comparison that guards a protected sheet
+    // doesn't see them go: a locked cell's are refused here, stored cell or not.
+    if matches!(what, "all" | "comments" | "hyperlinks") {
+        let d = s.doc()?;
+        if let Some(sh) = d.wb.sheet(sheet).filter(|sh| sh.is_protected()) {
+            for &whole in &ranges {
+                let skip = filtered_rows(sh, whole);
+                let notes = matches!(what, "all" | "comments").then(|| sh.comments.keys()).into_iter().flatten();
+                let links = matches!(what, "all" | "hyperlinks").then(|| sh.hyperlinks.keys()).into_iter().flatten();
+                for &c in notes.chain(links).filter(|c| whole.contains(**c) && !skip.contains(&c.row)) {
+                    if d.wb.styles.get(sh.style_id(c)).protection.locked
+                        && let Some(m) = locked_cell_error(sh, c, &d.unlocked_ranges)
+                    {
+                        return Err(EngineError::Other(m));
+                    }
+                }
+            }
+        }
+    }
     edit(s, |cx| {
         for &whole in &ranges {
             // Cells are only stored inside the used range. Notes and links are kept apart from
@@ -274,6 +293,18 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
                     }
                 }
             }
+            let cleared = |c: CellRef| whole.contains(c) && !skip.contains(&c.row);
+            // A removed note or link changes its cell even with nothing stored there.
+            let metadata: Vec<CellRef> = cx
+                .wb
+                .sheet(sheet)
+                .map(|sh| {
+                    let notes = matches!(what, "all" | "comments").then(|| sh.comments.keys()).into_iter().flatten();
+                    let links = matches!(what, "all" | "hyperlinks").then(|| sh.hyperlinks.keys()).into_iter().flatten();
+                    notes.chain(links).copied().filter(|c| cleared(*c)).collect()
+                })
+                .unwrap_or_default();
+            cx.changed.extend(metadata.into_iter().map(|c| (sheet, c)));
             let sh = cx.sheet_mut(sheet)?;
             for (c, mut cell) in cells {
                 match what {
@@ -290,7 +321,6 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
                     _ => {}
                 }
             }
-            let cleared = |c: CellRef| whole.contains(c) && !skip.contains(&c.row);
             if matches!(what, "all" | "comments") {
                 sh.comments.retain(|c, _| !cleared(*c));
             }

crates/engine/src/tests.rs

@@ -5477,3 +5477,51 @@ fn clear_hyperlinks_removes_links_on_cells_with_nothing_stored() {
     s.execute("edit.clearHyperlinks", json!({"range": "A1"})).unwrap();
     assert!(!s.doc().unwrap().wb.active().unwrap().hyperlinks.contains_key(&a1), "the link on an empty sheet is removed");
 }
+
+/// Notes and links aren't cell contents, so the cell comparison that guards a protected sheet
+/// doesn't see them go: Clear Hyperlinks and Clear Comments refuse locked cells themselves,
+/// stored or not.
+#[test]
+fn clearing_links_and_notes_respects_sheet_protection() {
+    let mut s = s();
+    s.execute("cell.set", json!({"cell": "A1", "input": "x"})).unwrap();
+    let link = || gridcraft_model::Hyperlink { target: "https://example.com".into(), tooltip: None, external: true };
+    let (a1, c5) = (CellRef::parse("A1").unwrap(), CellRef::parse("C5").unwrap());
+    {
+        let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap();
+        sh.hyperlinks.insert(a1, link());
+        sh.hyperlinks.insert(c5, link());
+    }
+    s.execute("review.newNote", json!({"cell": "C5", "text": "note", "author": "Example"})).unwrap();
+    s.execute("review.protectSheet", json!({})).unwrap();
+    for (cmd, range) in [("edit.clearHyperlinks", "A1"), ("edit.clearHyperlinks", "C5"), ("edit.clearComments", "C5")] {
+        assert!(s.execute(cmd, json!({"range": range})).is_err(), "{cmd} on locked {range}");
+    }
+    let sh = s.doc().unwrap().wb.active().unwrap().clone();
+    assert!(sh.hyperlinks.contains_key(&a1) && sh.hyperlinks.contains_key(&c5) && sh.comments.contains_key(&c5), "nothing removed");
+
+    s.execute("review.unprotectSheet", json!({})).unwrap();
+    s.execute("edit.clearHyperlinks", json!({"range": "A1:C5"})).unwrap();
+    s.execute("edit.clearComments", json!({"range": "C5"})).unwrap();
+    let sh = s.doc().unwrap().wb.active().unwrap().clone();
+    assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty());
+}
+
+/// A removed link or note is a change to its cell even when nothing is stored there, so a
+/// recorded macro's footprint lists it.
+#[test]
+fn clearing_links_and_notes_records_their_cells() {
+    let mut s = s();
+    let c5 = CellRef::parse("C5").unwrap();
+    std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb)
+        .sheet_mut(0)
+        .unwrap()
+        .hyperlinks
+        .insert(c5, gridcraft_model::Hyperlink { target: "https://example.com".into(), tooltip: None, external: true });
+    s.execute("review.newNote", json!({"cell": "D7", "text": "note", "author": "Example"})).unwrap();
+    s.doc_mut().unwrap().touched = Some(Vec::new());
+    s.execute("edit.clearHyperlinks", json!({"range": "C5"})).unwrap();
+    s.execute("edit.clearComments", json!({"range": "D7"})).unwrap();
+    let touched: Vec<String> = s.doc().unwrap().touched.clone().unwrap_or_default().into_iter().map(|(_, c)| c.a1()).collect();
+    assert!(touched.contains(&"C5".to_string()) && touched.contains(&"D7".to_string()), "{touched:?}");
+}