From 561199664d7ce46c8e5c301caf62b377019cdfb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 20:39:40 +0000 Subject: [PATCH 1/2] Add before-change and record-deleted triggers Before-change triggers run before a create or update is written. Their actions may set fields (new setField step), check conditions, read queries and fail, so a rejection never leaves a partial write. Deleted triggers run after a delete, sync or async. Both fire on action-query writes: deletes and replaces report removed rows, and before-change triggers decide on pending rows through a two-call flow with a fingerprint. Config version 5. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_012iLZgQTvMnEZufP6GdoWy2 --- docs/decisions/action-queries.md | 37 +- docs/decisions/archive-format.md | 1 + docs/decisions/async-trigger-queue.md | 53 ++- golden/crm/app.yaml | 2 +- golden/inventory/app.yaml | 2 +- golden/work-orders/app.yaml | 2 +- golden/work-orders/v2/app.yaml | 2 +- src-tauri/src/access/tests/convert.rs | 6 + src-tauri/src/archive.rs | 2 +- src-tauri/src/automation.rs | 87 +++- src-tauri/src/queries/action.rs | 146 ++++++- src-tauri/src/queries/action_before.rs | 354 +++++++++++++++ src-tauri/src/queries/action_before_tests.rs | 343 +++++++++++++++ src-tauri/src/queries/action_exec.rs | 130 +++++- src-tauri/src/queries/action_tests.rs | 22 +- src-tauri/src/queries/mod.rs | 3 + src-tauri/src/recordstore/commands.rs | 19 +- src/automation/StepEditor.tsx | 13 + src/automation/TriggersPanel.tsx | 81 ++-- src/automation/runner.ts | 36 +- src/automation/steps.ts | 3 + src/automation/triggers.ts | 163 ++++++- src/automation/types.ts | 26 +- src/lib/records.ts | 81 +++- .../before-change-triggers.test.tsx | 231 ++++++++++ tests/integration/shell.test.tsx | 2 +- tests/unit/before-change-triggers.test.ts | 403 ++++++++++++++++++ 27 files changed, 2108 insertions(+), 142 deletions(-) create mode 100644 src-tauri/src/queries/action_before.rs create mode 100644 src-tauri/src/queries/action_before_tests.rs create mode 100644 tests/integration/before-change-triggers.test.tsx create mode 100644 tests/unit/before-change-triggers.test.ts diff --git a/docs/decisions/action-queries.md b/docs/decisions/action-queries.md index cce87347..34017302 100644 --- a/docs/decisions/action-queries.md +++ b/docs/decisions/action-queries.md @@ -79,11 +79,14 @@ foreign keys and `ON DELETE CASCADE` are enforced by SQLite on every platform. PostgreSQL runs every kind directly, `postgres_scanner` handles every type. -**Triggers.** When the target has enabled `created` or `updated` triggers, the -run finds the changed rows: inserts compare key sets before and after, -updates compare every column by key, and `replace` reports every row as -created. `run_action_query` returns the created identities, the updated -identities with their old values, and one sync-trigger grant per event. +**Triggers.** When the target has enabled `created`, `updated` or `deleted` +triggers, the run finds the changed rows: inserts compare key sets before and +after, updates compare every column by key, deletes keep every column of the +rows that are gone, and `replace` reports every old row as deleted and every +new row as created. `run_action_query` returns the created identities, the +updated identities with their old values, the deleted rows with their values, +and one sync-trigger grant per event. Rows an SQL cascade removes from other +tables do not fire those tables' triggers. `src/lib/records.ts` `runActionQuery` calls each record hook once with a single write whose `meta.bulk` carries those rows, and `src/automation/triggers.ts` fires the triggers once per row, in order, as if each row had been written @@ -91,6 +94,23 @@ alone. Each use of a grant extends it (`trigger_auth::verify`), so one grant covers all rows of an event. It is released when they are done. A table with triggers needs a primary key. +**Before-change triggers.** They run in TypeScript, so a run on a table with +enabled before-change triggers (any kind but delete) takes two calls +(`queries::action_before`). The first runs the statement, collects every row +it would create or update (all new values, and the old values of updated +rows), rolls back, and returns them as `pending` with a fingerprint. The +caller runs the triggers on each row; a rejection ends the run there, with +nothing written. The second call carries `before`: the fingerprint and the +fields set per row, by position. It runs the statement again, refuses with +CONFLICT and rolls back if the rows differ, and applies the fields in the +same transaction: an UPDATE by key on PostgreSQL, an UPDATE of the copy +before it is compared for an embedded-SQLite update, and for rows an +embedded-SQLite insert created, a DELETE and INSERT of the row (the +attachment cannot UPDATE a date column). The fingerprint leaves out new rows' +defaulted and identity columns, whose values can change between the calls, +which is also why fields are matched by position. Trigger fields cannot +change a row's key. A dry run skips the triggers. + **Permissions.** A run needs the role's table operations: `create` for insert, `update`, `delete`, and both `delete` and `create` for replace (`trigger_auth::action_query_ops`). The user-mode trigger precheck runs as for @@ -137,6 +157,11 @@ becomes a `runQuery` step. key, inserts with date defaults, dry runs and constraint rollback on a real SQLite file, foreign keys and cascades with the journal mode kept, validation, and (ignored, CI runs it) the PostgreSQL path. +- `src-tauri/src/queries/action_before_tests.rs`: deleted rows of deletes + and replaces, pending rows and applied trigger fields for inserts and + embedded updates (a date column included), refused stale fingerprints, key + columns and unknown rows, and (ignored, CI runs it) the PostgreSQL path + with an identity column. - `src-tauri/src/trigger_auth_tests.rs`: a grant extended by each use. - `src-tauri/src/access/tests/translate.rs` and `convert.rs`: Access action statements translated and run in DuckDB. An imported template's append, @@ -157,3 +182,5 @@ becomes a `runQuery` step. - 2026-10-10: PRD §9.4, §10 and §12.1 now allow the DuckDB write path for user-written action queries only, and require store constraints, including foreign keys, to hold as they do for RecordStore writes. +- 2026-10-10: deleted triggers fire on delete and replace queries, and + before-change triggers run on each pending row through a second call. diff --git a/docs/decisions/archive-format.md b/docs/decisions/archive-format.md index 2ecc8a65..71e2a4c3 100644 --- a/docs/decisions/archive-format.md +++ b/docs/decisions/archive-format.md @@ -229,3 +229,4 @@ checkpoints are validated archive copies under and added the matching `archive_io` and `recovery.rs` tests to Evidence. - 2026-10-05 (after merging #36): re-checked the incremental-save text and evidence #36 added (`archive_io/reuse.rs`, `reuse_tests.rs`, `durability_tests/incremental.rs`, `Snapshot::reuse_from`, `DataStamp`); they match the code. No changes. - 2026-10-09: config version 4 adds `SavedQuery.action` (action queries); version 3 configs upgrade on load. +- 2026-10-10: config version 5 adds the `deleted` and `beforeChange` trigger events and the `setField` step; version 4 configs upgrade on load. diff --git a/docs/decisions/async-trigger-queue.md b/docs/decisions/async-trigger-queue.md index 35efe688..d82a9344 100644 --- a/docs/decisions/async-trigger-queue.md +++ b/docs/decisions/async-trigger-queue.md @@ -1,13 +1,14 @@ # Record triggers and the local async job queue -Status: accepted. Covers PRD §17.3 and the Phase 0 local async trigger queue -spike. +Status: accepted. Covers PRD §17.3 (including the Phase 7 before-change and +record-deleted triggers) and the Phase 0 local async trigger queue spike. ## Context -The MVP supports record-created and record-updated triggers. A trigger runs an -action either inside the initiating workflow or later through a durable local -queue. The queue only runs while the desktop app is open, and it must provide +The MVP supports record-created and record-updated triggers. Phase 7 adds +before-change triggers, which can set fields of a record or reject its save, +and record-deleted triggers. A trigger runs an action either inside the +initiating workflow or later through a durable local queue. The queue only runs while the desktop app is open, and it must provide retries, status, attempt history, cancellation, and idempotency keys. Schedules, webhooks, and always-on workers are deferred. @@ -33,6 +34,38 @@ forms, grids, dashboards, and actions passes through trigger matching in - **Async triggers** enqueue a job and return at once. - Each write carries its trigger depth. A chain deeper than 5 fails, which stops trigger loops. +- **Deleted triggers** (`event: "deleted"`) run after a delete commits, sync or + async. The hook reads the row before the delete (or takes `meta.old`), and + `record` and `old` are that row. Rust issues a grant for sync app-mode + deleted triggers like it does for created and updated ones. + +### Before-change triggers + +A trigger with `event: "beforeChange"` runs before a create or update is sent +to Rust, from the `before` record hook. It is always sync (validation refuses +async). `record` is the record about to be saved: the supplied values for a +create, the stored row merged with the changed values for an update. `old` is +the stored row, or null for a create. Triggers run in order, and each sees the +fields earlier ones set. + +- Its action may only use `setField` (sets a field of the pending record; + later steps see it in `record`), `condition`, `fail`, `runQuery` on a read + query, and `runAction` with the same limits. This follows Access's Before + Change data macros. Validation flags other steps, and the runner fails them. + `setField` outside a before-change trigger fails. +- A failing action rejects the save with `BeforeChangeRejected`: "Not saved:" + and the fail step's message, or the trigger's name and error. Nothing has + been sent, and the action cannot write, so a rejection never leaves a + partial write. In `writeRecordBatch` it aborts the whole batch. +- The fields it sets replace or join the write's values. Rust applies the + same validation and permissions to them as to the caller's own values. +- `runAs` keeps its meaning. App mode skips the role's checks in TypeScript + (reads still go through Rust's role checks) and needs no grant, since the + action writes nothing. User mode checks the role, and Rust refuses an + insert or update up front when the role could not run a user-mode + before-change trigger (`check_user_triggers` with `BeforeChange`). +- Action queries hand each row they would create or update to the triggers + through the `beforeBulk` hook (see [action queries](./action-queries.md)). ### Execution identity @@ -144,6 +177,15 @@ authorized against both the active role and the role it was enqueued under ## Evidence +- `src-tauri/src/automation.rs` tests: before-change triggers limited to + their step kinds and to sync, `setField` refused elsewhere. +- `tests/unit/before-change-triggers.test.ts`: fields set on inserts and + updates, rejections with no write (single and batch), refused steps, + chained triggers, deleted triggers sync and async, and the action query + two-call flow. +- `tests/integration/before-change-triggers.test.tsx`: the same through the + real bridge, including a date set on every row of an embedded-SQLite update + query and a rejection that leaves the table unchanged. - `src-tauri/src/jobs.rs` tests: idempotent enqueue per store, atomic and exclusive claims, exponential backoff until failed, completion with log, cancel and retry, expired leases recovered after restart, `active_lease` @@ -176,3 +218,4 @@ authorized against both the active role and the role it was enqueued under - 2026-10-05: Stated the worker's polling and wake conditions, the role check for user-mode jobs, and silent `STALE_LEASE` handling; added the `active_lease` test to Evidence. +- 2026-10-10: Phase 7 before-change and record-deleted triggers. diff --git a/golden/crm/app.yaml b/golden/crm/app.yaml index 2ad3b8a9..01a089d6 100644 --- a/golden/crm/app.yaml +++ b/golden/crm/app.yaml @@ -2,7 +2,7 @@ # Pure definition data: the same DocumentConfig YAML the Settings › YAML tab edits. # Records come from seed.sql, applied as the "Seed sample data" migration. # Object ids are fixed UUIDv7 literals. A trailing comment names each top-level object. -version: 4 +version: 5 name: CRM activeMode: run settings: diff --git a/golden/inventory/app.yaml b/golden/inventory/app.yaml index 16de249d..ea24bed8 100644 --- a/golden/inventory/app.yaml +++ b/golden/inventory/app.yaml @@ -3,7 +3,7 @@ # {{asset:}} placeholders are replaced with the ids of the template's assets # when a document is created from the template. # Object ids are fixed UUIDv7 literals. A trailing comment names each top-level object. -version: 4 +version: 5 name: Inventory activeMode: run settings: diff --git a/golden/work-orders/app.yaml b/golden/work-orders/app.yaml index e2a98673..50f46a63 100644 --- a/golden/work-orders/app.yaml +++ b/golden/work-orders/app.yaml @@ -2,7 +2,7 @@ # Pure definition data: the same DocumentConfig YAML the Settings › YAML tab edits. # v2/app.yaml is the next version of this application (migration fixture). # Object ids are fixed UUIDv7 literals. A trailing comment names each top-level object. -version: 4 +version: 5 name: Work orders activeMode: run settings: diff --git a/golden/work-orders/v2/app.yaml b/golden/work-orders/v2/app.yaml index cd5e2b4f..3a9b0f4b 100644 --- a/golden/work-orders/v2/app.yaml +++ b/golden/work-orders/v2/app.yaml @@ -4,7 +4,7 @@ # work_orders.due_on, and the definitions that use it. Applying this YAML to a # version 1 document and running pending migrations upgrades it in place. # Object ids are fixed UUIDv7 literals. A trailing comment names each top-level object. -version: 4 +version: 5 name: Work orders activeMode: run settings: diff --git a/src-tauri/src/access/tests/convert.rs b/src-tauri/src/access/tests/convert.rs index bddd5856..1fd56f6d 100644 --- a/src-tauri/src/access/tests/convert.rs +++ b/src-tauri/src/access/tests/convert.rs @@ -541,6 +541,12 @@ fn action_queries_convert_and_run(m: &DocumentManager, config: &crate::archive:: kind, watch: false, dry_run: false, + deleted: false, + before: Default::default(), + sqlite: true, + logical: vec![], + generated: vec![], + defaulted: vec![], }; let Plan::Direct(sql) = plan(q("MakeCustomerTotals"), &make, "main", true) .unwrap() diff --git a/src-tauri/src/archive.rs b/src-tauri/src/archive.rs index 251cae7a..9e5625a2 100644 --- a/src-tauri/src/archive.rs +++ b/src-tauri/src/archive.rs @@ -18,7 +18,7 @@ pub use crate::archive_io::{ read_archive, read_header, write_archive, ArchiveHeader, FORMAT_VERSION, MIN_FORMAT_VERSION, }; /// Current `DocumentConfig.version`. Version 2 configs load through serde defaults. -pub const CONFIG_VERSION: u32 = 4; +pub const CONFIG_VERSION: u32 = 5; #[derive(Debug, thiserror::Error)] pub enum ArchiveError { diff --git a/src-tauri/src/automation.rs b/src-tauri/src/automation.rs index 12e79616..1a96e471 100644 --- a/src-tauri/src/automation.rs +++ b/src-tauri/src/automation.rs @@ -45,12 +45,16 @@ pub struct ActionDef { pub on_error: OnError, } +/// `beforeChange` runs before a create or update is written and may set its +/// fields or reject it; the others run after the write commits. #[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "camelCase")] pub enum TriggerEvent { #[default] Created, Updated, + Deleted, + BeforeChange, } #[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq)] @@ -138,8 +142,14 @@ const STEP_KINDS: &[&str] = &[ "condition", "runAction", "fail", + "setField", ]; +/// The step kinds a before-change trigger's action may use: it computes field +/// values or rejects the save, and never writes records or touches the UI. +pub const BEFORE_CHANGE_STEPS: &[&str] = + &["setField", "condition", "fail", "runQuery", "runAction"]; + fn text<'a>(fields: &'a Map, key: &str) -> &'a str { fields.get(key).and_then(Value::as_str).unwrap_or("") } @@ -311,6 +321,10 @@ fn check_step(config: &DocumentConfig, action: &ActionDef, step: &Step, issues: "confirm" => need(issues, "message", "message expression"), "message" => need(issues, "text", "text expression"), "fail" => need(issues, "message", "message expression"), + "setField" => { + need(issues, "field", "field"); + need(issues, "value", "value expression"); + } "runAction" => { let set = config.actions.iter().map(|a| a.id.as_str()).collect(); missing(issues, "action", text(f, "actionId"), &set); @@ -387,6 +401,27 @@ pub fn validate(config: &DocumentConfig) -> Vec { if t.max_attempts == 0 { err("max attempts must be at least 1".into()); } + let steps = crate::trigger_auth::action_steps(config, &t.action_id); + if t.event == TriggerEvent::BeforeChange { + if t.mode == TriggerMode::Async { + err("a before-change trigger runs before the save, so it cannot be async".into()); + } + for step in &steps { + let action_query = step.kind == "runQuery" + && config.saved_queries.iter().any(|q| { + Some(q.id.as_str()) == step.fields.get("queryId").and_then(Value::as_str) + && q.action.is_some() + }); + if !BEFORE_CHANGE_STEPS.contains(&step.kind.as_str()) || action_query { + err(format!( + "a before-change trigger can only set fields, check conditions, fail, read queries and run actions, not {} steps", + if action_query { "action query" } else { step.kind.as_str() } + )); + } + } + } else if steps.iter().any(|s| s.kind == "setField") { + err("setField steps only run in before-change triggers".into()); + } if t.condition.as_deref().is_some_and(|c| c.trim().is_empty()) { err("condition expression is empty".into()); } @@ -539,6 +574,56 @@ mod tests { } } + #[test] + fn before_change_triggers_only_set_fields_check_and_read() { + let mut c = config( + json!([ + {"id": "stamp", "name": "Stamp", "steps": [ + {"id": "1", "kind": "setField", "field": "status", "value": "upper(record.status)"}, + {"id": "2", "kind": "fail", "message": "'no'", "when": "record.total < 0"}, + {"id": "3", "kind": "runAction", "actionId": "log"} + ]}, + {"id": "log", "name": "Log", "steps": [ + {"id": "4", "kind": "createRecord", "table": "audit", "values": {"m": "'x'"}}, + {"id": "5", "kind": "setField", "field": " ", "value": ""} + ]} + ]), + json!([ + {"id": "b", "name": "B", "table": "orders", "event": "beforeChange", "actionId": "stamp", "mode": "async"}, + {"id": "d", "name": "D", "table": "orders", "event": "deleted", "actionId": "log"} + ]), + ); + let all = messages(&validate(&c)); + for expected in [ + "trigger \"B\": a before-change trigger runs before the save, so it cannot be async", + "trigger \"B\": a before-change trigger can only set fields, check conditions, fail, read queries and run actions, not createRecord steps", + "trigger \"D\": setField steps only run in before-change triggers", + "setField step: field is empty", + "setField step: value expression is empty", + ] { + assert!(all.iter().any(|m| m == expected || m.ends_with(expected)), "missing {expected:?} in {all:#?}"); + } + c.actions[1] + .steps + .retain(|s| s.kind != "createRecord" && s.kind != "setField"); + c.actions[1].steps.push( + serde_json::from_value( + json!({"id": "6", "kind": "condition", "when": "true", "then": [], "else": []}), + ) + .unwrap(), + ); + c.triggers.retain(|t| t.id == "b"); + c.triggers[0].mode = TriggerMode::Sync; + assert!(validate(&c).is_empty(), "{:#?}", validate(&c)); + let t: Trigger = + serde_json::from_value(json!({"id": "t", "name": "T", "event": "beforeChange"})) + .unwrap(); + assert_eq!( + serde_json::to_value(t.event).unwrap(), + json!("beforeChange") + ); + } + #[test] fn detects_run_action_cycles_through_conditions() { let c = config( diff --git a/src-tauri/src/queries/action.rs b/src-tauri/src/queries/action.rs index 40524aae..0ce3587d 100644 --- a/src-tauri/src/queries/action.rs +++ b/src-tauri/src/queries/action.rs @@ -3,17 +3,19 @@ //! the datasource attached read-write (`data::write`), in one transaction. //! //! A run checks the role's table permission and the user-mode trigger -//! precheck, like a record write. When the target table has enabled `created` -//! or `updated` triggers, the run copies the table before the statement and -//! compares it after, by primary key, so the caller can fire those triggers -//! once per changed row. The run returns the created identities, the updated -//! identities with their old values, and one sync-trigger grant per event. +//! precheck, like a record write. When the target table has enabled +//! `created`, `updated` or `deleted` triggers, the run copies the table before +//! the statement and compares it after, by primary key, so the caller can fire +//! those triggers once per changed row. The run returns the created +//! identities, the updated identities with their old values, the deleted rows +//! with their values, and one sync-trigger grant per event. Before-change +//! triggers take a second call (`action_before`). use super::{resolve_params, rewrite_placeholders}; use crate::archive::{ActionKind, ActionSpec, DocumentConfig, SavedQuery}; use crate::automation::TriggerEvent; use crate::data::{self, sqltext, DataValue, NamedValue}; use crate::manager::AppError; -use serde::Serialize; +use serde::{Deserialize, Serialize}; /// Words an action statement may not contain: DDL, catalog, settings and file access. const FORBIDDEN: [&str; 37] = [ @@ -235,13 +237,60 @@ pub fn target<'a>(sql: &'a str, spec: &ActionSpec, schema: &str) -> Result>, pub updated: Vec, + pub deleted: Vec, +} + +#[derive(Debug, Clone, Serialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct DeletedRow { + pub identity: Vec, + /// Every column's value before the statement. + pub values: Vec, +} + +/// A row before-change triggers decide on: its identity after the statement, +/// every column's new value, and the old values of an updated row. +#[derive(Debug, Clone, Serialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct PendingRow { + pub identity: Vec, + pub values: Vec, + pub old: Option>, +} + +/// The rows a run would create or update, for before-change triggers. +#[derive(Debug, Clone, Serialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct Pending { + /// Hash of `rows` (without the defaulted columns of new rows); the second call must reproduce it. + pub fingerprint: String, + pub rows: Vec, +} + +/// What the caller's before-change triggers decided, for the second call. +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct BeforeChange { + pub fingerprint: String, + /// Fields the triggers set, per row. + #[serde(default)] + pub overrides: Vec, +} + +/// Fields set on one pending row, found by its position in `Pending::rows` +/// (a new row's key can differ between the calls, e.g. a PostgreSQL sequence). +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct Override { + pub row: usize, + pub values: Vec, } #[derive(Debug, Clone, Serialize, PartialEq)] @@ -264,17 +313,22 @@ pub struct ActionRun { pub table: String, #[serde(flatten)] pub rows: ChangedRows, - /// Sync app-mode trigger grants for the `created` and `updated` rows. + /// Sync app-mode trigger grants for the `created`, `updated` and `deleted` rows. pub created_grant: Option, pub updated_grant: Option, + pub deleted_grant: Option, + /// Rows awaiting before-change triggers; nothing was written (see `action_before`). + #[serde(skip_serializing_if = "Option::is_none")] + pub pending: Option, } /// The trigger events a kind of action query can raise. fn events(kind: ActionKind) -> &'static [TriggerEvent] { match kind { - ActionKind::Insert | ActionKind::Replace => &[TriggerEvent::Created], + ActionKind::Insert => &[TriggerEvent::Created], + ActionKind::Replace => &[TriggerEvent::Deleted, TriggerEvent::Created], ActionKind::Update => &[TriggerEvent::Updated], - ActionKind::Delete => &[], + ActionKind::Delete => &[TriggerEvent::Deleted], } } @@ -365,12 +419,15 @@ pub fn plan( Ok((plan, rewritten.names)) } -/// Runs an action query. `dry_run` rolls the transaction back and reports the counts. +/// Runs an action query. `dry_run` rolls the transaction back and reports the +/// counts. On a table with before-change triggers, a run without `before` +/// returns the pending rows instead of writing (see `ActionRun::pending`). pub fn run( window: &str, id: &str, supplied: &[NamedValue], dry_run: bool, + before: Option, ) -> Result { let manager = crate::manager()?; let config = manager.config(window)?; @@ -389,6 +446,11 @@ pub fn run( for &event in events(spec.kind) { crate::trigger_auth::precheck(window, &spec.table, event)?; } + let before_change = spec.kind != ActionKind::Delete + && has_triggers(&config, &spec.table, TriggerEvent::BeforeChange); + if before_change { + crate::trigger_auth::precheck(window, &spec.table, TriggerEvent::BeforeChange)?; + } let custom = crate::recordstore::entity_policy(&config, &spec.table) .is_some_and(|e| e.concurrency == "customAction"); if custom && spec.kind != ActionKind::Insert { @@ -403,9 +465,17 @@ pub fn run( let schema = schema_of(&config); let (plan, names) = plan(query, &spec, &schema, sqlite)?; let values = resolve_params(&names, &query.parameters, supplied, true)?; - let watch = events(spec.kind) - .iter() - .any(|&e| has_triggers(&config, &spec.table, e)); + let watch = [TriggerEvent::Created, TriggerEvent::Updated] + .into_iter() + .any(|e| events(spec.kind).contains(&e) && has_triggers(&config, &spec.table, e)); + let deleted = events(spec.kind).contains(&TriggerEvent::Deleted) + && has_triggers(&config, &spec.table, TriggerEvent::Deleted); + let (before, dry_run) = match (before_change, dry_run, before) { + (false, _, _) | (true, true, _) => (super::action_exec::Before::Off, dry_run), + (true, false, None) => (super::action_exec::Before::Collect, true), + (true, false, Some(decided)) => (super::action_exec::Before::Apply(decided), false), + }; + let collecting = matches!(before, super::action_exec::Before::Collect); let (workspace, extension, def) = manager.with_session(window, |s| { let def = s .reader @@ -423,7 +493,20 @@ pub fn run( .iter() .map(|c| (c.name.clone(), c.logical_type.clone())) .collect(); - if watch && keys.is_empty() && !matches!(plan, Plan::Copy(_)) { + let generated = def + .columns + .iter() + .filter(|c| c.generated_expression.is_some()) + .map(|c| c.name.clone()) + .collect(); + let defaulted = def + .columns + .iter() + .filter(|c| c.identity || c.default_expression.is_some()) + .map(|c| c.name.clone()) + .collect(); + let needs_rows = watch || deleted || !matches!(before, super::action_exec::Before::Off); + if needs_rows && keys.is_empty() && !matches!(plan, Plan::Copy(_)) { return Err(invalid(format!( "The table \"{}\" has triggers but no primary key, so an action query cannot tell which rows changed", spec.table @@ -436,7 +519,13 @@ pub fn run( rowid: sqlite && !def.without_rowid, kind: spec.kind, watch, + deleted, + before, dry_run, + sqlite, + logical: logical.clone(), + generated, + defaulted, }; let outcome = crate::recordstore::blocking(|| { let _gate = if sqlite { @@ -479,7 +568,13 @@ pub fn run( rows: outcome.rows, created_grant: None, updated_grant: None, + deleted_grant: None, + pending: outcome.pending, }; + if collecting { + // Nothing was written: the caller runs the before-change triggers first. + run.rows = ChangedRows::default(); + } if !dry_run { crate::recordstore::commands::after_write(window)?; let now = std::time::Instant::now(); @@ -501,6 +596,15 @@ pub fn run( now, ); } + if !run.rows.deleted.is_empty() { + run.deleted_grant = crate::trigger_auth::issue( + window, + &config, + &spec.table, + TriggerEvent::Deleted, + now, + ); + } } Ok(run) } @@ -513,8 +617,18 @@ pub async fn run_action_query( id: String, params: Vec, dry_run: Option, + before: Option, ) -> Result { - super::blocking(move || run(&window_label, &id, ¶ms, dry_run.unwrap_or(false))).await + super::blocking(move || { + run( + &window_label, + &id, + ¶ms, + dry_run.unwrap_or(false), + before, + ) + }) + .await } /// Checks an action query's SQL before it is saved (the guard and placeholders). diff --git a/src-tauri/src/queries/action_before.rs b/src-tauri/src/queries/action_before.rs new file mode 100644 index 00000000..06c1b8b6 --- /dev/null +++ b/src-tauri/src/queries/action_before.rs @@ -0,0 +1,354 @@ +//! Before-change triggers on action queries (`docs/decisions/action-queries.md`). +//! +//! The triggers run in TypeScript, so a run on a table with before-change +//! triggers takes two calls. The first computes the rows the statement would +//! create or update, rolls back, and returns them with a fingerprint. The +//! caller runs the triggers on each row: any rejection ends there, before +//! anything is written. The second call runs the statement again, checks +//! that it changes the same rows to the same values (else CONFLICT, rolled +//! back), and applies the fields the triggers set in the same transaction. +//! +//! Overrides are applied by key: PostgreSQL rows and the embedded-SQLite +//! update copy with an UPDATE, rows an embedded-SQLite insert created by +//! deleting and inserting them again (the attachment cannot UPDATE a date +//! column, see the record). +use super::action::{BeforeChange, Pending, PendingRow, WORK}; +use super::action_exec::{ + any_differs, column_types, list, named, run_statement, same_keys, select_rows, Job, BEFORE, + ROWID, +}; +use crate::archive::ActionKind; +use crate::data::{self, DataValue, NamedValue}; +use crate::manager::AppError; +use duckdb::types::Value as DuckValue; +use sha2::{Digest, Sha256}; + +fn invalid(message: String) -> AppError { + AppError::new("VALIDATION_ERROR", message) +} + +/// Pending rows of a direct run, in key order: every row an insert added (by +/// the key snapshot), every row a replace wrote, or every row an update changed +/// with its old values (by the full snapshot). +pub fn pending_direct(connection: &duckdb::Connection, job: &Job) -> Result { + let table = &job.table; + let order = list(&job.keys, "t"); + let rows = match job.kind { + ActionKind::Update => { + let columns = column_types(connection, BEFORE)?.0; + let sql = format!( + "SELECT {}, {} FROM {table} t JOIN {BEFORE} b ON {} WHERE {} ORDER BY {order}", + list(&columns, "t"), + list(&columns, "b"), + same_keys(&job.keys), + any_differs(&columns, "t", "b") + ); + select_rows(connection, &sql)? + .1 + .into_iter() + .map(|row| { + let (new, old) = row.split_at(columns.len()); + pending_row(job, &columns, new, Some(old)) + }) + .collect() + } + _ => { + let columns = column_types(connection, table)?.0; + let from = match job.kind { + ActionKind::Insert => { + format!("{table} t ANTI JOIN {BEFORE} b ON {}", same_keys(&job.keys)) + } + _ => format!("{table} t"), + }; + let sql = format!( + "SELECT {} FROM {from} ORDER BY {order}", + list(&columns, "t") + ); + select_rows(connection, &sql)? + .1 + .into_iter() + .map(|row| pending_row(job, &columns, &row, None)) + .collect() + } + }; + Ok(seal(job, rows)) +} + +/// Pending rows of an embedded-SQLite UPDATE, from the copy it ran on. A row +/// is found by its new key, or its rowid when the table has no key. +pub fn pending_copy( + connection: &duckdb::Connection, + job: &Job, + columns: &[String], +) -> Result { + let (key, join) = if job.keys.is_empty() { + (format!("t.{ROWID}"), format!("t.{ROWID} = b.rowid")) + } else { + (list(&job.keys, "t"), same_keys(&job.keys)) + }; + let sql = format!( + "SELECT {key}, {}, {} FROM {WORK} t JOIN {} b ON {join} WHERE {} ORDER BY {key}", + list(columns, "t"), + list(columns, "b"), + job.table, + any_differs(columns, "t", "b") + ); + let width = job.keys.len().max(1); + let rows = select_rows(connection, &sql)? + .1 + .into_iter() + .map(|row| { + let (identity, rest) = row.split_at(width); + let (new, old) = rest.split_at(columns.len()); + PendingRow { + identity: identity.to_vec(), + values: named(columns, new), + old: Some(named(columns, old)), + } + }) + .collect(); + Ok(seal(job, rows)) +} + +fn pending_row( + job: &Job, + columns: &[String], + new: &[DataValue], + old: Option<&[DataValue]>, +) -> PendingRow { + let identity = job + .keys + .iter() + .filter_map(|k| columns.iter().position(|c| c == k).map(|i| new[i].clone())) + .collect(); + PendingRow { + identity, + values: named(columns, new), + old: old.map(|o| named(columns, o)), + } +} + +/// The rows with the fingerprint the second call must reproduce. Defaulted +/// columns of new rows (sequences, timestamps) and their identities are left +/// out: they can differ between the calls without the triggers' input changing. +fn seal(job: &Job, rows: Vec) -> Pending { + let stable: Vec<(&[NamedValue], Vec<&NamedValue>)> = rows + .iter() + .map(|r| match &r.old { + Some(old) => (old.as_slice(), r.values.iter().collect()), + None => ( + &[][..], + r.values + .iter() + .filter(|v| !job.defaulted.contains(&v.column)) + .collect(), + ), + }) + .collect(); + let json = serde_json::to_string(&stable).unwrap_or_default(); + Pending { + fingerprint: format!("{:x}", Sha256::digest(json.as_bytes())), + rows, + } +} + +/// Refuses the run when the statement no longer changes the rows the triggers saw. +pub fn verify(before: &BeforeChange, pending: &Pending) -> Result<(), AppError> { + if before.fingerprint == pending.fingerprint { + return Ok(()); + } + Err(AppError::new( + "CONFLICT", + "The rows this action query changes changed while its before-change triggers ran; run it again", + )) +} + +/// One override, checked: the pending row it sets and its normalized values. +struct Change<'a> { + row: &'a PendingRow, + values: Vec, +} + +fn changes<'a>( + job: &Job, + before: &BeforeChange, + pending: &'a Pending, +) -> Result>, AppError> { + let mut out = vec![]; + for o in before.overrides.iter().filter(|o| !o.values.is_empty()) { + let row = pending.rows.get(o.row).ok_or_else(|| { + invalid( + "A before-change trigger set fields of a row this action query does not change" + .into(), + ) + })?; + let mut values = vec![]; + for v in &o.values { + if job.keys.contains(&v.column) { + return Err(invalid(format!( + "A before-change trigger cannot change the key column {} of a row an action query writes", + v.column + ))); + } + if !row.values.iter().any(|c| c.column == v.column) { + return Err(invalid(format!("{} has no column {}", job.name, v.column))); + } + let value = match job.logical.iter().find(|(n, _)| *n == v.column) { + Some((_, l)) => l.normalize(&v.column, &v.value).map_err(invalid)?, + None => v.value.clone(), + }; + values.push(NamedValue { + column: v.column.clone(), + value, + }); + } + // Several overrides of one row merge, later fields winning. + match out + .iter_mut() + .find(|c: &&mut Change| c.row.identity == row.identity) + { + Some(existing) => { + existing + .values + .retain(|v| !values.iter().any(|n| n.column == v.column)); + existing.values.extend(values); + } + None => out.push(Change { row, values }), + } + } + Ok(out) +} + +/// The key columns of a pending row with their values. +fn key_values(job: &Job, row: &PendingRow) -> Vec { + job.keys + .iter() + .zip(&row.identity) + .map(|(k, v)| NamedValue { + column: k.clone(), + value: v.clone(), + }) + .collect() +} + +fn bind(value: &DataValue) -> Result { + data::read::duck_bind(value).map_err(invalid) +} + +/// `"c" = CAST(? AS type)` for each value, and the bound values. +fn casts( + values: &[NamedValue], + types: &(Vec, Vec), + separator: &str, +) -> Result<(String, Vec), AppError> { + let type_of = |c: &str| { + types + .0 + .iter() + .position(|n| n == c) + .map(|i| types.1[i].clone()) + .unwrap_or_else(|| "VARCHAR".into()) + }; + let sql = values + .iter() + .map(|v| format!("{} = CAST(? AS {})", data::q(&v.column), type_of(&v.column))) + .collect::>() + .join(separator); + let bound = values + .iter() + .map(|v| bind(&v.value)) + .collect::>()?; + Ok((sql, bound)) +} + +/// Applies the triggers' fields to the rows a direct run changed, in its transaction. +pub fn apply_direct( + connection: &duckdb::Connection, + job: &Job, + before: &BeforeChange, + pending: &Pending, +) -> Result<(), AppError> { + let types = column_types(connection, &job.table)?; + for change in changes(job, before, pending)? { + let (find, found) = casts(&key_values(job, change.row), &types, " AND ")?; + if job.sqlite { + // The attachment cannot UPDATE a date column: write the row again instead. + run_statement( + connection, + &format!("DELETE FROM {} WHERE {find}", job.table), + &found, + )?; + let row: Vec = change + .row + .values + .iter() + .filter(|v| !job.generated.contains(&v.column)) + .map(|v| { + change + .values + .iter() + .find(|o| o.column == v.column) + .unwrap_or(v) + .clone() + }) + .collect(); + let (_, bound) = casts(&row, &types, ", ")?; + let columns: Vec = row.iter().map(|v| data::q(&v.column)).collect(); + let slots: Vec = row + .iter() + .map(|v| { + let i = types.0.iter().position(|n| *n == v.column); + let ty = i.map(|i| types.1[i].as_str()).unwrap_or("VARCHAR"); + format!("CAST(? AS {ty})") + }) + .collect(); + run_statement( + connection, + &format!( + "INSERT INTO {} ({}) VALUES ({})", + job.table, + columns.join(", "), + slots.join(", ") + ), + &bound, + )?; + } else { + let (set, mut bound) = casts(&change.values, &types, ", ")?; + bound.extend(found); + run_statement( + connection, + &format!("UPDATE {} SET {set} WHERE {find}", job.table), + &bound, + )?; + } + } + Ok(()) +} + +/// Applies the triggers' fields to the copy an embedded-SQLite UPDATE ran on, +/// before it is compared with the live table. +pub fn apply_copy( + connection: &duckdb::Connection, + job: &Job, + before: &BeforeChange, + pending: &Pending, +) -> Result<(), AppError> { + let types = column_types(connection, WORK)?; + for change in changes(job, before, pending)? { + let (set, mut bound) = casts(&change.values, &types, ", ")?; + let find = if job.keys.is_empty() { + bound.push(bind(&change.row.identity[0])?); + format!("{ROWID} = ?") + } else { + let (find, found) = casts(&key_values(job, change.row), &types, " AND ")?; + bound.extend(found); + find + }; + run_statement( + connection, + &format!("UPDATE {WORK} SET {set} WHERE {find}"), + &bound, + )?; + } + Ok(()) +} diff --git a/src-tauri/src/queries/action_before_tests.rs b/src-tauri/src/queries/action_before_tests.rs new file mode 100644 index 00000000..61f9b132 --- /dev/null +++ b/src-tauri/src/queries/action_before_tests.rs @@ -0,0 +1,343 @@ +//! Deleted rows and before-change triggers on action queries, on a real SQLite file. +use super::action::{BeforeChange, Override, Pending}; +use super::action_exec::{compute_update, direct, Before, Job}; +use super::action_tests::{apply, copy_sql, job, rows, workspace, writer}; +use crate::archive::ActionKind; +use crate::data::logical::LogicalType; +use crate::data::{DataValue, NamedValue}; + +fn shipments(dir: &std::path::Path) { + rusqlite::Connection::open(dir.join("data.db")) + .unwrap() + .execute_batch( + "CREATE TABLE shipments(id INTEGER PRIMARY KEY, order_id INTEGER, shipped DATE, note TEXT, + label TEXT GENERATED ALWAYS AS ('#' || id) VIRTUAL); + INSERT INTO shipments (id, order_id, note) VALUES (1, 2, 'old');", + ) + .unwrap(); +} + +fn shipments_job(kind: ActionKind, before: Before) -> Job { + let declared = [ + ("id", "INTEGER"), + ("order_id", "INTEGER"), + ("shipped", "DATE"), + ("note", "TEXT"), + ]; + Job { + table: "data.\"main\".\"shipments\"".into(), + name: "shipments".into(), + before, + logical: declared + .iter() + .map(|(n, t)| (n.to_string(), LogicalType::from_sqlite_declared(t))) + .collect(), + generated: vec!["label".into()], + ..job(kind, false, false) + } +} + +fn set(row: usize, values: &[(&str, DataValue)]) -> Override { + Override { + row, + values: values + .iter() + .map(|(c, v)| NamedValue { + column: c.to_string(), + value: v.clone(), + }) + .collect(), + } +} + +fn shipped(dir: &std::path::Path) -> Vec<(i64, Option, Option)> { + let c = rusqlite::Connection::open(dir.join("data.db")).unwrap(); + let mut stmt = c + .prepare("SELECT id, shipped, note FROM shipments ORDER BY id") + .unwrap(); + stmt.query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?))) + .unwrap() + .map(Result::unwrap) + .collect() +} + +const INSERT: &str = "INSERT INTO data.main.shipments (id, order_id) SELECT id + 100, id FROM data.main.orders WHERE paid = 0"; + +fn collect(dir: &std::path::Path) -> Pending { + let c = writer(dir); + let mut collecting = shipments_job(ActionKind::Insert, Before::Collect); + collecting.dry_run = true; + direct(&c, &[INSERT.into()], &[], &collecting) + .unwrap() + .pending + .unwrap() +} + +#[test] +fn deletes_and_replaces_report_the_rows_they_removed() { + let dir = workspace(); + let c = writer(&dir); + let deleting = Job { + deleted: true, + ..job(ActionKind::Delete, false, false) + }; + let out = direct( + &c, + &["DELETE FROM data.main.orders WHERE paid = 0".into()], + &[], + &deleting, + ) + .unwrap(); + let ids: Vec<_> = out + .rows + .deleted + .iter() + .map(|r| r.identity.clone()) + .collect(); + assert_eq!(ids, [[DataValue::Integer(1)], [DataValue::Integer(3)]]); + let total = out.rows.deleted[1] + .values + .iter() + .find(|v| v.column == "total") + .unwrap(); + assert_eq!(total.value, DataValue::Real(30.0)); + let replace = vec![ + "DELETE FROM data.main.orders".to_string(), + "INSERT INTO data.main.orders BY NAME (SELECT 7 AS id, 1 AS customer)".to_string(), + ]; + let replacing = Job { + kind: ActionKind::Replace, + ..deleting + }; + let out = direct(&c, &replace, &[], &replacing).unwrap(); + assert_eq!(out.rows.deleted.len(), 1); + assert_eq!(out.rows.deleted[0].identity, [DataValue::Integer(2)]); + drop(c); + assert_eq!(rows(&dir, "SELECT id FROM orders"), [[7]]); +} + +#[test] +fn an_insert_collects_pending_rows_then_applies_the_fields_triggers_set() { + let dir = workspace(); + shipments(&dir); + let pending = collect(&dir); + // The first call rolls back and returns every new row in key order. + assert_eq!(shipped(&dir).len(), 1); + let ids: Vec<_> = pending.rows.iter().map(|r| r.identity.clone()).collect(); + assert_eq!(ids, [[DataValue::Integer(101)], [DataValue::Integer(103)]]); + assert!(pending.rows.iter().all(|r| r.old.is_none())); + assert!(pending.rows[0] + .values + .iter() + .any(|v| v.column == "order_id" && v.value == DataValue::Integer(1))); + let decided = BeforeChange { + fingerprint: pending.fingerprint.clone(), + overrides: vec![ + set(0, &[("shipped", DataValue::Text("2024-05-01".into()))]), + set(0, &[("note", DataValue::Text("rush".into()))]), + ], + }; + let c = writer(&dir); + let out = direct( + &c, + &[INSERT.into()], + &[], + &shipments_job(ActionKind::Insert, Before::Apply(decided)), + ) + .unwrap(); + assert_eq!(out.changed, 2); + drop(c); + assert_eq!( + shipped(&dir), + [ + (1, None, Some("old".into())), + (101, Some("2024-05-01".into()), Some("rush".into())), + (103, None, None), + ] + ); +} + +#[test] +fn a_changed_result_keys_and_unknown_rows_are_refused_with_nothing_written() { + let dir = workspace(); + shipments(&dir); + let pending = collect(&dir); + let run = |overrides: Vec, fingerprint: &str| { + let decided = BeforeChange { + fingerprint: fingerprint.into(), + overrides, + }; + let c = writer(&dir); + direct( + &c, + &[INSERT.into()], + &[], + &shipments_job(ActionKind::Insert, Before::Apply(decided)), + ) + .map(|_| ()) + .unwrap_err() + }; + assert_eq!(run(vec![], "stale").code, "CONFLICT"); + let key = run( + vec![set(0, &[("id", DataValue::Integer(5))])], + &pending.fingerprint, + ); + assert!(key.message.contains("key column id"), "{}", key.message); + let other = run( + vec![set(5, &[("note", DataValue::Text("x".into()))])], + &pending.fingerprint, + ); + assert!( + other.message.contains("does not change"), + "{}", + other.message + ); + let bad = run( + vec![set(0, &[("shipped", DataValue::Text("soon".into()))])], + &pending.fingerprint, + ); + assert_eq!(bad.code, "VALIDATION_ERROR"); + assert_eq!(shipped(&dir).len(), 1); +} + +#[test] +fn an_embedded_update_applies_trigger_fields_to_its_copy() { + let dir = workspace(); + shipments(&dir); + rusqlite::Connection::open(dir.join("data.db")) + .unwrap() + .execute_batch("INSERT INTO shipments (id, order_id, note) VALUES (2, 3, 'old')") + .unwrap(); + let sql = copy_sql("UPDATE shipments SET note = 'packed'", "shipments"); + let c = writer(&dir); + let mut collecting = shipments_job(ActionKind::Update, Before::Collect); + collecting.watch = true; + let computed = compute_update(&c, &sql, &[], &collecting).unwrap(); + let pending = computed.outcome.pending.unwrap(); + assert_eq!(pending.rows.len(), 2); + let old = pending.rows[0].old.as_ref().unwrap(); + assert!(old + .iter() + .any(|v| v.column == "note" && v.value == DataValue::Text("old".into()))); + let decided = BeforeChange { + fingerprint: pending.fingerprint, + overrides: vec![set(1, &[("shipped", DataValue::Text("2024-06-02".into()))])], + }; + let mut applying = shipments_job(ActionKind::Update, Before::Apply(decided)); + applying.watch = true; + let computed = compute_update(&c, &sql, &[], &applying).unwrap(); + drop(c); + assert_eq!(computed.outcome.rows.updated.len(), 2); + apply(&dir.join("data.db"), &applying, &computed.changes).unwrap(); + assert_eq!( + shipped(&dir), + [ + (1, None, Some("packed".into())), + (2, Some("2024-06-02".into()), Some("packed".into())), + ] + ); +} + +#[test] +#[ignore = "needs IXTABLE_TEST_POSTGRES_URL; CI runs it with --include-ignored"] +fn postgres_trigger_fields_and_deleted_rows_go_through_the_attachment() { + let Some(url) = crate::test_env::postgres_url() else { + return; + }; + let schema = format!("ixt_{}", uuid::Uuid::new_v4().simple()); + let mut admin = ::postgres::Client::connect(&url, ::postgres::NoTls).unwrap(); + admin + .batch_execute(&format!( + "CREATE SCHEMA {schema}; + CREATE TABLE {schema}.tasks(id BIGINT PRIMARY KEY, done BOOLEAN NOT NULL DEFAULT false, due DATE, + seq BIGINT GENERATED ALWAYS AS IDENTITY); + INSERT INTO {schema}.tasks (id, done) VALUES (1, false), (2, true);" + )) + .unwrap(); + let dir = std::env::temp_dir().join(format!("ixtable-action-pg-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + let extension = crate::data::extensions::sqlite_extension_path().unwrap(); + let target = crate::data::read::ReadTarget::Postgres { + conninfo: url.clone(), + schema: schema.clone(), + }; + let c = crate::data::write::open_writer(&dir, &extension, &target, false).unwrap(); + let tasks = |kind, before| Job { + table: format!("data.\"{schema}\".\"tasks\""), + name: "tasks".into(), + rowid: false, + sqlite: false, + before, + logical: vec![("due".into(), LogicalType::Date)], + generated: vec![], + defaulted: vec!["done".into(), "seq".into()], + ..job(kind, false, false) + }; + let insert = vec!["INSERT INTO tasks (id) VALUES (3), (4)".to_string()]; + let mut collecting = tasks(ActionKind::Insert, Before::Collect); + collecting.dry_run = true; + let pending = direct(&c, &insert, &[], &collecting) + .unwrap() + .pending + .unwrap(); + assert_eq!(pending.rows.len(), 2); + let decided = BeforeChange { + fingerprint: pending.fingerprint, + overrides: vec![set(1, &[("due", DataValue::Text("2024-07-01".into()))])], + }; + direct( + &c, + &insert, + &[], + &tasks(ActionKind::Insert, Before::Apply(decided)), + ) + .unwrap(); + let update = vec!["UPDATE tasks SET done = true WHERE id = 1".to_string()]; + let mut collecting = tasks(ActionKind::Update, Before::Collect); + collecting.dry_run = true; + let pending = direct(&c, &update, &[], &collecting) + .unwrap() + .pending + .unwrap(); + assert!(pending.rows[0].old.is_some()); + let decided = BeforeChange { + fingerprint: pending.fingerprint, + overrides: vec![set(0, &[("due", DataValue::Text("2024-07-02".into()))])], + }; + direct( + &c, + &update, + &[], + &tasks(ActionKind::Update, Before::Apply(decided)), + ) + .unwrap(); + let deleting = Job { + deleted: true, + ..tasks(ActionKind::Delete, Before::Off) + }; + let delete = vec!["DELETE FROM tasks WHERE id = 2".to_string()]; + let out = direct(&c, &delete, &[], &deleting).unwrap(); + assert_eq!(out.rows.deleted.len(), 1); + assert_eq!(out.rows.deleted[0].identity, [DataValue::Integer(2)]); + drop(c); + let due: Vec<(i64, Option)> = admin + .query( + &format!("SELECT id, due::text FROM {schema}.tasks ORDER BY id"), + &[], + ) + .unwrap() + .iter() + .map(|r| (r.get(0), r.get(1))) + .collect(); + assert_eq!( + due, + [ + (1, Some("2024-07-02".into())), + (3, None), + (4, Some("2024-07-01".into())) + ] + ); + admin + .batch_execute(&format!("DROP SCHEMA {schema} CASCADE")) + .unwrap(); +} diff --git a/src-tauri/src/queries/action_exec.rs b/src-tauri/src/queries/action_exec.rs index 9d54feb4..44e988c9 100644 --- a/src-tauri/src/queries/action_exec.rs +++ b/src-tauri/src/queries/action_exec.rs @@ -7,15 +7,16 @@ //! returns the changed rows, joined to the live table by rowid, so even a //! changed primary key is found; it never writes the file. `write_back` then //! writes them in one DuckDB transaction (see `write_back`). -use super::action::{ChangedRows, UpdatedRow, WORK}; +use super::action::{BeforeChange, ChangedRows, DeletedRow, Pending, UpdatedRow, WORK}; +use super::action_before; use crate::archive::ActionKind; use crate::data::logical::LogicalType; use crate::data::{self, DataValue, NamedValue}; use crate::manager::AppError; use duckdb::types::Value as DuckValue; -const BEFORE: &str = "temp.main.__ixtable_before"; -const ROWID: &str = "__ixtable_rowid"; +pub(super) const BEFORE: &str = "temp.main.__ixtable_before"; +pub(super) const ROWID: &str = "__ixtable_rowid"; /// What a run works on. pub struct Job { @@ -28,9 +29,33 @@ pub struct Job { /// Whether the table has a rowid (SQLite tables not declared WITHOUT ROWID). pub rowid: bool, pub kind: ActionKind, - /// Collect the changed rows for triggers. + /// Collect the created and updated rows for triggers. pub watch: bool, + /// Collect the deleted rows, with their values, for `deleted` triggers. + pub deleted: bool, + /// What before-change triggers need from this run. + pub before: Before, pub dry_run: bool, + /// The embedded SQLite file (else PostgreSQL). + pub sqlite: bool, + /// Each column's logical type, for normalizing trigger values. + pub logical: Vec<(String, LogicalType)>, + /// Generated columns, which an insert never names. + pub generated: Vec, + /// Columns with a default or identity: a new row's value can differ between calls. + pub defaulted: Vec, +} + +/// Before-change triggers' part in a run (`action_before`). +#[derive(Debug, Clone, Default)] +pub enum Before { + /// The table has none. + #[default] + Off, + /// Return the pending rows; the run is a dry run. + Collect, + /// Check the pending rows and apply the fields the triggers set. + Apply(BeforeChange), } #[derive(Debug, Default)] @@ -38,6 +63,8 @@ pub struct Outcome { pub changed: u64, pub removed: u64, pub rows: ChangedRows, + /// Rows for before-change triggers (`Before::Collect`). + pub pending: Option, } /// A computed embedded-SQLite UPDATE: the changes `write_back` still has to write. @@ -67,7 +94,7 @@ pub fn database_error(message: &str) -> AppError { AppError::new(code, message.to_string()) } -fn failed(e: duckdb::Error) -> AppError { +pub(super) fn failed(e: duckdb::Error) -> AppError { database_error(&e.to_string()) } @@ -92,7 +119,7 @@ fn in_transaction( Ok(out) } -fn run_statement( +pub(super) fn run_statement( connection: &duckdb::Connection, sql: &str, values: &[DuckValue], @@ -104,7 +131,7 @@ fn run_statement( .map_err(failed)? as u64) } -fn select_rows( +pub(super) fn select_rows( connection: &duckdb::Connection, sql: &str, ) -> Result<(Vec, Vec>), AppError> { @@ -124,7 +151,7 @@ fn select_rows( Ok((columns, rows)) } -fn list(columns: &[String], alias: &str) -> String { +pub(super) fn list(columns: &[String], alias: &str) -> String { columns .iter() .map(|c| format!("{alias}.{}", data::q(c))) @@ -132,14 +159,14 @@ fn list(columns: &[String], alias: &str) -> String { .join(", ") } -fn same_keys(keys: &[String]) -> String { +pub(super) fn same_keys(keys: &[String]) -> String { keys.iter() .map(|k| format!("t.{0} IS NOT DISTINCT FROM b.{0}", data::q(k))) .collect::>() .join(" AND ") } -fn any_differs(columns: &[String], new: &str, old: &str) -> String { +pub(super) fn any_differs(columns: &[String], new: &str, old: &str) -> String { columns .iter() .map(|c| format!("{new}.{0} IS DISTINCT FROM {old}.{0}", data::q(c))) @@ -160,12 +187,15 @@ pub fn direct( ) -> Result { in_transaction(connection, !job.dry_run, || { let table = &job.table; - if job.watch && matches!(job.kind, ActionKind::Insert | ActionKind::Update) { - // Updates compare every column; inserts only need the keys that existed. - let snapshot = match job.kind { - ActionKind::Update => "*".to_string(), - _ => list(&job.keys, "t"), - }; + let before = !matches!(job.before, Before::Off); + // Updates and deletes keep every column; inserts only need the keys that existed. + let snapshot = match job.kind { + ActionKind::Update if job.watch || before => Some("*".to_string()), + ActionKind::Delete | ActionKind::Replace if job.deleted => Some("*".to_string()), + ActionKind::Insert if job.watch || before => Some(list(&job.keys, "t")), + _ => None, + }; + if let Some(snapshot) = snapshot { connection .execute_batch(&format!( "CREATE OR REPLACE TEMP TABLE __ixtable_before AS SELECT {snapshot} FROM {table} t" @@ -181,7 +211,7 @@ pub fn direct( [n] => (0, *n), _ => (0, 0), }; - let rows = match (job.watch, job.kind) { + let mut rows = match (job.watch, job.kind) { (false, _) | (_, ActionKind::Delete) => ChangedRows::default(), (true, ActionKind::Update) => updated_since_snapshot(connection, job)?, (true, ActionKind::Replace) => ChangedRows { @@ -190,7 +220,7 @@ pub fn direct( &format!("SELECT {} FROM {table} t", list(&job.keys, "t")), )? .1, - updated: vec![], + ..Default::default() }, (true, ActionKind::Insert) => ChangedRows { created: select_rows( @@ -202,17 +232,60 @@ pub fn direct( ), )? .1, - updated: vec![], + ..Default::default() }, }; + if job.deleted { + rows.deleted = deleted_since_snapshot(connection, job)?; + } + let pending = match &job.before { + Before::Off => None, + Before::Collect => Some(action_before::pending_direct(connection, job)?), + Before::Apply(decided) => { + let pending = action_before::pending_direct(connection, job)?; + action_before::verify(decided, &pending)?; + action_before::apply_direct(connection, job, decided, &pending)?; + None + } + }; Ok(Outcome { changed, removed, rows, + pending, }) }) } +/// Rows of the snapshot that are gone after a delete (every row, for a replace). +fn deleted_since_snapshot( + connection: &duckdb::Connection, + job: &Job, +) -> Result, AppError> { + let columns = columns_of(connection, BEFORE)?; + let sql = match job.kind { + ActionKind::Replace => format!("SELECT {} FROM {BEFORE} b", list(&columns, "b")), + _ => format!( + "SELECT {} FROM {BEFORE} b ANTI JOIN {} t ON {}", + list(&columns, "b"), + job.table, + same_keys(&job.keys) + ), + }; + Ok(select_rows(connection, &sql)? + .1 + .into_iter() + .map(|row| DeletedRow { + identity: job + .keys + .iter() + .filter_map(|k| columns.iter().position(|c| c == k).map(|i| row[i].clone())) + .collect(), + values: named(&columns, &row), + }) + .collect()) +} + /// Rows with the same key and some different column, with their old values. fn updated_since_snapshot( connection: &duckdb::Connection, @@ -239,12 +312,12 @@ fn updated_since_snapshot( }) .collect(); Ok(ChangedRows { - created: vec![], updated, + ..Default::default() }) } -fn named(columns: &[String], values: &[DataValue]) -> Vec { +pub(super) fn named(columns: &[String], values: &[DataValue]) -> Vec { columns .iter() .zip(values) @@ -287,6 +360,16 @@ pub fn compute_update( )) .map_err(failed)?; let changed = run_statement(connection, sql, values)?; + let pending = match &job.before { + Before::Off => None, + Before::Collect => Some(action_before::pending_copy(connection, job, &columns)?), + Before::Apply(decided) => { + let pending = action_before::pending_copy(connection, job, &columns)?; + action_before::verify(decided, &pending)?; + action_before::apply_copy(connection, job, decided, &pending)?; + None + } + }; // The key that finds the row: its rowid, else its old primary key. let type_of = |c: &str| { columns @@ -363,8 +446,8 @@ pub fn compute_update( }; let rows = if job.watch { ChangedRows { - created: vec![], updated, + ..Default::default() } } else { ChangedRows::default() @@ -374,13 +457,14 @@ pub fn compute_update( changed, removed: 0, rows, + pending, }, changes, }) }) } -fn column_types( +pub(super) fn column_types( connection: &duckdb::Connection, table: &str, ) -> Result<(Vec, Vec), AppError> { diff --git a/src-tauri/src/queries/action_tests.rs b/src-tauri/src/queries/action_tests.rs index 6e1a6631..c22ca53d 100644 --- a/src-tauri/src/queries/action_tests.rs +++ b/src-tauri/src/queries/action_tests.rs @@ -137,7 +137,7 @@ fn embedded_sqlite_updates_run_on_a_copy_under_the_table_name() { } /// A workspace with `data.db` holding customers and orders. -fn workspace() -> std::path::PathBuf { +pub(crate) fn workspace() -> std::path::PathBuf { let dir = std::env::temp_dir().join(format!("ixtable-action-{}", uuid::Uuid::new_v4())); std::fs::create_dir_all(&dir).unwrap(); let c = rusqlite::Connection::open(dir.join("data.db")).unwrap(); @@ -153,12 +153,12 @@ fn workspace() -> std::path::PathBuf { dir } -fn writer(dir: &std::path::Path) -> duckdb::Connection { +pub(crate) fn writer(dir: &std::path::Path) -> duckdb::Connection { let extension = data::extensions::sqlite_extension_path().unwrap(); data::write::open_writer(dir, &extension, &data::read::ReadTarget::Sqlite, false).unwrap() } -fn rows(dir: &std::path::Path, sql: &str) -> Vec> { +pub(crate) fn rows(dir: &std::path::Path, sql: &str) -> Vec> { let c = rusqlite::Connection::open(dir.join("data.db")).unwrap(); let mut stmt = c.prepare(sql).unwrap(); let n = stmt.column_count(); @@ -170,7 +170,7 @@ fn rows(dir: &std::path::Path, sql: &str) -> Vec> { const ORDERS: &str = "data.\"main\".\"orders\""; -fn job(kind: ActionKind, watch: bool, dry_run: bool) -> Job { +pub(crate) fn job(kind: ActionKind, watch: bool, dry_run: bool) -> Job { Job { table: ORDERS.into(), name: "orders".into(), @@ -179,10 +179,16 @@ fn job(kind: ActionKind, watch: bool, dry_run: bool) -> Job { kind, watch, dry_run, + deleted: false, + before: Default::default(), + sqlite: true, + logical: vec![], + generated: vec![], + defaulted: vec![], } } -fn copy_sql(sql: &str, table: &str) -> String { +pub(crate) fn copy_sql(sql: &str, table: &str) -> String { let query = SavedQuery { sql: sql.into(), ..Default::default() @@ -458,6 +464,12 @@ fn postgres_action_queries_write_through_the_attachment() { kind, watch: true, dry_run, + deleted: false, + before: Default::default(), + sqlite: false, + logical: vec![], + generated: vec![], + defaulted: vec![], }; let query = SavedQuery { sql: "UPDATE visits SET day = day + 1, seen = coalesce(seen, TIMESTAMP '2024-03-01 08:00'), paid = true WHERE total < $max".into(), diff --git a/src-tauri/src/queries/mod.rs b/src-tauri/src/queries/mod.rs index 1a3380ee..990d9c2c 100644 --- a/src-tauri/src/queries/mod.rs +++ b/src-tauri/src/queries/mod.rs @@ -189,6 +189,9 @@ pub fn prepare_sql(sql: &str) -> Result { } pub mod action; +pub(crate) mod action_before; +#[cfg(test)] +mod action_before_tests; pub(crate) mod action_exec; #[cfg(test)] pub(crate) mod action_tests; diff --git a/src-tauri/src/recordstore/commands.rs b/src-tauri/src/recordstore/commands.rs index 6e79472d..50ab559e 100644 --- a/src-tauri/src/recordstore/commands.rs +++ b/src-tauri/src/recordstore/commands.rs @@ -342,11 +342,11 @@ fn op_kind(op: &WriteOp) -> crate::authz::Op { WriteOp::Delete { .. } => crate::authz::Op::Delete, } } -fn trigger_event(op: &WriteOp) -> Option { +fn trigger_event(op: &WriteOp) -> crate::automation::TriggerEvent { match op { - WriteOp::Insert { .. } => Some(crate::automation::TriggerEvent::Created), - WriteOp::Update { .. } => Some(crate::automation::TriggerEvent::Updated), - WriteOp::Delete { .. } => None, + WriteOp::Insert { .. } => crate::automation::TriggerEvent::Created, + WriteOp::Update { .. } => crate::automation::TriggerEvent::Updated, + WriteOp::Delete { .. } => crate::automation::TriggerEvent::Deleted, } } @@ -370,8 +370,10 @@ fn authorize_ops( for (i, op) in ops.iter().enumerate() { let auth = triggers.get(i).and_then(Option::as_ref); crate::trigger_auth::authorize(window, op.table(), op_kind(op), auth)?; - if let Some(event) = trigger_event(op) { - crate::trigger_auth::precheck(window, op.table(), event)?; + crate::trigger_auth::precheck(window, op.table(), trigger_event(op))?; + if !matches!(op, WriteOp::Delete { .. }) { + let before = crate::automation::TriggerEvent::BeforeChange; + crate::trigger_auth::precheck(window, op.table(), before)?; } } Ok(()) @@ -388,9 +390,8 @@ fn with_grants( .iter() .zip(outcomes) .map(|(op, outcome)| { - let trigger_grant = trigger_event(op).and_then(|event| { - crate::trigger_auth::issue(window, &config, op.table(), event, now) - }); + let trigger_grant = + crate::trigger_auth::issue(window, &config, op.table(), trigger_event(op), now); TriggeredOutcome { outcome, trigger_grant, diff --git a/src/automation/StepEditor.tsx b/src/automation/StepEditor.tsx index 4041d11a..9b4deb23 100644 --- a/src/automation/StepEditor.tsx +++ b/src/automation/StepEditor.tsx @@ -389,6 +389,19 @@ function StepFields({ {when} ); + case "setField": + return ( +
+ set({ field: v })} /> + set({ value: v })} + /> + {when} +
+ ); case "condition": return ( <> diff --git a/src/automation/TriggersPanel.tsx b/src/automation/TriggersPanel.tsx index 8e2d11d0..80a66fbe 100644 --- a/src/automation/TriggersPanel.tsx +++ b/src/automation/TriggersPanel.tsx @@ -12,6 +12,7 @@ export function TriggersPanel({ focusId }: { focusId?: string }) { const triggers = config.triggers ?? []; const [selected, setSelected] = useState(focusId ?? triggers[0]?.id ?? null); const trigger = triggers.find((t) => t.id === selected) ?? null; + const before = trigger?.event === "beforeChange"; const enabledId = useId(); const tables = objects .filter((o) => o.objectType === "table" && !o.name.startsWith("_ixtable_")) @@ -102,10 +103,14 @@ export function TriggersPanel({ focusId }: { focusId?: string }) { edit({ event })} + onChange={(event) => + edit(event === "beforeChange" ? { event, mode: "sync" } : { event }) + } options={[ + { value: "beforeChange", label: "Before a record is saved" }, { value: "created", label: "Record created" }, { value: "updated", label: "Record updated" }, + { value: "deleted", label: "Record deleted" }, ]} /> -
- edit({ mode })} - options={[ - { value: "sync", label: "Synchronously, inside the write" }, - { value: "async", label: "Asynchronously, on the job queue" }, - ]} - /> - {trigger.mode === "async" && ( - <> - edit({ maxAttempts })} - /> - edit({ backoffMs })} - /> - - )} -
- {trigger.mode === "async" && ( + {before ? ( +

+ Runs before each new or changed record is saved, including rows an action query + writes. Its action can set fields with Set field steps, read queries, and reject the + save with a Fail step; it cannot write records or open anything. In the condition and + action, old is the row before an update and empty for a new record. +

+ ) : ( +
+ edit({ mode })} + options={[ + { value: "sync", label: "Synchronously, inside the write" }, + { value: "async", label: "Asynchronously, on the job queue" }, + ]} + /> + {trigger.mode === "async" && ( + <> + edit({ maxAttempts })} + /> + edit({ backoffMs })} + /> + + )} +
+ )} + {!before && trigger.mode === "async" && ( )}

