PivotTables: Show Details refuses an out-of-date report, and reports its errors instead of editing the cell

crates/engine/src/cmd/pivot.rs

@@ -753,10 +753,10 @@ fn show_details(s: &mut Session, p: &Json) -> Result<Json> {
         .pivots
         .iter()
         .find(|pt| pt.last_range.is_some_and(|r| r.contains(at)))
-        .ok_or_else(|| EngineError::Disabled("pivot.showDetails".into(), "select a value cell in a PivotTable".into()))?;
+        .ok_or_else(|| EngineError::Disabled("pivot.showDetails".into(), pv::NOT_A_VALUE_CELL.into()))?;
     let origin = pt.last_range.map(|r| r.start).unwrap_or_default();
     let found = pv::detail_records(&d.wb, pt, active, at.row.saturating_sub(origin.row), at.col.saturating_sub(origin.col)).map_err(other)?;
-    let (src, records) = found.ok_or_else(|| EngineError::Disabled("pivot.showDetails".into(), "select a value cell in a PivotTable".into()))?;
+    let (src, records) = found.ok_or_else(|| EngineError::Disabled("pivot.showDetails".into(), pv::NOT_A_VALUE_CELL.into()))?;
     if d.wb.protected_structure {
         return Err(other("The workbook is protected and cannot be changed."));
     }
@@ -2541,6 +2541,22 @@ mod tests {
         assert!(rows.iter().skip(1).all(|row| row[0] == "East" && row[1] == "Apples"), "{rows:?}");
     }
 
+    #[test]
+    fn show_details_refuses_a_report_its_source_has_moved_on_from() {
+        let mut s = sales();
+        s.execute("insert.pivotTable", json!({"rows": ["Region"], "columns": ["Product"], "values": ["Sales"]})).unwrap();
+        // B5 still shows East × Apples (160), but the first East record is now Central's.
+        s.execute("range.setValues", json!({"range": "Sheet1!A2", "values": [["Central"]]})).unwrap();
+        assert_eq!(val(&s, "B5"), Value::Number(160.0));
+        let e = s.execute("pivot.showDetails", json!({"cell": "B5"})).unwrap_err().to_string();
+        assert!(e.contains("out of date"), "{e}");
+        assert_eq!(s.doc().unwrap().wb.sheets.len(), 2);
+        // After a refresh East × Apples (now B6) has the one record left.
+        s.execute("pivot.refresh", json!({})).unwrap();
+        assert_eq!(text(&s, "A6"), "East");
+        assert_eq!(s.execute("pivot.showDetails", json!({"cell": "B6"})).unwrap()["records"], 1);
+    }
+
     #[test]
     fn show_details_needs_a_value_cell() {
         let mut s = sales();

crates/engine/src/pivot.rs

@@ -806,14 +806,32 @@ pub fn compute(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize) -> Result<Out
     compute_from(wb, pt, &src, None)
 }
 
+/// Why Show Details turns a cell down when it isn't a value cell of a PivotTable (the front end
+/// edits that cell instead).
+pub const NOT_A_VALUE_CELL: &str = "select a value cell in a PivotTable";
+
 /// Show Details: the source list and the indices of its records behind the value cell at
 /// (`row`, `col`) of the report grid (relative to its top-left), in source order. `None` when
-/// that cell holds no value (a label, a header, a blank).
+/// that cell holds no value (a label, a header, a blank). An error when the report on the sheet
+/// is no longer what the source computes: the cell's records would be another cell's.
 pub fn detail_records(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, row: u32, col: u32) -> Result<Option<(Source, Vec<usize>)>, String> {
     let src = read_source(wb, &pt.source, pivot_sheet)?;
     let mut probe = Probe { at: (row as usize, col as usize), hit: None, paths: Vec::new() };
-    compute_from(wb, pt, &src, Some(&mut probe))?;
+    let out = compute_from(wb, pt, &src, Some(&mut probe))?;
     let Some((rp, cp)) = probe.hit else { return Ok(None) };
+    // Excel reads its PivotTable cache; without one, the report written by the last refresh
+    // must still be what the source gives.
+    let sh = wb.sheet(pivot_sheet).ok_or("There's no such sheet.")?;
+    let current = pt.last_range.is_some_and(|r| {
+        r.height() == out.height().max(1)
+            && r.width() == out.width.max(1)
+            && out.grid.iter().enumerate().all(|(ri, line)| {
+                line.iter().enumerate().all(|(ci, oc)| sh.value(CellRef::new(r.start.row + ri as u32, r.start.col + ci as u32)) == oc.value)
+            })
+    });
+    if !current {
+        return Err("The PivotTable report is out of date. Refresh it first.".into());
+    }
     let records = probe.paths.into_iter().filter(|(_, r, c)| r.starts_with(&rp) && c.starts_with(&cp)).map(|(i, _, _)| i).collect();
     Ok(Some((src, records)))
 }

crates/ui-egui/src/grid.rs

@@ -2388,10 +2388,17 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
             app.grid.last_click = None;
             // A PivotTable's value cell shows its source records on a new sheet (Excel's Show
             // Details); its labels and headers still edit.
-            if sh.pivots.iter().any(|pt| pt.last_range.is_some_and(|r| r.contains(c)))
-                && app.run("pivot.showDetails", json!({"cell": c.a1()})).is_ok()
-            {
-                return;
+            if sh.pivots.iter().any(|pt| pt.last_range.is_some_and(|r| r.contains(c))) {
+                let not_a_value_cell =
+                    gridcraft_engine::EngineError::Disabled("pivot.showDetails".into(), gridcraft_engine::pivot::NOT_A_VALUE_CELL.into()).to_string();
+                match app.run("pivot.showDetails", json!({"cell": c.a1()})) {
+                    Ok(_) => return,
+                    Err(e) if e == not_a_value_cell => {}
+                    Err(e) => {
+                        app.message = Some(("GridCraft".into(), crate::clean_error(&e)));
+                        return;
+                    }
+                }
             }
             // Following a hyperlink requires a single click in Excel; double-click edits.
             app.begin_edit(None, false);

crates/ui-egui/tests/it/pivot_show_details.rs

@@ -74,3 +74,17 @@ fn double_click_on_a_row_label_still_edits() {
     assert!(app.editor.is_some());
     assert_eq!(app.session.active().unwrap().wb.sheets.len(), 1);
 }
+
+#[test]
+fn double_click_on_a_value_cell_of_a_protected_workbook_says_why_and_does_not_edit() {
+    let mut h = harness();
+    h.state_mut().session.execute("review.protectWorkbook", json!({"on": true})).unwrap();
+    h.run_steps(2);
+    let pos = center(&h, "E4");
+    double_click(&mut h, pos);
+
+    let app = h.state();
+    assert!(app.editor.is_none(), "the value cell must not open for editing");
+    assert!(app.message.as_ref().is_some_and(|(_, m)| m.contains("protected")), "{:?}", app.message);
+    assert_eq!(app.session.active().unwrap().wb.sheets.len(), 1);
+}