Refuse DeleteView of an absent view and AddFormula into an absent category (improvise-40j4)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-22 21:43:02 -07:00
parent 8fe55e0b4a
commit 07d9732b7a
2 changed files with 56 additions and 5 deletions
+11 -4
View File
@@ -208,11 +208,13 @@ impl Effect for AddFormula {
// grid; _Measure targets arrive via `measure_item_names()`.
if !crate::model::category::CategoryKind::of_name(&formula.target_category)
.is_measure()
&& let Err(e) = app
.model_state
.workbook
.add_item(&formula.target_category, &formula.target)
{
let _ = app
.model_state
.workbook
.add_item(&formula.target_category, &formula.target);
// Its category may be gone: store no formula for it (40j4).
return refused(app, e.to_string());
}
app.model_state.workbook.model.add_formula(formula);
}
@@ -341,6 +343,11 @@ impl Effect for CreateView {
pub struct DeleteView(pub String);
impl Effect for DeleteView {
fn apply(&self, app: &mut App) {
// `delete_view` answers Ok for a name it lacks, which is a failure to
// report, not a deletion (improvise-40j4).
if !app.model_state.workbook.views.contains_key(&self.0) {
return refused(app, format!("No view named \"{}\"", self.0));
}
if let Err(e) = app.model_state.workbook.delete_view(&self.0) {
return refused(app, e.to_string());
}
+45 -1
View File
@@ -1907,6 +1907,38 @@ mod tests {
);
}
/// improvise-40j4: the formula was stored for a category the frame's own
/// first command removed, and it survived save and reload.
#[test]
fn a_frame_adding_a_formula_to_a_category_its_first_command_removed_is_refused() {
let mut host = host();
let (asker, _) = host.connect(10, 10);
let answer = host.handle(
asker,
two_command_frame(
&host,
ModelCommand::RemoveCategory("Region".into()),
ModelCommand::AddFormula {
raw: "Total = 1".into(),
target_category: "Region".into(),
},
),
);
assert_eq!(
to(&answer, asker),
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
"a formula for a vanished category was accepted: {answer:?}"
);
let model = &host.app().model_state.workbook.model;
assert!(model.category("Region").is_some(), "the removal must be undone");
assert!(
model.formulas().iter().all(|f| f.target != "Total"),
"and the formula must not be stored"
);
}
/// A host whose storage holds one file per way `ImportJsonHeadless` can
/// fail, plus nothing at the two paths that must fail to read.
fn host_with_import_fixtures() -> Host {
@@ -1923,7 +1955,10 @@ mod tests {
crate::storage::Storage::write(&storage, Path::new(name), bytes)
.expect("fixture writes");
}
let mut app = App::new(workbook(), None);
// A second view, so deleting an absent one is not refused as "the last".
let mut wb = workbook();
wb.create_view("Scratch");
let mut app = App::new(wb, None);
app.storage = Box::new(storage);
app.rebuild_layout();
Host::new(app)
@@ -1963,6 +1998,15 @@ mod tests {
("ImportJsonHeadless of JSON that does not parse", import("bad.json", None)),
("ImportJsonHeadless at a path with no array", import("nested.json", Some("nope"))),
("ImportJsonHeadless that builds no model", import("measures.json", None)),
// improvise-40j4: `delete_view` answers Ok for a name it lacks.
("DeleteView of a view the document lacks", ModelCommand::DeleteView("Ghost".into())),
(
"AddFormula into a category the document lacks",
ModelCommand::AddFormula {
raw: "Total = 1".into(),
target_category: "Ghost".into(),
},
),
];
for (what, failing) in cases {
let mut host = host_with_import_fixtures();