- Sync triggers run after the record is saved, as part of the same operation; a failing - action reports an error to whoever saved the record, but the save stays. Async triggers - run in the background while the app is open, with retries. Run as app lets the trigger - make its own writes even when the user's role cannot; run as the signed-in user refuses - the save up front when the user's role cannot run the trigger. + Created, updated and deleted triggers see the record as record (a deleted + record as it was). Sync triggers run after the record is saved, as part of the same + operation; a failing action reports an error to whoever saved the record, but the save + stays. Async triggers run in the background while the app is open, with retries. Run as + app lets the trigger make its own writes even when the user's role cannot; run as the + signed-in user refuses the save up front when the user's role cannot run the trigger.

) : ( diff --git a/src/automation/runner.ts b/src/automation/runner.ts index d2a8b0fc..efdc6462 100644 --- a/src/automation/runner.ts +++ b/src/automation/runner.ts @@ -45,7 +45,16 @@ import * as queryApi from "../query/api"; import { ACTION_QUERY_OPS, type ActionSpec } from "../query/types"; import { customActionFor, customScope } from "./custom"; import { afterWrite, currentRow, type FoundRow, matchRows, rowKey, withKeys } from "./rows"; -import type { ActionDef, MatchSpec, OnError, Step, StepLog, ValueMap } from "./types"; +import { STEP_LABELS } from "./steps"; +import { + type ActionDef, + BEFORE_CHANGE_STEPS, + type MatchSpec, + type OnError, + type Step, + type StepLog, + type ValueMap, +} from "./types"; import { rowToObject, toNamedValues } from "./values"; export { ActionPicker } from "./ActionPicker"; @@ -87,6 +96,11 @@ export interface ActionContext { snapshot?: Record; /** Original values a write of this row must match (custom actions: what the caller edited). */ current?: { table: string; identity: DataValue[]; expected: NamedValue[] }; + /** + * Set while a before-change trigger runs: setField steps collect the record's + * new field values in `set`, and steps that write or touch the UI are refused. + */ + beforeChange?: { set: Record }; } export interface ActionResult { @@ -94,6 +108,8 @@ export interface ActionResult { error?: string; /** True when a confirm step was declined. */ cancelled?: boolean; + /** True when a fail step ended the action (`error` is its message). */ + aborted?: boolean; steps: StepLog[]; /** Values stored by steps with `storeAs`. */ results: Record; @@ -342,6 +358,7 @@ function effect(frame: Frame, run: () => void) { async function runStep(step: Step, frame: Frame, path: string): Promise { const { ctx, scope } = frame; + if (ctx.beforeChange) refuseInBeforeChange(step, ctx.config); switch (step.kind) { case "createRecord": { authorize(frame, "table", step.table, "create"); @@ -521,11 +538,28 @@ async function runStep(step: Step, frame: Frame, path: string): Promise { const text = expr(step.message, frame, "Failure message"); throw new Aborted(String(text ?? "") || "The action failed"); } + case "setField": { + if (!ctx.beforeChange) throw new Error("Set field steps only run in before-change triggers"); + if (!step.field?.trim()) throw new Error("Field is not set"); + const value = expr(step.value, frame, `Value for ${step.field}`); + ctx.beforeChange.set[step.field] = value; + scope.record = { ...(scope.record as Record | null), [step.field]: value }; + return; + } default: throw new Error(`Unknown step kind ${(step as { kind: string }).kind}`); } } +/** A before-change trigger only computes fields or rejects: it never writes or touches the UI. */ +function refuseInBeforeChange(step: Step, config: DocumentConfig) { + const actionQuery = + step.kind === "runQuery" && config.savedQueries.some((q) => q.id === step.queryId && q.action); + if (BEFORE_CHANGE_STEPS.includes(step.kind) && !actionQuery) return; + const what = actionQuery ? "Run action query" : STEP_LABELS[step.kind]; + throw new Aborted(`A before-change trigger cannot run "${what}" steps; use an after trigger`); +} + function exists(config: DocumentConfig, kind: string, id: string) { if (!id) throw new Error(`No ${kind} selected`); const list: { id: string }[] | undefined = diff --git a/src/automation/steps.ts b/src/automation/steps.ts index 57770114..35d0aa88 100644 --- a/src/automation/steps.ts +++ b/src/automation/steps.ts @@ -16,6 +16,7 @@ export const STEP_LABELS: Record = { condition: "If / else", runAction: "Run action", fail: "Fail with message", + setField: "Set field (before-change)", }; export function newStep(kind: StepKind): Step { @@ -49,5 +50,7 @@ export function newStep(kind: StepKind): Step { return { id, kind, actionId: "" }; case "fail": return { id, kind, message: "" }; + case "setField": + return { id, kind, field: "", value: "" }; } } diff --git a/src/automation/triggers.ts b/src/automation/triggers.ts index bba4591a..228264af 100644 --- a/src/automation/triggers.ts +++ b/src/automation/triggers.ts @@ -1,7 +1,13 @@ /** * Record triggers (PRD §17.3), hooked into src/lib/records.ts. * - * Sync triggers run their action after the initiating write commits, inside the + * Before-change triggers run before a create or update is sent: their actions + * may set fields of the record (setField) or reject the save (a fail step or + * any failing step), and cannot write records, so a rejection writes nothing. + * Action queries hand them each pending row through `beforeBulk`. + * + * Created, updated and deleted triggers run after the write commits. Sync + * triggers run their action after the initiating write commits, inside the * same workflow: the caller's insertRecord/updateRecord awaits them and a failing * trigger makes that call reject. The write itself is already committed then; * rollback-mode actions only undo their own writes. @@ -18,7 +24,9 @@ import { evaluate, evaluateBoolean } from "../expr"; import { inspectTable, readTablePage } from "../lib/api"; import { type BulkChange, + type PendingRow, type RecordWrite, + type RowOverride, registerRecordHook, type WriteExtra, } from "../lib/records"; @@ -27,7 +35,14 @@ import type { DataValue, DocumentConfig, Filter } from "../lib/types"; import { enqueueJob, notifyJobsChanged, releaseTriggerGrant } from "./api"; import { type ActionContext, runAction } from "./runner"; import type { Trigger, TriggerEvent } from "./types"; -import { fromDataValue, namedToObject, rowToObject, stableHash, toDataValue } from "./values"; +import { + fromDataValue, + namedToObject, + rowToObject, + stableHash, + toDataValue, + toNamedValues, +} from "./values"; export const MAX_TRIGGER_DEPTH = 5; @@ -56,19 +71,36 @@ export interface TriggerEnv { const enabledFor = (config: DocumentConfig, table: string, event: TriggerEvent) => (config.triggers ?? []).filter((t) => t.enabled && t.table === table && t.event === event); +/** Thrown when a before-change trigger rejects a save; nothing was written. */ +export class BeforeChangeRejected extends Error {} + /** * Registers the trigger hook on record writes; returns its unregister function. - * Before an update that has `updated` triggers, it reads the row so `old` is - * available even when the caller did not pass `meta.old`. + * Before an update with `updated` or `beforeChange` triggers, or a delete with + * `deleted` triggers, it reads the row so `old` is available even when the + * caller did not pass `meta.old`. Before-change triggers then decide the write. */ export function installTriggers(env: TriggerEnv): () => void { const previous = new WeakMap | null>(); return registerRecordHook({ before: async (write) => { - if (write.operation !== "update" || write.meta?.old || !write.identity) return; - if (!enabledFor(env.getConfig(), write.table, "updated").length) return; - previous.set(write, await readRow(write.table, write.identity)); + const config = env.getConfig(); + const has = (event: TriggerEvent) => enabledFor(config, write.table, event).length > 0; + const needsOld = + write.operation === "update" + ? has("updated") || has("beforeChange") + : write.operation === "delete" && has("deleted"); + const old = + write.meta?.old ?? + (needsOld && write.identity ? await readRow(write.table, write.identity) : null); + const next = + write.operation === "delete" || !has("beforeChange") + ? write + : await beforeChange(write, old, env); + if (needsOld && !write.meta?.old) previous.set(next, old); + return next; }, + beforeBulk: (table, rows, depth) => beforeChangeBulk(table, rows, depth, env), after: (write, result, extra) => write.meta?.bulk ? dispatchBulk(write, write.meta.bulk, env) @@ -108,7 +140,88 @@ export async function dispatchTriggers( } /** - * Fires an action query's triggers once per created or updated row, in order, + * Runs the before-change triggers of a create or update: returns the write with + * the fields they set, or throws BeforeChangeRejected. + */ +async function beforeChange( + write: RecordWrite, + old: Record | null, + env: TriggerEnv, +): Promise { + const values = namedToObject(write.values); + const record = write.operation === "update" ? { ...old, ...values } : values; + const isNew = write.operation === "insert"; + const set = await decide(write.table, record, isNew ? null : old, write.meta?.triggerDepth, env); + if (!Object.keys(set).length) return write; + return { + ...write, + values: [...write.values.filter((v) => !(v.column in set)), ...toNamedValues(set)], + }; +} + +/** Runs the before-change triggers on each row an action query is about to write. */ +async function beforeChangeBulk( + table: string, + rows: PendingRow[], + depth: number, + env: TriggerEnv, +): Promise { + if (!enabledFor(env.getConfig(), table, "beforeChange").length) return []; + const overrides: RowOverride[] = []; + for (const [i, row] of rows.entries()) { + const old = row.old ? namedToObject(row.old) : null; + const set = await decide(table, namedToObject(row.values), old, depth, env); + if (Object.keys(set).length) overrides.push({ row: i, values: toNamedValues(set) }); + } + return overrides; +} + +/** + * Runs a table's before-change triggers in order on `record` (each sees the + * fields earlier ones set) and returns the fields they set. A failing action + * rejects: a fail step with its own message, anything else naming the trigger. + */ +async function decide( + table: string, + record: Record, + old: Record | null, + triggerDepth: number | undefined, + env: TriggerEnv, +): Promise> { + const set: Record = {}; + const app = env.app?.() ?? {}; + for (const trigger of enabledFor(env.getConfig(), table, "beforeChange")) { + const current = { ...record, ...set }; + if ( + trigger.condition?.trim() && + !evaluateBoolean(trigger.condition, { record: current, old, app }) + ) + continue; + const outcome = await runAction( + trigger.actionId, + env.context({ + record: current, + old: old ?? undefined, + triggerDepth: triggerDepth ?? 0, + beforeChange: { set }, + // App mode skips the role's checks; it holds no grant, since the action cannot write. + ...(trigger.runAs !== "user" && { triggerAuth: { triggerId: trigger.id, grant: "" } }), + }), + ); + if (!outcome.ok) + throw new BeforeChangeRejected( + `Not saved: ${ + outcome.aborted + ? outcome.error + : `trigger "${trigger.name}" failed: ${outcome.error ?? "unknown error"}` + }`, + ); + } + return set; +} + +/** + * Fires an action query's triggers once per created, updated or deleted row, in order, * as if each row had been written alone. The rows of one event share Rust's * grant, released when all of them have run. Failures are collected so one * bad row does not stop the others' triggers. @@ -148,8 +261,24 @@ async function dispatchBulk(write: RecordWrite, bulk: BulkChange, env: TriggerEn 1, bulk.updatedGrant ?? undefined, ); + for (const [i, row] of (bulk.deleted ?? []).entries()) + await each( + { + operation: "delete", + table: write.table, + values: [], + identity: row.identity, + meta: { + ...base, + writeId: `${write.meta?.writeId}:d${i}`, + old: namedToObject(row.values), + }, + }, + 1, + bulk.deletedGrant ?? undefined, + ); } finally { - for (const grant of [bulk.createdGrant, bulk.updatedGrant]) + for (const grant of [bulk.createdGrant, bulk.updatedGrant, bulk.deletedGrant]) if (grant) await releaseTriggerGrant(grant).catch(() => undefined); } if (failures.length) throw new Error(failures.join("; ")); @@ -162,8 +291,8 @@ async function dispatch( oldRow: Record | undefined, grant: string | undefined, ): Promise { - if (write.operation === "delete") return; - const event: TriggerEvent = write.operation === "insert" ? "created" : "updated"; + const event: TriggerEvent = + write.operation === "insert" ? "created" : write.operation === "update" ? "updated" : "deleted"; const triggers = enabledFor(env.getConfig(), write.table, event); if (!triggers.length) return; const depth = (write.meta?.triggerDepth ?? 0) + 1; @@ -174,11 +303,15 @@ async function dispatch( const identity = (write.operation === "insert" ? (result as DataValue[] | null) : write.identity) ?? []; const values = namedToObject(write.values); - const record = (await readRow(write.table, identity)) ?? { - ...values, - ...(identity.length === 1 && { rowid: fromDataValue(identity[0]) }), - }; const old = write.meta?.old ?? oldRow ?? null; + // A deleted row is gone: its triggers see the values it had. + const record = + event === "deleted" + ? (old ?? {}) + : ((await readRow(write.table, identity)) ?? { + ...values, + ...(identity.length === 1 && { rowid: fromDataValue(identity[0]) }), + }); const app = env.app?.() ?? {}; for (const trigger of triggers) { const scope = { record, old, app }; diff --git a/src/automation/types.ts b/src/automation/types.ts index 77ebac39..e12d0a7b 100644 --- a/src/automation/types.ts +++ b/src/automation/types.ts @@ -33,7 +33,9 @@ export type StepBody = | { kind: "condition"; then: Step[]; else: Step[] } | { kind: "runAction"; actionId: string } /** Business-rule abort: ends the action with ok: false and this message (nothing commits under rollback). */ - | { kind: "fail"; message: Expr }; + | { kind: "fail"; message: Expr } + /** Before-change triggers only: sets a field of the record about to be saved. */ + | { kind: "setField"; field: string; value: Expr }; export type StepKind = StepBody["kind"]; @@ -55,14 +57,31 @@ export interface ActionDef { onError: OnError; } -export type TriggerEvent = "created" | "updated"; +/** + * created/updated/deleted run after the write commits. beforeChange runs before a + * create or update is written (always sync): its action may set fields of the + * record (setField) or reject the save (fail, or any failing step). + */ +export type TriggerEvent = "created" | "updated" | "deleted" | "beforeChange"; + +/** The step kinds a before-change trigger's action may use (mirrors automation.rs). */ +export const BEFORE_CHANGE_STEPS: readonly StepKind[] = [ + "setField", + "condition", + "fail", + "runQuery", + "runAction", +]; export interface Trigger { id: string; name: string; table: string; event: TriggerEvent; - /** Optional expression over `record` (and `old` for updates); false/null skips the trigger. */ + /** + * Optional expression over `record` (and `old`: the row before an update, the + * deleted row, null on a before-change create); false/null skips the trigger. + */ condition?: Expr; actionId: string; /** sync: runs inside the initiating write workflow. async: enqueued on the durable local job queue. */ @@ -138,4 +157,5 @@ export const STEP_KINDS: StepKind[] = [ "condition", "runAction", "fail", + "setField", ]; diff --git a/src/lib/records.ts b/src/lib/records.ts index ec31953d..98d3ea7c 100644 --- a/src/lib/records.ts +++ b/src/lib/records.ts @@ -36,13 +36,31 @@ export interface RecordWriteMeta { bulk?: BulkChange; } -/** The rows an action query created or updated, for firing triggers once per row. */ +/** The rows an action query created, updated or deleted, for firing triggers once per row. */ export interface BulkChange { created: DataValue[][]; updated: { identity: DataValue[]; old: NamedValue[] }[]; + /** Deleted rows with every column's value before the statement. */ + deleted?: { identity: DataValue[]; values: NamedValue[] }[]; /** Sync app-mode trigger grants Rust issued for each event; shared by its rows. */ createdGrant?: string | null; updatedGrant?: string | null; + deletedGrant?: string | null; +} + +/** A row an action query is about to create or update, for before-change triggers. */ +export interface PendingRow { + identity: DataValue[]; + /** Every column's new value. */ + values: NamedValue[]; + /** Every column's value before an update; null for a created row. */ + old: NamedValue[] | null; +} + +/** Fields before-change triggers set on one pending row, by its position in `pending.rows`. */ +export interface RowOverride { + row: number; + values: NamedValue[]; } /** What `run_action_query` returns (src-tauri/src/queries/action.rs). */ @@ -53,6 +71,8 @@ export interface ActionQueryRun extends BulkChange { removed: number; dryRun: boolean; table: string; + /** Rows awaiting before-change triggers: nothing was written yet (see `runActionQuery`). */ + pending?: { fingerprint: string; rows: PendingRow[] }; } /** What an app-mode trigger presents to Rust: its sync grant, or its job lease. */ @@ -99,8 +119,10 @@ export function setRecordRouter(next: RecordRouter): () => void { } export interface RecordHook { - // Runs before the write; throwing aborts it. - before?: (write: RecordWrite) => void | Promise; + // Runs before the write; throwing aborts it, returning a write replaces it (before-change triggers). + before?: (write: RecordWrite) => void | RecordWrite | Promise; + // Runs before an action query writes its rows (before-change triggers); throwing aborts the query. + beforeBulk?: (table: string, rows: PendingRow[], triggerDepth: number) => Promise; // Runs after the write commits, with the command result. after?: (write: RecordWrite, result: unknown, extra?: WriteExtra) => void | Promise; } @@ -184,20 +206,28 @@ async function write( if (routed) return (await routed) as T; } const active = [...hooks]; - for (const hook of active) await hook.before?.(record); - const outcome = await send(record); + const final = await beforeAll(record, active); + const outcome = await send(final); announceChange(); - const [result] = await afterCommit([record], [outcome], active); + const [result] = await afterCommit([final], [outcome], active); return result as T; } +/** Runs the before hooks in turn; each may replace the write. */ +async function beforeAll(record: RecordWrite, active: RecordHook[]): Promise { + let current = record; + for (const hook of active) current = (await hook.before?.(current)) ?? current; + return current; +} + const trigger = (record: RecordWrite) => record.meta?.trigger ?? null; /** Inserts a row and resolves with the new row's identity values. */ export const insertRecord = (table: string, values: NamedValue[], meta?: RecordWriteMeta) => write( { operation: "insert", table, values, identity: null, ...(meta && { meta }) }, - (record) => call("insert_row", { table, values, trigger: trigger(record) }), + (record) => + call("insert_row", { table, values: record.values, trigger: trigger(record) }), ); /** @@ -213,7 +243,7 @@ export const updateRecord = ( write({ operation: "update", table, values, identity, ...(meta && { meta }) }, (record) => call("update_row", { table, - values, + values: record.values, identity, expected: meta?.expected ?? null, trigger: trigger(record), @@ -242,9 +272,9 @@ export const deleteRecord = (table: string, identity: DataValue[], meta?: Record * actions here; the action runner routes them before batching. */ export async function writeRecordBatch(input: RecordWrite[]): Promise { - const writes = input.map(withWriteId); const active = [...hooks]; - for (const record of writes) for (const hook of active) await hook.before?.(record); + const writes: RecordWrite[] = []; + for (const record of input.map(withWriteId)) writes.push(await beforeAll(record, active)); const outcomes = await call("execute_write_batch", { ops: writes.map(toOp), triggers: writes.map((w) => w.meta?.trigger ?? null), @@ -255,22 +285,33 @@ export async function writeRecordBatch(input: RecordWrite[]): Promise /** * Runs a saved action query (docs/decisions/action-queries.md). With `dryRun` - * the changes roll back and only the counts come back. Otherwise every record - * hook runs once with a single write that stands for the whole query (its - * `meta.bulk` lists the created and updated rows); the trigger hook fires the - * table's triggers once per row. A failing trigger rejects with a - * CommittedWriteError: the query's changes are saved. + * the changes roll back and only the counts come back. On a table with + * before-change triggers the first call writes nothing and returns the pending + * rows: the `beforeBulk` hooks decide on them (a rejection ends the run with + * nothing written), and a second call writes the query with the fields they + * set. Then every record hook runs once with a single write that stands for + * the whole query (its `meta.bulk` lists the created, updated and deleted + * rows); the trigger hook fires the table's triggers once per row. A failing + * trigger rejects with a CommittedWriteError: the query's changes are saved. */ export async function runActionQuery( queryId: string, params: NamedValue[], options: { dryRun?: boolean; triggerDepth?: number } = {}, ): Promise { - const run = await call("run_action_query", { - id: queryId, - params, - dryRun: options.dryRun ?? false, - }); + const request = { id: queryId, params, dryRun: options.dryRun ?? false }; + let run = await call("run_action_query", request); + if (run.pending) { + const active = [...hooks]; + const overrides: RowOverride[] = []; + for (const hook of active) + overrides.push( + ...((await hook.beforeBulk?.(run.table, run.pending.rows, options.triggerDepth ?? 0)) ?? + []), + ); + const before = { fingerprint: run.pending.fingerprint, overrides }; + run = await call("run_action_query", { ...request, before }); + } if (run.dryRun) return run; announceChange(); const operation: RecordOperation = run.updated.length ? "update" : "insert"; diff --git a/tests/integration/before-change-triggers.test.tsx b/tests/integration/before-change-triggers.test.tsx new file mode 100644 index 00000000..0f18ae34 --- /dev/null +++ b/tests/integration/before-change-triggers.test.tsx @@ -0,0 +1,231 @@ +import { screen } from "@testing-library/react"; +import { invoke } from "@tauri-apps/api/core"; +import { expect, it } from "vitest"; +import { deleteRecord, insertRecord, runActionQuery } from "../../src/lib/records"; +import { createTable, insertRow, readPage, renderNewDocument, value } from "./helpers"; + +const LONG = { timeout: 20_000 }; +type User = Awaited>; +const text = (v: string) => value("text", v); +const today = () => new Date().toISOString().slice(0, 10); + +async function setup() { + await createTable("orders", [ + { name: "id", declaredType: "INTEGER", primaryKeyPosition: 1 }, + { name: "total", declaredType: "REAL" }, + { name: "due", declaredType: "DATE" }, + { name: "status", declaredType: "TEXT" }, + ]); + await createTable("audit", [ + { name: "id", declaredType: "INTEGER", primaryKeyPosition: 1 }, + { name: "message", declaredType: "TEXT" }, + ]); + for (const [id, total] of [ + [1, 10], + [2, 20], + [3, 30], + ] as const) + await insertRow("orders", [ + { column: "id", value: value("integer", id) }, + { column: "total", value: value("real", total) }, + { column: "due", value: { type: "date", value: "2024-01-31" } }, + { column: "status", value: text("open") }, + ]); +} + +// Applies config changes the way the other integration tests do, then reloads the Entities tab. +async function editConfig(user: User, extra: (config: Record) => object) { + await user.click(screen.getByRole("button", { name: "Settings" })); + await user.click(await screen.findByRole("tab", { name: "Entities" }, LONG)); + await screen.findByRole("combobox", { name: "Concurrency policy for orders" }, LONG); + const config = await invoke>("read_document_config", { + windowLabel: "main", + }); + await invoke("update_document_config", { + windowLabel: "main", + config: { ...config, ...extra(config) }, + }); + await user.click(screen.getByRole("tab", { name: "Datasource" })); + await user.click(screen.getByRole("tab", { name: "Entities" })); + await screen.findByRole("heading", { name: "Entities" }, LONG); +} + +const action = (id: string, steps: Array>) => ({ + id, + name: id, + onError: "stop", + steps: steps.map((s, i) => ({ id: `${id}-${i}`, ...s })), +}); +const trigger = (id: string, event: string, actionId: string, extra: object = {}) => ({ + id, + name: id, + table: "orders", + event, + actionId, + mode: "sync", + enabled: true, + maxAttempts: 1, + backoffMs: 0, + ...extra, +}); +const column = async (table: string, name: string) => { + const page = await readPage(table); + const index = page.columns.findIndex((c) => c.name === name); + return page.rows.map((r) => r[index]); +}; +const auditMessages = async () => (await column("audit", "message")).map((v) => v.value).sort(); + +it("stores a field set by a before-change trigger on insert and rejects without a row", async () => { + const user = await renderNewDocument(); + await setup(); + await editConfig(user, () => ({ + actions: [ + action("a-stamp", [ + { kind: "setField", field: "status", value: "upper(record.status)" }, + { kind: "fail", message: "'Total must be positive'", when: "record.total <= 0" }, + ]), + ], + triggers: [trigger("t-before", "beforeChange", "a-stamp")], + })); + await insertRecord("orders", [ + { column: "id", value: value("integer", 4) }, + { column: "total", value: value("real", 5) }, + { column: "status", value: text("new") }, + ]); + expect((await column("orders", "status"))[3]).toEqual(text("NEW")); + + await expect( + insertRecord("orders", [ + { column: "id", value: value("integer", 5) }, + { column: "total", value: value("real", -1) }, + { column: "status", value: text("bad") }, + ]), + ).rejects.toThrow("Not saved: Total must be positive"); + expect((await readPage("orders")).total).toBe(4); +}); + +it("writes an audit row from a deleted trigger with the deleted row's values", async () => { + const user = await renderNewDocument(); + await setup(); + await editConfig(user, () => ({ + actions: [ + action("a-audit", [ + { + kind: "createRecord", + table: "audit", + values: { message: "'deleted ' & record.id & ' ' & record.status & ' ' & old.total" }, + }, + ]), + ], + triggers: [trigger("t-del", "deleted", "a-audit")], + })); + await deleteRecord("orders", [value("integer", 2)], { + expected: [ + { column: "id", value: value("integer", 2) }, + { column: "total", value: value("real", 20) }, + { column: "due", value: { type: "date", value: "2024-01-31" } }, + { column: "status", value: text("open") }, + ], + }); + expect((await readPage("orders")).total).toBe(2); + expect(await auditMessages()).toEqual(["deleted 2 open 20"]); +}); + +it("applies before-change fields to every row of an update action query, or rejects it", async () => { + const user = await renderNewDocument(); + await setup(); + await editConfig(user, () => ({ + savedQueries: [ + { + id: "q-close", + name: "Close orders", + sql: "UPDATE orders SET status = 'closed' WHERE total < $max", + parameters: [{ name: "max", logicalType: "number" }], + action: { kind: "update", table: "orders" }, + }, + ], + actions: [ + action("a-date", [ + { kind: "setField", field: "due", value: "today()" }, + { kind: "fail", message: "'Orders over 25 stay open'", when: "record.total > 25" }, + ]), + ], + triggers: [trigger("t-before", "beforeChange", "a-date")], + })); + const before = await readPage("orders"); + + // Order 3 is rejected, so the whole table stays as it was. + await expect( + runActionQuery("q-close", [{ column: "max", value: value("integer", 100) }]), + ).rejects.toThrow("Not saved: Orders over 25 stay open"); + expect((await readPage("orders")).rows).toEqual(before.rows); + + const run = await runActionQuery("q-close", [{ column: "max", value: value("integer", 25) }]); + expect([run.dryRun, run.changed]).toEqual([false, 2]); + const page = await readPage("orders"); + expect(page.rows.map((r) => [r[2], r[3]])).toEqual([ + [{ type: "date", value: today() }, text("closed")], + [{ type: "date", value: today() }, text("closed")], + [{ type: "date", value: "2024-01-31" }, text("open")], + ]); +}, 60_000); + +it("applies a before-change field to rows an insert action query creates", async () => { + const user = await renderNewDocument(); + await setup(); + await editConfig(user, () => ({ + savedQueries: [ + { + id: "q-copy", + name: "Copy orders", + sql: "INSERT INTO orders (id, total, status) SELECT id + 100, total, 'copy' FROM orders", + parameters: [], + action: { kind: "insert", table: "orders" }, + }, + ], + actions: [ + action("a-set", [{ kind: "setField", field: "status", value: "record.status & '!'" }]), + ], + triggers: [trigger("t-before", "beforeChange", "a-set")], + })); + const run = await runActionQuery("q-copy", []); + // Only created/updated/deleted triggers make Rust list the created rows. + expect(run.changed).toBe(3); + const page = await readPage("orders"); + expect((await column("orders", "status")).slice(3)).toEqual([ + text("copy!"), + text("copy!"), + text("copy!"), + ]); + expect(page.total).toBe(6); +}, 60_000); + +it("fires deleted triggers once per row of a delete action query", async () => { + const user = await renderNewDocument(); + await setup(); + await editConfig(user, () => ({ + savedQueries: [ + { + id: "q-purge", + name: "Purge small orders", + sql: "DELETE FROM orders WHERE total <= 20", + parameters: [], + action: { kind: "delete", table: "orders" }, + }, + ], + actions: [ + action("a-audit", [ + { + kind: "createRecord", + table: "audit", + values: { message: "'purged ' & record.id & ' total ' & old.total" }, + }, + ]), + ], + triggers: [trigger("t-del", "deleted", "a-audit")], + })); + const run = await runActionQuery("q-purge", []); + expect(run.deleted?.length).toBe(2); + expect((await readPage("orders")).total).toBe(1); + expect(await auditMessages()).toEqual(["purged 1 total 10", "purged 2 total 20"]); +}, 60_000); diff --git a/tests/integration/shell.test.tsx b/tests/integration/shell.test.tsx index 374d32e5..c7a3f102 100644 --- a/tests/integration/shell.test.tsx +++ b/tests/integration/shell.test.tsx @@ -106,7 +106,7 @@ it("edits config YAML, reports YAML errors, and undoes an applied YAML change", expect(await screen.findByText("YAML applied.", {}, LONG)).toBeInTheDocument(); expect(screen.queryByRole("alert")).not.toBeInTheDocument(); expect(await screen.findByText("Inventory")).toBeInTheDocument(); - await waitFor(() => expect(editor).toHaveDisplayValue(/version: 4/)); + await waitFor(() => expect(editor).toHaveDisplayValue(/version: 5/)); await user.click(screen.getByRole("button", { name: "Undo" })); await waitFor(async () => expect((await readConfig()).name).toBe("Untitled"), LONG); diff --git a/tests/unit/before-change-triggers.test.ts b/tests/unit/before-change-triggers.test.ts new file mode 100644 index 00000000..599474fd --- /dev/null +++ b/tests/unit/before-change-triggers.test.ts @@ -0,0 +1,403 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { DocumentConfig } from "../../src/lib/types"; +import { createFakeBackend } from "./automation-fakes"; + +const state = vi.hoisted(() => ({ + backend: null as null | { invoke: (c: string, a?: Record) => Promise }, +})); +vi.mock("@tauri-apps/api/core", () => ({ + invoke: (command: string, args?: Record) => state.backend?.invoke(command, args), +})); + +const { installTriggers, BeforeChangeRejected } = await import("../../src/automation/triggers"); +const { insertRecord, updateRecord, deleteRecord, writeRecordBatch, runActionQuery } = await import( + "../../src/lib/records" +); +const { browserContext } = await import("../../src/automation/context"); +type Trigger = import("../../src/automation/types").Trigger; +type ActionDef = import("../../src/automation/types").ActionDef; +type Step = import("../../src/automation/types").Step; + +let db: ReturnType; +let config: DocumentConfig; +let uninstall: () => void; + +const text = (value: string) => ({ type: "text" as const, value }); +const int = (value: number) => ({ type: "integer" as const, value }); +const named = (column: string, value: string) => ({ column, value: text(value) }); +const action = (id: string, steps: Step[]): ActionDef => ({ id, name: id, onError: "stop", steps }); +const set = (id: string, field: string, value: string): Step => ({ + id, + kind: "setField", + field, + value, +}); +const trigger = (t: Partial): Trigger => ({ + id: "t1", + name: "Check orders", + table: "orders", + event: "beforeChange", + actionId: "stamp", + mode: "sync", + enabled: true, + maxAttempts: 1, + backoffMs: 0, + ...t, +}); +const auditAction = action("audit", [ + { + id: "a1", + kind: "createRecord", + table: "audit", + values: { message: "'deleted ' & record.id & ' ' & record.status & ' old=' & old.status" }, + }, +]); + +beforeEach(() => { + db = createFakeBackend(); + state.backend = db; + db.addTable("orders", ["id", "status", "note"], [{ id: 1, status: "open", note: null }]); + db.addTable("audit", ["id", "message"]); + config = { + actions: [action("stamp", [set("s1", "note", "'stamped ' & record.status")]), auditAction], + triggers: [], + savedQueries: [], + design: { forms: [] }, + } as unknown as DocumentConfig; + uninstall = installTriggers({ + getConfig: () => config, + context: (base) => browserContext(config, () => undefined, base), + }); +}); +afterEach(() => uninstall()); + +describe("beforeChange", () => { + it("sets a field on insert and the stored row has it", async () => { + config.triggers = [trigger({})]; + await insertRecord("orders", [named("status", "new")]); + expect(db.rows("orders")[1]).toMatchObject({ status: "new", note: "stamped new" }); + const sent = db.calls.find((c) => c.command === "insert_row" && c.args.table === "orders"); + expect(sent?.args.values).toContainEqual({ column: "note", value: text("stamped new") }); + }); + + it("sets a field on update from the old row merged with the new values", async () => { + config.actions = [ + action("stamp", [set("s1", "note", "old.status & '->' & record.status & ' #' & record.id")]), + ]; + config.triggers = [trigger({})]; + await updateRecord("orders", [named("status", "closed")], [int(1)]); + expect(db.rows("orders")[0]).toMatchObject({ status: "closed", note: "open->closed #1" }); + }); + + it("gives inserts a null old row", async () => { + config.actions = [action("stamp", [set("s1", "note", "coalesce(old.status, 'none')")])]; + config.triggers = [trigger({})]; + await insertRecord("orders", [named("status", "new")]); + expect(db.rows("orders")[1].note).toBe("none"); + }); + + it("rejects with the fail step's message and writes nothing", async () => { + config.actions = [ + action("stamp", [{ id: "f", kind: "fail", message: "'Status is required'", when: "true" }]), + ]; + config.triggers = [trigger({})]; + const error = await insertRecord("orders", [named("status", "x")]).catch((e) => e); + expect(error).toBeInstanceOf(BeforeChangeRejected); + expect(error.message).toBe("Not saved: Status is required"); + expect(db.rows("orders")).toHaveLength(1); + expect(db.calls.some((c) => c.command === "insert_row")).toBe(false); + }); + + it("rejects an update with a failing condition-gated fail step and leaves the row", async () => { + config.actions = [ + action("stamp", [ + { + id: "f", + kind: "fail", + message: "'Closed orders are final'", + when: "old.status = 'closed'", + }, + ]), + ]; + config.triggers = [trigger({})]; + db.rows("orders")[0].status = "closed"; + await expect(updateRecord("orders", [named("status", "open")], [int(1)])).rejects.toThrow( + "Not saved: Closed orders are final", + ); + expect(db.rows("orders")[0].status).toBe("closed"); + expect(db.calls.some((c) => c.command === "update_row")).toBe(false); + }); + + it("names the trigger when a step errors", async () => { + config.actions = [action("stamp", [set("s1", "note", "1 +")])]; + config.triggers = [trigger({})]; + await expect(insertRecord("orders", [named("status", "x")])).rejects.toThrow( + /^Not saved: trigger "Check orders" failed: /, + ); + expect(db.rows("orders")).toHaveLength(1); + }); + + it("refuses steps that write, with a message naming the step", async () => { + config.actions = [ + action("stamp", [ + { id: "c", kind: "createRecord", table: "audit", values: { message: "'x'" } }, + ]), + ]; + config.triggers = [trigger({})]; + await expect(insertRecord("orders", [named("status", "x")])).rejects.toThrow( + 'Not saved: A before-change trigger cannot run "Create record" steps; use an after trigger', + ); + expect(db.rows("orders")).toHaveLength(1); + expect(db.rows("audit")).toHaveLength(0); + }); + + it("skips a trigger whose condition is false", async () => { + config.triggers = [trigger({ condition: "record.status = 'open'" })]; + await insertRecord("orders", [named("status", "draft")]); + await insertRecord("orders", [named("status", "open")]); + expect(db.rows("orders").map((r) => r.note ?? null)).toEqual([null, null, "stamped open"]); + }); + + it("skips disabled triggers", async () => { + config.triggers = [trigger({ enabled: false })]; + await insertRecord("orders", [named("status", "open")]); + expect(db.rows("orders")[1].note ?? null).toBeNull(); + }); + + it("chains triggers in order, each seeing the fields earlier ones set", async () => { + config.actions = [ + action("first", [set("s1", "note", "'one'")]), + action("second", [set("s2", "note", "record.note & '+two'")]), + ]; + config.triggers = [ + trigger({ id: "t1", actionId: "first" }), + trigger({ id: "t2", actionId: "second" }), + ]; + await insertRecord("orders", [named("status", "x")]); + expect(db.rows("orders")[1].note).toBe("one+two"); + }); + + it("sees later steps' updated record within one action", async () => { + config.actions = [ + action("stamp", [set("s1", "note", "'a'"), set("s2", "status", "record.note & 'b'")]), + ]; + config.triggers = [trigger({})]; + await insertRecord("orders", [named("status", "x")]); + expect(db.rows("orders")[1]).toMatchObject({ note: "a", status: "ab" }); + }); + + it("stops the chain when an earlier trigger rejects", async () => { + config.actions = [ + action("first", [{ id: "f", kind: "fail", message: "'no'" }]), + action("second", [set("s2", "note", "'two'")]), + ]; + config.triggers = [ + trigger({ id: "t1", actionId: "first" }), + trigger({ id: "t2", actionId: "second" }), + ]; + await expect(insertRecord("orders", [named("status", "x")])).rejects.toThrow("Not saved: no"); + }); + + it("rejects a whole write batch and writes nothing", async () => { + config.actions = [ + action("stamp", [ + { id: "f", kind: "fail", message: "'bad row'", when: "record.status = 'bad'" }, + set("s1", "note", "'ok'"), + ]), + ]; + config.triggers = [trigger({})]; + const insert = (status: string) => ({ + operation: "insert" as const, + table: "orders", + values: [named("status", status)], + identity: null, + }); + await expect(writeRecordBatch([insert("good"), insert("bad")])).rejects.toThrow( + "Not saved: bad row", + ); + expect(db.rows("orders")).toHaveLength(1); + expect(db.calls.some((c) => c.command === "execute_write_batch")).toBe(false); + + await writeRecordBatch([insert("good"), insert("fine")]); + expect(db.rows("orders").map((r) => r.note)).toEqual([null, "ok", "ok"]); + }); +}); + +describe("deleted", () => { + it("runs sync triggers after the delete with the deleted row as record and old", async () => { + config.triggers = [trigger({ event: "deleted", actionId: "audit" })]; + await deleteRecord("orders", [int(1)]); + expect(db.rows("orders")).toHaveLength(0); + expect(db.rows("audit").map((r) => r.message)).toEqual(["deleted 1 open old=open"]); + }); + + it("uses meta.old when the caller passes it", async () => { + config.triggers = [trigger({ event: "deleted", actionId: "audit" })]; + await deleteRecord("orders", [int(1)], { old: { id: 1, status: "passed" } }); + expect(db.rows("audit").map((r) => r.message)).toEqual(["deleted 1 passed old=passed"]); + }); + + it("honors the trigger condition against the deleted row", async () => { + config.triggers = [ + trigger({ event: "deleted", actionId: "audit", condition: "old.status = 'x'" }), + ]; + await deleteRecord("orders", [int(1)]); + expect(db.rows("audit")).toHaveLength(0); + }); + + it("enqueues a job for async triggers with event deleted and the row", async () => { + config.triggers = [trigger({ event: "deleted", actionId: "audit", mode: "async" })]; + await deleteRecord("orders", [int(1)]); + expect(db.jobs).toHaveLength(1); + expect(db.jobs[0]).toMatchObject({ + triggerId: "t1", + actionId: "audit", + payload: { + table: "orders", + event: "deleted", + record: { id: 1, status: "open" }, + old: { id: 1, status: "open" }, + identity: [1], + }, + }); + expect(db.rows("audit")).toHaveLength(0); + }); +}); + +describe("action queries", () => { + const pendingRows = [1, 2].map((id) => ({ + identity: [int(id)], + values: [ + { column: "id", value: int(id) }, + { column: "status", value: text("paid") }, + ], + old: [ + { column: "id", value: int(id) }, + { column: "status", value: text("open") }, + ], + })); + const run = (extra: object = {}) => ({ + changed: 2, + removed: 0, + dryRun: false, + table: "orders", + created: [], + updated: [], + createdGrant: null, + updatedGrant: null, + ...extra, + }); + const queryCalls = () => db.calls.filter((c) => c.command === "run_action_query"); + + beforeEach(() => { + db.addTable( + "orders", + ["id", "status", "note"], + [ + { id: 1, status: "open", note: null }, + { id: 2, status: "open", note: null }, + ], + ); + db.on("release_trigger_grant", () => null); + }); + + it("runs beforeBulk hooks on pending rows and sends overrides with the fingerprint", async () => { + config.triggers = [trigger({})]; + db.on("run_action_query", (args) => + args.before + ? run({ + updated: pendingRows.map((r) => ({ identity: r.identity, old: r.old })), + }) + : run({ dryRun: true, pending: { fingerprint: "fp-1", rows: pendingRows } }), + ); + const result = await runActionQuery("pay", []); + expect(result.dryRun).toBe(false); + expect(queryCalls()).toHaveLength(2); + expect(queryCalls()[0].args.before).toBeUndefined(); + expect(queryCalls()[1].args.before).toEqual({ + fingerprint: "fp-1", + overrides: [ + { row: 0, values: [{ column: "note", value: text("stamped paid") }] }, + { row: 1, values: [{ column: "note", value: text("stamped paid") }] }, + ], + }); + }); + + it("gives each pending row's old values to the trigger", async () => { + config.actions = [action("stamp", [set("s1", "note", "old.status & '>' & record.status")])]; + config.triggers = [trigger({})]; + db.on("run_action_query", (args) => + args.before + ? run() + : run({ dryRun: true, pending: { fingerprint: "fp", rows: pendingRows } }), + ); + await runActionQuery("pay", []); + const overrides = (queryCalls()[1].args.before as { overrides: { values: unknown[] }[] }) + .overrides; + expect(overrides.map((o) => o.values)).toEqual([ + [{ column: "note", value: text("open>paid") }], + [{ column: "note", value: text("open>paid") }], + ]); + }); + + it("sends no overrides entry for rows the triggers leave alone", async () => { + config.triggers = [trigger({ condition: "record.id = 2" })]; + db.on("run_action_query", (args) => + args.before + ? run() + : run({ dryRun: true, pending: { fingerprint: "fp", rows: pendingRows } }), + ); + await runActionQuery("pay", []); + const before = queryCalls()[1].args.before as { overrides: { row: number }[] }; + expect(before.overrides.map((o) => o.row)).toEqual([1]); + }); + + it("makes no second call when a trigger rejects", async () => { + config.actions = [ + action("stamp", [{ id: "f", kind: "fail", message: "'No paying'", when: "record.id = 2" }]), + ]; + config.triggers = [trigger({})]; + db.on("run_action_query", () => + run({ dryRun: true, pending: { fingerprint: "fp", rows: pendingRows } }), + ); + await expect(runActionQuery("pay", [])).rejects.toThrow("Not saved: No paying"); + expect(queryCalls()).toHaveLength(1); + }); + + it("does not run before-change hooks on a dry run", async () => { + config.triggers = [trigger({})]; + db.on("run_action_query", (args) => run({ dryRun: !!args.dryRun })); + const result = await runActionQuery("pay", [], { dryRun: true }); + expect(result.dryRun).toBe(true); + expect(queryCalls()).toHaveLength(1); + }); + + it("fires deleted triggers once per deleted row and releases the grant once", async () => { + config.triggers = [trigger({ event: "deleted", actionId: "audit" })]; + db.on("run_action_query", () => + run({ + removed: 2, + changed: 0, + deleted: pendingRows.map((r) => ({ identity: r.identity, values: r.old })), + deletedGrant: "grant-d", + }), + ); + await runActionQuery("purge", []); + expect(db.rows("audit").map((r) => r.message)).toEqual([ + "deleted 1 open old=open", + "deleted 2 open old=open", + ]); + const released = db.calls.filter((c) => c.command === "release_trigger_grant"); + expect(released.map((c) => c.args.grant)).toEqual(["grant-d"]); + }); + + it("enqueues one async deleted job per deleted row", async () => { + config.triggers = [trigger({ event: "deleted", actionId: "audit", mode: "async" })]; + db.on("run_action_query", () => + run({ deleted: pendingRows.map((r) => ({ identity: r.identity, values: r.old })) }), + ); + await runActionQuery("purge", []); + expect(db.jobs.map((j) => (j.payload as { identity: unknown[] }).identity)).toEqual([[1], [2]]); + expect(db.jobs.every((j) => (j.payload as { event: string }).event === "deleted")).toBe(true); + }); +}); From 5139699e5f2bb3c7b3144a2b4fe6ed9a263cada5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 05:35:45 +0000 Subject: [PATCH 2/2] Add App QA screenshots for before-change and deleted triggers Show only the before-change help text on a before-change trigger. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_012iLZgQTvMnEZufP6GdoWy2 --- scripts/screenshot/specs/triggers.spec.tsx | 169 +++++++++++++++++++++ src/automation/TriggersPanel.tsx | 23 +-- 2 files changed, 183 insertions(+), 9 deletions(-) create mode 100644 scripts/screenshot/specs/triggers.spec.tsx diff --git a/scripts/screenshot/specs/triggers.spec.tsx b/scripts/screenshot/specs/triggers.spec.tsx new file mode 100644 index 00000000..2cdf46b3 --- /dev/null +++ b/scripts/screenshot/specs/triggers.spec.tsx @@ -0,0 +1,169 @@ +import { screen, waitFor, within } from "@testing-library/react"; +import { invoke } from "@tauri-apps/api/core"; +import { expect, it } from "vitest"; +import { captureDocument } from "../capture"; +import { LONG, openMode, startFromTemplate, type User } from "./fixtures"; + +const runtimePage = () => screen.getByRole("region", { name: "Application page" }); + +const titles = async () => + ( + await invoke<{ rows: Array> }>("execute_read_query", { + windowLabel: "main", + sql: "SELECT title FROM work_orders WHERE number LIKE 'WO-2%' ORDER BY number", + }) + ).rows.map((r) => r[0]?.value); + +async function addStep(user: User, kind: string) { + await user.selectOptions(screen.getByRole("combobox", { name: "New step kind" }), kind); + await user.click(screen.getByRole("button", { name: "Add step" })); +} + +async function newWorkOrder(user: User, number: string, title: string) { + const page = await screen.findByRole("region", { name: "Application page" }, LONG); + const nav = await screen.findByRole("navigation", { name: "Application navigation" }, LONG); + await user.click(await within(nav).findByRole("button", { name: "Work orders" }, LONG)); + await user.click(await within(page).findByRole("button", { name: "New work order" }, LONG)); + const create = await within(page).findByRole("form", { name: "New Work order" }, LONG); + await user.type(within(create).getByRole("textbox", { name: "Number" }), number); + await user.type(within(create).getByRole("textbox", { name: "Title" }), title); + for (const [label, option] of [ + ["Asset", "Rooftop HVAC unit"], + ["Priority", "High"], + ]) { + const select = await within(create).findByRole("combobox", { name: label }, LONG); + await within(select).findByRole("option", { name: option }, LONG); + await user.selectOptions(select, option); + } + await user.click(within(create).getByRole("button", { name: "Create" })); + return create; +} + +it("sets fields and rejects saves with a before-change trigger, and edits a deleted trigger", async () => { + const user = await startFromTemplate("Work orders"); + await openMode(user, "Automation"); + await screen.findByRole("heading", { name: "Automation" }, LONG); + + // An action that upper-cases the title and refuses short titles. + await user.click(screen.getByRole("tab", { name: "Actions" })); + await user.click(await screen.findByRole("button", { name: "New action" }, LONG)); + const name = await screen.findByRole("textbox", { name: "Action name" }, LONG); + await user.clear(name); + await user.type(name, "Check work order"); + await addStep(user, "fail"); + const fail = await screen.findByRole("group", { name: "Step 1: Fail with message" }, LONG); + await user.type( + within(fail).getByRole("textbox", { name: "Failure message" }), + "'Titles need at least 5 characters'", + ); + await user.type( + within(fail).getByRole("textbox", { name: "Only when" }), + "len(record.title) < 5", + ); + await addStep(user, "setField"); + const setField = await screen.findByRole( + "group", + { name: "Step 2: Set field (before-change)" }, + LONG, + ); + await user.type(within(setField).getByRole("textbox", { name: "Field" }), "title"); + await user.type(within(setField).getByRole("textbox", { name: "Value" }), "upper(record.title)"); + await waitFor( + () => + expect(within(setField).getByRole("textbox", { name: "Value" })).toHaveValue( + "upper(record.title)", + ), + LONG, + ); + await captureDocument(document, { + name: "triggers-01-set-field-action", + expectations: [ + "The Check work order action has a Fail with message step guarded by len(record.title) < 5.", + "Its second step is 'Set field (before-change)' setting title to upper(record.title).", + ], + }); + + // A before-change trigger on work_orders that runs it. + await user.click(screen.getByRole("tab", { name: "Triggers" })); + await user.click(await screen.findByRole("button", { name: "New trigger" }, LONG)); + const triggerName = await screen.findByRole("textbox", { name: "Trigger name" }, LONG); + await user.clear(triggerName); + await user.type(triggerName, "Check work orders before saving"); + await user.selectOptions(screen.getByRole("combobox", { name: "Table" }), "work_orders"); + await user.selectOptions(screen.getByRole("combobox", { name: "Event" }), "beforeChange"); + await user.selectOptions(screen.getByRole("combobox", { name: "Action" }), "Check work order"); + expect(screen.queryByLabelText("Run")).toBeNull(); + expect(screen.getByText(/Runs before each new or changed record is saved/)).toBeInTheDocument(); + expect(screen.queryByText(/Sync triggers run after the record is saved/)).toBeNull(); + expect( + within(screen.getByRole("combobox", { name: "Event" })) + .getAllByRole("option") + .map((o) => o.textContent), + ).toEqual(["Before a record is saved", "Record created", "Record updated", "Record deleted"]); + await captureDocument(document, { + name: "triggers-02-before-change-editor", + expectations: [ + "The new trigger's Event reads 'Before a record is saved' on table work_orders with action Check work order.", + "The sync/async Run select is gone; the only help text says it can set fields or reject the save and cannot write records.", + ], + }); + + // Runtime: a short title is refused with the trigger's message and nothing is saved. + await openMode(user, "Runtime"); + const refused = await newWorkOrder(user, "WO-2001", "Fix"); + await waitFor( + () => + expect( + within(refused) + .queryAllByRole("alert") + .map((a) => a.textContent) + .join("\n"), + ).toMatch("Not saved: Titles need at least 5 characters"), + LONG, + ); + expect(await titles()).toEqual([]); + await captureDocument(document, { + name: "triggers-03-save-rejected", + expectations: [ + "The New Work order form stays open with the error 'Not saved: Titles need at least 5 characters'.", + "The entered number WO-2001 and title Fix are still in the form.", + ], + }); + + // A valid title is saved upper-cased by the Set field step. + await newWorkOrder(user, "WO-2002", "replace hvac belt"); + const saved = await within(runtimePage()).findByRole("form", { name: "Work order" }, LONG); + await waitFor( + () => + expect(within(saved).getByRole("textbox", { name: "Title" })).toHaveValue( + "REPLACE HVAC BELT", + ), + LONG, + ); + expect(await titles()).toEqual(["REPLACE HVAC BELT"]); + await captureDocument(document, { + name: "triggers-04-field-set-on-save", + expectations: [ + "The saved work order WO-2002 shows its title as REPLACE HVAC BELT, upper-cased by the trigger.", + ], + }); + + // A record-deleted trigger keeps the Run and Run as options of an after trigger. + await openMode(user, "Automation"); + await user.click(await screen.findByRole("tab", { name: "Triggers" }, LONG)); + await user.click(await screen.findByRole("button", { name: "New trigger" }, LONG)); + const deletedName = await screen.findByRole("textbox", { name: "Trigger name" }, LONG); + await user.clear(deletedName); + await user.type(deletedName, "Log deleted work orders"); + await user.selectOptions(screen.getByRole("combobox", { name: "Table" }), "work_orders"); + await user.selectOptions(screen.getByRole("combobox", { name: "Event" }), "deleted"); + await user.selectOptions(screen.getByRole("combobox", { name: "Action" }), "Log new work order"); + expect(screen.getByLabelText("Run")).toHaveValue("sync"); + await captureDocument(document, { + name: "triggers-05-deleted-editor", + expectations: [ + "The new trigger's Event reads 'Record deleted' on table work_orders with action Log new work order.", + "Run as and the sync/async Run select are shown, and the help text says a deleted record is seen as it was.", + ], + }); +}); diff --git a/src/automation/TriggersPanel.tsx b/src/automation/TriggersPanel.tsx index 80a66fbe..fb48dfe2 100644 --- a/src/automation/TriggersPanel.tsx +++ b/src/automation/TriggersPanel.tsx @@ -138,7 +138,9 @@ export function TriggersPanel({ focusId }: { focusId?: string }) { Runs before each new or changed record is saved, including rows an action query writes. Its action can set fields with Set field steps, read queries, and reject the save with a Fail step; it cannot write records or open anything. In the condition and - action, old is the row before an update and empty for a new record. + action, old is the row before an update and empty for a new record. Run + as app skips the user's role checks; run as the signed-in user refuses the save up + front when the user's role cannot run the trigger.

) : (
@@ -176,14 +178,17 @@ export function TriggersPanel({ focusId }: { focusId?: string }) { onChange={(idempotencyKey) => edit({ idempotencyKey: idempotencyKey || undefined })} /> )} -

- Created, updated and deleted triggers see the record as record (a deleted - record as it was). Sync triggers run after the record is saved, as part of the same - operation; a failing action reports an error to whoever saved the record, but the save - stays. Async triggers run in the background while the app is open, with retries. Run as - app lets the trigger make its own writes even when the user's role cannot; run as the - signed-in user refuses the save up front when the user's role cannot run the trigger. -

+ {!before && ( +

+ Created, updated and deleted triggers see the record as record (a deleted + record as it was). Sync triggers run after the record is saved, as part of the same + operation; a failing action reports an error to whoever saved the record, but the save + stays. Async triggers run in the background while the app is open, with retries. Run + as app lets the trigger make its own writes even when the user's role cannot; run as + the signed-in user refuses the save up front when the user's role cannot run the + trigger. +

+ )}
) : (