diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 324b09f3..0bed8ba5 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -20,7 +20,8 @@ // Re-export the main Workbook and Worksheet types from controller/api pub use logisheets_controller::api::{ - BlockSortOrder, CellInfo, CellRefRange, CfRuleInfo, DependentCell, FillRange, ReproducibleCell, + BlockSortOrder, CellInfo, CellRefRange, CfRuleInfo, DependentCell, FieldValidationVerdict, + FillRange, ReproducibleCell, SaveFileResult, SheetCoordinate, SheetDimension, Workbook, Worksheet, }; diff --git a/crates/api/src/rpc/controller.rs b/crates/api/src/rpc/controller.rs index ed429f4b..44d16b4a 100644 --- a/crates/api/src/rpc/controller.rs +++ b/crates/api/src/rpc/controller.rs @@ -296,6 +296,27 @@ pub fn may_modify_block( .map_err(ErrorMessage::from) } +/// Whether writing `proposed` into this cell would break its field's +/// validation rule — see [`CheckFieldValidationParams`]. +/// +/// The companion to [`may_modify_block`] for `BlockOp::OverrideValidation`: +/// that answers "may this actor write a violating value", this answers +/// "would this value be a violating one". Neither writes anything. +/// +/// [`CheckFieldValidationParams`]: crate::rpc::message::CheckFieldValidationParams +pub fn check_field_validation( + mgr: &mut Manager, + id: usize, + sheet_idx: usize, + row: usize, + col: usize, + proposed: String, +) -> Result { + let wb = mgr.get_mut_workbook(&id).unwrap(); + wb.check_field_validation(sheet_idx, row, col, proposed) + .map_err(ErrorMessage::from) +} + /// A block's governance metadata, without its cells. See /// [`GetBlockModifyInfoParams`]. /// diff --git a/crates/api/src/rpc/message.rs b/crates/api/src/rpc/message.rs index 3a377fbb..5931919e 100644 --- a/crates/api/src/rpc/message.rs +++ b/crates/api/src/rpc/message.rs @@ -4,6 +4,7 @@ use crate::BlockId; use crate::{ ActionEffect, AppData, AppendixWithCell, BlockActor, BlockDataRow, BlockField, BlockInfo, BlockModifyInfo, BlockOp, BlockSortOrder, CellCoordinateWithSheet, CellImageInfo, CellInfo, + FieldValidationVerdict, CellInput, CellPosition, CellRefRange, CfRuleInfo, ChartInfo, ColId, Comment, DependentCell, DisplayWindow, DisplayWindowWithStartPoint, EditPayload, ErrorMessage, FormulaDisplayInfo, LinkInfo, MergeCell, ReproducibleCell, RowId, RowInfo, SaveFileResult, ShadowCellInfo, @@ -62,6 +63,7 @@ pub enum Message { GetBlockValues(GetBlockValuesParams), GetBlockSortOrder(GetBlockSortOrderParams), MayModifyBlock(MayModifyBlockParams), + CheckFieldValidation(CheckFieldValidationParams), GetBlockModifyInfo(GetBlockModifyInfoParams), GetShadowCellId(GetShadowCellIdParams), GetShadowCellIds(GetShadowCellIdsParams), @@ -522,6 +524,27 @@ pub struct MayModifyBlockParams { pub actor: BlockActor, } +/// Ask whether a value would break a block field's validation rule, before +/// writing it. +/// +/// The pairing with [`MayModifyBlockParams`] is the whole point: a host that +/// wants to enforce `BlockOp::OverrideValidation` needs both halves — is this +/// write a violation, and is this actor allowed to make one. +#[derive(Debug, Clone, TS)] +#[ts( + file_name = "check_field_validation_params.ts", + rename_all = "camelCase" +)] +pub struct CheckFieldValidationParams { + pub sheet_idx: usize, + /// Sheet-absolute coordinates, not block-relative — this is asked from the + /// grid's write path, which speaks in sheet coordinates. + pub row: usize, + pub col: usize, + /// The value the caller is about to write, exactly as the user typed it. + pub proposed: String, +} + /// A block's governance metadata on its own — owner, default policy, /// per-operation overrides, description — without its cells. /// @@ -871,6 +894,10 @@ pub struct WorkbookMethods { ) -> Result, pub may_modify_block: fn(params: MayModifyBlockParams, book_id: Option) -> Result, + pub check_field_validation: fn( + params: CheckFieldValidationParams, + book_id: Option, + ) -> Result, pub get_block_modify_info: fn( params: GetBlockModifyInfoParams, book_id: Option, diff --git a/crates/controller/src/api/field_validation.rs b/crates/controller/src/api/field_validation.rs new file mode 100644 index 00000000..f1597a9c --- /dev/null +++ b/crates/controller/src/api/field_validation.rs @@ -0,0 +1,236 @@ +//! Ask, before writing, whether a value would violate a block field's +//! validation rule. +//! +//! The rule itself is already live: a field with a `validation_formula` gets a +//! `ShadowKind::Validation` shadow per record, wired into the dependency graph +//! with `#PLACEHOLDER` bound to the cell it guards, and readers use its value +//! to draw the warning marker. That is a verdict on the value the cell *has*. +//! +//! [`BlockOp::OverrideValidation`](crate::edit_action::BlockOp) needs the +//! verdict on a value the cell does not have yet — the host has to know whether +//! the write it is about to allow is a violating one *before* it happens, or +//! the only way to refuse would be to write and then undo. +//! +//! So this evaluates the shadow's existing AST against a proposed value: swap +//! the value into the container, run the calculator on that one node, put the +//! old value back. Nothing is registered, nothing is dirtied, and no history +//! entry is made — the workbook is unchanged when this returns. Re-using the +//! shadow's AST (rather than re-parsing the template) also means the answer +//! cannot drift from what the marker will say once the write lands. + +use std::collections::{HashMap, HashSet}; + +use gents_derives::TS; +use logisheets_base::{Addr, CellId, CellValue, SheetId, TextId, errors::BasicError}; + +// NB: do not `use crate::errors::Result` here — the local alias would shadow +// `std::result::Result` and break the serde impls the `TS` derive generates for +// `FieldValidationVerdict` (the same trap `sort_block` documents). Reference it +// fully-qualified in the signature instead. +use crate::{ + calc_engine::calculator::{calc_vertex::CalcValue, calc_vertex::Value, calculator::calc}, + cell::Cell, + connectors::CalcConnector, + errors::Error, + sid_assigner::ShadowKind, +}; + +use super::Workbook; + +/// What a proposed write would do to a cell's validation rule. +#[derive(Debug, Clone, Default, TS)] +#[ts(file_name = "field_validation_verdict.ts", rename_all = "camelCase")] +pub struct FieldValidationVerdict { + /// `false` when the cell has no validation rule at all — the other two + /// fields are then meaningless and the caller has nothing to gate on. + pub has_rule: bool, + /// `true` when the proposed value fails the rule. Only meaningful when + /// `has_rule`. + pub violates: bool, + /// The rule's raw template, so a refusal can say what was expected. + /// Empty when the field carries no rule. + pub rule: String, +} + +/// How a computed validation result is read as a verdict. +/// +/// Mirrors `interpretValidation` in `logisheets-core` deliberately: a rule is +/// satisfied when it evaluates truthy, and an error (`#VALUE!`, `#NAME?`, a +/// rule that cannot resolve) counts as a violation rather than as a pass — a +/// rule nobody can evaluate is not a rule anybody has met. A blank result is +/// treated as passing, so an optional field is not flagged for being empty. +fn scalar_violates(v: &Value) -> bool { + match v { + Value::Boolean(b) => !*b, + Value::Number(n) => *n == 0.0, + Value::Error(_) => true, + Value::Blank => false, + Value::Text(_) => false, + } +} + +fn violates(value: &CalcValue) -> bool { + match value { + CalcValue::Scalar(v) => scalar_violates(v), + // A rule that yields a range or a union is malformed for this purpose; + // judge it on its first element rather than silently passing, which + // would let a broken rule read as "everything is fine". + CalcValue::Range(m) => match m.visit(0, 0) { + Ok(v) => scalar_violates(v), + Err(v) => scalar_violates(&v), + }, + CalcValue::Cube(_) => true, + CalcValue::Union(parts) => parts.first().map(|p| violates(p)).unwrap_or(false), + } +} + +impl Workbook { + /// Whether writing `proposed` into the cell at (`row`, `col`) would break + /// its field's validation rule. + /// + /// Returns `has_rule: false` — never an error — for every cell that is not + /// a validated block cell: cells outside a block, fields with no rule, and + /// rows whose shadow has not been installed yet. A caller gating a write + /// wants one question answered, and "there is nothing to check here" is an + /// answer, not a failure. + pub fn check_field_validation( + &mut self, + sheet_idx: usize, + row: usize, + col: usize, + proposed: String, + ) -> std::result::Result { + let sheet_id = self + .controller + .status + .sheet_info_manager + .get_sheet_id(sheet_idx) + .ok_or(BasicError::SheetIdxExceed(sheet_idx))?; + + let cell_id = self + .controller + .status + .navigator + .fetch_cell_id(&sheet_id, row, col)?; + let CellId::BlockCell(bcid) = cell_id else { + return Ok(FieldValidationVerdict::default()); + }; + + let Some(rule) = self + .controller + .status + .block_schema_manager + .validation_for_block_cell(sheet_id, &bcid) + else { + return Ok(FieldValidationVerdict::default()); + }; + + // The shadow carries the rule already substituted for this record — + // its `#FIELD` siblings and `#KEY` resolved against this row. Without + // one there is nothing to evaluate; that happens only in the window + // between a bind and the calc pass, so report the rule but no verdict + // rather than guessing at one. + let unknown = FieldValidationVerdict { + has_rule: true, + violates: false, + rule: rule.clone(), + }; + let Some(eid) = + self.controller + .sid_assigner + .find_shadow_id(sheet_id, cell_id, ShadowKind::Validation) + else { + return Ok(unknown); + }; + let Some(ast) = self + .controller + .status + .formula_manager + .formulas + .get(&(sheet_id, CellId::EphemeralCell(eid))) + .cloned() + else { + return Ok(unknown); + }; + + let violated = self.eval_against(sheet_id, cell_id, proposed, &ast); + Ok(FieldValidationVerdict { + has_rule: true, + violates: violated, + rule, + }) + } + + /// Evaluate `ast` with `cell_id` temporarily holding `proposed`, then put + /// the cell back exactly as it was. + /// + /// The restore is unconditional and covers the "cell had no entry" case + /// too: a validated-but-empty cell has nothing in the container, and + /// leaving a blank one behind would turn a question into an edit. + fn eval_against( + &mut self, + sheet_id: SheetId, + cell_id: CellId, + proposed: String, + ast: &logisheets_parser::ast::Node, + ) -> bool { + let status = &mut self.controller.status; + let proposed_value = { + let text_ids = &mut status.text_id_manager; + CellValue::from_string(proposed, &mut |t| -> TextId { + text_ids.get_or_register_id(t) + }) + }; + + let saved: Option = status.container.get_cell(sheet_id, &cell_id).cloned(); + status + .container + .update_value(sheet_id, cell_id, proposed_value); + + let (row, col) = status + .navigator + .fetch_cell_idx(&sheet_id, &cell_id) + .unwrap_or((0, 0)); + + let mut dirty_next: imbl::HashSet<(SheetId, CellId)> = imbl::HashSet::new(); + let mut calc_cells: HashSet<(SheetId, CellId)> = HashSet::new(); + let mut async_func_manager = crate::async_func_manager::AsyncFuncManager::default(); + let async_funcs: HashSet = HashSet::new(); + + let value = { + let mut connector = CalcConnector { + range_manager: &status.range_manager, + cube_manager: &status.cube_manager, + navigator: &mut status.navigator, + container: &mut status.container, + ext_links: &mut status.external_links_manager, + text_id_manager: &mut status.text_id_manager, + func_id_manager: &status.func_id_manager, + sheet_id_manager: &status.sheet_id_manager, + names_storage: HashMap::new(), + cells_storage: HashMap::new(), + sheet_pos_manager: &status.sheet_info_manager, + async_func_manager: &mut async_func_manager, + async_funcs: &async_funcs, + active_sheet: sheet_id, + curr_addr: Addr { row, col }, + dirty_cells_in_next_run: &mut dirty_next, + calc_cells: &mut calc_cells, + block_schema_manager: &status.block_schema_manager, + formula_manager: &status.formula_manager, + name_id_manager: &status.name_id_manager, + ext_ref_manager: &status.ext_ref_manager, + }; + calc(ast, &mut connector) + }; + + match saved { + Some(cell) => status.container.add_cell(sheet_id, cell_id, cell), + None => { + status.container.remove_cell(sheet_id, &cell_id); + } + } + + violates(&value) + } +} diff --git a/crates/controller/src/api/mod.rs b/crates/controller/src/api/mod.rs index 1c83c80e..d51202dc 100644 --- a/crates/controller/src/api/mod.rs +++ b/crates/controller/src/api/mod.rs @@ -5,6 +5,7 @@ pub use crate::{ errors::{Error, ErrorMessage, Result}, }; mod cell_positioner; +mod field_validation; mod fill; mod sort_block; mod types; @@ -13,6 +14,7 @@ mod worksheet; #[cfg(test)] mod test; +pub use field_validation::FieldValidationVerdict; pub use fill::FillRange; pub use logisheets_base::BlockId; pub use sort_block::BlockSortOrder; diff --git a/crates/controller/src/api/test.rs b/crates/controller/src/api/test.rs index b4b3fb6b..6077ac09 100644 --- a/crates/controller/src/api/test.rs +++ b/crates/controller/src/api/test.rs @@ -1313,6 +1313,230 @@ fn clearing_field_rule_purges_stale_shadow_value() { } } +// A field with a value formula owns every cell in its column. The grid's write +// path is `CellInput` (not `BlockInput`), and it used to sail straight through: +// the container wrote the typed value and the formula executor then REMOVED the +// materialized formula, so one keystroke permanently un-computed that row. The +// column is supposed to be read-only to people; assert every user-facing write +// payload leaves it exactly as the schema left it. +#[test] +fn a_templated_field_refuses_every_user_write() { + use crate::controller::display::Value; + use crate::edit_action::{BindFormSchema, CellClear, CellInput}; + + let mut wb = Workbook::default(); + let bid = wb.get_available_block_id(0).unwrap(); + + // 2x2 block at A1. Field 0 ("qty") is free-form and doubles as the key; + // field 1 ("total") is derived: total = qty * 2. + wb.handle_action(EditAction::Payloads(PayloadsAction { + payloads: vec![ + EditPayload::CreateBlock(CreateBlock { + sheet_idx: 0, + id: bid, + master_row: 0, + master_col: 0, + row_cnt: 2, + col_cnt: 2, + owner: None, + modify_policy: None, + permissions: None, + description: None, + }), + EditPayload::BindFormSchema(BindFormSchema { + ref_name: "rec".into(), + sheet_idx: 0, + block_id: bid, + field_from: 0, + key_idx: 0, + fields: vec!["qty".into(), "total".into()], + render_ids: vec!["r0".into(), "r1".into()], + row: true, + field_formulas: vec![None, Some("=#FIELD(\"qty\")*2".into())], + validation_formulas: vec![], + editability_formulas: vec![], + }), + EditPayload::CellInput(CellInput { + sheet_idx: 0, + row: 0, + col: 0, + content: "10".to_string(), + }), + ], + undoable: false, + init: false, + })); + + let total = + |wb: &Workbook| -> Value { wb.get_sheet_by_idx(0).unwrap().get_value(0, 1).unwrap() }; + let formula = + |wb: &Workbook| -> String { wb.get_sheet_by_idx(0).unwrap().get_formula(0, 1).unwrap() }; + + assert!( + matches!(total(&wb), Value::Number(n) if n == 20.0), + "the template should have computed 10*2, got {:?}", + total(&wb) + ); + let materialized = formula(&wb); + assert!( + !materialized.is_empty(), + "the templated cell should carry a real formula" + ); + + // Every one of these is a person interacting with the grid. + for (label, payload) in [ + ( + "a plain value", + EditPayload::CellInput(CellInput { + sheet_idx: 0, + row: 0, + col: 1, + content: "999".to_string(), + }), + ), + ( + "a literal formula", + EditPayload::CellInput(CellInput { + sheet_idx: 0, + row: 0, + col: 1, + content: "=42".to_string(), + }), + ), + ( + "a clear", + EditPayload::CellClear(CellClear { + sheet_idx: 0, + row: 0, + col: 1, + }), + ), + ] { + wb.handle_action(EditAction::Payloads(PayloadsAction { + payloads: vec![payload], + undoable: false, + init: false, + })); + assert!( + matches!(total(&wb), Value::Number(n) if n == 20.0), + "{label} must not change a templated cell's value, got {:?}", + total(&wb) + ); + assert_eq!( + formula(&wb), + materialized, + "{label} must leave the field's formula on the cell" + ); + } + + // And the column still tracks its input: the formula is live, not a + // frozen leftover that merely survived the writes above. + wb.handle_action(EditAction::Payloads(PayloadsAction { + payloads: vec![EditPayload::CellInput(CellInput { + sheet_idx: 0, + row: 0, + col: 0, + content: "7".to_string(), + })], + undoable: false, + init: false, + })); + assert!( + matches!(total(&wb), Value::Number(n) if n == 14.0), + "the field formula should still recompute from its input, got {:?}", + total(&wb) + ); +} + +// `BlockOp::OverrideValidation` asks the host to gate a write BEFORE it lands, +// so the engine has to be able to judge a value the cell does not hold yet. +// The check runs the field's live validation shadow against the proposed value +// and must leave nothing behind — a question that quietly edits the workbook is +// worse than no question at all. +#[test] +fn a_proposed_value_is_judged_without_touching_the_workbook() { + use crate::controller::display::Value; + use crate::edit_action::{BindFormSchema, CellInput}; + + let mut wb = Workbook::default(); + let bid = wb.get_available_block_id(0).unwrap(); + + // 2x2 block at A1: key in col 0, `qty` in col 1 validated as > 0. + wb.handle_action(EditAction::Payloads(PayloadsAction { + payloads: vec![ + EditPayload::CreateBlock(CreateBlock { + sheet_idx: 0, + id: bid, + master_row: 0, + master_col: 0, + row_cnt: 2, + col_cnt: 2, + owner: None, + modify_policy: None, + permissions: None, + description: None, + }), + EditPayload::BindFormSchema(BindFormSchema { + ref_name: "rec".into(), + sheet_idx: 0, + block_id: bid, + field_from: 0, + key_idx: 0, + fields: vec!["key".into(), "qty".into()], + render_ids: vec!["r0".into(), "r1".into()], + row: true, + field_formulas: vec![], + validation_formulas: vec![None, Some("#PLACEHOLDER>0".into())], + editability_formulas: vec![], + }), + EditPayload::CellInput(CellInput { + sheet_idx: 0, + row: 0, + col: 1, + content: "5".to_string(), + }), + ], + undoable: false, + init: false, + })); + + let read = + |wb: &Workbook| -> Value { wb.get_sheet_by_idx(0).unwrap().get_value(0, 1).unwrap() }; + assert!( + matches!(read(&wb), Value::Number(n) if n == 5.0), + "sanity: the cell should hold the value that was written" + ); + + let ok = wb.check_field_validation(0, 0, 1, "7".to_string()).unwrap(); + assert!(ok.has_rule, "the field carries a validation rule"); + assert!(!ok.violates, "7 > 0 passes the rule"); + assert_eq!(ok.rule, "#PLACEHOLDER>0"); + + let bad = wb + .check_field_validation(0, 0, 1, "-3".to_string()) + .unwrap(); + assert!(bad.violates, "-3 fails `#PLACEHOLDER>0`"); + + // Neither call may have moved anything: not the cell it asked about, and + // not the verdict the marker is currently showing. + assert!( + matches!(read(&wb), Value::Number(n) if n == 5.0), + "checking a proposed value must leave the cell holding its own value, got {:?}", + read(&wb) + ); + + // A field with no rule, and a cell outside any block, both answer + // "nothing to check" rather than failing. + let keyless = wb + .check_field_validation(0, 0, 0, "anything".to_string()) + .unwrap(); + assert!(!keyless.has_rule, "the key field declares no validation"); + let outside = wb + .check_field_validation(0, 9, 9, "anything".to_string()) + .unwrap(); + assert!(!outside.has_rule, "a cell outside every block has no rule"); +} + // The CreateLink edit payload, driven through the public API, for the real app // flow: the seller's SUM(A1:A2) formula ALREADY exists (reading literal cells), // THEN the user links A1:A2 to a block. The existing formula must redirect to the diff --git a/crates/controller/src/api/workbook.rs b/crates/controller/src/api/workbook.rs index 08321374..d8265840 100644 --- a/crates/controller/src/api/workbook.rs +++ b/crates/controller/src/api/workbook.rs @@ -32,7 +32,10 @@ const CALC_CONDITION_EPHEMERAL_ID: u64 = 225715; pub(crate) type CellPositionerDefault = CellPositioner<1000>; pub struct Workbook { - controller: Controller, + /// `pub(crate)` so sibling api modules (`field_validation`) can run a + /// one-shot calculation against the live status; nothing outside the crate + /// reaches the controller directly. + pub(crate) controller: Controller, cell_positioners: Locked>>, } diff --git a/crates/controller/src/connectors/formula_connector.rs b/crates/controller/src/connectors/formula_connector.rs index 3b7aaca1..b70534c7 100644 --- a/crates/controller/src/connectors/formula_connector.rs +++ b/crates/controller/src/connectors/formula_connector.rs @@ -381,6 +381,12 @@ impl<'a> FormulaExecCtx for FormulaConnector<'a> { self.block_schema_manager.cell_role(sheet_id, cell) } + fn is_block_cell_templated(&self, sheet_id: SheetId, cell: &BlockCellId) -> bool { + self.block_schema_manager + .formula_for_block_cell(sheet_id, cell) + .is_some() + } + fn block_cell_template( &self, sheet_id: SheetId, diff --git a/crates/controller/src/container/executor.rs b/crates/controller/src/container/executor.rs index 4b3fc0d3..063ba8bd 100644 --- a/crates/controller/src/container/executor.rs +++ b/crates/controller/src/container/executor.rs @@ -12,6 +12,18 @@ use crate::{ use super::{DataContainer, ctx::ContainerExecCtx}; +/// True when `cell_id` is a block cell sitting in a field that carries a +/// value-formula template. Such a cell is engine-owned: the schema's +/// template decides what it holds, and a write from anywhere else — a +/// person typing in the grid, a paste, a clear — has to be dropped, or +/// the row silently loses the column's formula. +fn is_templated_cell(ctx: &C, sheet_id: SheetId, cell_id: &CellId) -> bool { + match cell_id { + CellId::BlockCell(bcid) => ctx.is_block_cell_templated(sheet_id, bcid), + _ => false, + } +} + pub struct ContainerExecutor { pub container: DataContainer, pub cells_removed: Vec<(SheetId, CellId)>, @@ -197,6 +209,13 @@ impl ContainerExecutor { cell.coordinate.row - anchor_x + p.start_row, cell.coordinate.col - anchor_y + p.start_col, )?; + // Paste / fill lands on whatever the target rectangle + // covers, which may include a block's computed column. + // Skip those cells rather than refusing the whole + // operation — the rest of the paste is legitimate. + if is_templated_cell(ctx, sheet_id, &cell_id) { + continue; + } let cell_value = match cell.value { crate::Value::Str(s) => { CellValue::from_string(s, &mut |t| -> TextId { ctx.fetch_text_id(t) }) @@ -236,6 +255,15 @@ impl ContainerExecutor { let cell_value = CellValue::from_string(p.content, &mut |t| -> TextId { ctx.fetch_text_id(t) }); let cell_id = ctx.fetch_cell_id(&sheet_id, p.row, p.col)?; + // A templated block cell belongs to its field's formula. + // `BlockInput` already drops writes here; `CellInput` is + // the path a person typing into the grid takes, so it has + // to drop them too — otherwise a stray keystroke replaces + // the computed value and the formula executor tears the + // row's formula out behind it. + if is_templated_cell(ctx, sheet_id, &cell_id) { + return Ok((self, false)); + } self.container.update_value(sheet_id, cell_id, cell_value); self.value_changed.push((sheet_id, cell_id)); Ok((self, true)) @@ -269,6 +297,12 @@ impl ContainerExecutor { .fetch_sheet_id_by_index(p.sheet_idx) .map_err(|l| BasicError::SheetIdxExceed(l))?; let cell_id = ctx.fetch_cell_id(&sheet_id, p.row, p.col)?; + // Clearing a templated cell would blank a value the schema + // is about to recompute anyway — and removing the cell drops + // the formula with it. Leave it alone. + if is_templated_cell(ctx, sheet_id, &cell_id) { + return Ok((self, false)); + } self.container.remove_cell(sheet_id, &cell_id); self.value_changed.push((sheet_id, cell_id)); Ok((self, true)) diff --git a/crates/controller/src/edit_action/mod.rs b/crates/controller/src/edit_action/mod.rs index 6513b4ff..f1de73ba 100644 --- a/crates/controller/src/edit_action/mod.rs +++ b/crates/controller/src/edit_action/mod.rs @@ -751,18 +751,31 @@ pub enum BlockOp { SortByField, /// Rewrite the prose description. ModifyDescription, + /// Write a value into a block cell that its field's validation rule + /// rejects. + /// + /// Distinct from `CellInput` because the two answer different questions: + /// `CellInput` is "may this actor write here at all", this one is "may + /// this actor write something the schema says is wrong". A craft that + /// maintains a table often has to seed a row it knows is incomplete — + /// a required field it fills in on the next round — while a person typing + /// into the same block should be held to the rule. Left at `All` (the + /// default), a violating write lands and is flagged, which is what every + /// block did before this op existed. + OverrideValidation, } impl BlockOp { /// Every operation, so a caller can render or check the whole set without /// having to keep its own list in step with this one. - pub const ALL: [BlockOp; 6] = [ + pub const ALL: [BlockOp; 7] = [ BlockOp::InsertDeleteLines, BlockOp::RemoveBlock, BlockOp::ModifySchema, BlockOp::CellInput, BlockOp::SortByField, BlockOp::ModifyDescription, + BlockOp::OverrideValidation, ]; /// Attribute name used for .xlsx persistence. @@ -774,6 +787,7 @@ impl BlockOp { BlockOp::CellInput => "cellInput", BlockOp::SortByField => "sortByField", BlockOp::ModifyDescription => "modifyDescription", + BlockOp::OverrideValidation => "overrideValidation", } } } @@ -793,6 +807,7 @@ pub struct BlockPermissions { pub cell_input: Option, pub sort_by_field: Option, pub modify_description: Option, + pub override_validation: Option, } impl BlockPermissions { @@ -810,6 +825,7 @@ impl BlockPermissions { BlockOp::CellInput => self.cell_input = policy, BlockOp::SortByField => self.sort_by_field = policy, BlockOp::ModifyDescription => self.modify_description = policy, + BlockOp::OverrideValidation => self.override_validation = policy, } } @@ -828,6 +844,7 @@ impl BlockPermissions { BlockOp::CellInput => self.cell_input, BlockOp::SortByField => self.sort_by_field, BlockOp::ModifyDescription => self.modify_description, + BlockOp::OverrideValidation => self.override_validation, } } } diff --git a/crates/controller/src/file_loader/mod.rs b/crates/controller/src/file_loader/mod.rs index f0d606ab..1e156a89 100644 --- a/crates/controller/src/file_loader/mod.rs +++ b/crates/controller/src/file_loader/mod.rs @@ -210,6 +210,7 @@ pub fn load_file(wb: Wb, book_name: String) -> Controller { cell_input: parse_perm(&block_range.perm_cell_input), sort_by_field: parse_perm(&block_range.perm_sort_by_field), modify_description: parse_perm(&block_range.perm_modify_description), + override_validation: parse_perm(&block_range.perm_override_validation), }; let block_place = BlockPlace::new( master_cell_id, diff --git a/crates/controller/src/file_saver/worksheet.rs b/crates/controller/src/file_saver/worksheet.rs index b4f41938..68ff98aa 100644 --- a/crates/controller/src/file_saver/worksheet.rs +++ b/crates/controller/src/file_saver/worksheet.rs @@ -154,6 +154,7 @@ pub fn save_sheets( perm_cell_input: perm(BlockOp::CellInput), perm_sort_by_field: perm(BlockOp::SortByField), perm_modify_description: perm(BlockOp::ModifyDescription), + perm_override_validation: perm(BlockOp::OverrideValidation), row_infos, col_infos, }) diff --git a/crates/controller/src/formula_manager/ctx.rs b/crates/controller/src/formula_manager/ctx.rs index c981e0e8..5ea407b6 100644 --- a/crates/controller/src/formula_manager/ctx.rs +++ b/crates/controller/src/formula_manager/ctx.rs @@ -32,6 +32,11 @@ pub trait FormulaExecCtx: /// caller can dirty the right virtual node. fn block_cell_role(&self, sheet_id: SheetId, cell: &BlockCellId) -> BlockCellRole; + /// True if this block cell sits in a field carrying a value-formula + /// template. Cheaper than {@link block_cell_template} (no sibling map, + /// no key lookup) and used by the write-guard on user-facing payloads. + fn is_block_cell_templated(&self, sheet_id: SheetId, cell: &BlockCellId) -> bool; + /// If this block cell sits in a field with a value-formula template, /// return everything the executor needs to substitute and parse it: /// 1. the raw template (still including `=` if the author wrote one) diff --git a/crates/controller/src/formula_manager/executors/mod.rs b/crates/controller/src/formula_manager/executors/mod.rs index 2cc54d7c..6856451f 100644 --- a/crates/controller/src/formula_manager/executors/mod.rs +++ b/crates/controller/src/formula_manager/executors/mod.rs @@ -35,6 +35,24 @@ pub struct FormulaExecutor { pub ephemeral_shadows_cleared: HashSet<(SheetId, CellId)>, } +/// True when the cell at `(row, col)` is a block cell whose field carries a +/// value-formula template. The template owns that cell, so user-facing write +/// payloads have to leave it alone. +fn is_templated_at( + ctx: &mut C, + sheet_idx: usize, + row: usize, + col: usize, +) -> Result { + let sheet = ctx + .fetch_sheet_id_by_index(sheet_idx) + .map_err(|l| BasicError::SheetIdxExceed(l))?; + match ctx.fetch_cell_id(&sheet, row, col) { + Ok(CellId::BlockCell(bcid)) => Ok(ctx.is_block_cell_templated(sheet, &bcid)), + _ => Ok(false), + } +} + impl FormulaExecutor { pub fn execute( self, @@ -43,6 +61,17 @@ impl FormulaExecutor { ) -> Result { let executor = match payload { EditPayload::CellInput(mut cell_input) => { + // A cell in a field with a value-formula template is + // engine-owned. `CellInput` is the grid's write path, so + // this is where a person typing lands: refuse both a plain + // value (which would be overwritten on the next recalc + // anyway) and a literal `=formula` (which would silently + // replace the field's template for this one row). Crafts + // that genuinely want the per-row escape hatch still have + // it through `BlockInput`. + if is_templated_at(ctx, cell_input.sheet_idx, cell_input.row, cell_input.col)? { + return Ok(self); + } if cell_input.content.starts_with("=") { let formula = cell_input.content.split_off(1); input_formula( @@ -257,7 +286,14 @@ impl FormulaExecutor { p.col_cnt, ctx, ), - EditPayload::CellClear(p) => remove_formula(self, p.sheet_idx, p.row, p.col, ctx), + EditPayload::CellClear(p) => { + // Clearing a templated cell must not strip the field's + // formula off that row — the container drops the clear too. + if is_templated_at(ctx, p.sheet_idx, p.row, p.col)? { + return Ok(self); + } + remove_formula(self, p.sheet_idx, p.row, p.col, ctx) + } EditPayload::CreateLink(_) => { // Creating a link remaps existing formulas' ranges to the block // (in the range executor). Their dependency edges were built diff --git a/crates/wasms/server/src/rpc.rs b/crates/wasms/server/src/rpc.rs index 5c3492f5..caa9452f 100644 --- a/crates/wasms/server/src/rpc.rs +++ b/crates/wasms/server/src/rpc.rs @@ -334,6 +334,14 @@ pub fn handle(msg: JsValue, book_id: Option) -> JsValue { params.op, params.actor, )), + Message::CheckFieldValidation(params) => res_to_js(controller::check_field_validation( + &mut mgr, + id, + params.sheet_idx, + params.row, + params.col, + params.proposed, + )), Message::GetBlockModifyInfo(params) => res_to_js(controller::get_block_modify_info( &mgr, id, diff --git a/crates/workbook/src/logisheets.rs b/crates/workbook/src/logisheets.rs index 8ea8bd73..a3d7b9f7 100644 --- a/crates/workbook/src/logisheets.rs +++ b/crates/workbook/src/logisheets.rs @@ -94,6 +94,8 @@ pub struct BlockRange { pub perm_sort_by_field: Option, #[xmlserde(name = b"permModifyDescription", ty = "attr")] pub perm_modify_description: Option, + #[xmlserde(name = b"permOverrideValidation", ty = "attr")] + pub perm_override_validation: Option, #[xmlserde(name = b"rowInfos", ty = "child")] pub row_infos: Vec, #[xmlserde(name = b"colInfos", ty = "child")] diff --git a/crates/workbook/src/ooxml/drawing_part.rs b/crates/workbook/src/ooxml/drawing_part.rs index 9bd1c760..1d151376 100644 --- a/crates/workbook/src/ooxml/drawing_part.rs +++ b/crates/workbook/src/ooxml/drawing_part.rs @@ -289,10 +289,9 @@ pub struct CtGeomGuideList {} // --- graphicFrame (charts) ------------------------------------------------- // // Modeled enough to (a) read a chart reference on load and (b) regenerate the -// anchor on save. `nvGraphicFramePr` and `graphic/graphicData/c:chart` are -// typed; `xfrm` is preserved opaquely (Excel recomputes it from the anchor's -// from/to on a twoCellAnchor). Non-chart graphicData (e.g. SmartArt) is not -// modeled and would not round-trip — charts are the supported case. +// anchor on save. `nvGraphicFramePr`, `xfrm` and `graphic/graphicData/c:chart` +// are typed. Non-chart graphicData (e.g. SmartArt) is not modeled and would not +// round-trip — charts are the supported case. #[derive(Debug, Default, XmlSerialize, XmlDeserialize)] pub struct CtGraphicFrame { #[xmlserde(name = b"xdr:nvGraphicFramePr", ty = "child")] @@ -300,11 +299,51 @@ pub struct CtGraphicFrame { pub nv_graphic_frame_pr: Option, #[xmlserde(name = b"xdr:xfrm", ty = "child")] #[xmlserde(alias(b"xfrm"))] - pub xfrm: Option, + pub xfrm: Option, #[xmlserde(name = b"a:graphic", ty = "child")] pub graphic: Option, } +/// `` on a graphicFrame — DrawingML's `a:CT_Transform2D`. +/// +/// The schema requires it, and so does Excel: a graphicFrame without one makes +/// Excel repair the drawing part on open. Excel does recompute the geometry +/// from the anchor's from/to, so the element may be *empty* — real producers +/// write a bare `` rather than omitting it, which is what +/// `tests/one_cell_anchor.xlsx` contains — but it has to be present. +/// +/// Typed rather than kept opaque because a chart we generate has to emit one +/// and `xmlserde::Unparsed` cannot be constructed from outside that crate. The +/// three attributes and two children below are the whole of `CT_Transform2D`, +/// so nothing is dropped from a file that already had one. +/// +/// Distinct from `drawings::CtTransform2D`, which names its children without +/// the `a:` prefix; this module writes every tag prefixed. +#[derive(Debug, Default, XmlSerialize, XmlDeserialize)] +pub struct CtGraphicFrameXfrm { + #[xmlserde(name = b"rot", ty = "attr")] + pub rot: Option, + #[xmlserde(name = b"flipH", ty = "attr")] + pub flip_h: Option, + #[xmlserde(name = b"flipV", ty = "attr")] + pub flip_v: Option, + #[xmlserde(name = b"a:off", ty = "child")] + #[xmlserde(alias(b"off"))] + pub off: Option, + #[xmlserde(name = b"a:ext", ty = "child")] + #[xmlserde(alias(b"ext"))] + pub ext: Option, +} + +/// `` — an offset in EMUs. +#[derive(Debug, XmlSerialize, XmlDeserialize)] +pub struct CtPoint2D { + #[xmlserde(name = b"x", ty = "attr")] + pub x: i64, + #[xmlserde(name = b"y", ty = "attr")] + pub y: i64, +} + #[derive(Debug, Default, XmlSerialize, XmlDeserialize)] pub struct CtGraphicFrameNonVisual { #[xmlserde(name = b"xdr:cNvPr", ty = "child")] @@ -464,7 +503,9 @@ impl CtTwoCellAnchor { }), c_nv_graphic_frame_pr: Some(CtNvGraphicFrameProps::default()), }), - xfrm: None, + // Empty, but present: Excel recomputes the geometry from + // the anchor and repairs the part if the element is absent. + xfrm: Some(CtGraphicFrameXfrm::default()), graphic: Some(CtGraphicalObject { graphic_data: Some(CtGraphicalObjectData { uri: String::from("http://schemas.openxmlformats.org/drawingml/2006/chart"), @@ -522,7 +563,9 @@ impl CtOneCellAnchor { }), c_nv_graphic_frame_pr: Some(CtNvGraphicFrameProps::default()), }), - xfrm: None, + // Empty, but present: Excel recomputes the geometry from + // the anchor and repairs the part if the element is absent. + xfrm: Some(CtGraphicFrameXfrm::default()), graphic: Some(CtGraphicalObject { graphic_data: Some(CtGraphicalObjectData { uri: String::from("http://schemas.openxmlformats.org/drawingml/2006/chart"), diff --git a/crates/workbook/src/writer.rs b/crates/workbook/src/writer.rs index ea9897ad..3dbfb808 100644 --- a/crates/workbook/src/writer.rs +++ b/crates/workbook/src/writer.rs @@ -755,11 +755,18 @@ fn write_content_types( let overides = proofs .into_iter() .fold(Vec::::new(), |mut prev, p| { - let c = CtOverride { - part_name: format!("/{}", String::from(p.path)), - content_type: get_content_type(p.rtype).into(), - }; - prev.push(c); + let content_type = get_content_type(p.rtype); + // An `Override` with an empty ContentType is not a lax package, it + // is an invalid one — OPC requires a media type here, and Excel + // rejects the whole file rather than repairing a part. A part whose + // type we cannot name is better left to the `xml` Default above, + // which is what it would have resolved to anyway. + if !content_type.is_empty() { + prev.push(CtOverride { + part_name: format!("/{}", String::from(p.path)), + content_type: content_type.into(), + }); + } prev }); // Entries carried by preserved parts, deduped against what we emit anyway: @@ -811,7 +818,16 @@ fn get_content_type(rtype: RType) -> &'static str { "application/vnd.openxmlformats-officedocument.spreadsheetml.pivotTable+xml" } TABLE => "application/vnd.openxmlformats-officedocument.spreadsheetml.table+xml", - _ => "", + // Our own part. Not an OOXML type, but a part still needs a real + // content type: an `Override` with an empty one makes the whole package + // invalid, and Excel then refuses the file with a generic "we found a + // problem" rather than a per-part repair record. + LOGISHEETS_APP_DATA => "application/xml", + // Nothing else should reach here. Returning "" would emit an invalid + // Override, so name the generic XML type — which is what the `xml` + // Default resolves to anyway, and which `write_content_types` is free + // to drop as redundant. + _ => "application/xml", } } @@ -930,6 +946,138 @@ mod tests { assert!(ct.contains("/xl/charts/colors1.xml")); } + /// Every `Override` must name a real media type. + /// + /// An empty `ContentType` is not a lax package but an invalid one: Excel + /// rejects the whole file with a generic "we found a problem" and no + /// per-part repair record, because the failure is at the OPC layer rather + /// than inside any one part. `logisheets/data.xml` shipped like that — its + /// relationship type was missing from `get_content_type`, whose catch-all + /// arm returned "". + #[test] + fn every_content_type_override_names_a_media_type() { + use crate::logisheets::LogiSheetsData; + use crate::workbook::Wb; + let buf = fs::read("../../tests/graph.xlsx").unwrap(); + let mut wb = Wb::from_file(&buf).unwrap(); + // The regression was in `logisheets/data.xml`, which is only written + // for a workbook that carries LogiSheets data — every workbook this + // engine authors, and none of the plain-Excel fixtures. Attach some, or + // the part under test is never emitted. + wb.logisheets = Some(LogiSheetsData { + sheets: vec![], + apps: vec![], + field_renders: vec![], + }); + let out = write(wb).unwrap(); + let ct = read_zip_entry(&out, "[Content_Types].xml"); + assert!( + ct.contains("/logisheets/data.xml"), + "the part under test was not written: {ct}" + ); + + // Every declared type is `type/subtype`, per the media-type grammar OPC + // requires. This catches the empty string too. + let mut checked = 0; + for chunk in ct.split("ContentType=\"").skip(1) { + let value = chunk.split('"').next().unwrap_or(""); + let parts = value.split('/').collect::>(); + assert!( + parts.len() == 2 && !parts[0].is_empty() && !parts[1].is_empty(), + "not a media type: {value:?} in {ct}" + ); + checked += 1; + } + assert!(checked > 0, "no content types declared at all"); + } + + /// A generated chart's `graphicFrame` must carry `xdr:xfrm`. + /// + /// The schema requires it, and Excel repairs the drawing part without it. + /// Excel recomputes the geometry from the anchor, so an empty element is + /// enough — which is what real producers write (see + /// `tests/one_cell_anchor.xlsx`) — but it has to be there. + #[test] + fn a_generated_chart_anchor_carries_an_xfrm() { + use crate::ooxml::drawing_part::{CtMarker, CtOneCellAnchor, CtPositiveSize2D, CtTwoCellAnchor}; + + let two = CtTwoCellAnchor::new_chart_anchor( + CtMarker::new(5, 4), + CtMarker::new(13, 16), + 2, + String::from("Chart 1"), + String::from("rId1"), + ); + assert!( + two.graphic_frame + .as_ref() + .expect("graphic frame") + .xfrm + .is_some(), + "twoCellAnchor: a generated graphicFrame needs an xfrm or Excel repairs the drawing" + ); + + let one = CtOneCellAnchor::new_chart_anchor( + CtMarker::new(5, 4), + CtPositiveSize2D { + cx: 4572000, + cy: 2743200, + }, + 2, + String::from("Chart 1"), + String::from("rId1"), + ); + assert!( + one.graphic_frame + .as_ref() + .expect("graphic frame") + .xfrm + .is_some(), + "oneCellAnchor: a generated graphicFrame needs an xfrm" + ); + } + + /// The typed `xfrm` must not lose geometry a real file already had. + /// + /// `tests/graph.xlsx` was written by Excel and carries a populated + /// ``. Modelling the element — it used + /// to be opaque passthrough — is only safe if those values survive a save. + #[test] + fn a_populated_xfrm_survives_the_round_trip() { + use crate::workbook::Wb; + let buf = fs::read("../../tests/graph.xlsx").unwrap(); + + let read_xfrm = |bytes: &[u8]| { + let wb = Wb::from_file(bytes).unwrap(); + let d = wb + .xl + .worksheets + .values() + .filter_map(|w| w.drawing.as_ref()) + .find(|d| !d.chart_parts.is_empty()) + .expect("drawing with a chart"); + let frame = d + .content + .two_cell_anchors + .iter() + .find_map(|a| a.graphic_frame.as_ref()) + .expect("graphicFrame"); + let xfrm = frame.xfrm.as_ref().expect("xfrm read from a real file"); + ( + xfrm.off.as_ref().map(|o| (o.x, o.y)), + xfrm.ext.as_ref().map(|e| (e.cx, e.cy)), + ) + }; + + let before = read_xfrm(&buf); + assert!( + before.0.is_some() && before.1.is_some(), + "fixture should have a populated xfrm, got {before:?}" + ); + let out = write(Wb::from_file(&buf).unwrap()).unwrap(); + assert_eq!(before, read_xfrm(&out), "xfrm geometry changed on save"); + } + #[test] fn pivot_round_trips() { use crate::ooxml::simple_types::StSourceType; diff --git a/docs/craft/writing-a-craft.md b/docs/craft/writing-a-craft.md index 6d9450b1..faa7a1c4 100644 --- a/docs/craft/writing-a-craft.md +++ b/docs/craft/writing-a-craft.md @@ -258,10 +258,21 @@ await ctx.workbook.handleTransaction({ ``` The operations are `insertDeleteLines`, `removeBlock`, `modifySchema`, -`cellInput`, `sortByField` and `modifyDescription`. Each takes `all`, -`ownerAndUser` (the person at the keyboard, but no other craft) or `ownerOnly`. -An operation you leave out follows `modifyPolicy`. **`owner` must be set** — a -policy on an unowned block has nobody to privilege, so it is read as `all`. +`cellInput`, `sortByField`, `modifyDescription` and `overrideValidation`. Each +takes `all`, `ownerAndUser` (the person at the keyboard, but no other craft) or +`ownerOnly`. An operation you leave out follows `modifyPolicy`. **`owner` must +be set** — a policy on an unowned block has nobody to privilege, so it is read +as `all`. + +`overrideValidation` is the narrow one: it governs writing a value that a +field's `validationFormulas` rule rejects, not writing at all. Left at `all` a +violating value lands and the cell is flagged, which is what every block did +before the op existed. Reserved to the owner, the person at the keyboard is +held to the rule while you can still seed a row you know is incomplete — a +required field you fill in on the next round. Hosts ask +`checkFieldValidation({sheetIdx, row, col, proposed})` for the verdict on a +value before writing it, then `mayModifyBlock` with this op; the engine +evaluates the rule against the proposed value without touching the cell. `removeBlock` is worth reserving even on a block that is otherwise open: deleting it takes the records, the schema and this policy along with it, and diff --git a/packages/core/src/ops/index.ts b/packages/core/src/ops/index.ts index 536973ea..b2268b9a 100644 --- a/packages/core/src/ops/index.ts +++ b/packages/core/src/ops/index.ts @@ -72,6 +72,16 @@ export interface FormBlockField { renderId: string /** Per-field value-formula template (#FIELD("X") / #KEY); '' if free-form. */ valueFormula?: string + /** + * Per-field validation template (#PLACEHOLDER for the value under test, + * #FIELD("X") for a same-row sibling); '' when the field has no rule. + * + * This goes into the schema rather than staying host-side so the engine + * installs the per-record shadow itself — on bind AND on every row added + * later — and so one answer serves every reader: the warning marker, the + * `overrideValidation` write gate, and any other host. + */ + validationFormula?: string /** Whether the field renders via a host-drawn (DIY) overlay. */ diyRender: boolean /** Number format applied to the field's render info. */ @@ -480,7 +490,9 @@ export class WorkbookOps { fields: fields.map((f) => f.name), renderIds: fields.map((f) => f.renderId), fieldFormulas: fields.map((f) => f.valueFormula ?? ''), - validationFormulas: [], + validationFormulas: fields.map( + (f) => f.validationFormula ?? '' + ), editabilityFormulas: [], }, }, @@ -548,7 +560,9 @@ export class WorkbookOps { fields: fields.map((f) => f.name), renderIds: fields.map((f) => f.renderId), fieldFormulas: fields.map((f) => f.valueFormula ?? ''), - validationFormulas: [], + validationFormulas: fields.map( + (f) => f.validationFormula ?? '' + ), editabilityFormulas: [], }, }, @@ -624,7 +638,9 @@ export class WorkbookOps { fields: fields.map((f) => f.name), renderIds: fields.map((f) => f.renderId), fieldFormulas: fields.map((f) => f.valueFormula ?? ''), - validationFormulas: [], + validationFormulas: fields.map( + (f) => f.validationFormula ?? '' + ), editabilityFormulas: [], }, }) @@ -641,6 +657,43 @@ export class WorkbookOps { await this.apply(payloads, true) } + /** + * Rewrite ONE kind of per-field rule on a block, leaving the others alone. + * + * `formulas` is one entry per field, in the schema's field order — a rule + * or `''` for none. The other two rule kinds are sent empty, which the + * engine reads as "don't touch these", so editing a validation rule cannot + * clear the value formulas standing next to it. + * + * Every existing row is re-materialized from the new rule, so this is also + * how a rule is removed: pass `''` for that field. + */ + async setFieldRules(opts: { + sheetIdx: number + blockId: number + kind: 'value' | 'validation' | 'editability' + formulas: readonly string[] + }): Promise { + const {sheetIdx, blockId, kind, formulas} = opts + const forKind = (k: typeof kind) => + kind === k ? formulas.map((f) => f ?? '') : [] + await this.apply( + [ + { + type: 'upsertFieldFormulas', + value: { + sheetIdx, + blockId, + fieldFormulas: forKind('value'), + validationFormulas: forKind('validation'), + editabilityFormulas: forKind('editability'), + }, + }, + ], + true + ) + } + // ---- generic / temp-branch ----------------------------------------- /** diff --git a/packages/engine/src/lib/block/field_manager.ts b/packages/engine/src/lib/block/field_manager.ts index c7bddce5..627be5c3 100644 --- a/packages/engine/src/lib/block/field_manager.ts +++ b/packages/engine/src/lib/block/field_manager.ts @@ -145,9 +145,11 @@ export class FieldManager { blockId, }; // (Pre-Phase-1+2 we forced `userEditable=false` here when - // valueFormula was set. The Rust engine now rejects BlockInput on - // templated cells in the container layer, so the TS-side enforcement - // is redundant and FieldInfo no longer carries valueFormula.) + // valueFormula was set. The Rust engine now drops every user-facing + // write to a templated cell — CellInput, CellClear, paste/fill and + // BlockInput alike — and the host reads the template off + // `BlockInfo.schema` to gate the UI, so FieldInfo no longer needs to + // carry either the formula or a flag standing in for it.) this.fields.set(fieldId, fieldInfo); diff --git a/packages/engine/src/lib/clients/workbook.ts b/packages/engine/src/lib/clients/workbook.ts index 709c9544..84be4bb3 100644 --- a/packages/engine/src/lib/clients/workbook.ts +++ b/packages/engine/src/lib/clients/workbook.ts @@ -692,6 +692,11 @@ export class WorkbookClient implements Client { Client['mayModifyBlock'] > + checkFieldValidation: Client['checkFieldValidation'] = (params) => + this._call(MethodName.CheckFieldValidation, params) as ReturnType< + Client['checkFieldValidation'] + > + getDiyCellIdWithBlockId: Client['getDiyCellIdWithBlockId'] = (params) => this._call(MethodName.GetDiyCellIdWithBlockId, params) as ReturnType< Client['getDiyCellIdWithBlockId'] diff --git a/packages/engine/src/lib/worker/types.ts b/packages/engine/src/lib/worker/types.ts index 9051fa3d..e925d098 100644 --- a/packages/engine/src/lib/worker/types.ts +++ b/packages/engine/src/lib/worker/types.ts @@ -98,6 +98,7 @@ export enum MethodName { GetBlockValues = 'getBlockValues', GetBlockSortOrder = 'getBlockSortOrder', MayModifyBlock = 'mayModifyBlock', + CheckFieldValidation = 'checkFieldValidation', GetBlockModifyInfo = 'getBlockModifyInfo', GetAvailableBlockId = 'getAvailableBlockId', diff --git a/packages/engine/src/lib/worker/workbook.worker.ts b/packages/engine/src/lib/worker/workbook.worker.ts index d79d3f6c..d2f4a543 100644 --- a/packages/engine/src/lib/worker/workbook.worker.ts +++ b/packages/engine/src/lib/worker/workbook.worker.ts @@ -37,6 +37,8 @@ import type { BlockSortOrder, GetBlockSortOrderParams, MayModifyBlockParams, + CheckFieldValidationParams, + FieldValidationVerdict, GetBlockModifyInfoParams, BlockModifyInfo, } from 'logisheets-web' @@ -411,6 +413,16 @@ export class WorkbookWorkerService implements IWorkbookWorker { return this.workbook.mayModifyBlock(params) } + /** + * Whether a proposed value would break its field's validation rule. + * Read-only — the cell is left holding whatever it already held. + */ + public checkFieldValidation( + params: CheckFieldValidationParams + ): Result { + return this.workbook.checkFieldValidation(params) + } + /** A block's governance metadata, without its cells. */ public getBlockModifyInfo( params: GetBlockModifyInfoParams @@ -808,6 +820,9 @@ export class WorkbookWorkerService implements IWorkbookWorker { case MethodName.MayModifyBlock: result = this.mayModifyBlock(args) break + case MethodName.CheckFieldValidation: + result = this.checkFieldValidation(args) + break case MethodName.GetAvailableBlockId: result = this.getAvailableBlockId(args) break diff --git a/packages/formula-editor/src/lib/inline.ts b/packages/formula-editor/src/lib/inline.ts index 5f6f687b..46f180c8 100644 --- a/packages/formula-editor/src/lib/inline.ts +++ b/packages/formula-editor/src/lib/inline.ts @@ -115,6 +115,13 @@ export interface InlineCellEditorOptions { origin?: {x: number; y: number} /** Permission gate before opening. Default: always editable. */ canEdit?: (sheetIdx: number, row: number, col: number, grid: Grid) => boolean + /** + * Called instead of opening the editor when {@link canEdit} refuses. + * Without it a keystroke on a read-only cell does nothing at all, which + * reads as a broken grid rather than a locked one — hosts use this to say + * why. Default: no-op. + */ + onEditRefused?: (sheetIdx: number, row: number, col: number) => void /** Autocomplete / signature functions. Default: bundled built-ins. */ formulaFunctions?: FormulaFunction[] /** Maps a reference index to a CSS color. Default: built-in palette. */ @@ -177,6 +184,7 @@ export function createInlineCellEditor( coordinator, origin = {x: 32, y: 24}, canEdit = () => true, + onEditRefused, formulaFunctions = builtinFormulaFunctions, getHighlightColor = getCellRefColor, editorConfig, @@ -416,7 +424,10 @@ export function createInlineCellEditor( ) { if (!grid) return const editSheetIdx = getViewSheetIdx() - if (!canEdit(editSheetIdx, row, col, grid)) return + if (!canEdit(editSheetIdx, row, col, grid)) { + onEditRefused?.(editSheetIdx, row, col) + return + } if (editing) finishEditing() const position = getCellRect(grid, row, col, { diff --git a/packages/logician/src/tools/builder.ts b/packages/logician/src/tools/builder.ts index 663a5195..6d124f13 100644 --- a/packages/logician/src/tools/builder.ts +++ b/packages/logician/src/tools/builder.ts @@ -59,6 +59,7 @@ const BLOCK_OPS: readonly BlockOp[] = [ 'cellInput', 'sortByField', 'modifyDescription', + 'overrideValidation', ] /** Narrow the workbook client to the concrete `Client` from logisheets-web. @@ -3230,7 +3231,7 @@ export const setBlockPermissions: Tool< description: [ 'Set who may do what to a block, one operation at a time.', '', - 'Use it on a block you built that must stay under your control: leave `cellInput` open so the user keeps entering data, and reserve `insertDeleteLines`, `removeBlock` and `modifySchema` to the owner so a stray edit cannot take the table out of your hands. `removeBlock` is worth reserving even when the rest is open — deleting the block takes its records, its schema and this policy with it. `ownerOnly` means only the owning craft; `ownerAndUser` adds the person at the keyboard but no other craft; `all` means anyone.', + "Use it on a block you built that must stay under your control: leave `cellInput` open so the user keeps entering data, and reserve `insertDeleteLines`, `removeBlock` and `modifySchema` to the owner so a stray edit cannot take the table out of your hands. `removeBlock` is worth reserving even when the rest is open — deleting the block takes its records, its schema and this policy with it. `overrideValidation` is narrower than `cellInput`: reserving it lets the user keep typing but holds them to the fields' validation rules, while you can still seed a row you know is incomplete. `ownerOnly` means only the owning craft; `ownerAndUser` adds the person at the keyboard but no other craft; `all` means anyone.", '', "REPLACES the whole set: an operation you leave out goes back to following `default_policy`. State the block's full stance each time.", '', @@ -3286,6 +3287,7 @@ export const setBlockPermissions: Tool< cellInput: policy('cellInput'), sortByField: policy('sortByField'), modifyDescription: policy('modifyDescription'), + overrideValidation: policy('overrideValidation'), }, modifyPolicy: input.default_policy, }, diff --git a/packages/web/src/api/workbook.ts b/packages/web/src/api/workbook.ts index 0b30b12b..b1e2ff18 100644 --- a/packages/web/src/api/workbook.ts +++ b/packages/web/src/api/workbook.ts @@ -7,6 +7,8 @@ import { BlockSortOrder, GetBlockSortOrderParams, MayModifyBlockParams, + CheckFieldValidationParams, + FieldValidationVerdict, GetBlockModifyInfoParams, BlockModifyInfo, FormulaDisplayInfo, @@ -669,6 +671,29 @@ export class Workbook { ) } + /** + * Whether writing `proposed` into this cell would break its field's + * validation rule. + * + * The companion to {@link mayModifyBlock} for `overrideValidation`: that + * answers whether this actor may write a violating value, this answers + * whether the value in hand is a violating one. A host gating a write + * needs both, and needs them BEFORE the write — the marker on the cell + * only ever judges a value the cell already holds. + * + * Reads nothing into the workbook: the rule is evaluated against the + * proposed value and the cell is left exactly as it was. + */ + public checkFieldValidation( + params: CheckFieldValidationParams + ): Result { + return rpc( + 'checkFieldValidation', + params as unknown as Record, + this._id + ) + } + /** * A block's governance metadata on its own — owner, default policy, * per-operation overrides, description. `getBlockInfo` carries the same diff --git a/src/components/block-composer/config_panel.tsx b/src/components/block-composer/config_panel.tsx index 140d90f7..86c04d24 100644 --- a/src/components/block-composer/config_panel.tsx +++ b/src/components/block-composer/config_panel.tsx @@ -29,6 +29,11 @@ import { } from '@mui/icons-material' import type {FieldSetting, EnumValue, FieldTypeEnum} from 'logisheets-core' import {COLORS} from './types' +import { + FieldRuleEditor, + RULE_HELPER_TEXT, + RULE_PLACEHOLDER, +} from './field-rule-editor' import {buttonSx, primaryButtonSx, cardSx, sectionLabelSx} from './styles' import {EnumSetManager, FieldManager} from 'logisheets-engine' import {useToast} from '@/ui/notification/useToast' @@ -88,6 +93,11 @@ export const FieldConfigPanel = ({ ) const availableEnumSets = enumSetManager.getAll() + + // Rule editors address fields by name; the panel keeps the list here so + // both boxes offer the same siblings. + const fieldNames = localFields.map((f) => f.name) + const showEnumSection = field.type === 'enum' || field.type === 'multiSelect' @@ -1140,35 +1150,31 @@ export const FieldConfigPanel = ({ } /> - - onUpdate({ - ...field, - validation: e.target.value, - }) + onChange={(validation) => + onUpdate({...field, validation}) } - placeholder="e.g., #PLACEHOLDER > 0 && #PLACEHOLDER < 100" - helperText="Use #PLACEHOLDER to reference the input value" + placeholder={ + RULE_PLACEHOLDER.validation + } + helperText={RULE_HELPER_TEXT.validation} /> - - onUpdate({ - ...field, - valueFormula: e.target.value, - }) - } - placeholder={`e.g., =#FIELD("amount") * #FIELD("price")`} - helperText={ - 'When set, this column is derived (not editable). Use #FIELD("name") for same-row siblings and #KEY for the row key.' + onChange={(valueFormula) => + onUpdate({...field, valueFormula}) } + placeholder={RULE_PLACEHOLDER.value} + helperText={RULE_HELPER_TEXT.value} /> diff --git a/src/components/block-composer/field-formula.test.ts b/src/components/block-composer/field-formula.test.ts new file mode 100644 index 00000000..410346c7 --- /dev/null +++ b/src/components/block-composer/field-formula.test.ts @@ -0,0 +1,127 @@ +import {describe, expect, it} from 'vitest' +import type {FieldSetting} from 'logisheets-core' +import { + firstFieldFormulaError, + referencedFieldNames, + validateFieldFormula, + validateValidationFormula, +} from './field-formula' + +function field( + name: string, + valueFormula?: string, + validation?: string +): FieldSetting { + return { + id: `id-${name}`, + name, + type: 'number', + required: false, + unique: false, + primary: false, + valueFormula, + validation, + } as FieldSetting +} + +describe('referencedFieldNames', () => { + it('reads names out of both quote styles and spacing variants', () => { + expect( + referencedFieldNames( + `=#FIELD("qty") * #field( 'unit price' ) + #FIELD("qty")` + ) + ).toEqual([ + {name: 'qty', keyed: false}, + {name: 'unit price', keyed: false}, + {name: 'qty', keyed: false}, + ]) + }) + + it('marks the keyed form, which addresses another row', () => { + expect(referencedFieldNames(`=#FIELD("total", "a-1")`)).toEqual([ + {name: 'total', keyed: true}, + ]) + }) + + it('finds nothing in a formula with no field refs', () => { + expect(referencedFieldNames('=#KEY & "-x"')).toEqual([]) + }) +}) + +describe('validateFieldFormula', () => { + const qty = field('qty') + const price = field('price') + + it('accepts an empty formula — most fields have none', () => { + expect(validateFieldFormula(field('total'), [qty, price])).toBeNull() + }) + + it('accepts refs to declared siblings', () => { + const total = field('total', '=#FIELD("qty")*#FIELD("price")') + expect(validateFieldFormula(total, [qty, price, total])).toBeNull() + }) + + it('rejects a ref to a field the block does not have', () => { + const total = field('total', '=#FIELD("qty")*#FIELD("discount")') + expect(validateFieldFormula(total, [qty, price, total])).toMatch( + /discount/ + ) + }) + + it('rejects an unkeyed self-reference — it resolves to this very cell', () => { + const total = field('total', '=#FIELD("total")+1') + expect(validateFieldFormula(total, [qty, total])).toMatch(/itself/) + }) + + it('allows a keyed self-reference — that one reaches another record', () => { + const total = field('total', '=#FIELD("total", "a-1")+1') + expect(validateFieldFormula(total, [qty, total])).toBeNull() + }) +}) + +describe('validateValidationFormula', () => { + const qty = field('qty') + + it('accepts a rule that only uses #PLACEHOLDER', () => { + const f = field( + 'total', + undefined, + 'AND(#PLACEHOLDER>0,#PLACEHOLDER<10)' + ) + expect(validateValidationFormula(f, [qty, f])).toBeNull() + }) + + it('rejects a ref to a field the block does not have', () => { + const f = field('total', undefined, '#PLACEHOLDER<#FIELD("cap")') + expect(validateValidationFormula(f, [qty, f])).toMatch(/cap/) + }) + + it('allows a rule to name its own field — that is not a self-loop', () => { + // A rule is evaluated against the cell rather than computing it, so + // `#FIELD("total")` here is just a longhand `#PLACEHOLDER`. + const f = field('total', undefined, '#FIELD("total")>0') + expect(validateValidationFormula(f, [qty, f])).toBeNull() + }) +}) + +describe('firstFieldFormulaError', () => { + it('returns nothing when every field checks out', () => { + const qty = field('qty') + const total = field('total', '=#FIELD("qty")*2') + expect(firstFieldFormulaError([qty, total])).toBeNull() + }) + + it('names the offending field so the dialog can select it', () => { + const qty = field('qty') + const total = field('total', '=#FIELD("nope")') + expect(firstFieldFormulaError([qty, total])?.field.name).toBe('total') + }) + + it('catches a bad validation rule too, not just a bad value formula', () => { + const qty = field('qty') + const total = field('total', '=#FIELD("qty")*2', '#FIELD("nope")>0') + const bad = firstFieldFormulaError([qty, total]) + expect(bad?.field.name).toBe('total') + expect(bad?.message).toMatch(/nope/) + }) +}) diff --git a/src/components/block-composer/field-formula.ts b/src/components/block-composer/field-formula.ts new file mode 100644 index 00000000..e6df4f5c --- /dev/null +++ b/src/components/block-composer/field-formula.ts @@ -0,0 +1,115 @@ +// Local checks for a field's value-formula template, so the composer can say +// what's wrong while the person is still typing it. +// +// The engine validates the same thing at bind time and refuses the whole +// payload — correct, but it arrives as an aborted save with a message written +// for a caller, after the dialog has already collected everything else. These +// checks are deliberately narrow: only the mistakes that are certain without +// parsing (an unknown field name, a self-reference). Everything else — syntax, +// function names, cross-block refs — stays the engine's call. + +import type {FieldSetting} from 'logisheets-core' + +/** + * `#FIELD("name")` / `#FIELD('name')`, capturing the name and whether a second + * (row-key) argument follows. The keyed form addresses another row, so it is + * the one case where a field may legitimately name itself. + */ +const FIELD_REF = /#FIELD\s*\(\s*(["'])(.*?)\1\s*(,)?/gi + +/** Every `#FIELD(...)` reference in a template, in order. */ +export function referencedFieldNames( + formula: string +): {name: string; keyed: boolean}[] { + const refs: {name: string; keyed: boolean}[] = [] + for (const m of formula.matchAll(FIELD_REF)) + refs.push({name: m[2], keyed: m[3] === ','}) + return refs +} + +/** Which rule is being written — they differ in what they may reference. */ +export type FieldRuleKind = 'value' | 'validation' + +/** + * What's wrong with a rule as typed, or `null` when it's fine (or empty — a + * field with no rule is the normal case, not an error). + * + * Text-level so both the composer, which edits a block that does not exist + * yet, and the grid's per-field dialog, which edits a live one, can ask the + * same question. `selfName` is the field the rule belongs to; `allNames` every + * field of its block, including that one. + */ +export function validateRuleText( + kind: FieldRuleKind, + text: string | undefined, + selfName: string, + allNames: readonly string[] +): string | null { + const formula = (text ?? '').trim() + if (formula === '') return null + + const declared = new Set(allNames) + for (const {name, keyed} of referencedFieldNames(formula)) { + if (!declared.has(name)) + return `#FIELD("${name}") refers to a field this block doesn’t have.` + // In a VALUE formula an unkeyed self-reference resolves to the very + // cell being computed — a self-loop. Keyed (`#FIELD("x", "some-key")`) + // it reaches another row, which is the supported way to read across + // records. + // + // A VALIDATION rule may name its own field freely: it is evaluated + // against the cell rather than computing it, so `#FIELD("qty")` inside + // `qty`'s rule is just a longhand `#PLACEHOLDER`. + if (kind === 'value' && name === selfName && !keyed) + return `#FIELD("${name}") resolves to this cell itself. Add a row key — #FIELD("${name}", "…") — to read another record.` + } + return null +} + +/** + * What's wrong with `field`'s value formula, or `null` when it's fine. + * + * `allFields` is every field of the block being composed, including `field` + * itself. + */ +export function validateFieldFormula( + field: FieldSetting, + allFields: readonly FieldSetting[] +): string | null { + return validateRuleText( + 'value', + field.valueFormula, + field.name, + allFields.map((f) => f.name) + ) +} + +/** What's wrong with `field`'s validation rule, or `null` when it's fine. */ +export function validateValidationFormula( + field: FieldSetting, + allFields: readonly FieldSetting[] +): string | null { + return validateRuleText( + 'validation', + field.validation, + field.name, + allFields.map((f) => f.name) + ) +} + +/** + * The first field whose rules don't check out, with its message. Used to stop + * a save before it reaches the engine, which would otherwise refuse the whole + * bind — taking every other edit in the dialog with it. + */ +export function firstFieldFormulaError( + allFields: readonly FieldSetting[] +): {field: FieldSetting; message: string} | null { + for (const field of allFields) { + const value = validateFieldFormula(field, allFields) + if (value) return {field, message: value} + const validation = validateValidationFormula(field, allFields) + if (validation) return {field, message: validation} + } + return null +} diff --git a/src/components/block-composer/field-rule-editor.tsx b/src/components/block-composer/field-rule-editor.tsx new file mode 100644 index 00000000..0fe63a56 --- /dev/null +++ b/src/components/block-composer/field-rule-editor.tsx @@ -0,0 +1,159 @@ +// One field rule — a value formula or a validation rule — as a text box with +// the block's own placeholders offered underneath. +// +// Shared by the composer (authoring a block that does not exist yet) and the +// grid's per-field dialog (editing a live one) so the two cannot drift on what +// a rule may say or how it explains itself. + +import {useEffect, useRef} from 'react' +import {Box, Chip, Stack, TextField, Typography} from '@mui/material' +import {FieldRuleKind, validateRuleText} from './field-formula' + +export interface FieldRuleEditorProps { + kind: FieldRuleKind + /** The field this rule belongs to — excluded from the sibling chips. */ + fieldName: string + /** Every field name in the block, including `fieldName`. */ + allFieldNames: readonly string[] + value: string + onChange: (next: string) => void + /** Rendered under the box when the rule checks out. */ + helperText: string + label?: string + placeholder?: string + autoFocus?: boolean +} + +/** The error to show for a rule, or `null`. Exported so a save can reuse it. */ +export function ruleError( + kind: FieldRuleKind, + value: string, + fieldName: string, + allFieldNames: readonly string[] +): string | null { + return validateRuleText(kind, value, fieldName, allFieldNames) +} + +export const FieldRuleEditor = ({ + kind, + fieldName, + allFieldNames, + value, + onChange, + helperText, + label, + placeholder, + autoFocus, +}: FieldRuleEditorProps) => { + const inputRef = useRef(null) + const pendingCaret = useRef(null) + + const error = ruleError(kind, value, fieldName, allFieldNames) + + // Splice a placeholder in at the caret (or append when the box isn't + // focused), then put the caret after it so several inserts compose into + // one expression. The box is controlled, so the caret has to be restored + // AFTER React has written the new value into the DOM — hence the ref plus + // effect rather than setting it here, which the re-render would undo. + const insert = (snippet: string) => { + const input = inputRef.current + const at = + input && document.activeElement === input + ? input.selectionStart ?? value.length + : value.length + pendingCaret.current = at + snippet.length + onChange(value.slice(0, at) + snippet + value.slice(at)) + } + + useEffect(() => { + const caret = pendingCaret.current + if (caret === null) return + pendingCaret.current = null + const el = inputRef.current + if (!el) return + el.focus() + el.setSelectionRange(caret, caret) + }) + + return ( + + onChange(e.target.value)} + placeholder={placeholder} + error={!!error} + helperText={error ?? helperText} + /> + {/* The placeholders are the whole point of a field rule and are + easy to get subtly wrong by hand (a field since renamed, a + quote missed), so offer them as the block actually defines + them. */} + + + Insert: + + {kind === 'validation' && ( + insert('#PLACEHOLDER')} + /> + )} + {allFieldNames + .filter( + // A value formula cannot read its own column — that is + // the cell being computed. A validation rule can, but + // `#PLACEHOLDER` says it better, so leave it out of + // both. + (n) => n !== fieldName && n.trim() !== '' + ) + .map((n) => ( + + insert(`#FIELD("${n.replace(/"/g, '""')}")`) + } + /> + ))} + insert('#KEY')} + /> + + + ) +} + +/** The helper text each rule kind shows when it checks out. */ +export const RULE_HELPER_TEXT: Record = { + value: 'When set, this column is derived — the engine computes every row and nobody can type over it.', + validation: + 'Flags a value that breaks the rule. #PLACEHOLDER is the value being checked; the block can also refuse such a write outright — see its permissions.', +} + +/** The placeholder each rule kind shows in an empty box. */ +export const RULE_PLACEHOLDER: Record = { + value: 'e.g., =#FIELD("amount") * #FIELD("price")', + validation: 'e.g., AND(#PLACEHOLDER>0, #PLACEHOLDER<100)', +} diff --git a/src/components/block-composer/field_list.tsx b/src/components/block-composer/field_list.tsx index c5968c0b..0a1b673f 100644 --- a/src/components/block-composer/field_list.tsx +++ b/src/components/block-composer/field_list.tsx @@ -7,6 +7,7 @@ import { ListItem, ListItemButton, ListItemText, + Tooltip, } from '@mui/material' import { Add as AddIcon, @@ -206,6 +207,29 @@ export const FieldList = ({ > {field.name} + {field.valueFormula?.trim() ? ( + + + + ) : null} {field.primary && ( {field.name} + {field.valueFormula?.trim() ? ( + + + + ) : null} {field.primary && ( { default: break } + // The validation rule goes into the SCHEMA, not just the host + // FieldInfo: the engine then installs the per-record shadow itself + // (including on rows added later), and the same rule answers both + // the warning marker and the `overrideValidation` write gate. + // Only some field types carry one — the rest send ''. + const validationFormula = + 'validation' in ty ? ty.validation ?? '' : '' return { name: field.name, renderId, valueFormula: field.valueFormula ?? '', + validationFormula, diyRender, numFmt, } @@ -404,8 +413,22 @@ export const BlockComposerComponent = (props: BlockComposerProps) => { return null } + /** + * Field formulas that name a field the block doesn't have make the engine + * refuse the whole bind, taking every other edit in the dialog with it. + * Catch it here and say which field is at fault. + */ + const validateFieldFormulas = (): boolean => { + const bad = firstFieldFormulaError(fields) + if (!bad) return true + setSelectedFieldId(bad.field.id) + toast(`“${bad.field.name}” — ${bad.message}`, {type: 'error'}) + return false + } + const handleSaveEdit = async () => { if (!editTarget || !editMeta) return + if (!validateFieldFormulas()) return const refErr = await validateRefName() if (refErr) { toast(refErr, {type: 'error'}) @@ -449,6 +472,7 @@ export const BlockComposerComponent = (props: BlockComposerProps) => { ) return } + if (!validateFieldFormulas()) return const refErr = await validateRefName() if (refErr) { toast(refErr, {type: 'error'}) diff --git a/src/components/block-interface/field-rule-dialog.tsx b/src/components/block-interface/field-rule-dialog.tsx new file mode 100644 index 00000000..9f39ac40 --- /dev/null +++ b/src/components/block-interface/field-rule-dialog.tsx @@ -0,0 +1,103 @@ +// Edit ONE rule on ONE field of a live block, straight from the grid. +// +// The composer can do this too, but it opens the block's whole schema to reach +// it. Wanting to change what a column computes, or what it accepts, is a small +// and frequent enough thing to deserve its own door — and it is the one edit +// that is safe to make on a block already full of records, because the engine +// re-materializes every row from the new rule. + +import {useEffect, useState} from 'react' +import { + Button, + Dialog, + DialogActions, + DialogContent, + DialogTitle, + Typography, +} from '@mui/material' +import { + FieldRuleEditor, + RULE_HELPER_TEXT, + RULE_PLACEHOLDER, + ruleError, +} from '@/components/block-composer/field-rule-editor' +import type {FieldRuleKind} from '@/components/block-composer/field-formula' + +export interface FieldRuleDialogProps { + kind: FieldRuleKind + fieldName: string + /** Every field name in the block, including `fieldName`. */ + allFieldNames: readonly string[] + /** The rule as it currently stands; '' when the field has none. */ + initialValue: string + onSave: (rule: string) => void + onClose: () => void +} + +const TITLE: Record = { + value: 'Field formula', + validation: 'Validation rule', +} + +export const FieldRuleDialog = ({ + kind, + fieldName, + allFieldNames, + initialValue, + onSave, + onClose, +}: FieldRuleDialogProps) => { + const [value, setValue] = useState(initialValue) + + // Re-seed when the dialog is pointed at a different field or rule without + // unmounting in between. + useEffect(() => { + setValue(initialValue) + }, [initialValue, kind, fieldName]) + + const error = ruleError(kind, value, fieldName, allFieldNames) + const unchanged = value.trim() === initialValue.trim() + + return ( + + + {TITLE[kind]} + + {fieldName} + + + + + + + + + + + ) +} diff --git a/src/components/block-interface/index.tsx b/src/components/block-interface/index.tsx index d6dcfff6..1ed9acc1 100644 --- a/src/components/block-interface/index.tsx +++ b/src/components/block-interface/index.tsx @@ -9,6 +9,8 @@ import { Add as AddIcon, ArrowUpward as ArrowUpwardIcon, ArrowDownward as ArrowDownwardIcon, + Functions as FunctionsIcon, + RuleOutlined as RuleIcon, } from '@mui/icons-material' import { Grid, @@ -24,7 +26,13 @@ import {MenuComponent} from './menu' import {BlockComposerComponent} from '@/components/block-composer' import {useEngine, useOps, useDataService} from '@/core/engine/provider' import type {FieldInfo} from 'logisheets-engine' -import {BlockCellInfo, BlockDisplayInfo} from 'logisheets-engine' +import { + BlockCellInfo, + BlockDisplayInfo, + BlockSchemaFieldEntry, +} from 'logisheets-engine' +import {FieldRuleDialog} from './field-rule-dialog' +import type {FieldRuleKind} from '@/components/block-composer/field-formula' import {LeftTop} from '@/core/settings' import {BlockCellProps, RenderedCellSpec, buildRenderedCells} from './cell' import {EnumCell} from './enum-cell' @@ -192,6 +200,7 @@ export const BlockInterfaceComponent = (props: BlockInterfaceProps) => { cells={info.cells} grid={grid} title={info.schema.name} + schemaFields={info.schema.fields} /> ) })} @@ -218,6 +227,13 @@ interface BlockInterfaceInternalProps { cells: readonly BlockCellInfo[] grid: Grid title: string + /** + * The block's schema fields IN SCHEMA ORDER, not display order. + * `UpsertFieldFormulas` takes one rule per field in exactly that order, so + * the per-field rule dialog has to rebuild its vector from this list + * rather than from the (idx-sorted) `fieldInfo` above. + */ + schemaFields: readonly BlockSchemaFieldEntry[] } const BlockInterface = observer((props: BlockInterfaceInternalProps) => { @@ -240,6 +256,7 @@ const BlockInterface = observer((props: BlockInterfaceInternalProps) => { canvasStartY, cells, grid, + schemaFields, } = props const ops = useOps() @@ -282,6 +299,42 @@ const BlockInterface = observer((props: BlockInterfaceInternalProps) => { ) } } + + // Which per-field rule the header menu opened, if any. + const [ruleDialog, setRuleDialog] = useState<{ + kind: FieldRuleKind + field: string + } | null>(null) + + const ruleOf = (field: string, kind: FieldRuleKind): string => { + const entry = schemaFields.find((f) => f.field === field) + const raw = + kind === 'value' ? entry?.valueFormula : entry?.validationFormula + return raw ?? '' + } + + const handleSaveRule = async ( + kind: FieldRuleKind, + field: string, + rule: string + ) => { + setRuleDialog(null) + // One entry per field, in the schema's own order — the engine replaces + // the whole vector for this rule kind, so every field that is not the + // one being edited has to carry its rule through unchanged. + const formulas = schemaFields.map((f) => + f.field === field ? rule : ruleOf(f.field, kind) + ) + try { + await ops.setFieldRules({sheetIdx, blockId, kind, formulas}) + } catch (e) { + toast.error( + `Failed to set the ${ + kind === 'value' ? 'field formula' : 'validation rule' + } on "${field}": ${e instanceof Error ? e.message : String(e)}` + ) + } + } const [clickMousePosition, setClickMousePosition] = useState({x: 0, y: 0}) const [descriptorUrl, setDescriptorUrl] = useState() const [error, setError] = useState() @@ -958,8 +1011,58 @@ const BlockInterface = observer((props: BlockInterfaceInternalProps) => { > Sort descending + {/* The two rules a field can carry. Labelled by whether one + is already in force, so the menu says what the column + does without having to open anything. */} + } + onClick={() => { + if (!sortMenu) return + setRuleDialog({ + kind: 'value', + field: sortMenu.field, + }) + setSortMenu(null) + }} + > + {sortMenu && ruleOf(sortMenu.field, 'value') !== '' + ? 'Edit field formula…' + : 'Set field formula…'} + + } + onClick={() => { + if (!sortMenu) return + setRuleDialog({ + kind: 'validation', + field: sortMenu.field, + }) + setSortMenu(null) + }} + > + {sortMenu && ruleOf(sortMenu.field, 'validation') !== '' + ? 'Edit validation rule…' + : 'Set validation rule…'} + + {ruleDialog && ( + f.field)} + initialValue={ruleOf(ruleDialog.field, ruleDialog.kind)} + onSave={(rule) => + handleSaveRule( + ruleDialog.kind, + ruleDialog.field, + rule + ) + } + onClose={() => setRuleDialog(null)} + /> + )} + {/* Add row button (bottom) — hover only, like the settings button. */} {isHover && ( diff --git a/src/components/block-interface/validation-cell.tsx b/src/components/block-interface/validation-cell.tsx index 6d0bd9e1..eea53166 100644 --- a/src/components/block-interface/validation-cell.tsx +++ b/src/components/block-interface/validation-cell.tsx @@ -36,9 +36,18 @@ export const ValidationCell = (props: BlockCellProps) => { const validation = t.validation if (validation !== '' && shadowValue === undefined) { - // We haven't set a shadow cell for calculating the validation. - // The orchestration lives in logisheets-core's WorkbookOps so the - // browser and the Node runtime establish validation identically. + // Legacy path. A rule declared in the SCHEMA + // (`BindFormSchema.validationFormulas`) is installed by the engine on + // every record, so `shadowValue` is already populated and this branch + // never runs — that is now how every producer writes a rule: the + // composer, Watson's block builder and the crafts alike. + // + // It stays for blocks that predate that: a file saved when the rule + // lived only in the host's FieldInfo has nothing in its schema to + // install from, and without this its markers would silently vanish on + // reload. Installing here keeps them, at the cost of a rule the + // `overrideValidation` gate cannot see — such a block enforces + // nothing, it only warns, exactly as it did before. ops.setValidationRule(sheetIdx, rowIdx, colIdx, validation).catch(() => toast.error('Failed to set validation rule') ) diff --git a/src/components/content/edit-bar.tsx b/src/components/content/edit-bar.tsx index 2989dcce..08a5d3a1 100644 --- a/src/components/content/edit-bar.tsx +++ b/src/components/content/edit-bar.tsx @@ -14,7 +14,12 @@ import {isErrorMessage} from 'logisheets-engine' import {TransformOutlined, RuleOutlined} from '@mui/icons-material' import {IconButton, Tooltip} from '@mui/material' import {callerRegistry} from 'logisheets-core' -import {isCellUserEditableSync} from '@/core/permissions/field-editable' +import { + editRefusedMessage, + getCellFieldFormula, + isCellUserEditableSync, +} from '@/core/permissions/field-editable' +import {useToast} from '@/ui/notification/useToast' import { FormulaEditor, FormulaEditorRef, @@ -43,6 +48,7 @@ export const EditBarComponent = observer(function EditBarComponent({ }: EditBarProps) { const engine = useEngine() const dataSvc = engine.getDataService() + const {toast} = useToast() const ops = useOps() // Target the active view: the edit bar reads/writes the view the user // last focused (highlighted), not always the main one. Falls back to the @@ -68,6 +74,24 @@ export const EditBarComponent = observer(function EditBarComponent({ const [sheetName, setSheetName] = useState('') const editorRef = useRef(null) + // The field-formula template governing the selected cell, if any. The + // cell's own formula is this template with `#FIELD` already resolved to + // this row's coordinates — true, but not the thing a person would edit, + // and misleading to offer in an editable box. Show the template instead, + // read-only, and point at where it can actually be changed. + const fieldFormula = useMemo(() => { + const selectedCell = getSelectedCellRange(selectedData) + if (!selectedCell) return undefined + return getCellFieldFormula( + selectedCell.startRow, + selectedCell.startCol, + engine.getGrid() + ) + // `selectedDataContentChanged` is in the deps on purpose: binding or + // clearing a field formula doesn't move the selection, so nothing else + // would trigger a recompute. + }, [selectedData, selectedDataContentChanged, engine]) + // One-call engine binding: getDisplayUnits + the app's function list. const source = useMemo( () => @@ -181,21 +205,15 @@ export const EditBarComponent = observer(function EditBarComponent({ // Mirror the previous behavior: don't fire CellInput if the buffer is // empty (e.g., user opened the bar but typed nothing). if (value !== '') { - // Guard: block cells whose field is declared - // `userEditable: false` are not writable from the formula - // bar either. Engine-side patch would reject anyway; this - // failing fast here avoids a confusing silent commit - // (and keeps the cell's previous value visible without a - // round-trip). + // Guard: a cell computed by its field's formula, or in a field + // declared `userEditable: false`, is not writable from the + // formula bar either. The engine rejects the write anyway; the + // point of failing fast here is that the rejection is otherwise + // invisible — the bar would clear and the old value would come + // back with no explanation. const cell = getFirstCell(selectedData) - if ( - isCellUserEditableSync( - sheetIdx, - cell.y, - cell.x, - engine.getGrid() - ) - ) { + const grid = engine.getGrid() + if (isCellUserEditableSync(sheetIdx, cell.y, cell.x, grid)) { // Await the write, then re-read so the bar shows the committed // value. The selectedDataContentChanged bump can fire before // the input is applied, which would otherwise leave the bar @@ -206,6 +224,8 @@ export const EditBarComponent = observer(function EditBarComponent({ setFormulaText(cellToDisplayText(c)) setRawValue(c.getText()) } + } else { + toast(editRefusedMessage(cell.y, cell.x, grid), {type: 'info'}) } } setIsEditing(false) @@ -268,6 +288,16 @@ export const EditBarComponent = observer(function EditBarComponent({ value={validationText} readOnly /> + ) : showFormula && fieldFormula ? ( + + + ) : showFormula ? (
(null) @@ -74,6 +79,8 @@ export function InlineCellEditor({ setViewSheet, onContentChanged, ops, + grid, + toast, }) latest.current = { sheetName, @@ -82,6 +89,8 @@ export function InlineCellEditor({ setViewSheet, onContentChanged, ops, + grid, + toast, } useEffect(() => { @@ -103,6 +112,11 @@ export function InlineCellEditor({ setViewSheet: (i) => latest.current.setViewSheet(i), coordinator: formulaEditCoordinator, canEdit: (s, r, c, g) => isCellUserEditableSync(s, r, c, g), + onEditRefused: (_s, r, c) => + latest.current.toast( + editRefusedMessage(r, c, latest.current.grid), + {type: 'info'} + ), getHighlightColor: (i) => getHighlightColor(i).css(), onInvalidFormula: () => setInvalidOpen(true), onContentChanged: () => latest.current.onContentChanged?.(), diff --git a/src/core/permissions/field-editable.ts b/src/core/permissions/field-editable.ts index 891bfa47..78bcf909 100644 --- a/src/core/permissions/field-editable.ts +++ b/src/core/permissions/field-editable.ts @@ -12,6 +12,8 @@ * that forget the check, etc.). * * Decision: + * - Block cell whose field carries a value formula → NOT editable. The + * engine owns those cells; see `getCellFieldFormula`. * - Non-block cell → editable (cells outside any block have no field * constraint). * - Block cell with field `userEditable: true` → editable. @@ -28,10 +30,76 @@ * through the async permission patch instead. */ -import type {Grid} from 'logisheets-engine' +import type {BlockDisplayInfo, Grid} from 'logisheets-engine' import {getEngine} from '@/core/engine' import {callerRegistry, isFieldUserEditable} from 'logisheets-core' +/** + * The block whose rectangle covers a sheet-absolute coordinate, or + * `undefined` when the cell sits outside every block. + */ +function blockAt( + grid: Grid | null, + row: number, + col: number +): BlockDisplayInfo | undefined { + return grid?.blockInfos?.find((block: BlockDisplayInfo) => { + const i = block.info + return ( + row >= i.rowStart && + row < i.rowStart + i.rowCnt && + col >= i.colStart && + col < i.colStart + i.colCnt + ) + }) +} + +/** + * The raw field-formula template governing this cell (e.g. + * `=#FIELD("qty")*#FIELD("price")`), or `undefined` when the cell isn't in a + * block or its field is free-form. + * + * Read straight off `grid.blockInfos` — the engine ships each block's schema + * with the display window, so this needs no RPC and, unlike the host-side + * `FieldInfo`, it is present for blocks nobody in this session authored: + * loaded from a file, created by a craft, or written by another client. + */ +export function getCellFieldFormula( + row: number, + col: number, + grid: Grid | null +): string | undefined { + const block = blockAt(grid, row, col) + const schema = block?.info.schema + if (!block || !schema || schema.schemaType === 'random') return undefined + // `idx` is the field's offset along the schema's field axis: columns for + // a row schema (one field per column), rows for a col schema. + const axisIdx = + schema.schemaType === 'row' + ? col - block.info.colStart + : row - block.info.rowStart + const formula = schema.fields.find((f) => f.idx === axisIdx)?.valueFormula + return formula && formula.trim() !== '' ? formula : undefined +} + +/** + * Why a write to this cell was refused, phrased for the person who just tried. + * + * A guard that silently swallows a keystroke is indistinguishable from a + * broken grid, and a computed column is exactly the case where the reason is + * both non-obvious and actionable — the value is editable, just not here. + */ +export function editRefusedMessage( + row: number, + col: number, + grid: Grid | null +): string { + const formula = getCellFieldFormula(row, col, grid) + if (formula) + return `This column is computed by its field formula (${formula}). Change the block’s field formula to edit it.` + return 'This cell is read-only.' +} + /** * Resolve editability for a sheet-absolute (row, col) coordinate. * Returns `true` when the cell is permitted to be edited by the user; @@ -45,6 +113,12 @@ export function isCellUserEditableSync( grid: Grid | null ): boolean { if (!grid?.blockInfos) return true + // A field formula owns its whole column: the engine recomputes those cells + // from the schema and refuses writes to them, so the UI must not offer an + // editor that would silently do nothing. This is checked first because it + // holds for every block the engine knows about, not just ones this session + // registered a FieldInfo for. + if (getCellFieldFormula(row, col, grid) !== undefined) return false for (const block of grid.blockInfos) { const info = block.info if ( diff --git a/src/core/permissions/patch.ts b/src/core/permissions/patch.ts index 8156b68d..68d71ae6 100644 --- a/src/core/permissions/patch.ts +++ b/src/core/permissions/patch.ts @@ -51,6 +51,7 @@ export function blockOpForPayload(type: string): BlockOp | undefined { // could unlock a block simply by asking to. case 'bindFormSchema': case 'bindRandomSchema': + case 'upsertFieldFormulas': case 'upsertFieldRenderInfo': case 'blockLineNameFieldUpdate': case 'setBlockPermissions': @@ -157,6 +158,10 @@ function isBlockPayload(payload: Payload): boolean { 'convertBlock', 'bindFormSchema', 'bindRandomSchema', + // Rewriting a field's value or validation rule changes what the block + // computes and what it accepts — as much a schema change as rebinding + // it, and governed the same way. + 'upsertFieldFormulas', 'upsertFieldRenderInfo', 'blockStyleUpdate', 'blockLineStyleUpdate', @@ -252,6 +257,84 @@ async function validateCellInput( payload: Payload, callerUuid: string, modifyInfoCache: Map +): Promise { + if (!(await mayWriteHere(client, payload, callerUuid, modifyInfoCache))) { + return false + } + // Being allowed to write here is a separate question from being allowed to + // write THIS — a value the field's validation rule rejects. Asked last, so + // a write refused for any other reason never reaches the extra RPC. + return mayWriteAViolatingValue(client, payload, callerUuid) +} + +/** + * Whether the caller may write a value the field's validation rule rejects. + * + * `true` whenever there is nothing to gate: no rule on this field, or a value + * that satisfies it. When the value does violate, the block's + * `overrideValidation` policy decides — left unstated it falls back to the + * block's own policy, which for an ordinary block is `all`, i.e. exactly what + * happened before this gate existed: the value lands and the marker shows. + * + * A craft seeding a row it knows is incomplete is the case this protects: the + * owner keeps writing while a person typing into the same block is held to the + * rule. + */ +async function mayWriteAViolatingValue( + client: WorkbookClient, + payload: Payload, + callerUuid: string +): Promise { + const v = payload.value as { + sheetIdx: number + row: number + col: number + content: string + } + const verdict = await client.checkFieldValidation({ + sheetIdx: v.sheetIdx, + row: v.row, + col: v.col, + proposed: v.content ?? '', + }) + if (isErrorMessage(verdict)) return true + if (!verdict.hasRule || !verdict.violates) return true + + const sheetCellId = await client.getCellId({ + sheetIdx: v.sheetIdx, + rowIdx: v.row, + colIdx: v.col, + }) + if (isErrorMessage(sheetCellId)) return true + if (sheetCellId.cellId.type !== 'blockCell') return true + + const allowed = await mayCallerModify( + client, + v.sheetIdx, + sheetCellId.cellId.value.blockId, + 'overrideValidation', + callerUuid + ) + // `undefined` means the caller could not be identified; don't turn an + // unknown actor into a refusal on the strength of a validation rule. + if (allowed === false) { + // The one refusal in this file worth spelling out: the person can see + // the cell, the value looks reasonable to them, and the rule that + // rejected it lives in the block's schema where they will not think to + // look. Quote it. + toast.error( + `“${v.content}” doesn’t satisfy this field’s validation rule (${verdict.rule}), and this block doesn’t allow overriding it.` + ) + return false + } + return true +} + +async function mayWriteHere( + client: WorkbookClient, + payload: Payload, + callerUuid: string, + modifyInfoCache: Map ): Promise { const v = payload.value as { sheetIdx: number diff --git a/src/core/permissions/use-editable.ts b/src/core/permissions/use-editable.ts index db70eb35..c611ce76 100644 --- a/src/core/permissions/use-editable.ts +++ b/src/core/permissions/use-editable.ts @@ -3,6 +3,9 @@ * * Post-Phase-1+2 architecture: * + * - A field with a value formula is never editable — the engine owns + * those cells and refuses every write to them. This is checked + * first and overrides everything below. * - The static boolean form lives on `FieldInfo.userEditable` * (read synchronously). * - The dynamic formula form lives in the Rust schema @@ -26,6 +29,7 @@ import {useEffect, useState} from 'react' import type {FieldInfo, SheetCellId, Value} from 'logisheets-engine' import {isErrorMessage} from 'logisheets-engine' import {useEngine} from '@/core/engine/provider' +import {getCellFieldFormula} from '@/core/permissions/field-editable' // Module-level cache of last-known shadow values, keyed by // `sheetIdx:row:col`. Updated on every refresh. @@ -59,6 +63,12 @@ export function useEditable( const engine = useEngine() const ue = fieldInfo?.userEditable + // A field formula outranks both the shadow and the static flag: the engine + // computes those cells and drops writes to them, so a widget that let the + // user pick a value would just be lying about what happens next. + const computed = + getCellFieldFormula(rowIdx, colIdx, engine.getGrid()) !== undefined + const cacheKey = `${sheetIdx}:${rowIdx}:${colIdx}` const cached = _shadowValueCache.get(cacheKey) const initial = cached !== undefined ? cached : staticFlag(ue) @@ -118,5 +128,5 @@ export function useEditable( } }, [engine, ue, sheetIdx, rowIdx, colIdx, cacheKey]) - return editable + return computed ? false : editable } diff --git a/tests/block_schema_roundtrip.rs b/tests/block_schema_roundtrip.rs index d5fb7004..195ee116 100644 --- a/tests/block_schema_roundtrip.rs +++ b/tests/block_schema_roundtrip.rs @@ -111,6 +111,9 @@ fn authored() -> Workbook { cell_input: Some(ModifyPolicy::All), sort_by_field: None, modify_description: Some(ModifyPolicy::OwnerOnly), + // Typing is open, but a value the schema says is wrong is + // the craft's call — the pairing the op exists for. + override_validation: Some(ModifyPolicy::OwnerOnly), }), description: Some( "Customer orders, one per row. `total` is qty * price and is \ @@ -580,6 +583,11 @@ fn check(wb: &Workbook, stage: &str) { Some(ModifyPolicy::OwnerOnly), "[{stage}] modifyDescription" ); + assert_eq!( + perms.explicit(BlockOp::OverrideValidation), + Some(ModifyPolicy::OwnerOnly), + "[{stage}] overrideValidation" + ); // ...and that the unstated one still resolves through the default. assert_eq!( perms.policy_for(BlockOp::SortByField, modify.modify_policy), @@ -905,6 +913,80 @@ fn a_second_save_produces_the_same_data_part() { ); } +/// A field formula is LogiSheets' own concept, but the cells it computes have +/// to leave the building as ordinary spreadsheet cells: an `` with a plain +/// A1 expression and a `` holding the last computed value, one per record. +/// Anything else and a workbook opened elsewhere shows a column of blanks (or +/// a column of frozen numbers with no formula behind them), which is exactly +/// the state a person would discover only after sending the file to someone. +#[test] +fn a_field_formula_reaches_the_file_as_a_cell_formula_per_row() { + let wb = authored(); + let xml = part(&wb.save().unwrap(), "xl/worksheets/sheet1.xml"); + + // `total` is column D; the three records are rows 1..3. + for (cell, qty, price) in [("D1", "B1", "C1"), ("D2", "B2", "C2"), ("D3", "B3", "C3")] { + let f = + cell_formula(&xml, cell).unwrap_or_else(|| panic!("no written for {cell}:\n{xml}")); + assert!( + f.contains(qty) && f.contains(price), + "{cell}'s formula should reference this row's qty/price cells ({qty}, {price}) — the template's #FIELD refs are substituted at parse time, so what lands on disk is ordinary A1. Got {f:?}" + ); + assert!( + !f.contains("#FIELD") && !f.contains('#'), + "{cell} must not carry a LogiSheets placeholder into the sheet part — no other reader understands one. Got {f:?}" + ); + } + + // The cached values ship too, so a reader that doesn't recalculate on open + // still shows the right numbers. + for (cell, want) in [("D1", "7"), ("D2", "5"), ("D3", "10")] { + let v = + cell_value(&xml, cell).unwrap_or_else(|| panic!("no written for {cell}:\n{xml}")); + let got: f64 = v.parse().unwrap_or_else(|_| panic!("{cell} value {v:?}")); + let want: f64 = want.parse().unwrap(); + assert!( + (got - want).abs() < 1e-9, + "{cell} should carry its computed value {want}, got {got}" + ); + } +} + +/// The `` element for one cell, as raw XML. +fn cell_element<'a>(xml: &'a str, reference: &str) -> Option<&'a str> { + let needle = format!("r=\"{reference}\""); + let start = xml.find(&needle)?; + let start = xml[..start].rfind("").map(|e| start + e)?; + Some(&xml[start..end]) +} + +fn cell_formula<'a>(xml: &'a str, reference: &str) -> Option<&'a str> { + let c = cell_element(xml, reference)?; + let start = c.find("")? + "".len(); + let end = c[start..].find("").map(|e| start + e)?; + Some(&c[start..end]) +} + +fn cell_value<'a>(xml: &'a str, reference: &str) -> Option<&'a str> { + let c = cell_element(xml, reference)?; + let start = c.find("")? + "".len(); + let end = c[start..].find("").map(|e| start + e)?; + Some(&c[start..end]) +} + +/// Read one part of a saved workbook as text. +fn part(bytes: &[u8], name: &str) -> String { + use std::io::Read; + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes.to_vec())).expect("a zip"); + let mut f = zip + .by_name(name) + .unwrap_or_else(|_| panic!("no {name} in the saved file")); + let mut s = String::new(); + f.read_to_string(&mut s).expect("utf-8 part"); + s +} + fn data_xml(bytes: &[u8]) -> String { use std::io::Read; let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes.to_vec())).expect("a zip"); @@ -1171,6 +1253,7 @@ fn set_description_and_permissions_survive_a_trip() { cell_input: None, sort_by_field: Some(ModifyPolicy::OwnerAndUser), modify_description: Some(ModifyPolicy::OwnerOnly), + override_validation: None, }, // Raising the default at the same time, which is the other // half of that payload.