diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70f8690..eafe610 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,10 +21,18 @@ jobs: test: runs-on: ubuntu-latest + name: test (ruby ${{ matrix.ruby }}, rails ${{ matrix.rails || 'latest' }}) strategy: fail-fast: false + # The gemspec supports Rails >= 7.1, < 9.0. Pin one Rails per entry so + # that range is exercised rather than asserted; a bare RAILS_VERSION + # resolves to the newest release. matrix: - ruby: ["3.2", "3.3", "3.4"] + include: + - { ruby: "3.2", rails: "7.1" } + - { ruby: "3.3", rails: "7.2" } + - { ruby: "3.4", rails: "8.0" } + - { ruby: "3.4", rails: "" } services: postgres: image: postgres:16 @@ -39,6 +47,7 @@ jobs: --health-retries 5 env: DATABASE_URL: postgres://postgres:postgres@localhost:5432 + RAILS_VERSION: ${{ matrix.rails }} steps: - uses: actions/checkout@v7 - uses: ruby/setup-ruby@v1 @@ -46,6 +55,16 @@ jobs: ruby-version: ${{ matrix.ruby }} bundler-cache: true - name: Prepare database - run: bin/rails app:db:test:prepare + # Migrate rather than load schema.rb, which carries the Rails version + # that dumped it and so cannot be read by an older Rails in this matrix. + # The engine's and the dummy's migrations interleave by timestamp and + # run from separate paths, hence the second engine pass. + env: + RAILS_ENV: test + run: | + bin/rails app:db:create + bin/rails app:db:migrate + (cd test/dummy && bin/rails db:migrate) + bin/rails app:db:migrate - name: Run tests run: bin/rails app:test diff --git a/.gitignore b/.gitignore index ddb15bc..fb27d6a 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,10 @@ # OS / editor cruft. .DS_Store + +# Agent worktrees created by tooling — never part of the repo. +/.claude/ + +# The dummy app's schema is dumped with whichever Rails ran the migration, which +# no older Rails in the CI matrix can read. Everything migrates from scratch. +/test/dummy/db/schema.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b0b4177..f268e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,155 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.1.0] - 2026-08-29 + +Three things 1.0 left you to build: authoring a flow, editing routing rules in a +browser, and telling anyone there was something waiting. Nothing in 1.0 changed +behaviour, and every new surface is off until you switch it on. + +### Added + +- `ApprovalEngine.define_flow(name, tenant:, model:)` — a declarative DSL for a + whole flow (the template, its ordered steps, and the rules that route to it) + in one block, instead of three `create!` calls whose relationship you have to + already know: + + ApprovalEngine.define_flow "High-value invoice", tenant: "acme", model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo", timeout_after: 2.days + end + + Each `step` is its own layer in declaration order; a `parallel(approvals_required:)` + block puts several in one layer, under one consensus policy. `on` takes a + lifecycle symbol resolved through `model:` or any event name you fire + yourself, plus `when:`, `priority:` and `active:`. Idempotent on + `(tenant_id, name)`: it reconciles the same rows rather than creating a second + flow, so a seed file can re-run on every deploy — steps are matched by name and + updated in place so the ids the admin UI links to survive, and a rule the block + stopped declaring is deactivated rather than destroyed. It needs a live + connection, so it belongs in `db/seeds.rb` or a rake task, not an initializer. + **In-flight approvals are untouched** — they were stamped from the blueprint at + build time and never re-read it. `TrackTemplate` / `TemplateStep` / + `TriggerRule` remain public API and are unchanged; `define_flow` writes exactly + the rows you would have. `ApprovalEngine.flow(name, tenant:)` reads the + template back, for `run_approval!(templates: [...])`. +- `ApprovalEngine::Condition` — the translation layer between a + field/operator/value triple and the JSON Logic AST stored in + `TriggerRule#condition`, shared by the DSL's `when:` sugar and the admin rule + builder. It covers `eq`, `not_eq`, `gt`, `gte`, `lt`, `lte`, `in` and + `not_in`, ANDs multiple conditions together, and casts values to the types + `exposes_for_approval` declared so a rule can't compare a number against a + string and quietly never match. `Condition.parse` returns `nil` — not an empty + result — for an AST it cannot represent, so **a rule someone hand-wrote is + never silently flattened into something it isn't.** +- A built-in admin UI for templates, their steps, and their trigger rules, + mounted under `/admin` inside the engine — the code behind the README's + standing claim that admins change routing without a deploy. Gated behind + `config.admin_enabled` (**default `false`**, so an existing 1.0 mount stays + read-only across the upgrade), re-checked per request rather than trusted from + boot, and drawn once at boot so it needs a restart. The rule editor is a + zero-JavaScript field/operator/value form whose field list comes from + `exposes_for_approval`, with an automatic fallback to a raw JSON Logic textarea + for any condition the simple form cannot represent. **Turning it on is not + authentication**: it decides whether the write routes exist, not who may reach + them, so the mount still needs your own constraint. The approvals dashboard + itself stays read-only. +- Built-in notifications — six messages (`step_activated`, `step_reminder`, + `step_reassigned`, `changes_requested`, `approval_approved`, + `approval_rejected`) rendered in HTML and text by + `ApprovalEngine::NotificationMailer`, dispatched by `ApprovalEngine::Notifier` + from inside `ProcessOutboxJob` so mail rides the existing transactional outbox: + a flaky SMTP host retries with backoff and never touches the approval. Off + unless `config.notifications_enabled = true`: **upgrading does not start + emailing your users.** Enabling it also needs a From address (`mailer_from`, or + a `parent_mailer` that declares one) and `approval_url_builder`, without which + the mail arrives with nowhere to go; the three approval-level messages stay + silent until `approval_recipients` says who asked. Recipients otherwise resolve + off the actor via `actor_email_method` (`:email` by default), and an actor with + no address is skipped rather than raised on. Views are overridden by dropping + copies into `app/views/approval_engine/notification_mailer/`. Delivery is + at-least-once, as with every outbox event. The `approval_engine.*` + `ActiveSupport::Notifications` remain the escape hatch for hosts that would + rather send their own. +- `ApprovalEngine::ReminderSweepJob` — nudges approvers who have gone quiet for + longer than `config.reminder_after` (nil by default, so scheduling the job + before setting it does nothing). Idempotent via a new `reminded_at` column on + steps, so each step is nudged at most once and running the sweep hourly only + makes the nudge arrive sooner. +- `config.approval_groups` — an optional allowlist of the group names + `define_flow` may route to. `nil`, the default, means no vocabulary is declared + and nothing is checked; set it and a `group:` typo stops the seed instead of + resolving zero actors at build time. + +### Changed + +- `spec.summary` and `spec.description` now lead with what the gem does rather + than how it is built, so the RubyGems snippet reads as a problem instead of a + spec sheet. `documentation_uri` points at the cookbook, `source_code_uri` is + pinned to the repository rather than derived from `homepage`, and + `docs/ARCHITECTURE.md` / `docs/COOKBOOK.md` now ship inside the gem. +- Rewrote the README around the problem it solves: the job in the first line, + screenshots of a real approval above the fold, a single-block 60-second + quickstart built on `define_flow`, an honest "what to do when a boolean flag + stops fitting" instead of a bullet sending most readers away, and an + `Alternatives` section naming the neighbours. `test/docs_test.rb` now fails the + build if the README documents a config key, an `ApprovalEngine` method, a + generator, or an image that does not exist. + +### Changed + +- **Minimum Rails is now 7.1**, up from 7.0.8. The outbox relay's retry backoff + uses `:polynomially_longer`, which 7.0 does not know, and 7.0 is past security + support. CI now runs the whole supported range rather than one version of it. + +### Added + +- `ApprovalEngine::TestHelpers` — test support for host applications. + `approve_approval!` walks a flow to a decision, `reject_approval!` stops at + whatever is waiting, `pending_approval_steps` is the inbox for one record, and + `drain_approval_outbox!` relays the side-effects, without which `after_approved` + has genuinely not run yet. Actions and queries rather than assertions, so they + read the same under Minitest and RSpec. + +### Fixed + +- Step notifications ignored delegations. A delegate can act on a step, so + mailing only the assignee told the person who was away and not the person who + could do something about it. Both are notified now. +- The `changes_requested` email named the step's assignee rather than whoever + actually asked for changes, which is recorded on the ledger and is who the + reader needs. +- The condition size cap applied only to the raw JSON editor, so the simple + builder could store an unbounded condition on the same hot path. +- Approvals attached to a host record whose primary key is a UUID silently lost + their target. The polymorphic reference columns were created by + `t.references`, which defaults to bigint, so a UUID cast to `0`: the row + saved, no error was raised, and `approval.target` read back `nil`. They are + string columns now, which fit a bigint, a UUID and a ULID alike. + +### Upgrading from 1.0 + +Nothing to do, and nothing to migrate before the gem will run. Both new +surfaces default to off: no mail is sent and no write endpoint exists until you +ask for them. **Nothing that touches your users or calls your code turns itself +on because you bumped a version.** + +One thing does change without asking, and it is worth knowing about. The ledger +now emits a `step.activated` outbox event when a step becomes actionable — +roughly one extra outbox row and relay job per step. It reaches your app only if +you subscribe to it: `ActiveSupport::Notifications` subscribers matching +`approval_engine.*` will start seeing `approval_engine.step.activated`, and a +model that happens to define `after_step_activated(step)` will start having it +called. If neither is true of your app, the only difference is the extra rows. + +There is one new migration, and only the reminder sweep reads the column it +adds, so it is needed when you want reminders and not before: + + bin/rails approval_engine:install:migrations + bin/rails db:migrate + ## [1.0.0] - 2026-06-17 First public release. The API below is stable. @@ -135,4 +284,5 @@ First public release. The API below is stable. - A misconfigured `actor_class` now raises an actionable `BuilderError` naming the setting, instead of a raw `NameError`. +[1.1.0]: https://github.com/Harry-kp/approval_engine/releases/tag/v1.1.0 [1.0.0]: https://github.com/Harry-kp/approval_engine/releases/tag/v1.0.0 diff --git a/Gemfile b/Gemfile index 666c5ac..b41a9ca 100644 --- a/Gemfile +++ b/Gemfile @@ -4,6 +4,14 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } # Specify your gem's dependencies in approval_engine.gemspec. gemspec +# CI pins a Rails version per matrix entry so the gemspec's supported range is +# actually exercised, not just asserted. Unset resolves to the newest Rails. +if (rails_version = ENV["RAILS_VERSION"]) && !rails_version.empty? + gem "rails", "~> #{rails_version}.0" + # Rails 7.x predates Minitest 6, whose runner signature it calls incorrectly. + gem "minitest", "~> 5.25" if rails_version.start_with?("7.") +end + gem "puma" gem "pg" diff --git a/README.md b/README.md index afc025a..da9a5c3 100644 --- a/README.md +++ b/README.md @@ -4,16 +4,104 @@ [![Downloads](https://img.shields.io/gem/dt/approval_engine?logo=rubygems&color=blue)](https://rubygems.org/gems/approval_engine) [![CI](https://github.com/Harry-kp/approval_engine/actions/workflows/ci.yml/badge.svg)](https://github.com/Harry-kp/approval_engine/actions/workflows/ci.yml) ![Ruby](https://img.shields.io/badge/ruby-%3E%3D%203.1-CC342D) -![Rails](https://img.shields.io/badge/rails-%3E%3D%207.0.8-D30001) +![Rails](https://img.shields.io/badge/rails-%3E%3D%207.1-D30001) ![License: MIT](https://img.shields.io/badge/license-MIT-blue) -Multi-tenant, immutable-ledger human approval flows for Rails. +**A manager approves, then the CFO. Unless it's over $50k — then Legal too.** -Use it when a manager approves an invoice, then a CFO. Or Legal and IT -in parallel. Or "any two of five reviewers." ApprovalEngine supplies the -generic machinery: an append-only ledger, race-safe transitions, runtime -routing rules, and async side-effects. You decide what gets approved, who -approves, and what happens next. +Every app eventually grows an approval step. It starts as an `approved` boolean +and an `approved_by_id`. Then Finance wants a second signature over $10k. Then +Legal and IT have to review the same contract at the same time. Then someone +goes on leave and their queue backs up. Then an auditor asks who approved +invoice #4471 and on whose behalf — and the honest answer is that it is spread +across three callbacks and a `status` column you have already overwritten twice. + +ApprovalEngine is the generic machinery for that, extracted: an append-only +ledger of who decided what, routing rules your admins change without a deploy, +and side-effects that can't roll back an approval. You decide what gets +approved, who approves, and what happens next. + +![The mounted dashboard: six approvals across pending, approved and rejected, filterable by status](https://raw.githubusercontent.com/Harry-kp/approval_engine/main/assets/screenshot-dashboard.png) + +![One approval in detail: layer 1 approved, Legal and IT both pending at layer 2 under all-must-approve consensus, the CFO waiting at layer 3, and the audit trail below](https://raw.githubusercontent.com/Harry-kp/approval_engine/main/assets/screenshot-approval.png) + +- **A ledger, not a flag.** `Approval → Track → Step`, append-only. A rejection + or a request for changes appends a new iteration; nothing is overwritten, so + the history survives the question. +- **Rules, not `if`s.** Routing lives in the database as JSON Logic, scoped per + tenant. "Over $10k" becomes "over $25k, unless EMEA" without a deploy. +- **An outbox, not callbacks.** Side-effects run through a transactional outbox + on ActiveJob, so a down payment API can never roll back an approval. + +## 60-second quickstart + +Needs PostgreSQL (the routing engine uses `jsonb`), Rails 7.1+, Ruby 3.1+. + +```sh +bundle add approval_engine +bin/rails generate approval_engine:install +bin/rails db:migrate +``` + +Then four small edits, and a $20,000 invoice routes itself to a manager and +then a CFO: + +```ruby +# config/initializers/approval_engine.rb — the generator wrote this; fill in two lines +ApprovalEngine.configure do |config| + config.actor_class = "User" # who approves + config.current_tenant_method = -> { Current.account } # anything with #id + + # `Current` here is your own ActiveSupport::CurrentAttributes class — the + # engine only ever calls `#id` on what you return. Not multi-tenant? Return a + # constant instead: `-> { Struct.new(:id).new("default") }`. +end + +# app/models/user.rb — who is in a group, resolved at runtime against your own models +class User < ApplicationRecord + def self.resolve_approval_group(group_name, target) + where(role: group_name) # `target` is the record being approved + end +end + +# app/models/invoice.rb — arm the model, and declare what a rule may read +class Invoice < ApplicationRecord + has_approvals + + exposes_for_approval do + attribute :amount, type: :decimal + end + + def after_approved + PaymentService.disburse_funds!(self) + end +end + +# db/seeds.rb — the flow itself. Idempotent, so re-running it on every deploy is safe. +account = Account.find_by!(subdomain: "acme") + +ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" +end + +# Anywhere in your app — a controller, a job, a console. +Current.account = account +invoice = Invoice.create!(amount: 20_000) # over $10k, so it routes itself + +step = ApprovalEngine::Step.actionable_by(current_user).first +step.approve!(by: current_user) # the CFO's step opens; once they approve too, + # `after_approved` fires through the outbox +``` + +Create the approvers first: a group that resolves to nobody raises at build +time rather than starting an approval nobody can act on. + +Nothing triggered? `invoice.preview_approval(event: "invoice.created")` tells +you what would have happened, before you go any further — see +[Gotchas](#gotchas). ## Is this for you? @@ -26,12 +114,35 @@ Use it when you have: Look elsewhere when: -- You just need a boolean `approved` flag. A column and a method are simpler. +- One person says yes, once, and nobody will ever ask who. A boolean column and + a method is smaller, and smaller is right until it stops fitting — see below + for what to do when it does. - You need a state machine for non-approval domains. Try [AASM](https://github.com/aasm/aasm) or [state_machines](https://github.com/state-machines/state_machines). - You're not on PostgreSQL. The routing engine needs `jsonb` and `gin`. +### When a boolean flag stops fitting + +You will know: a second approver appears, or somebody asks *who* approved it, or +the threshold has to differ per customer. You don't have to give the column up. +It stops being the source of truth and becomes a projection of the ledger, so +every query, view, and scope you already wrote keeps working: + +```ruby +class Invoice < ApplicationRecord + has_approvals + + # The flag you already had. Now it is derived, not decided here. + def after_approved + update_column(:approved, true) + end +end +``` + +Day one, nothing else in your app changes. Day thirty, you add the second layer +and the audit trail is already there. + ## Installation Add this line to your application's **Gemfile**: @@ -50,7 +161,66 @@ rails db:migrate The generator copies migrations and an initializer, and prints next steps. -## Quickstart + +## Defining a flow + +A flow is three records: a template, its ordered steps, and the rule that says +which event fires it. You can still write those by hand — the admin UI does +exactly that — but for the flows that live in your codebase, `define_flow` says +the same thing in the shape of the policy it encodes: + +```ruby +ApprovalEngine.define_flow "Vendor contract", tenant: account.id, model: Contract do + on "contract.submitted", when: { value: { gte: 50_000 } } + + step "Manager sign-off", group: "manager" + + parallel(approvals_required: :all) do + step "Legal review", group: "legal" + step "IT review", group: "it" + end + + step "CFO sign-off", group: "cfo", timeout_after: 2.days +end +``` + +Each `step` is its own layer, in the order written. Steps inside a `parallel` +block share a layer and open together, under one consensus policy declared on +the block. Everything a template step takes, `step` takes: `approvals_required` +(`:any`, `:all`, `:majority`, a percentage like `"60%"`, or a count) and +`timeout_after`. + +`on` takes a lifecycle symbol (`:create`, resolved through `model:` into the +conventional event name) or any event name you fire yourself, plus `when:`, +`priority:` and `active:`. The `when:` sugar — `{ amount: { gt: 10_000 } }` — +compiles to the JSON Logic actually stored, using `eq`, `not_eq`, `gt`, `gte`, +`lt`, `lte`, `in` and `not_in`; several keys AND together, and anything more +involved can be passed as raw JSON Logic instead. + +`define_flow` is idempotent, keyed on the tenant and the flow name. Run it from +`db/seeds.rb`, a rake task, or a deploy hook and re-run it as often as you like: +it reconciles the blueprint in place instead of stacking a fourth copy of the +same template. It needs a live database connection, so it does not belong in an +initializer. + +Editing a flow does not disturb work already in flight. An approval is stamped +from the blueprint when it is built and never reads it again — the steps a +manager is looking at right now are the steps that existed when the invoice was +created. New runs pick up the new shape. + +Two smaller seams. `ApprovalEngine.flow(name, tenant:)` hands back the +`TrackTemplate` a definition owns, which is what +`run_approval!(templates: [...])` wants when several flows should run as +parallel tracks of one approval. And `config.approval_groups` is an optional +allowlist of the group names a flow may route to — `nil` by default, meaning +unchecked, so a typo in `group:` stops the seed only once you have declared a +vocabulary. + +Prefer the records? `TrackTemplate` / `TemplateStep` / `TriggerRule` are still +public API and still work exactly as they did in 1.0 — `define_flow` writes the +same rows you would have written. + +## Wiring it into your app Teach your actor class to resolve approval groups. The engine creates one step per returned record. `target` is the record being approved (e.g. the @@ -81,21 +251,8 @@ class Invoice < ApplicationRecord end ``` -Define a template, its ordered steps, and the rule that triggers it. - -```ruby -template = ApprovalEngine::TrackTemplate.create!( - tenant_id: "acme", name: "High-value invoice", status: "active" -) -template.template_steps.create!(name: "Manager", layer: 1, assigned_group: "manager") -template.template_steps.create!(name: "CFO", layer: 2, assigned_group: "cfo") -template.trigger_rules.create!( - tenant_id: "acme", event_name: "invoice.created", - condition: { ">" => [{ "var" => "amount" }, 10_000] } -) -``` - -Trigger a run. +Trigger a run. `has_approvals` auto-routes on create once +`current_tenant_method` is set; anywhere else, fire the event yourself. ```ruby invoice = Invoice.create!(amount: 20_000) @@ -126,29 +283,41 @@ trigger, `preview_approval(...).triggered?` tells you why. Check: - `config.current_tenant_method` is set. Until then, auto-routing on create is a no-op, so pass `tenant_id:` explicitly. -## See it live +`define_flow` is the exception, and deliberately so. It is setup code you run at +deploy, so it raises on a typo instead of failing closed — an unexposed +attribute, an unknown lifecycle, a duplicate step name, and a group outside +`config.approval_groups` all stop the seed with a sentence explaining themselves. -Run the demo against a clone of this repo, not your own app. It needs -PostgreSQL running. +## Testing your flows -```sh -bin/demo -# seeds sample data and boots the dashboard at -# http://localhost:3000/approval_engine +The engine ships helpers so a host app can drive an approval without hand-rolling +fixtures. They are actions and queries rather than assertions, so they read the +same under Minitest and RSpec. + +```ruby +# test/test_helper.rb +require "approval_engine/test_helpers" + +class ActiveSupport::TestCase + include ApprovalEngine::TestHelpers +end ``` -Or explore the API in a console preloaded with sample data. +```ruby +invoice = Invoice.create!(amount: 20_000) + +assert_equal "Manager sign-off", pending_approval_steps(invoice).sole.name -```sh -bin/console ->> Rails.application.load_seed ->> ApprovalEngine::Step.pending.first.approve!(by: User.find_by(role: "manager")) +approve_approval_and_settle!(invoice) # walk every layer, then relay the outbox +assert_equal "paid", invoice.reload.state ``` -The mounted dashboard lists every approval, filters by status, and drills -into tracks, steps, and the full audit trail. It is read-only, with -bundled styling. It has no auth of its own — when you mount it in a real app, -wrap it in a `constraints`/authenticated route ([recipe](docs/COOKBOOK.md#ui--monitoring)). +`approve_approval!` walks each layer as it opens, approving with the step's own +assigned actor unless you pass `by:`. `reject_approval!` stops at whatever is +waiting. **`drain_approval_outbox!` is the one to remember** — side-effects reach +your model through the outbox, relayed by ActiveJob, so until the queue runs +`after_approved` has genuinely not fired and a test asserting on it will fail for +the right reason and look like the wrong one. ## Configuration @@ -159,6 +328,7 @@ ApprovalEngine.configure do |config| config.current_tenant_method = -> { Current.account } # anything with #id config.outbox_queue = :default # ActiveJob queue for side-effects config.raise_on_rule_errors = false # fail closed in production + config.approval_groups = nil # nil = any group name is allowed end ``` @@ -170,6 +340,171 @@ Single-tenant apps can return a constant, e.g. No Redis or Sidekiq required. Side-effects run through ActiveJob, so SolidQueue, Sidekiq, or the async adapter all work. +The two surfaces added in 1.1 — notifications and the admin UI — have their own +keys, and both are off until you set them. See below. + +## Notifications + +An approval only works if the approver finds out. In 1.0 that was yours to +build: the engine published the events and you wrote the mailer. It still can — +but for the common case, the engine will now send the mail. It is **off by +default**, and upgrading from 1.0 does not turn it on: your `Step` rows are +assigned to real people, and a gem upgrade must not start mailing them. + +```ruby +ApprovalEngine.configure do |config| + config.notifications_enabled = true + config.mailer_from = "approvals@acme.com" + config.approval_url_builder = ->(approval) do + Rails.application.routes.url_helpers.invoice_url(approval.target) + end +end +``` + +Those three are the minimum. Without a From address — either `mailer_from`, or a +`parent_mailer` that already declares `default from:` — Action Mailer has +nothing to hand the SMTP server. Without `approval_url_builder` the mail arrives +with nowhere to go. + +Six messages ship, listed in `config.notification_events`: an approver is told +they have work waiting (`step_activated`), nudged if they go quiet +(`step_reminder`), and told when a step is handed to them (`step_reassigned`); +the requesting side is told when changes are requested (`changes_requested`) and +when the approval is approved or rejected (`approval_approved`, +`approval_rejected`). Drop one to silence it: + +```ruby +config.notification_events -= [ :step_reassigned ] +``` + +The last three go wherever `config.approval_recipients` says — the engine knows +who approves, but only you know who asked, so while that is nil those three stay +silent. The rest resolve off the actor via `config.actor_email_method` +(`:email` by default); an actor with no address is skipped, never raised on. +`config.actor_label_method` and `config.target_label_method` decide how a person +and a record name themselves in an inbox, and fall back to something readable +rather than an object inspection. + +The mails go out through the same transactional outbox as every other +side-effect, so a dead SMTP host can never touch the approval: the transition +has already committed, and only the delivery fails. Delivery is at-least-once, +which for email means an approver may occasionally get the same nudge twice — +the tradeoff for never getting zero. The engine hands the message to +`deliver_later` and imposes no retry policy of its own, so what happens to a +bounced job is whatever your Active Job backend already does with a failure. + +Reminders are a separate, opt-in sweep. Set how long a step may sit unanswered +and schedule the job with whatever recurring mechanism you already run: + +```ruby +config.reminder_after = 2.days.to_i # nil (the default) = reminders off +``` + +```ruby +ApprovalEngine::ReminderSweepJob.perform_later # all tenants +ApprovalEngine::ReminderSweepJob.perform_later(tenant_id: account.id) +``` + +Each step is nudged at most once, so running the sweep hourly only makes the +nudge arrive sooner rather than nagging. That guarantee is on the *emission*: +like every other notification it rides the at-least-once outbox, so a relay +retry can still deliver a duplicate copy of the same nudge. + +Be deliberate about the first run. `reminder_after` is measured from when a step +became actionable, not from when you switched reminders on, so the first sweep +nudges *every* step already past the threshold — on an app with a backlog that +is one large batch of email. Check what it will pick up before you schedule it: + +```ruby +ApprovalEngine::Step.remindable(after: 2.days.to_i).count +``` +The stamp that makes that true lives in a column added in 1.1, so an app +upgrading from 1.0 needs `bin/rails approval_engine:install:migrations` and +`bin/rails db:migrate` before scheduling the sweep — nothing else in the engine +reads it. + +Make the mail yours by overriding the views: drop your own copies into +`app/views/approval_engine/notification_mailer/` and your app's view path is +searched before the engine's. Override *both* formats of an action together — +Action Mailer takes an action's templates from the first view path that has any +of them, so a lone `.html.erb` silences the engine's `.text.erb` and leaves you +with a single-part message. To take a message over wholesale, subclass +`ApprovalEngine::NotificationMailer` and point `config.mailer_class` at it, or +point `config.parent_mailer` at your `ApplicationMailer` to inherit your layout +and sender. If you would rather send everything yourself, leave +`notifications_enabled` off and subscribe to the `approval_engine.*` +[notifications](docs/COOKBOOK.md#notify-another-system-without-coupling-to-my-model) +as before. Escalation and what "late" means are still yours: the engine tells +you what happened, not what it should mean. + +## Dashboard + +Mount the engine and you get an ops dashboard: every approval, filterable by +status, drilling into tracks, steps, the full audit trail, and a time-in-step +column so a stuck approval is visible instead of reported. + +```ruby +# config/routes.rb +authenticate :admin_user, ->(u) { u.super_admin? } do + mount ApprovalEngine::Engine => "/approval_engine" +end +``` + +It is read-only and stays that way — its job is to surface stuck, quarantined +and in-flight work without anyone writing SQL, not to act on it. It has no auth +of its own; wrapping the mount is your call and your app's +([recipe](docs/COOKBOOK.md#ui--monitoring)). + +Want to see it without installing anything? Clone this repo and run: + +```sh +bin/demo +# seeds sample data and boots the dashboard at +# http://localhost:3000/approval_engine +``` + +Or explore the API in a console preloaded with the same data. + +```sh +bin/console +>> Rails.application.load_seed +>> ApprovalEngine::Step.pending.first.approve!(by: User.find_by(role: "manager")) +``` + +## Admin UI + +The README has claimed since 1.0 that admins change routing rules at runtime, +without a deploy. Since 1.1 there is a UI for it: templates, their steps, and +their trigger rules, editable in the browser under `/admin` inside the mount. +It is **off by default** — 1.0 shipped a read-only dashboard that hosts mounted +behind whatever auth they had, and a gem upgrade must never turn that into a +write surface. + +```ruby +ApprovalEngine.configure do |config| + config.admin_enabled = true +end +``` + +Routes are drawn once at boot, so this belongs in an initializer and needs a +restart. **Turning it on is not authentication.** It decides whether the write +routes exist at all, not who may reach them — anyone who can reach the mount can +change what routes where, so keep the mount behind your own constraint either +way. + +![The admin rule editor: a field dropdown showing amount (decimal), an operator dropdown showing is greater than, a value field, and a Simple / Raw JSON Logic toggle](https://raw.githubusercontent.com/Harry-kp/approval_engine/main/assets/screenshot-rule-builder.png) + +Rules get a form instead of a JSON blob: pick a field, an operator, and a value, +and the engine stores the JSON Logic. The field dropdown is populated from +`exposes_for_approval`, so it offers the names a rule may actually read — an +unexposed variable is a clean non-match, which is the single most common way a +hand-typed rule silently never fires. There is no JavaScript involved. + +A rule too complex for the simple form — an `or`, a nested group, a substring +`in` — opens in a raw JSON Logic textarea instead of being quietly flattened +into something it isn't. That fallback is the point: the form is a convenience +over the storage format, never a lossy replacement for it. + ## Core concepts | Term | What it is | @@ -186,8 +521,10 @@ A single-track run is an approval with one track, not a special path. You never build that chain by hand: start a run with `run_approval!` and act on a step with `step.approve!`. The layers surface only when you need them, such as parallel tracks or the -dashboard. For a single-track approval, `approval.track` and -`approval.step` read it back without `.first`. +dashboard. For a single-track approval, `approval.track` reads it back without +`.first`, and `approval.step` does the same for an approval that has exactly one +step — it is `sole`, so it raises rather than guess when there are two, which is +the case for the manager-then-CFO flow above. `approvals_required` is one idea used at two levels: within a layer (how many of its steps), and across the parallel tracks of a scatter-gather (how many tracks @@ -225,6 +562,28 @@ way. Set `config.raise_on_rule_errors = true` to surface errors loudly. For the full design — the model hierarchy, the consensus/rework model, and why the outbox exists — see **[docs/ARCHITECTURE.md](docs/ARCHITECTURE.md)**. +## Alternatives + +"Which gem" is a real question, so: honest neighbours. + +- **A boolean column.** Genuinely the right answer for one approver and one + yes/no. See [above](#when-a-boolean-flag-stops-fitting) for what to do when it + stops being. +- **[AASM](https://github.com/aasm/aasm) / [state_machines](https://github.com/state-machines/state_machines).** + Better than this gem at modelling *your* domain's states. They don't know what + an approver is, so multi-party consensus, delegation, and the audit trail stay + yours to write. +- **[ringlesoft/laravel-process-approval](https://github.com/ringlesoft/laravel-process-approval).** + The same idea in Laravel, and worth reading if you are comparing across + ecosystems. +- **A workflow or BPM service.** If your approvals are one node in a much larger + orchestrated process, buy that instead of building it on ActiveRecord. + +This gem is young, and worth saying so plainly: 1.0 shipped in June 2026 and was +never announced, so real-world usage is close to none. The API is stable and +every documented recipe is exercised by the test suite, but it has not met your +edge cases yet. + ## Development ApprovalEngine needs Ruby 3.1+ and PostgreSQL. diff --git a/RELEASING.md b/RELEASING.md index d1e6949..376483d 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -20,8 +20,15 @@ Configure this repo as a **trusted publisher** for the gem (no API key needed): ## Cutting a release 1. Update `lib/approval_engine/version.rb`. -2. Move the `CHANGELOG.md` `[Unreleased]` notes under a new - `## [x.y.z] - YYYY-MM-DD` heading. +2. Add a `## [x.y.z] - YYYY-MM-DD` section to `CHANGELOG.md`, directly above the + previous release, plus a matching `[x.y.z]: .../releases/tag/vX.Y.Z` link + reference at the bottom of the file (`test/docs_test.rb` fails the build if a + version heading has no link reference). This file keeps no `[Unreleased]` + section: notes are written under their version when the release is cut. + + The date must be the day you push the tag. `release.yml` fires on `v*` and + publishes within seconds, so a stale date is public before you notice and can + only be corrected by another commit. 3. Commit, then tag and push: ```sh diff --git a/app/controllers/approval_engine/admin/base_controller.rb b/app/controllers/approval_engine/admin/base_controller.rb new file mode 100644 index 0000000..cd2d8ca --- /dev/null +++ b/app/controllers/approval_engine/admin/base_controller.rb @@ -0,0 +1,74 @@ +module ApprovalEngine + module Admin + # Base for the opt-in admin — the only place in the engine that edits + # blueprints. Inherits the dashboard's layout and CSRF posture. + class BaseController < ApplicationController + helper ApprovalEngine::ApplicationHelper + helper ApprovalEngine::Admin::BaseHelper + + before_action :ensure_admin_enabled + + helper_method :exposed_attribute_groups, :known_event_names, :default_tenant_id + + private + + # Routes are drawn once at boot, so re-check per request: a host that + # flips the flag off must not still have a write surface. + def ensure_admin_enabled + head :not_found unless ApprovalEngine.config.admin_enabled + end + + # What a rule may read, so the dropdown offers real names. An unexposed + # var is a clean non-match — a rule that silently never fires. + def exposed_attribute_groups + @exposed_attribute_groups ||= approvable_models.to_h { |model| [ model.name, exposed_options_for(model) ] } + .reject { |_name, options| options.empty? } + end + + # Nothing exposed means nothing a rule can read, so omit it entirely. + def exposed_options_for(model) + model.approval_exposure.attributes.values.map do |attribute| + [ "#{attribute.name} (#{attribute.type})", attribute.name.to_s ] + end + end + + # Conventional names plus ones already in use. A datalist, not a select — + # any string is still allowed. + def known_event_names + @known_event_names ||= begin + conventional = approvable_models.flat_map do |model| + ApprovalEngine::Approvable::LIFECYCLE_EVENTS.each_key.map { |lifecycle| model.approval_event_name(lifecycle) } + end + (conventional | TriggerRule.distinct.pluck(:event_name)).compact.sort + end + end + + # So a typed value is stored as the type the payload carries — otherwise + # `amount > "10000"` compares a number to a string and never matches. + # Scoped by event prefix, falling back to every exposed attribute. + def attribute_types(event_name) + element = event_name.to_s.split(".").first + models = approvable_models.select { |model| model.model_name.element == element } + models = approvable_models if models.empty? + models.reverse.reduce({}) { |types, model| types.merge(model.approval_exposure.attributes.transform_values(&:type)) } + end + + # In development classes load on demand, so a cold-booted admin would + # offer an empty field list. Eager loading once is the price. + def approvable_models + @approvable_models ||= begin + Rails.application.eager_load! unless Rails.application.config.eager_load + ApprovalEngine.approvable_models + end + end + + # Prefill only. The dashboard is a cross-tenant ops surface and stays one: + # `current_tenant_method` is nil by default and an ops mount has no tenant + # in scope, so scoping reads to it would usually show an empty page. + def default_tenant_id + tenant = ApprovalEngine.current_tenant + tenant.respond_to?(:id) ? tenant.id : tenant + end + end + end +end diff --git a/app/controllers/approval_engine/admin/template_steps_controller.rb b/app/controllers/approval_engine/admin/template_steps_controller.rb new file mode 100644 index 0000000..a807ab5 --- /dev/null +++ b/app/controllers/approval_engine/admin/template_steps_controller.rb @@ -0,0 +1,63 @@ +module ApprovalEngine + module Admin + # Runtime CRUD for the layers of a template. Steps have no page of their + # own: the template's show page is their index, so every action lands back + # there and the admin always sees a layer in the context of its siblings. + class TemplateStepsController < BaseController + before_action :set_track_template + before_action :set_template_step, only: %i[edit update destroy] + + def new + # Layers are 1-based and ordered; the next one is almost always what an + # admin means by "add a step", so offer it rather than making them count. + @template_step = @track_template.template_steps.build( + layer: (@track_template.template_steps.maximum(:layer) || 0) + 1, + approvals_required: "any" + ) + end + + def create + @template_step = @track_template.template_steps.build(template_step_params) + if @template_step.save + redirect_to admin_track_template_path(@track_template), notice: "Step added." + else + render :new, status: :unprocessable_entity + end + end + + def edit + end + + def update + if @template_step.update(template_step_params) + redirect_to admin_track_template_path(@track_template), notice: "Step updated." + else + render :edit, status: :unprocessable_entity + end + end + + def destroy + @template_step.destroy + redirect_to admin_track_template_path(@track_template), notice: "Step deleted." + end + + private + + def set_track_template + @track_template = TrackTemplate.find(params[:track_template_id]) + end + + # Scoped through the parent so an id from another template is a 404 rather + # than a cross-template edit. + def set_template_step + @template_step = @track_template.template_steps.find(params[:id]) + end + + # `approval_engine_track_template_id` is deliberately absent: the parent + # comes from the nested route, never from the form. + def template_step_params + params.require(:template_step).permit(:name, :layer, :assigned_group, :approvals_required, :timeout_after) + end + end + end +end diff --git a/app/controllers/approval_engine/admin/track_templates_controller.rb b/app/controllers/approval_engine/admin/track_templates_controller.rb new file mode 100644 index 0000000..f527dee --- /dev/null +++ b/app/controllers/approval_engine/admin/track_templates_controller.rb @@ -0,0 +1,83 @@ +module ApprovalEngine + module Admin + # Runtime CRUD for the blueprint an approval is stamped from. This owns + # *what* happens; TriggerRulesController owns *when*. + class TrackTemplatesController < BaseController + PAGE_LIMIT = 100 + + before_action :set_track_template, only: %i[show edit update destroy] + + def index + @tenant_id = params[:tenant_id].presence + scope = TrackTemplate.order(:tenant_id, :name) + scope = scope.for_tenant(@tenant_id) if @tenant_id + @track_templates = scope.limit(PAGE_LIMIT).to_a + @total = scope.count + @tenant_ids = TrackTemplate.distinct.order(:tenant_id).pluck(:tenant_id) + # One grouped query for all the row counts, instead of N `.template_steps.size`. + ids = @track_templates.map(&:id) + @step_counts = TemplateStep.where(approval_engine_track_template_id: ids) + .group(:approval_engine_track_template_id).count + @rule_counts = TriggerRule.where(approval_engine_track_template_id: ids) + .group(:approval_engine_track_template_id).count + end + + def show + @template_steps = @track_template.template_steps + @trigger_rules = @track_template.trigger_rules.by_priority + end + + def new + @track_template = TrackTemplate.new(tenant_id: default_tenant_id, status: "draft") + end + + def create + @track_template = TrackTemplate.new(track_template_params) + if @track_template.save + redirect_to admin_track_template_path(@track_template), notice: "Template created." + else + render :new, status: :unprocessable_entity + end + end + + def edit + end + + def update + if @track_template.update(track_template_params) + redirect_to admin_track_template_path(@track_template), notice: "Template updated." + else + render :edit, status: :unprocessable_entity + end + end + + def destroy + # Deleting a template cascades to its rules, and deleting a rule nullifies + # the provenance column on every approval it ever routed — the ledger + # would quietly forget which rule started it. Archiving stops the + # template firing and keeps that history, so history wins over the delete. + if routed_approvals? + redirect_to admin_track_template_path(@track_template), + alert: "This template has already routed approvals. Archive it instead of deleting it." + else + @track_template.destroy + redirect_to admin_track_templates_path, notice: "Template deleted." + end + end + + private + + def set_track_template + @track_template = TrackTemplate.find(params[:id]) + end + + def routed_approvals? + Approval.where(approval_engine_trigger_rule_id: @track_template.trigger_rules.select(:id)).exists? + end + + def track_template_params + params.require(:track_template).permit(:tenant_id, :name, :status) + end + end + end +end diff --git a/app/controllers/approval_engine/admin/trigger_rules_controller.rb b/app/controllers/approval_engine/admin/trigger_rules_controller.rb new file mode 100644 index 0000000..d818508 --- /dev/null +++ b/app/controllers/approval_engine/admin/trigger_rules_controller.rb @@ -0,0 +1,177 @@ +module ApprovalEngine + module Admin + # Runtime CRUD for routing rules: which template an event picks, and under + # what condition, is data an admin edits rather than code someone deploys. + class TriggerRulesController < BaseController + PAGE_LIMIT = 100 + # "Add another condition" without JavaScript is just spare rows. + BLANK_CONDITION_ROWS = 3 + # jsonb will accept a novel; cap it so a paste can't slow every event. + MAX_CONDITION_BYTES = 8_192 + + before_action :set_track_template, except: :index + before_action :set_trigger_rule, only: %i[edit update destroy] + + def index + # The order the evaluator resolves them in. + @tenant_id = params[:tenant_id].presence + scope = TriggerRule.includes(:track_template).order(:event_name).by_priority + scope = scope.for_tenant(@tenant_id) if @tenant_id + @trigger_rules = scope.limit(PAGE_LIMIT).to_a + @total = scope.count + end + + def new + @trigger_rule = @track_template.trigger_rules.build(tenant_id: @track_template.tenant_id, priority: 0, active: true) + prepare_condition_editor + end + + def create + @trigger_rule = @track_template.trigger_rules.build(rule_attributes) + @trigger_rule.condition = submitted_condition + if @condition_error.nil? && @trigger_rule.save + redirect_to admin_track_template_path(@track_template), notice: "Rule created." + else + @trigger_rule.errors.add(:condition, @condition_error) if @condition_error + prepare_condition_editor + render :new, status: :unprocessable_entity + end + end + + def edit + prepare_condition_editor + end + + def update + @trigger_rule.assign_attributes(rule_attributes) + @trigger_rule.condition = submitted_condition + if @condition_error.nil? && @trigger_rule.save + redirect_to admin_track_template_path(@track_template), notice: "Rule updated." + else + @trigger_rule.errors.add(:condition, @condition_error) if @condition_error + prepare_condition_editor + render :edit, status: :unprocessable_entity + end + end + + def destroy + # The provenance FK is `on_delete: :nullify`, so deleting a rule + # silently erases which rule routed every approval it spawned. + # Deactivating keeps the history — as the template guard and + # FlowDefinition#reconcile_rules! both do. + if routed_approvals? + redirect_to admin_track_template_path(@track_template), + alert: "This rule has already routed approvals. Untick Active instead — " \ + "deleting it would erase which rule started them." + else + @trigger_rule.destroy + redirect_to admin_track_template_path(@track_template), notice: "Rule deleted." + end + end + + private + + def set_track_template + @track_template = TrackTemplate.find(params[:track_template_id]) + end + + def set_trigger_rule + @trigger_rule = @track_template.trigger_rules.find(params[:id]) + end + + def routed_approvals? + Approval.where(approval_engine_trigger_rule_id: @trigger_rule.id).exists? + end + + # Tolerant of a missing key: `new`/`edit` render the same editor. + def trigger_rule_params + @trigger_rule_params ||= params.fetch(:trigger_rule, ActionController::Parameters.new) + .permit(:tenant_id, :event_name, :priority, :active, + :condition_mode, :condition_json, + conditions: [ :field, :operator, :value ]) + end + + # Assembled, never mass-assigned — the form submits triples or raw JSON, + # and neither is the column's value. + def rule_attributes + trigger_rule_params.except(:condition_mode, :condition_json, :conditions) + end + + def advanced_mode? + trigger_rule_params[:condition_mode] == "advanced" + end + + # nil with `@condition_error` set when unreadable, so the action + # re-renders the form rather than blowing up on the admin. + def submitted_condition + condition = advanced_mode? ? parsed_raw_condition : built_condition + return if condition.nil? + + # Capped here rather than on the raw path alone: the simple builder can + # assemble an oversized condition too, and every rule is evaluated on + # the hot path of a host record's save. + if condition.to_json.bytesize > MAX_CONDITION_BYTES + @condition_error = "is too large (max #{MAX_CONDITION_BYTES} bytes)" + return + end + + condition + end + + def parsed_raw_condition + json = trigger_rule_params[:condition_json].to_s + parsed = JSON.parse(json) + # JSON Logic evaluates a non-object as a literal, so an array or bare + # number would match *every* event of its name. + raise ArgumentError, %(must be a JSON Logic object, e.g. {">": [{"var": "amount"}, 10000]}) unless parsed.is_a?(Hash) + + parsed + rescue JSON::ParserError => e + @condition_error = "isn't valid JSON: #{e.message}" + nil + rescue ArgumentError => e + @condition_error = e.message + nil + end + + def built_condition + types = attribute_types(trigger_rule_params[:event_name]) + conditions = submitted_condition_rows.map do |row| + ApprovalEngine::Condition.new( + field: row[:field], operator: row[:operator], value: row[:value], type: types[row[:field].to_s] + ) + end + ApprovalEngine::Condition.to_json_logic(conditions) + rescue ArgumentError => e + # Condition raises for a blank field, bad operator, or no rows. + @condition_error = e.message + nil + end + + # Blank rows are the JavaScript-free "add another", so drop them. + def submitted_condition_rows + Array(trigger_rule_params[:conditions]).reject { |row| row[:field].blank? } + end + + # `Condition.parse` returns nil for an AST the simple form can't hold — + # the signal to open the raw editor rather than mangle a hand-written rule. + def prepare_condition_editor + stored = @trigger_rule.condition + parsed = ApprovalEngine::Condition.parse(stored) + @condition_simple = stored.blank? || !parsed.nil? + @condition_mode = trigger_rule_params[:condition_mode].presence || (@condition_simple ? "simple" : "advanced") + @condition_json = trigger_rule_params[:condition_json].presence || + (stored.present? ? JSON.pretty_generate(stored) : "") + @condition_rows = submitted_condition_rows.presence || stored_condition_rows(parsed) + @blank_rows = BLANK_CONDITION_ROWS + end + + def stored_condition_rows(parsed) + Array(parsed).map do |condition| + { "field" => condition.field, "operator" => condition.operator.to_s, + "value" => helpers.condition_value_for_form(condition) } + end + end + end + end +end diff --git a/app/controllers/approval_engine/approvals_controller.rb b/app/controllers/approval_engine/approvals_controller.rb index b80dfea..9bb91e1 100644 --- a/app/controllers/approval_engine/approvals_controller.rb +++ b/app/controllers/approval_engine/approvals_controller.rb @@ -1,7 +1,8 @@ module ApprovalEngine # The dashboard's approval list and detail views. Read-only by design — its # job is to surface stuck, quarantined and in-flight work without anyone - # writing SQL, not to act on it. + # writing SQL, not to act on it. Writes live in the opt-in Admin namespace, + # and edit blueprints (templates and rules) rather than approvals in flight. class ApprovalsController < ApplicationController PAGE_LIMIT = 100 diff --git a/app/helpers/approval_engine/admin/base_helper.rb b/app/helpers/approval_engine/admin/base_helper.rb new file mode 100644 index 0000000..5c81863 --- /dev/null +++ b/app/helpers/approval_engine/admin/base_helper.rb @@ -0,0 +1,43 @@ +module ApprovalEngine + module Admin + # View-side vocabulary for the rule and step editors — how a value reads in + # a form, which concerns this admin and nothing else in the engine. + module BaseHelper + # A list round-trips as a comma-separated string, as Condition reads it. + def condition_value_for_form(condition) + condition.list? ? Array(condition.value).join(", ") : condition.value.to_s + end + + # A one-line reading for the index. What the simple form can't represent + # has no honest summary, so say so rather than print half the AST. + def condition_summary(rule) + parsed = ApprovalEngine::Condition.parse(rule.condition) + return "Custom JSON Logic" if parsed.nil? + + parsed.map do |condition| + "#{condition.field} #{ApprovalEngine::Condition::LABELS[condition.operator]} #{condition_value_for_form(condition)}" + end.join(" and ") + end + + # Plus a group for an attribute nobody exposes, when the rule names one: + # that reads as a clean non-match, so surface it rather than drop it. + def field_groups_including(field) + groups = exposed_attribute_groups + return groups if field.blank? || groups.any? { |_model, options| options.any? { |_label, value| value == field.to_s } } + + [ [ "Not exposed — this rule can never match", [ [ field.to_s, field.to_s ] ] ] ] + groups.to_a + end + + # "any" and "all" are about the layer's approvers, not how many layers. + def consensus_hint + "any = one approver is enough · all = every approver · majority · a percentage like 60% · a count like 2" + end + + def timeout_label(seconds) + return "—" if seconds.blank? + + ActiveSupport::Duration.build(seconds.to_i).inspect + end + end + end +end diff --git a/app/helpers/approval_engine/application_helper.rb b/app/helpers/approval_engine/application_helper.rb index 94ca05d..9a86224 100644 --- a/app/helpers/approval_engine/application_helper.rb +++ b/app/helpers/approval_engine/application_helper.rb @@ -4,9 +4,18 @@ module ApplicationHelper "pending" => "is-pending", "waiting" => "is-waiting", "approved" => "is-approved", "rejected" => "is-rejected", "changes_requested" => "is-changes-requested", "cancelled" => "is-cancelled", - "quarantined" => "is-quarantined" + "quarantined" => "is-quarantined", + # Template lifecycle, which the admin renders through the same badge. + "draft" => "is-waiting", "active" => "is-approved", "archived" => "is-cancelled" }.freeze + # The admin nav exists only when the host opted in *and* the routes were + # drawn for it — a flag flipped after boot leaves the helpers undefined, + # and a host that never opts in gets exactly the 1.0 read-only page. + def admin_ui? + ApprovalEngine.config.admin_enabled && respond_to?(:admin_track_templates_path) + end + def status_badge(status) tag.span(status.to_s.tr("_", " "), class: "ae-badge #{STATUS_TONES.fetch(status.to_s, "is-pending")}") end diff --git a/app/jobs/approval_engine/process_outbox_job.rb b/app/jobs/approval_engine/process_outbox_job.rb index 0bbf7c0..89ee370 100644 --- a/app/jobs/approval_engine/process_outbox_job.rb +++ b/app/jobs/approval_engine/process_outbox_job.rb @@ -45,6 +45,17 @@ def perform(outbox_event_id) def deliver(event) run_host_callbacks(event) broadcast_notification(event) + send_notifications(event) + end + + # Built-in mail is a courtesy layered on top of the delivery contract, not + # part of it. A missing template or an unreachable actor must never fail the + # event, because a retry would re-run the host callbacks above — turning a + # mailer problem into a double-disbursed invoice. + def send_notifications(event) + Notifier.deliver(event) + rescue StandardError => e + Rails.logger&.warn("[ApprovalEngine] notification skipped for outbox event #{event.id}: #{e.class}: #{e.message}") end # Invoke the matching host callback if the target model chose to define one. @@ -55,6 +66,8 @@ def run_host_callbacks(event) return unless target case event.event_name + when "step.activated" then try_call(target, :after_step_activated, record) + when "step.reminded" then try_call(target, :on_step_reminder, record) when "step.approved" then try_call(target, :after_step_approved, record) when "step.rejected" then try_call(target, :after_step_rejected, record) when "step.changes_requested" then try_call(target, :after_step_changes_requested, record) diff --git a/app/jobs/approval_engine/reminder_sweep_job.rb b/app/jobs/approval_engine/reminder_sweep_job.rb new file mode 100644 index 0000000..19792fd --- /dev/null +++ b/app/jobs/approval_engine/reminder_sweep_job.rb @@ -0,0 +1,17 @@ +module ApprovalEngine + # Nudges approvers who have gone quiet longer than `config.reminder_after`. + # Schedule it with whatever recurring mechanism you already run: + # + # ApprovalEngine::ReminderSweepJob.perform_later # all tenants + # ApprovalEngine::ReminderSweepJob.perform_later(tenant_id: account.id) + # + # Each step is nudged at most once, and it is a no-op until `reminder_after` + # is set, so running it often and scheduling it early are both harmless. + class ReminderSweepJob < ApplicationJob + queue_as { ApprovalEngine.config.outbox_queue } + + def perform(tenant_id: nil) + Step.sweep_reminders!(tenant_id: tenant_id) + end + end +end diff --git a/app/mailers/approval_engine/notification_mailer.rb b/app/mailers/approval_engine/notification_mailer.rb new file mode 100644 index 0000000..b45e849 --- /dev/null +++ b/app/mailers/approval_engine/notification_mailer.rb @@ -0,0 +1,88 @@ +module ApprovalEngine + # The built-in notifications. Dispatched by Notifier from inside + # ProcessOutboxJob, never a model callback, so a message is only built for + # state that has already committed. + # + # Every action takes an explicit `to:` and assigns only Strings and Integers, + # so no view calls a method on a host record — that is what makes these + # templates work for any application. + # + # To restyle one, drop your own copy at + # app/views/approval_engine/notification_mailer/.html.erb. Override + # *both* formats together: Action Mailer takes an action's templates from the + # first view path holding any of them, so a lone .html.erb silences the + # engine's .text.erb. To replace one wholesale, subclass and point + # `config.mailer_class` at it. + class NotificationMailer < ApprovalEngine.config.parent_mailer.constantize + # Only impose our layout when not inheriting the host's mailer — their + # layout and sender are the whole point of setting `parent_mailer`. + layout "approval_engine/mailer" if superclass == ActionMailer::Base + + # Action Mailer resolves templates from `self.class.mailer_name`, so without + # this a subclass looks under its own path and every action raises + # MissingTemplate. Pinned here, a subclass inherits the engine's views; a + # host's own still win, because its view path is searched first. + default template_path: "approval_engine/notification_mailer" + + def step_activated(step, to:) + assign_step(step) + notification_mail(to: to) + end + + def step_reminder(step, to:) + assign_step(step) + # Whole hours: "sitting for two days" is the point, not the arithmetic. + @waiting_hours = (step.waiting_for / 3600.0).round if step.waiting_for + notification_mail(to: to) + end + + def step_reassigned(step, to:) + assign_step(step) + notification_mail(to: to) + end + + def changes_requested(step, to:) + assign_step(step) + entry = step.audit_logs.where(event: "changes_requested").order(:created_at).last + @comment = entry&.comment + # Name whoever actually asked for changes. A delegate acting for the + # assignee is recorded on the ledger, and it is who the reader needs. + @actor_label = ApprovalEngine.config.actor_label(entry&.actual_actor || step.assigned_actor) + notification_mail(to: to) + end + + def approval_approved(approval, to:) + assign_approval(approval) + notification_mail(to: to) + end + + def approval_rejected(approval, to:, reason: nil) + assign_approval(approval) + @reason = reason + notification_mail(to: to) + end + + private + + def assign_step(step) + @step_name = step.name + @track_name = step.track.name + @actor_label = ApprovalEngine.config.actor_label(step.assigned_actor) + assign_approval(step.track.approval) + end + + def assign_approval(approval) + @target_label = ApprovalEngine.config.target_label(approval.target) + @event_name = approval.event_name + @url = ApprovalEngine.config.approval_url(approval) + end + + # One funnel for all six. `from` is set only when configured, so inheriting + # the host's mailer keeps their sender; subjects live in the locale file. + def notification_mail(to:) + headers = { to: to, subject: default_i18n_subject(target: @target_label) } + headers[:from] = ApprovalEngine.config.mailer_from if ApprovalEngine.config.mailer_from.present? + mail(headers) + end + end +end diff --git a/app/models/approval_engine/step.rb b/app/models/approval_engine/step.rb index a708272..1ec413f 100644 --- a/app/models/approval_engine/step.rb +++ b/app/models/approval_engine/step.rb @@ -36,6 +36,11 @@ class Step < ApplicationRecord before_update :guard_immutable_transition before_save :stamp_timing + # A step becoming actionable was the one ledger event the outbox never + # carried. Two callbacks rather than one `after_save`: `status` defaults to + # "pending" in the database, so a step built pending records no change. + after_create :emit_activation, if: :pending? + after_update :emit_activation, if: -> { saved_change_to_status? && pending? } scope :waiting, -> { where(status: "waiting") } scope :pending, -> { where(status: "pending") } @@ -48,6 +53,11 @@ class Step < ApplicationRecord scope :overdue, ->(as_of = Time.current) { pending.where(timed_out_at: nil).where.not(timeout_at: nil).where("timeout_at <= ?", as_of) } + # What the reminder sweep acts on. Each step is nudged at most once, so a + # minutely sweep is as safe as a nightly one. + scope :remindable, ->(after:, as_of: Time.current) { + pending.where(reminded_at: nil).where.not(activated_at: nil).where("activated_at <= ?", as_of - after) + } # An approver's inbox: pending steps the actor may act on — assigned to them # directly *plus* those they cover via an active delegation. The scope form @@ -137,6 +147,21 @@ def time_out! self end + # A nudge, not a verdict: the only write is the stamp that stops it + # repeating. Idempotent under concurrent sweeps, like time_out!. + def remind! + track.approval.with_lock do + reload + return self unless pending? && reminded_at.nil? + + update!(reminded_at: Time.current) + record_audit(event: "reminded", by: nil, comment: nil) + emit_outbox("step.reminded") + end + + self + end + # Honest denial when an approver never acted in time: the step becomes # `expired` (a distinct terminal state — never "approved", never a human # "rejected"), with no human actor on the ledger. Resolves the surrounding @@ -168,7 +193,9 @@ def reassign!(to:, by: nil, comment: nil) end record_audit(event: "reassigned", by: by, comment: comment) - update!(assigned_actor: to) + # Restart the reminder clock: otherwise whoever is handed a stalled + # approval is the one person the sweep never reminds. + update!(assigned_actor: to, reminded_at: nil) emit_outbox("step.reassigned") end self @@ -192,6 +219,23 @@ def self.sweep_timeouts!(tenant_id: nil) swept end + # Nudge every step quiet longer than `config.reminder_after`; a no-op while + # that is nil. Returns the number nudged. + # One step raising is skipped, not fatal — as the timeout sweep. + def self.sweep_reminders!(tenant_id: nil, after: ApprovalEngine.config.reminder_after) + return 0 if after.blank? + + scope = tenant_id ? remindable(after: after).for_tenant(tenant_id) : remindable(after: after) + reminded = 0 + scope.find_each do |step| + step.remind! + reminded += 1 + rescue StandardError => e + Rails.logger&.warn("[ApprovalEngine] reminder sweep skipped step #{step.id}: #{e.class}: #{e.message}") + end + reminded + end + private # The shared transition pipeline. We lock the *approval* — the aggregate @@ -234,6 +278,10 @@ def emit_outbox(event_name) OutboxEvent.create!(tenant_id: tenant_id, event_name: event_name, record: self) end + def emit_activation + emit_outbox("step.activated") + end + def guard_immutable_transition return unless will_save_change_to_status? diff --git a/app/models/approval_engine/template_step.rb b/app/models/approval_engine/template_step.rb index 3307290..b0c40c1 100644 --- a/app/models/approval_engine/template_step.rb +++ b/app/models/approval_engine/template_step.rb @@ -7,6 +7,10 @@ class TemplateStep < ApplicationRecord validates :name, :assigned_group, presence: true validates :layer, numericality: { greater_than: 0 } + # A zero or negative deadline expires the step the moment it opens. + # FlowDefinition already refuses one; this is the same guard for rows the + # admin writes, so both doors into this table agree. + validates :timeout_after, numericality: { greater_than: 0 }, allow_nil: true validate :approvals_required_is_valid scope :ordered, -> { order(:layer) } diff --git a/app/services/approval_engine/flow_definition.rb b/app/services/approval_engine/flow_definition.rb new file mode 100644 index 0000000..eb50244 --- /dev/null +++ b/app/services/approval_engine/flow_definition.rb @@ -0,0 +1,453 @@ +require "digest" + +module ApprovalEngine + # The declarative flow builder behind `ApprovalEngine.define_flow`. Reconciles + # a block of `on` / `step` / `parallel` declarations into ordinary + # TrackTemplate / TemplateStep / TriggerRule rows. + # + # Keyed on (tenant_id, name), so re-running is safe. Fails loudly at definition + # time — this is deploy-time setup code, unlike runtime rule evaluation, which + # fails closed and silent. In-flight approvals are never affected: + # ApprovalBuilder copies template facts onto the ledger rather than holding a + # reference to them. + class FlowDefinition + class DefinitionError < ApprovalEngine::Error; end + + # TriggerRule requires a condition, so "always" is spelled as a tautology. + ALWAYS = { "==" => [ 1, 1 ] }.freeze + + # Listed rather than taken as keywords: `when` is a Ruby keyword, so + # `def on(event, when: nil)` parses but the value is unreachable. + RULE_OPTIONS = %i[when priority active].freeze + + # Verbatim from TemplateStep's validation, so the message never diverges. + CONSENSUS_HINT = "must be :any, :all, :majority, a percentage like \"60%\", or a positive integer".freeze + + # Not `Step`/`Rule`: nothing here should read as the ledger's Step. + DeclaredStep = Struct.new(:name, :group, :layer, :approvals_required, :timeout_after, keyword_init: true) + DeclaredRule = Struct.new(:event_name, :condition, :priority, :active, keyword_init: true) + + def self.run(name:, tenant:, model:, status:, timeout_after:, &block) + definition = new(name: name, tenant: tenant, model: model, status: status, timeout_after: timeout_after) + definition.instance_eval(&block) if block + definition.commit! + end + + # As `Approvable#approval_tenant_id`, plus `.to_s` — tenant_id is a string + # column the reconciler compares in Ruby. + def self.tenant_id_for(tenant) + tenant.respond_to?(:id) ? tenant.id.to_s : tenant.to_s + end + + def initialize(name:, tenant:, model:, status:, timeout_after:) + @name = name.to_s + guard_name! + + @tenant_id = resolve_tenant!(tenant) + @model = model + @status = status.to_s + @timeout_after = seconds(timeout_after, "timeout_after") + @declared_steps = [] + @declared_rules = [] + @layer = 0 + + guard_status! + guard_model! + end + + # --- block DSL ------------------------------------------------------- + + # One layer. `group` goes to the host's `resolve_approval_group`, which the + # engine expands into one step per actor returned. + def step(name, group:, approvals_required: nil, timeout_after: nil) + if @open_layer && approvals_required + raise DefinitionError, + "step #{name.to_s.inspect} sets approvals_required inside a parallel block. Every step " \ + "in a layer shares one consensus policy (the engine reads a single spec per layer), so " \ + "declare it on the block: parallel(approvals_required: #{approvals_required.inspect})." + end + + spec = @open_layer_consensus || approvals_required || :any + guard_step_name!(name) + guard_group!(name, group) + guard_consensus!("step #{name.to_s.inspect}", spec) + + @declared_steps << DeclaredStep.new( + name: name.to_s, + group: group.to_s, + layer: @open_layer || next_layer!, + approvals_required: spec.to_s, + timeout_after: seconds(timeout_after, "step #{name.to_s.inspect}: timeout_after") || @timeout_after + ) + end + + # Steps that open together, as one layer. + # + # Consensus belongs on the block, never the steps inside: `Track#tally_for` + # reads one spec per layer and counts every actor in it. That also makes the + # count flat across the layer — `:all` over a 3-person and a 2-person group + # needs all five. "One from each group" is parallel *tracks* instead: + # `run_approval!(templates: [...])`. Defaults to `:all`, not the engine's + # `:any`, because "Legal and IT review" means both. + def parallel(approvals_required: :all, &block) + raise DefinitionError, "parallel needs a block of steps." unless block + raise DefinitionError, "parallel blocks can't be nested." if @open_layer + + guard_consensus!("parallel", approvals_required) + layer = next_layer! + @open_layer = layer + @open_layer_consensus = approvals_required + + begin + instance_eval(&block) + ensure + @open_layer = @open_layer_consensus = nil + end + + declared_here = @declared_steps.count { |declared| declared.layer == layer } + + if declared_here.zero? + raise DefinitionError, "a parallel block in flow #{@name.inspect} declares no steps — it would leave " \ + "an empty layer nothing can resolve." + end + + guard_absolute_count_across_groups!(approvals_required, declared_here) + end + + # A count reads as flat across the layer, but ApprovalBuilder validates it + # against each group's own members — so `parallel(approvals_required: 2)` + # over two one-person groups raises at every build. Refuse it here instead of + # writing a template that only fails later. + def guard_absolute_count_across_groups!(spec, step_count) + return unless step_count > 1 + return unless /\A\d+\z/.match?(spec.to_s) + + raise DefinitionError, + "parallel in flow #{@name.inspect} sets approvals_required #{spec.to_s.inspect} across " \ + "#{step_count} groups. A plain count is checked against each group's own members when an " \ + "approval is built, so this would raise at approval time unless every group had at least " \ + "#{spec} people. Use :all, :any, :majority or a percentage, which hold however the groups resolve." + end + + # A rule routing an event to this flow. `event` is an event name or a + # lifecycle symbol resolved through the model. May be called more than once. + def on(event, **options) + unknown = options.keys - RULE_OPTIONS + if unknown.any? + raise DefinitionError, "on: unknown option(s) #{unknown.inspect}. It takes when:, priority: and active:." + end + + event_name = resolve_event_name(event) + condition = compile_condition(event_name, options[:when]) + priority = integer_priority(options.fetch(:priority, 0)) + + if @declared_rules.any? { |rule| rule.event_name == event_name && rule.priority == priority } + raise DefinitionError, "flow #{@name.inspect} declares two rules for #{event_name.inspect} at " \ + "priority #{priority}. Give them different priorities so the order they " \ + "are tried in is explicit." + end + + warn_unrouted_lifecycle(event) + @declared_rules << DeclaredRule.new( + event_name: event_name, condition: condition, priority: priority, active: options.fetch(:active, true) + ) + end + + # --- reconciliation -------------------------------------------------- + + def commit! + guard_steps_declared! + guard_unique_step_names! + guard_actor_class! + + ActiveRecord::Base.transaction do + lock! + template = upsert_template! + reconcile_steps!(template) + reconcile_rules!(template) + template + end + end + + private + + # Two concurrent deploys both running db/seeds.rb would race + # upsert_template! into duplicate templates. Scoped per flow. + def lock! + key = Digest::SHA256.digest("approval_engine.flow:#{@tenant_id}:#{@name}").unpack1("q>") + # Injection-free: the key is an Integer, and neither tenant nor name + # reaches the SQL. `execute` because the function returns void. + ActiveRecord::Base.connection.execute("SELECT pg_advisory_xact_lock(#{key.to_i})") + end + + # No unique index on (tenant_id, name): an existing install may already hold + # duplicates, so adding one could break a production migrate. + def upsert_template! + matches = TrackTemplate.for_tenant(@tenant_id).where(name: @name).order(:created_at).to_a + + if matches.size > 1 + raise DefinitionError, + "tenant #{@tenant_id.inspect} already has #{matches.size} templates named #{@name.inspect} " \ + "(#{matches.map(&:id).join(", ")}). define_flow can't tell which one it owns — archive or " \ + "rename the extras first." + end + + template = matches.first || TrackTemplate.new(tenant_id: @tenant_id, name: @name) + template.update!(status: @status) + template + end + + # Matched by name and updated in place, so ids an admin UI links to survive. + # Nothing outside the template references a template step, so rewriting these + # cannot disturb an in-flight approval. A rename reads as remove plus add. + def reconcile_steps!(template) + # group_by, not index_by: duplicate names are possible in a hand-built + # template, and index_by would strand all but the last. + existing = template.template_steps.group_by(&:name) + + @declared_steps.each do |declared| + same_name = existing.delete(declared.name) || [] + record = same_name.shift || template.template_steps.new(name: declared.name) + same_name.each(&:destroy!) + + record.update!( + layer: declared.layer, + assigned_group: declared.group, + approvals_required: declared.approvals_required, + timeout_after: declared.timeout_after + ) + end + + existing.each_value { |orphans| orphans.each(&:destroy!) } + template.template_steps.reset + end + + # Deactivated, never destroyed: the provenance FK is `on_delete: :nullify`, + # so deleting a rule would silently erase which rule routed every approval it + # ever spawned. Asymmetric with steps on purpose — a template with no steps + # is a bug, but code-owned steps with admin-owned routing is legitimate, so a + # block with no `on` leaves rules alone. + def reconcile_rules!(template) + return if @declared_rules.empty? + + existing = template.trigger_rules.group_by { |rule| [ rule.event_name, rule.priority ] } + + @declared_rules.each do |declared| + same_key = existing.delete([ declared.event_name, declared.priority ]) || [] + record = same_key.shift || template.trigger_rules.new + same_key.each { |dupe| dupe.update!(active: false) } + + record.update!( + tenant_id: @tenant_id, + event_name: declared.event_name, + condition: declared.condition, + priority: declared.priority, + active: declared.active + ) + end + + existing.each_value { |retired| retired.each { |rule| rule.update!(active: false) } } + template.trigger_rules.reset + end + + # --- declaration helpers --------------------------------------------- + + def next_layer! + @layer += 1 + end + + def resolve_tenant!(tenant) + tenant = ApprovalEngine.current_tenant if tenant.nil? + + if tenant.nil? || (tenant.respond_to?(:empty?) && tenant.empty?) + raise DefinitionError, "define_flow needs a tenant: — pass one, or set config.current_tenant_method." + end + + self.class.tenant_id_for(tenant) + end + + def resolve_event_name(event) + return event.to_s unless event.is_a?(Symbol) + + unless @model + raise DefinitionError, "on(#{event.inspect}) needs the model: option so the flow can ask it for the " \ + "conventional event name — or pass the event name as a string." + end + + begin + @model.approval_event_name(event) + rescue KeyError + raise DefinitionError, "on(#{event.inspect}) isn't a lifecycle #{@model} understands — it knows " \ + "#{ApprovalEngine::Approvable::LIFECYCLE_EVENTS.keys.inspect}. Pass a string " \ + "for any other event name." + end + end + + # Wrap Condition.from's ArgumentError so callers rescue one error class. + def compile_condition(event_name, clause) + return ALWAYS if clause.blank? + + condition = ApprovalEngine::Condition.from(clause, types: condition_types) + guard_exposed_attributes!(event_name, condition) + condition + rescue ArgumentError => e + raise DefinitionError, "flow #{@name.inspect}: the when: for #{event_name.inspect} is invalid — #{e.message}." + end + + # Cast as the payload will be, so a rule never compares a number to a string + # and silently never matches. + def condition_types + return Hash.new(:raw) unless @model.respond_to?(:approval_exposure) + + @model.approval_exposure.attributes.each_with_object(Hash.new(:raw)) do |(attribute_name, attribute), types| + types[attribute_name] = attribute.type + end + end + + # Every attribute a condition reads. Condition.parse can't serve: it returns + # nil for an `or`, which is exactly where a typo hides. + def condition_vars(node) + case node + when Hash then node.key?("var") ? Array(var_name(node["var"])) : node.values.flat_map { |value| condition_vars(value) } + when Array then node.flat_map { |value| condition_vars(value) } + else [] + end + end + + # `var` is also written `["amount", 0]` for a default and `"a.b"` for nested + # data. Comparing either verbatim rejected valid rules. An empty operand + # means the whole payload, so it names nothing. + def var_name(operand) + key = Array(operand).first.to_s + return nil if key.empty? + + key.split(".").first + end + + def integer_priority(value) + Integer(value) + rescue ArgumentError, TypeError + raise DefinitionError, "on: priority must be an integer, got #{value.inspect}." + end + + # Durations and plain counts both land as Integer seconds. A non-positive + # deadline would expire the step as it opened, so it is rejected. + def seconds(value, label) + return nil if value.nil? + + converted = value.is_a?(Numeric) || value.is_a?(ActiveSupport::Duration) ? value.to_i : nil + return converted if converted&.positive? + + raise DefinitionError, "#{label} must be a positive number of seconds (or a duration like 2.days), " \ + "got #{value.inspect}." + end + + # --- guards ---------------------------------------------------------- + + def guard_name! + raise DefinitionError, "define_flow needs a name." if @name.empty? + end + + def guard_status! + return if TrackTemplate::STATUSES.include?(@status) + + raise DefinitionError, "define_flow status #{@status.inspect} is not one of " \ + "#{TrackTemplate::STATUSES.join(", ")}." + end + + def guard_model! + return if @model.nil? + return if @model.respond_to?(:approval_exposure) + + raise DefinitionError, "model: #{@model} hasn't called has_approvals, so it has no exposed attributes " \ + "or event names to check against." + end + + def guard_step_name!(name) + return if name.present? + + raise DefinitionError, "step needs a name — it is how a step is identified across re-runs." + end + + def guard_group!(step_name, group) + if group.blank? + raise DefinitionError, "step #{step_name.to_s.inspect} needs a group: — the name your " \ + "resolve_approval_group turns into actors." + end + + allowed = ApprovalEngine.config.approval_groups + return if allowed.nil? + + known = Array(allowed).map(&:to_s) + return if known.include?(group.to_s) + + raise DefinitionError, "step #{step_name.to_s.inspect} uses group #{group.to_s.inspect}, which isn't " \ + "in config.approval_groups (#{known.join(", ")})." + end + + # Consensus is what the model and the check constraint use, so the DSL + # cannot invent a second spelling. + def guard_consensus!(subject, spec) + return if Consensus.valid?(spec) + + raise DefinitionError, "#{subject} has approvals_required #{spec.to_s.inspect} — #{CONSENSUS_HINT}." + end + + def guard_steps_declared! + return if @declared_steps.any? + + raise DefinitionError, "flow #{@name.inspect} declares no steps. An approval built from it would " \ + "create a track with nothing to approve and sit pending forever." + end + + def guard_unique_step_names! + duplicate = @declared_steps.map(&:name).tally.find { |_name, count| count > 1 } + return unless duplicate + + raise DefinitionError, "flow #{@name.inspect} declares two steps named #{duplicate.first.inspect}. " \ + "Step names identify a step across re-runs, so they have to be unique within a flow." + end + + # ApprovalBuilder's check, brought forward — a missing resolver is certain, + # not possible, so the first approval shouldn't be the one to find out. + def guard_actor_class! + klass = begin + ApprovalEngine.config.actor_class_constant + rescue NameError => e + raise DefinitionError, "ApprovalEngine.config.actor_class is " \ + "#{ApprovalEngine.config.actor_class.inspect}, which doesn't resolve to a " \ + "loaded class (#{e.message})." + end + + return if klass.respond_to?(:resolve_approval_group) + + raise DefinitionError, "#{klass} must define `self.resolve_approval_group(group_name, target)`." + end + + # An unexposed var is a clean non-match at runtime, so the rule silently + # never fires — right behaviour, wrong thing to discover in production. + def guard_exposed_attributes!(event_name, condition) + return unless @model.respond_to?(:approval_exposure) + + exposed = @model.approval_exposure.attributes.keys + unknown = condition_vars(condition).uniq - exposed + return if unknown.empty? + + raise DefinitionError, + "flow #{@name.inspect}: the rule for #{event_name.inspect} reads #{unknown.join(", ")}, which " \ + "#{@model} doesn't expose (it exposes #{exposed.join(", ")}). Add it to exposes_for_approval or " \ + "fix the name — an unexposed var reads as a clean non-match, so the rule would never fire." + end + + # Not fatal: plenty of hosts fire run_approval!(event:) themselves. + def warn_unrouted_lifecycle(event) + return unless event.is_a?(Symbol) + return unless @model.respond_to?(:approval_trigger_events) + return if @model.approval_trigger_events.include?(event) + + Rails.logger&.warn("[ApprovalEngine] flow #{@name.inspect} routes #{@model}.#{event}, but #{@model} " \ + "declares has_approvals(on: #{@model.approval_trigger_events.inspect}) — nothing " \ + "will fire it automatically.") + end + end +end diff --git a/app/services/approval_engine/notifier.rb b/app/services/approval_engine/notifier.rb new file mode 100644 index 0000000..61fb807 --- /dev/null +++ b/app/services/approval_engine/notifier.rb @@ -0,0 +1,107 @@ +module ApprovalEngine + # Turns one relayed outbox event into one piece of mail — the only place that + # knows which event produces which notification, who gets it, and whether it + # is on at all. Sends nothing until `config.notifications_enabled` is true. + class Notifier + # Step notifications go to the assignee the engine already knows; the + # approval-level ones go wherever `config.approval_recipients` says, + # because the engine knows who approves but not who asked. + EVENTS = { + "step.activated" => :step_activated, + "step.reminded" => :step_reminder, + "step.reassigned" => :step_reassigned, + "step.changes_requested" => :changes_requested, + "approval.approved" => :approval_approved, + "approval.rejected" => :approval_rejected + }.freeze + + ACTOR_ADDRESSED = %i[step_activated step_reminder step_reassigned].freeze + + def self.deliver(event) + new(event).deliver + end + + def initialize(event) + @event = event + @notification = EVENTS[event.event_name] + end + + def deliver + return unless notification && ApprovalEngine.config.notification_enabled?(notification) + return unless record + + addresses = recipient_addresses + if addresses.empty? + Rails.logger&.warn("[ApprovalEngine] no recipient address for #{@event.event_name} on #{record.class}##{record.id} — notification skipped") + return + end + + warn_without_sender + addresses.each { |address| enqueue(address) } + end + + private + + attr_reader :notification + + def record + @event.record + end + + # Enqueued, not sent: a dead mail server must retry the *mail*, never the + # outbox event, which would re-run the host's callbacks with it. + def enqueue(address) + message(address).deliver_later(**delivery_options) + end + + def message(address) + mailer = ApprovalEngine.config.mailer + + if notification == :approval_rejected + mailer.approval_rejected(record, to: address, reason: @event.error_payload) + else + mailer.public_send(notification, record, to: address) + end + end + + # Without a sender the failure surfaces deep in the delivery job, far from + # the missing setting. Name it here; still enqueue, because dropping what + # the host asked for is worse than a job their queue reports on. + def warn_without_sender + return if ApprovalEngine.config.mailer_from.present? + return if ApprovalEngine.config.mailer.default_params[:from].present? + + Rails.logger&.warn("[ApprovalEngine] notifications are enabled but no From address is configured — set config.mailer_from, or point config.parent_mailer at a mailer with a `default from:`") + end + + def delivery_options + queue = ApprovalEngine.config.mailer_queue + queue ? { queue: queue } : {} + end + + def recipient_addresses + Array(recipients).filter_map { |actor| ApprovalEngine.config.actor_email(actor) }.uniq + end + + # A delegate can act on the step (`Step.actionable_by` includes them), so a + # notification that reaches only the assignee tells the wrong person. Both + # are mailed: the assignee stays informed, the delegate can act. + def recipients + if ACTOR_ADDRESSED.include?(notification) + [ record.assigned_actor, *active_delegates_for(record) ] + else + ApprovalEngine.config.approval_recipients_for(approval) + end + end + + def active_delegates_for(step) + return [] if step.assigned_actor.nil? + + Delegation.active_for(step.assigned_actor, tenant_id: step.tenant_id).map(&:delegatee) + end + + def approval + record.is_a?(Approval) ? record : record.approval + end + end +end diff --git a/app/views/approval_engine/admin/shared/_errors.html.erb b/app/views/approval_engine/admin/shared/_errors.html.erb new file mode 100644 index 0000000..d94c9f7 --- /dev/null +++ b/app/views/approval_engine/admin/shared/_errors.html.erb @@ -0,0 +1,10 @@ +<% if record.errors.any? %> +
+ <%= pluralize(record.errors.count, "problem") %> stopped this from being saved: + +
+<% end %> diff --git a/app/views/approval_engine/admin/template_steps/_form.html.erb b/app/views/approval_engine/admin/template_steps/_form.html.erb new file mode 100644 index 0000000..642312a --- /dev/null +++ b/app/views/approval_engine/admin/template_steps/_form.html.erb @@ -0,0 +1,44 @@ +<%= form_with model: [ :admin, @track_template, @template_step ], local: true, class: "ae-form" do |form| %> + <%= render "approval_engine/admin/shared/errors", record: @template_step %> + +
+
+ <%= form.label :name %> + <%= form.text_field :name %> +
+ +
+ <%= form.label :layer %> + <%= form.number_field :layer, min: 1 %> +

Layers run in order; every step sharing a layer runs at once.

+
+
+ +
+ <%= form.label :assigned_group %> + <%= form.text_field :assigned_group %> +

Passed to your actor class's resolve_approval_group, which decides who that name means.

+
+ +
+ <%= form.label :approvals_required %> + <%= form.text_field :approvals_required, list: "ae-consensus-options" %> + + <% %w[any all majority 60% 2].each do |option| %> + + <% end %> + +

<%= consensus_hint %>

+
+ +
+ <%= form.label :timeout_after, "Timeout after (seconds)" %> + <%= form.number_field :timeout_after, min: 0 %> +

Leave blank for no deadline. 172800 is two days.

+
+ +
+ <%= form.submit "Save step", class: "ae-btn" %> + <%= link_to "Cancel", admin_track_template_path(@track_template), class: "ae-btn is-secondary" %> +
+<% end %> diff --git a/app/views/approval_engine/admin/template_steps/edit.html.erb b/app/views/approval_engine/admin/template_steps/edit.html.erb new file mode 100644 index 0000000..2cf1984 --- /dev/null +++ b/app/views/approval_engine/admin/template_steps/edit.html.erb @@ -0,0 +1,4 @@ +<% content_for :title, "Edit step" %> +<%= link_to "← #{@track_template.name}", admin_track_template_path(@track_template), class: "ae-back" %> +

Edit <%= @template_step.name %>

+<%= render "form" %> diff --git a/app/views/approval_engine/admin/template_steps/new.html.erb b/app/views/approval_engine/admin/template_steps/new.html.erb new file mode 100644 index 0000000..1e9b10c --- /dev/null +++ b/app/views/approval_engine/admin/template_steps/new.html.erb @@ -0,0 +1,4 @@ +<% content_for :title, "New step" %> +<%= link_to "← #{@track_template.name}", admin_track_template_path(@track_template), class: "ae-back" %> +

New step

+<%= render "form" %> diff --git a/app/views/approval_engine/admin/track_templates/_form.html.erb b/app/views/approval_engine/admin/track_templates/_form.html.erb new file mode 100644 index 0000000..e9a262b --- /dev/null +++ b/app/views/approval_engine/admin/track_templates/_form.html.erb @@ -0,0 +1,25 @@ +<%= form_with model: [ :admin, @track_template ], local: true, class: "ae-form" do |form| %> + <%= render "approval_engine/admin/shared/errors", record: @track_template %> + +
+ <%= form.label :tenant_id %> + <%= form.text_field :tenant_id %> +

Rules are matched per tenant. A template and its rules should normally share one.

+
+ +
+ <%= form.label :name %> + <%= form.text_field :name %> +
+ +
+ <%= form.label :status %> + <%= form.select :status, ApprovalEngine::TrackTemplate::STATUSES %> +

Only active templates are ever selected by a rule — draft templates never fire, and archiving retires one without deleting the history it routed.

+
+ +
+ <%= form.submit "Save template", class: "ae-btn" %> + <%= link_to "Cancel", @track_template.persisted? ? admin_track_template_path(@track_template) : admin_track_templates_path, class: "ae-btn is-secondary" %> +
+<% end %> diff --git a/app/views/approval_engine/admin/track_templates/edit.html.erb b/app/views/approval_engine/admin/track_templates/edit.html.erb new file mode 100644 index 0000000..cd56b0e --- /dev/null +++ b/app/views/approval_engine/admin/track_templates/edit.html.erb @@ -0,0 +1,3 @@ +<% content_for :title, "Edit template" %> +

Edit <%= @track_template.name %>

+<%= render "form" %> diff --git a/app/views/approval_engine/admin/track_templates/index.html.erb b/app/views/approval_engine/admin/track_templates/index.html.erb new file mode 100644 index 0000000..8798f27 --- /dev/null +++ b/app/views/approval_engine/admin/track_templates/index.html.erb @@ -0,0 +1,41 @@ +<% content_for :title, "Templates" %> +

Templates

+ +
+ <%= link_to admin_track_templates_path, class: ("is-active" unless @tenant_id) do %> + All tenants <%= @total %> + <% end %> + <% @tenant_ids.each do |tenant_id| %> + <%= link_to admin_track_templates_path(tenant_id: tenant_id), class: ("is-active" if @tenant_id == tenant_id) do %> + <%= tenant_id %> + <% end %> + <% end %> +
+ +

<%= link_to "New template", new_admin_track_template_path, class: "ae-btn" %>

+ +<% if @track_templates.empty? %> +
No templates<%= " for tenant #{@tenant_id}" if @tenant_id %>.
+<% else %> + + + + + + <% @track_templates.each do |template| %> + + + + + + + + + + <% end %> + +
TenantNameStatusStepsRulesUpdated
<%= template.tenant_id %><%= template.name %><%= status_badge(template.status) %><%= @step_counts.fetch(template.id, 0) %><%= @rule_counts.fetch(template.id, 0) %><%= template.updated_at.to_fs(:short) %><%= link_to "Edit", admin_track_template_path(template) %>
+ <% if @total > @track_templates.size %> +

Showing the most recent <%= @track_templates.size %> of <%= @total %>.

+ <% end %> +<% end %> diff --git a/app/views/approval_engine/admin/track_templates/new.html.erb b/app/views/approval_engine/admin/track_templates/new.html.erb new file mode 100644 index 0000000..99ae8d6 --- /dev/null +++ b/app/views/approval_engine/admin/track_templates/new.html.erb @@ -0,0 +1,3 @@ +<% content_for :title, "New template" %> +

New template

+<%= render "form" %> diff --git a/app/views/approval_engine/admin/track_templates/show.html.erb b/app/views/approval_engine/admin/track_templates/show.html.erb new file mode 100644 index 0000000..c3a4efe --- /dev/null +++ b/app/views/approval_engine/admin/track_templates/show.html.erb @@ -0,0 +1,81 @@ +<% content_for :title, @track_template.name %> +<%= link_to "← All templates", admin_track_templates_path, class: "ae-back" %> + +

<%= @track_template.name %> <%= status_badge(@track_template.status) %>

+ +
+

+ Tenant <%= @track_template.tenant_id %> · + updated <%= @track_template.updated_at.to_fs(:short) %> +

+ <% if @track_template.status != "active" %> +

Only active templates are ever selected by a rule — a draft or archived template never fires.

+ <% end %> +
+ <%= link_to "Edit template", edit_admin_track_template_path(@track_template), class: "ae-btn is-secondary" %> + <%= button_to "Delete template", admin_track_template_path(@track_template), method: :delete, class: "ae-btn is-danger" %> +
+
+ +

Steps

+<% if @template_steps.empty? %> +
No steps yet. A template with no steps approves nothing.
+<% else %> + + + + + + <% @template_steps.each do |step| %> + + + + + + + + + <% end %> + +
LayerNameGroupApprovals requiredTimeout
<%= step.layer %><%= step.name %><%= step.assigned_group %><%= step.approvals_required %><%= timeout_label(step.timeout_after) %> +
+ <%= link_to "Edit", edit_admin_track_template_template_step_path(@track_template, step) %> + <%= button_to "Delete", admin_track_template_template_step_path(@track_template, step), method: :delete, class: "ae-btn is-danger" %> +
+
+<% end %> +

<%= link_to "Add step", new_admin_track_template_template_step_path(@track_template), class: "ae-btn" %>

+ +

Rules

+<% if @trigger_rules.empty? %> +
No rules yet. Nothing routes to this template until one exists.
+<% else %> + + + + + + <% @trigger_rules.each do |rule| %> + + + + + + + + + <% end %> + +
EventConditionPriorityTenantActive
<%= rule.event_name %><%= condition_summary(rule) %><%= rule.priority %> + <%= rule.tenant_id %> + <% if rule.tenant_id != @track_template.tenant_id %> +

Differs from the template's tenant — this rule stamps a cross-tenant approval.

+ <% end %> + 		<%= rule.active? ? "Yes" : "No" %> +
+ <%= link_to "Edit", edit_admin_track_template_trigger_rule_path(@track_template, rule) %> + <%= button_to "Delete", admin_track_template_trigger_rule_path(@track_template, rule), method: :delete, class: "ae-btn is-danger" %> +
+
+<% end %> +

<%= link_to "Add rule", new_admin_track_template_trigger_rule_path(@track_template), class: "ae-btn" %>

diff --git a/app/views/approval_engine/admin/trigger_rules/_condition_row.html.erb b/app/views/approval_engine/admin/trigger_rules/_condition_row.html.erb new file mode 100644 index 0000000..12d7615 --- /dev/null +++ b/app/views/approval_engine/admin/trigger_rules/_condition_row.html.erb @@ -0,0 +1,25 @@ +
+
+ <%= label_tag nil, "Field" %> + <% if exposed_attribute_groups.any? %> + <%= select_tag "trigger_rule[conditions][][field]", + grouped_options_for_select(field_groups_including(row["field"]), row["field"]), + include_blank: "—", disabled: !@condition_simple %> + <% else %> + <%# Nothing has called exposes_for_approval yet, so there is nothing honest + to offer — fall back to free text rather than an empty dropdown. %> + <%= text_field_tag "trigger_rule[conditions][][field]", row["field"], disabled: !@condition_simple %> +

No model declares exposes_for_approval yet.

+ <% end %> +
+
+ <%= label_tag nil, "Operator" %> + <%= select_tag "trigger_rule[conditions][][operator]", + options_for_select(ApprovalEngine::Condition::LABELS.map { |op, label| [ label, op.to_s ] }, row["operator"]), + disabled: !@condition_simple %> +
+
+ <%= label_tag nil, "Value" %> + <%= text_field_tag "trigger_rule[conditions][][value]", row["value"], disabled: !@condition_simple %> +
+
diff --git a/app/views/approval_engine/admin/trigger_rules/_form.html.erb b/app/views/approval_engine/admin/trigger_rules/_form.html.erb new file mode 100644 index 0000000..af729bb --- /dev/null +++ b/app/views/approval_engine/admin/trigger_rules/_form.html.erb @@ -0,0 +1,70 @@ +<%= form_with model: [ :admin, @track_template, @trigger_rule ], local: true, class: "ae-form" do |form| %> + <%= render "approval_engine/admin/shared/errors", record: @trigger_rule %> + +
+
+ <%= form.label :tenant_id %> + <%= form.text_field :tenant_id %> + <% if @trigger_rule.tenant_id.present? && @trigger_rule.tenant_id != @track_template.tenant_id %> +

This differs from the template's tenant (<%= @track_template.tenant_id %>). The rule matches on + its own tenant but stamps this template's approval, so the two should normally agree.

+ <% end %> +
+ +
+ <%= form.label :event_name %> + <%= form.text_field :event_name, list: "ae-event-options" %> + + <% known_event_names.each do |event_name| %> + + <% end %> + +

Any string your app triggers. The listed ones are what the engine fires by convention.

+
+
+ +
+
+ <%= form.label :priority %> + <%= form.number_field :priority %> +

Highest priority wins when several rules match one event.

+
+ +
+ <%= form.label :active, "Active" %> + <%= form.check_box :active %> +

An inactive rule is kept but never matched.

+
+
+ +
+ Condition + + <%= radio_button_tag "trigger_rule[condition_mode]", "simple", @condition_mode == "simple", + id: "ae-cond-simple", class: "ae-cond-toggle", disabled: !@condition_simple %> + <%= label_tag "ae-cond-simple", "Simple" %> + <%= radio_button_tag "trigger_rule[condition_mode]", "advanced", @condition_mode == "advanced", + id: "ae-cond-advanced", class: "ae-cond-toggle" %> + <%= label_tag "ae-cond-advanced", "Raw JSON Logic" %> + +
+ <% @condition_rows.each do |row| %><%= render "condition_row", row: row %><% end %> + <% @blank_rows.times do %><%= render "condition_row", row: {} %><% end %> +

Several conditions must all match. For "or", use raw JSON Logic.

+
+ +
+ <% unless @condition_simple %> +

This rule is more than the simple editor can show — an "or", a nested + group, or an operator it doesn't offer. Edit it as JSON Logic, or switch to Simple to + replace it entirely.

+ <% end %> + <%= text_area_tag "trigger_rule[condition_json]", @condition_json, rows: 8, class: "ae-mono" %> +
+
+ +
+ <%= form.submit "Save rule", class: "ae-btn" %> + <%= link_to "Cancel", admin_track_template_path(@track_template), class: "ae-btn is-secondary" %> +
+<% end %> diff --git a/app/views/approval_engine/admin/trigger_rules/edit.html.erb b/app/views/approval_engine/admin/trigger_rules/edit.html.erb new file mode 100644 index 0000000..de7fd25 --- /dev/null +++ b/app/views/approval_engine/admin/trigger_rules/edit.html.erb @@ -0,0 +1,19 @@ +<% content_for :title, "Edit rule" %> +<%= link_to "← #{@track_template.name}", admin_track_template_path(@track_template), class: "ae-back" %> +

Edit rule

+ +<% + # Editing a live rule changes future routing only — in-flight approvals carry + # stamped copies of the steps. But the approvals it already routed still point + # at this row, so an audit view of an old one will show the *new* condition. + routed_count = ApprovalEngine::Approval.where(approval_engine_trigger_rule_id: @trigger_rule.id).count +%> +<% if routed_count.positive? %> +
+

This rule has routed <%= pluralize(routed_count, "approval") %>. Editing it changes what happens + next, not what already happened — but those approvals point at this rule, so their provenance will read as + whatever you save here.

+
+<% end %> + +<%= render "form" %> diff --git a/app/views/approval_engine/admin/trigger_rules/index.html.erb b/app/views/approval_engine/admin/trigger_rules/index.html.erb new file mode 100644 index 0000000..9b88922 --- /dev/null +++ b/app/views/approval_engine/admin/trigger_rules/index.html.erb @@ -0,0 +1,29 @@ +<% content_for :title, "Rules" %> +

Rules

+

Listed the way the evaluator resolves them: by event, then highest priority first.

+ +<% if @trigger_rules.empty? %> +
No rules<%= " for tenant #{@tenant_id}" if @tenant_id %>.
+<% else %> + + + + + + <% @trigger_rules.each do |rule| %> + + + + + + + + + + <% end %> + +
EventPriorityConditionTemplateTenantActive
<%= rule.event_name %><%= rule.priority %><%= condition_summary(rule) %><%= link_to rule.track_template.name, admin_track_template_path(rule.track_template) %><%= rule.tenant_id %><%= rule.active? ? "Yes" : "No" %><%= link_to "Edit", edit_admin_track_template_trigger_rule_path(rule.track_template, rule) %>
+ <% if @total > @trigger_rules.size %> +

Showing the most recent <%= @trigger_rules.size %> of <%= @total %>.

+ <% end %> +<% end %> diff --git a/app/views/approval_engine/admin/trigger_rules/new.html.erb b/app/views/approval_engine/admin/trigger_rules/new.html.erb new file mode 100644 index 0000000..3dd13ec --- /dev/null +++ b/app/views/approval_engine/admin/trigger_rules/new.html.erb @@ -0,0 +1,4 @@ +<% content_for :title, "New rule" %> +<%= link_to "← #{@track_template.name}", admin_track_template_path(@track_template), class: "ae-back" %> +

New rule

+<%= render "form" %> diff --git a/app/views/approval_engine/notification_mailer/_call_to_action.html.erb b/app/views/approval_engine/notification_mailer/_call_to_action.html.erb new file mode 100644 index 0000000..b0e9172 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/_call_to_action.html.erb @@ -0,0 +1,5 @@ +<% if @url.present? %> +

Review this approval

+<% else %> +

Open <%= Rails.application.class.module_parent_name %> to review it — no approval link is configured yet (see config.approval_url_builder).

+<% end %> diff --git a/app/views/approval_engine/notification_mailer/_call_to_action.text.erb b/app/views/approval_engine/notification_mailer/_call_to_action.text.erb new file mode 100644 index 0000000..6764a73 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/_call_to_action.text.erb @@ -0,0 +1 @@ +<% if @url.present? %>Review it here: <%= @url %><% else %>Open <%= Rails.application.class.module_parent_name %> to review it — no approval link is configured yet (see config.approval_url_builder).<% end %> diff --git a/app/views/approval_engine/notification_mailer/_details.html.erb b/app/views/approval_engine/notification_mailer/_details.html.erb new file mode 100644 index 0000000..0ecd06c --- /dev/null +++ b/app/views/approval_engine/notification_mailer/_details.html.erb @@ -0,0 +1,3 @@ +<% if @step_name.present? || @track_name.present? %> +

<%= [ @step_name, @track_name ].select(&:present?).join(" — ") %>

+<% end %> diff --git a/app/views/approval_engine/notification_mailer/_details.text.erb b/app/views/approval_engine/notification_mailer/_details.text.erb new file mode 100644 index 0000000..62e5dfe --- /dev/null +++ b/app/views/approval_engine/notification_mailer/_details.text.erb @@ -0,0 +1,3 @@ +<% if @step_name.present? || @track_name.present? %> +<%= [ @step_name, @track_name ].select(&:present?).join(" — ") %> +<% end %> diff --git a/app/views/approval_engine/notification_mailer/approval_approved.html.erb b/app/views/approval_engine/notification_mailer/approval_approved.html.erb new file mode 100644 index 0000000..3053467 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/approval_approved.html.erb @@ -0,0 +1,2 @@ +

<%= @target_label %> has been fully approved. Nothing further is needed.

+<%= render partial: "call_to_action", formats: [ :html ] %> diff --git a/app/views/approval_engine/notification_mailer/approval_approved.text.erb b/app/views/approval_engine/notification_mailer/approval_approved.text.erb new file mode 100644 index 0000000..be86f99 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/approval_approved.text.erb @@ -0,0 +1,3 @@ +<%= @target_label %> has been fully approved. Nothing further is needed. + +<%= render partial: "call_to_action", formats: [ :text ] %> diff --git a/app/views/approval_engine/notification_mailer/approval_rejected.html.erb b/app/views/approval_engine/notification_mailer/approval_rejected.html.erb new file mode 100644 index 0000000..cb6d6f9 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/approval_rejected.html.erb @@ -0,0 +1,5 @@ +

<%= @target_label %> was not approved.

+<% if @reason.present? %> +

Reason: <%= @reason %>

+<% end %> +<%= render partial: "call_to_action", formats: [ :html ] %> diff --git a/app/views/approval_engine/notification_mailer/approval_rejected.text.erb b/app/views/approval_engine/notification_mailer/approval_rejected.text.erb new file mode 100644 index 0000000..fb4ff31 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/approval_rejected.text.erb @@ -0,0 +1,5 @@ +<%= @target_label %> was not approved. +<% if @reason.present? %> +Reason: <%= @reason %> +<% end %> +<%= render partial: "call_to_action", formats: [ :text ] %> diff --git a/app/views/approval_engine/notification_mailer/changes_requested.html.erb b/app/views/approval_engine/notification_mailer/changes_requested.html.erb new file mode 100644 index 0000000..b640b04 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/changes_requested.html.erb @@ -0,0 +1,6 @@ +

Changes were requested on <%= @target_label %><%= " by #{@actor_label}" if @actor_label.present? %>.

+<%= render partial: "details", formats: [ :html ] %> +<% if @comment.present? %> +
<%= @comment %>
+<% end %> +<%= render partial: "call_to_action", formats: [ :html ] %> diff --git a/app/views/approval_engine/notification_mailer/changes_requested.text.erb b/app/views/approval_engine/notification_mailer/changes_requested.text.erb new file mode 100644 index 0000000..a90840e --- /dev/null +++ b/app/views/approval_engine/notification_mailer/changes_requested.text.erb @@ -0,0 +1,7 @@ +Changes were requested on <%= @target_label %><%= " by #{@actor_label}" if @actor_label.present? %>. +<%= render partial: "details", formats: [ :text ] %><% if @comment.present? %> +Their comment: + +<%= @comment %> +<% end %> +<%= render partial: "call_to_action", formats: [ :text ] %> diff --git a/app/views/approval_engine/notification_mailer/step_activated.html.erb b/app/views/approval_engine/notification_mailer/step_activated.html.erb new file mode 100644 index 0000000..702d03d --- /dev/null +++ b/app/views/approval_engine/notification_mailer/step_activated.html.erb @@ -0,0 +1,4 @@ +

Hello<%= " #{@actor_label}" if @actor_label.present? %>,

+

<%= @target_label %> is waiting on your approval.

+<%= render partial: "details", formats: [ :html ] %> +<%= render partial: "call_to_action", formats: [ :html ] %> diff --git a/app/views/approval_engine/notification_mailer/step_activated.text.erb b/app/views/approval_engine/notification_mailer/step_activated.text.erb new file mode 100644 index 0000000..7c077f3 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/step_activated.text.erb @@ -0,0 +1,5 @@ +Hello<%= " #{@actor_label}" if @actor_label.present? %>, + +<%= @target_label %> is waiting on your approval. +<%= render partial: "details", formats: [ :text ] %> +<%= render partial: "call_to_action", formats: [ :text ] %> diff --git a/app/views/approval_engine/notification_mailer/step_reassigned.html.erb b/app/views/approval_engine/notification_mailer/step_reassigned.html.erb new file mode 100644 index 0000000..975e69d --- /dev/null +++ b/app/views/approval_engine/notification_mailer/step_reassigned.html.erb @@ -0,0 +1,4 @@ +

Hello<%= " #{@actor_label}" if @actor_label.present? %>,

+

<%= @target_label %> has been handed to you for approval. It is waiting on you now.

+<%= render partial: "details", formats: [ :html ] %> +<%= render partial: "call_to_action", formats: [ :html ] %> diff --git a/app/views/approval_engine/notification_mailer/step_reassigned.text.erb b/app/views/approval_engine/notification_mailer/step_reassigned.text.erb new file mode 100644 index 0000000..b7678b6 --- /dev/null +++ b/app/views/approval_engine/notification_mailer/step_reassigned.text.erb @@ -0,0 +1,5 @@ +Hello<%= " #{@actor_label}" if @actor_label.present? %>, + +<%= @target_label %> has been handed to you for approval. It is waiting on you now. +<%= render partial: "details", formats: [ :text ] %> +<%= render partial: "call_to_action", formats: [ :text ] %> diff --git a/app/views/approval_engine/notification_mailer/step_reminder.html.erb b/app/views/approval_engine/notification_mailer/step_reminder.html.erb new file mode 100644 index 0000000..cc6ce1b --- /dev/null +++ b/app/views/approval_engine/notification_mailer/step_reminder.html.erb @@ -0,0 +1,4 @@ +

Hello<%= " #{@actor_label}" if @actor_label.present? %>,

+

<%= @target_label %> is still waiting on your approval<%= ", and has been for about #{pluralize(@waiting_hours, "hour")}" if @waiting_hours %>.

+<%= render partial: "details", formats: [ :html ] %> +<%= render partial: "call_to_action", formats: [ :html ] %> diff --git a/app/views/approval_engine/notification_mailer/step_reminder.text.erb b/app/views/approval_engine/notification_mailer/step_reminder.text.erb new file mode 100644 index 0000000..d1ba05d --- /dev/null +++ b/app/views/approval_engine/notification_mailer/step_reminder.text.erb @@ -0,0 +1,5 @@ +Hello<%= " #{@actor_label}" if @actor_label.present? %>, + +<%= @target_label %> is still waiting on your approval<%= ", and has been for about #{pluralize(@waiting_hours, "hour")}" if @waiting_hours %>. +<%= render partial: "details", formats: [ :text ] %> +<%= render partial: "call_to_action", formats: [ :text ] %> diff --git a/app/views/layouts/approval_engine/dashboard.html.erb b/app/views/layouts/approval_engine/dashboard.html.erb index 22b6652..58f432e 100644 --- a/app/views/layouts/approval_engine/dashboard.html.erb +++ b/app/views/layouts/approval_engine/dashboard.html.erb @@ -35,14 +35,56 @@ .ae-mono { font-family:ui-monospace,SFMono-Regular,Menlo,monospace; font-size:12px; } .ae-card { background:var(--panel); border:1px solid var(--line); border-radius:10px; padding:16px 20px; margin-bottom:16px; } .ae-back { display:inline-block; margin-bottom:12px; } + /* Everything below is the opt-in admin. It is additive: a host that never + sets admin_enabled renders none of these classes. */ + .ae-nav { margin-left:auto; display:flex; gap:16px; } + .ae-nav a { font-weight:500; color:#cbd5e1; } + .ae-flash { background:#dcfce7; border:1px solid #bbf7d0; color:#166534; border-radius:8px; padding:10px 16px; margin-bottom:16px; } + .ae-flash.is-alert { background:#fee2e2; border-color:#fecaca; color:#991b1b; } + .ae-errors { background:#fee2e2; border:1px solid #fecaca; color:#991b1b; border-radius:10px; padding:12px 16px; margin-bottom:16px; } + .ae-errors ul { margin:6px 0 0; padding-left:18px; } + .ae-form { background:var(--panel); border:1px solid var(--line); border-radius:10px; padding:16px 20px; } + .ae-field { margin-bottom:14px; } + .ae-field label, .ae-form legend { display:block; font-size:12px; color:var(--muted); text-transform:uppercase; letter-spacing:.03em; margin-bottom:4px; } + .ae-form input[type=text], .ae-form input[type=number], .ae-form select, .ae-form textarea { + width:100%; padding:8px 10px; border:1px solid var(--line); border-radius:6px; font:inherit; color:var(--ink); background:#fff; } + .ae-form textarea { font-family:ui-monospace,SFMono-Regular,Menlo,monospace; font-size:12px; } + .ae-form fieldset { border:1px solid var(--line); border-radius:10px; padding:12px 16px; margin:0 0 16px; } + .ae-row { display:flex; gap:12px; flex-wrap:wrap; } + .ae-row .ae-field { flex:1 1 180px; } + .ae-hint { color:var(--muted); font-size:12px; margin:4px 0 0; } + .ae-actions { display:flex; gap:8px; align-items:center; margin-top:16px; } + .ae-btn { display:inline-block; background:var(--accent); color:#fff; border:0; border-radius:6px; padding:8px 16px; font:inherit; font-weight:600; text-decoration:none; cursor:pointer; } + .ae-btn.is-secondary { background:#fff; color:var(--ink); border:1px solid var(--line); } + .ae-btn.is-danger { background:#fff; color:#991b1b; border:1px solid #fecaca; } + .ae-cond-toggle { width:auto; } + .ae-cond label[for] { display:inline; text-transform:none; font-size:13px; color:var(--ink); margin-right:16px; } + /* The whole no-JavaScript trick: the radios are siblings of both panels, so + :checked ~ picks the one to show. Strip the CSS and both panels appear — + the server still obeys the submitted radio, so nothing is lost. */ + .ae-cond-panel { display:none; } + #ae-cond-simple:checked ~ .ae-cond-panel.is-simple { display:block; } + #ae-cond-advanced:checked ~ .ae-cond-panel.is-advanced { display:block; }
<%= link_to "ApprovalEngine", root_path %> ops dashboard + <% if admin_ui? %> + + <% end %>
+ <% if admin_ui? %> + <% flash.each do |type, message| %> +
"><%= message %>
+ <% end %> + <% end %> <%= yield %>
diff --git a/app/views/layouts/approval_engine/mailer.html.erb b/app/views/layouts/approval_engine/mailer.html.erb new file mode 100644 index 0000000..734dcbe --- /dev/null +++ b/app/views/layouts/approval_engine/mailer.html.erb @@ -0,0 +1,13 @@ + + + + + + <%= message.subject %> + + +
+ <%= yield %> +
+ + diff --git a/app/views/layouts/approval_engine/mailer.text.erb b/app/views/layouts/approval_engine/mailer.text.erb new file mode 100644 index 0000000..37f0bdd --- /dev/null +++ b/app/views/layouts/approval_engine/mailer.text.erb @@ -0,0 +1 @@ +<%= yield %> diff --git a/approval_engine.gemspec b/approval_engine.gemspec index 570c764..6c4dbfb 100644 --- a/approval_engine.gemspec +++ b/approval_engine.gemspec @@ -6,27 +6,50 @@ Gem::Specification.new do |spec| spec.authors = [ "Harry-kp" ] spec.email = [ "chaudharyharshit9@gmail.com" ] spec.homepage = "https://github.com/Harry-kp/approval_engine" - spec.summary = "Multi-tenant, immutable-ledger approval flows for Rails." + # `summary` is the snippet on rubygems.org search and in `gem search -d`, so it + # leads with the job rather than with implementation properties nobody types + # into a search box. `description` is indexed too, and is where the long-tail + # phrasing belongs. + spec.summary = "Multi-step approval workflows for Rails: an append-only ledger, runtime routing rules, and a full audit trail." spec.description = <<~DESC.strip - A mountable Rails engine for human-in-the-loop approval flows: an - append-only ledger, dynamic JSON-Logic routing, consensus (any/all/majority), - delegation, and a transactional outbox — without forcing Redis or Sidekiq. + ApprovalEngine is a mountable Rails engine for human-in-the-loop approval + processes — a manager then the CFO, Legal and IT in parallel, "any two of + five reviewers". It gives you an append-only approval ledger, tenant-scoped + routing rules stored as JSON Logic so admins can change them without a + deploy, a declarative define_flow DSL for the flows that live in your + codebase, consensus per layer (any/all/majority/percentage/count), + time-bound delegation with intended-vs-actual actor auditing, per-step + timeouts that never auto-approve, optional built-in notifications, and a + transactional outbox so a failing external API can never roll back an + approval. PostgreSQL and ActiveJob; no Redis or Sidekiq required. DESC spec.license = "MIT" spec.required_ruby_version = ">= 3.1.0" - spec.metadata["source_code_uri"] = spec.homepage + # `source_code_uri` is pinned to the repository rather than derived from + # `homepage`: moving the homepage some day must not silently repoint the + # "Source code" link on rubygems.org at a marketing page. There is no + # `homepage_uri` here on purpose — it would duplicate `spec.homepage`, which + # rubygems.org already renders, and `gem build` warns about the duplicate. + spec.metadata["source_code_uri"] = "https://github.com/Harry-kp/approval_engine" + spec.metadata["documentation_uri"] = "https://github.com/Harry-kp/approval_engine/blob/main/docs/COOKBOOK.md" spec.metadata["changelog_uri"] = "#{spec.homepage}/blob/main/CHANGELOG.md" spec.metadata["bug_tracker_uri"] = "#{spec.homepage}/issues" spec.metadata["rubygems_mfa_required"] = "true" # Ship only runtime files (no Rakefile/test harness/dev tooling) and only - # files, never directory entries — keep the published gem minimal. + # files, never directory entries — keep the published gem minimal. The two + # docs ride along because `bundle open approval_engine` should answer a + # question rather than send you back to a browser; they are named one by one + # so a future docs/ subdirectory can never be packaged by accident. spec.files = Dir.chdir(__dir__) do - Dir["{app,config,db,lib}/**/*", "MIT-LICENSE", "README.md", "CHANGELOG.md"].select { |f| File.file?(f) } + Dir[ + "{app,config,db,lib}/**/*", "MIT-LICENSE", "README.md", "CHANGELOG.md", + "docs/ARCHITECTURE.md", "docs/COOKBOOK.md" + ].select { |f| File.file?(f) } end - spec.add_dependency "rails", ">= 7.0.8", "< 9.0" + spec.add_dependency "rails", ">= 7.1", "< 9.0" spec.add_dependency "shiny_json_logic", "~> 0.3" end diff --git a/assets/screenshot-approval.png b/assets/screenshot-approval.png new file mode 100644 index 0000000..78ff60a Binary files /dev/null and b/assets/screenshot-approval.png differ diff --git a/assets/screenshot-dashboard.png b/assets/screenshot-dashboard.png new file mode 100644 index 0000000..e454cac Binary files /dev/null and b/assets/screenshot-dashboard.png differ diff --git a/assets/screenshot-rule-builder.png b/assets/screenshot-rule-builder.png new file mode 100644 index 0000000..a6c1f4a Binary files /dev/null and b/assets/screenshot-rule-builder.png differ diff --git a/assets/social-preview.html b/assets/social-preview.html new file mode 100644 index 0000000..2c22039 --- /dev/null +++ b/assets/social-preview.html @@ -0,0 +1,123 @@ + + + + + + + + + + +
+
+ +
+
Ruby gem · Rails engine
+ +
+
Approval
→ +
Track
→ +
Step
+
+ +

ApprovalEngine

+ +
+ A manager approves, then the CFO.
+ Unless it's over $50k — then Legal too. +
+ +
+
Rules admins change without a deploy
+
Append-only audit trail
+
Sequential, parallel, or “any 2 of 5”
+
No Redis or Sidekiq
+
+ +
+
$gem install approval_engine
+
github.com/Harry-kp/approval_engine
+
+
+ + diff --git a/assets/social-preview.png b/assets/social-preview.png index 6f058f6..3c76248 100644 Binary files a/assets/social-preview.png and b/assets/social-preview.png differ diff --git a/config/locales/approval_engine.en.yml b/config/locales/approval_engine.en.yml new file mode 100644 index 0000000..87eeac5 --- /dev/null +++ b/config/locales/approval_engine.en.yml @@ -0,0 +1,19 @@ +# Subjects for the built-in notifications. The engine's locale path is loaded +# before the application's, so a host reworded subject in their own en.yml wins +# without forking the mailer. `%{target}` is the label from +# `config.target_label_method`, already resolved to a plain String. +en: + approval_engine: + notification_mailer: + step_activated: + subject: "Approval needed: %{target}" + step_reminder: + subject: "Still waiting on you: %{target}" + step_reassigned: + subject: "Approval handed to you: %{target}" + changes_requested: + subject: "Changes requested: %{target}" + approval_approved: + subject: "Approved: %{target}" + approval_rejected: + subject: "Rejected: %{target}" diff --git a/config/routes.rb b/config/routes.rb index a708e5c..24b348a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,4 +5,19 @@ # end root to: "approvals#index" resources :approvals, only: %i[index show] + + # The runtime rule editor — the engine's only write surface, so it is drawn + # only when the host asks for it. Routes are drawn once at boot, well after + # config/initializers run, so reading the flag here is safe; a flag flipped + # later needs a restart (the controllers re-check it per request anyway). + if ApprovalEngine.config.admin_enabled + namespace :admin do + root to: "track_templates#index" + resources :track_templates do + resources :template_steps, only: %i[new create edit update destroy] + resources :trigger_rules, only: %i[new create edit update destroy] + end + resources :trigger_rules, only: %i[index] + end + end end diff --git a/db/migrate/20260614103434_create_approval_engine_core_tables.rb b/db/migrate/20260614103434_create_approval_engine_core_tables.rb index 924ba1e..ca796d0 100644 --- a/db/migrate/20260614103434_create_approval_engine_core_tables.rb +++ b/db/migrate/20260614103434_create_approval_engine_core_tables.rb @@ -1,10 +1,14 @@ class CreateApprovalEngineCoreTables < ActiveRecord::Migration[7.0] + # Host records are referenced polymorphically, so these columns have to hold + # whatever the approved models use as a primary key. String fits a bigint, a + # UUID and a ULID alike; `t.references`' bigint default would cast a UUID to 0 + # and silently lose the target. def change # The orchestrator. One per host record + event. Holds the parallel # track tracks and the overall outcome. create_table :approval_engine_approvals, id: :uuid do |t| t.string :tenant_id, null: false, index: true - t.references :target, polymorphic: true, null: false, index: true + t.references :target, polymorphic: true, type: :string, null: false, index: true t.string :status, null: false, default: "pending", index: true t.string :event_name @@ -33,7 +37,7 @@ def change t.string :status, null: false, default: "pending", index: true # How many approvals this step's layer needs: any | all | majority | "60%" | "2". t.string :approvals_required, null: false, default: "any" - t.references :assigned_actor, polymorphic: true, null: false + t.references :assigned_actor, polymorphic: true, type: :string, null: false # Cycle-time facts for latency/bottleneck reporting: when the step became # actionable (pending), and when a human resolved it. Both stay null while @@ -61,10 +65,10 @@ def change t.string :tenant_id, null: false t.references :approval_engine_step, null: false, foreign_key: true, type: :uuid t.string :event, null: false - t.references :intended_actor, polymorphic: true, null: true + t.references :intended_actor, polymorphic: true, type: :string, null: true # Nullable: system events (e.g. a step timing out) have no human actor — # the ledger records that honestly rather than fabricating one. - t.references :actual_actor, polymorphic: true, null: true + t.references :actual_actor, polymorphic: true, type: :string, null: true t.text :comment t.timestamps diff --git a/db/migrate/20260614103435_create_approval_engine_infrastructure_tables.rb b/db/migrate/20260614103435_create_approval_engine_infrastructure_tables.rb index 358c065..03b7a60 100644 --- a/db/migrate/20260614103435_create_approval_engine_infrastructure_tables.rb +++ b/db/migrate/20260614103435_create_approval_engine_infrastructure_tables.rb @@ -1,4 +1,8 @@ class CreateApprovalEngineInfrastructureTables < ActiveRecord::Migration[7.0] + # Host records are referenced polymorphically, so these columns have to hold + # whatever the approved models use as a primary key. String fits a bigint, a + # UUID and a ULID alike; `t.references`' bigint default would cast a UUID to 0 + # and silently lose the target. def change create_table :approval_engine_outbox_events, id: :uuid do |t| t.string :tenant_id, null: false @@ -13,8 +17,8 @@ def change create_table :approval_engine_delegations, id: :uuid do |t| t.string :tenant_id, null: false - t.references :delegator, polymorphic: true, null: false - t.references :delegatee, polymorphic: true, null: false + t.references :delegator, polymorphic: true, type: :string, null: false + t.references :delegatee, polymorphic: true, type: :string, null: false t.datetime :starts_at, null: false t.datetime :ends_at, null: false t.boolean :active, null: false, default: true diff --git a/db/migrate/20260618000001_add_reminded_at_to_steps.rb b/db/migrate/20260618000001_add_reminded_at_to_steps.rb new file mode 100644 index 0000000..25334cb --- /dev/null +++ b/db/migrate/20260618000001_add_reminded_at_to_steps.rb @@ -0,0 +1,19 @@ +class AddRemindedAtToSteps < ActiveRecord::Migration[7.0] + # When the sweep last nudged this step, so a nudge fires once per step. + # Nullable with no backfill — a metadata-only ALTER, and an app that upgrades + # without running it loses reminders, never approvals. + def change + add_column :approval_engine_steps, :reminded_at, :datetime + + # Builds with a lock, so on a very large table create it by hand first and + # let `if_not_exists` make this a no-op: + # + # CREATE INDEX CONCURRENTLY idx_ae_steps_remindable + # ON approval_engine_steps (activated_at) + # WHERE reminded_at IS NULL AND status = 'pending'; + add_index :approval_engine_steps, :activated_at, + where: "reminded_at IS NULL AND status = 'pending'", + name: "idx_ae_steps_remindable", + if_not_exists: true + end +end diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4dd7d9d..b2a76ae 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1,6 +1,6 @@ # Architecture & Engineering Document: `approval_engine` -**Enterprise-Grade Approval Orchestration for Ruby on Rails** +**Why the engine is shaped the way it is.** > **This is the design rationale, not a usage guide.** For how to install and > use the gem, see the [README](../README.md) and [Cookbook](COOKBOOK.md), which @@ -10,7 +10,7 @@ ## 1. Introduction & Philosophy -`approval_engine` is a Ruby on Rails engine designed specifically for B2B SaaS platforms. It solves the hardest problem in enterprise software—concurrent, multi-tenant state machines and dynamic approval routing—without sacrificing the developer happiness of "The Rails Way". +`approval_engine` is a Rails engine for multi-step human approvals. The problems it takes seriously are the ones that make people rewrite this by hand every few years: concurrent transitions that must never double-approve, routing rules that change without a deploy, and a record of who decided what that survives the decision being revised. **The Philosophy:** Be an absolute dictator about database integrity (immutable ledgers, pessimistic locking), but rely entirely on Rails conventions (Rich Models, ActiveJob, Duck Typing, Hotwire Generators) for the developer experience. @@ -25,6 +25,10 @@ * **Delegation of Authority (DoA):** Allow time-bound "Proxy" approvals (e.g., User A acts on behalf of User B while on leave). * **Anti-Corruption Macro:** Provide a class-level DSL (`exposes_for_approval`) to explicitly whitelist model attributes and relationships safe for the JSON Rules Engine. * **Asynchronous Side-Effects:** Guarantee that external API failures or slow mailers do not roll back the approval database transaction via the Transactional Outbox pattern. +* **Declarative Flow Definition:** Let a flow — its template, its ordered steps, and the rules that route to it — be written as one idempotent block (`ApprovalEngine.define_flow`) that reconciles the same rows on every deploy instead of stacking duplicates. +* **A Shared Authoring Vocabulary:** Provide one field/operator/value abstraction (`ApprovalEngine::Condition`) that renders to JSON Logic and reads back from it, so a rule written in a seed file and a rule written in a browser form are the same rule. +* **Runtime Rule Editing:** Provide an opt-in admin surface where a tenant's templates, steps, and routing rules are edited in the browser, with no JavaScript and no deploy — the claim the README makes, made true. +* **Approver Notification:** Ship the approver-facing half — mail on activation, reassignment, rework and outcome, plus an idempotent reminder sweep for approvers who have gone quiet — as an opt-in layer, never as an upgrade side-effect. --- @@ -34,6 +38,7 @@ * **Race-Condition Immunity:** Utilize database-level pessimistic locking (`lock!`) during state transitions to prevent double-approvals. * **Cryptographic-Style Auditability:** Maintain an append-only `AuditLogs` table tracking `intended_actor` vs. `actual_actor` for strict compliance. * **Safe Failures (Fail Closed):** A *missing* payload key is a clean non-match (JSON Logic treats it as false), so the approval simply doesn't start. Only a *malformed* rule (e.g. an unknown operator) quarantines the approval into a "System Failure" state, rather than crashing the web worker. +* **Silent Upgrades:** A new surface that can write, mail, or otherwise act on someone else's users ships switched off. `bundle update approval_engine` must never change observable behaviour on its own — see ADR 9 and ADR 10. * **Developer Ergonomics (Omakase):** No heavy Service Objects. Use Rich ActiveRecord models, bang methods, and conventional callbacks. --- @@ -64,10 +69,79 @@ * *Rationale:* Maximizes developer happiness by allowing standard Rails error handling (`rescue ActiveRecord::RecordInvalid`), scopes, and validations. +* **ADR 5: `Condition` as the Authoring Seam** +* *Context:* JSON Logic is the right format to *store* a rule in and the wrong format to *write* one in. Nobody should have to hand-type `{ ">" => [{ "var" => "amount" }, 10_000] }` into a form field or a seed file. +* *Decision:* One class — `ApprovalEngine::Condition` — renders a field/operator/value triple down to JSON Logic and reads it back up. Both authoring paths go through it: the `when:` sugar in `define_flow` and the admin rule builder. It reports `nil` from `.parse` for any AST it cannot represent. +* *Rationale:* One vocabulary (`eq`, `not_eq`, `gt`, `gte`, `lt`, `lte`, `in`, `not_in`, ANDed) means a rule written in `db/seeds.rb` and the same rule written in the browser are the same rule, with no second dialect to keep in sync. The honest `nil` is what lets the UI fall back to a raw textarea rather than silently rewriting a hand-written `or`, or a substring `in`, into something with different semantics. Values are cast against the declared `exposes_for_approval` type on the way in, because `amount > "10000"` compares a number against a string and quietly never matches — a failure mode with no error message anywhere. + + +* **ADR 6: `define_flow` Reconciles, It Never Inserts** +* *Context:* A flow that lives in the codebase has to survive being re-run on every deploy, from `db/seeds.rb`, by two deploys at once. +* *Decision:* A flow is keyed on `(tenant_id, name)` and reconciled under a per-flow Postgres advisory lock. Steps are matched **by name** and updated in place; a step no longer declared is destroyed. Rules are matched by `(event_name, priority)` and updated in place; a rule no longer declared is **deactivated, never destroyed**. A block that declares no `on` at all leaves existing rules alone. +* *Rationale:* The block is a *desired state*, not a migration, so re-running it with nothing changed must write nothing. Matching steps by name (rather than position) keeps the ids an admin UI links to. Rules are deactivated because an `Approval` records the `TriggerRule` that routed it as provenance on a `nullify` foreign key — destroying a rule would not error, it would quietly erase "which rule started this?" from every approval it ever spawned, and deactivating is what dropping an `on` actually means. The asymmetry with steps is deliberate: a template with no steps is always a bug, but a flow whose steps live in code and whose routing an admin owns in the UI is a legitimate shape. + + +* **ADR 7: The Ledger Copies the Blueprint, It Does Not Reference It** +* *Context:* If a `Step` held a foreign key to the `TemplateStep` it came from, editing a template would retroactively change what an approver is looking at, and deleting one would strand a running approval. +* *Decision:* `ApprovalBuilder` copies `name`, `layer`, `approvals_required` and `timeout_after` onto each `Step` row at build time, and resolves `assigned_group` into a concrete `assigned_actor` per step. `IterationBuilder` clones a rework iteration from the previous iteration's *ledger* rows, never from the template. No ledger row holds a foreign key to a blueprint row. +* *Rationale:* An approval is a historical fact and has to read the same in a year as it did on the day. This is also what makes ADR 6 possible: because nothing outside a template points at its steps, reconciliation can rewrite blueprints freely — no template versioning, no soft deletes, no "which version was this built from?" column, and no lock taken against running work. See [§5](#5-the-blueprintledger-boundary) for the full consequences, including the honest cost. + + +* **ADR 8: Notifications Ride the Outbox** +* *Context:* An approval nobody hears about is not a workflow, so 1.1 adds a mailer. Mail is the least reliable side-effect in the system and must not be able to disturb the most important one. +* *Decision:* `ApprovalEngine::Notifier` is dispatched from inside `ProcessOutboxJob` — never from a model callback — and it **enqueues** mail (`deliver_later`) rather than delivering it. A notification that raises is logged and swallowed. +* *Rationale:* Building the message inside the relay means mail is only ever built from state that has already committed. Enqueuing rather than delivering means a dead SMTP server retries the *mail* and never the outbox event — a retried event would re-run the host's callbacks alongside it, turning a mailer problem into a double-disbursed invoice. Swallowing follows from the same premise: built-in mail is a courtesy layered on top of the delivery contract, not part of it, so a missing template or an unreachable actor must never fail the event. The cost, stated where adopters read it rather than in a footnote: delivery is at-least-once, so an approver may occasionally receive the same nudge twice. + + +* **ADR 9: Notifications Are Off By Default** +* *Context:* 1.1 adds a mailer to a gem that shipped without one. Every install that upgrades already has `Step` rows assigned to real employees and customers, and their addresses are already reachable through `config.actor_class`. +* *Decision:* `config.notifications_enabled` defaults to `false` and nothing beneath it has any effect. Enabling it takes three deliberate settings — the switch, a From address (`mailer_from`, or a `parent_mailer` that already declares `default from:`), and `approval_url_builder`. The three approval-level messages additionally need `approval_recipients`. +* *Rationale:* The blast radius of getting this wrong is not a broken page; it is unsolicited mail to someone else's users, sent by a dependency they bumped a minor version of. A feature that can only be discovered by reading the changelog is a fair price for that. The other two settings are gates for the same reason at a smaller scale: without a sender there is nothing to hand the SMTP server, and without a URL the mail arrives with nowhere to go. `approval_recipients` has no safe default at all — the engine knows who *approves*, and only the host knows who *asked*. `notification_events` is the second switch: a host can keep the layer on and mute one message without writing a mailer or forking a view. + + +* **ADR 10: The Admin Is Off By Default** +* *Context:* 1.0 shipped a read-only ops dashboard, and hosts mounted it behind whatever authentication they already had — a `constraints` block, a Devise `authenticate`, an IP allowlist, a VPN. 1.1 adds template, step and rule CRUD to that same mount. +* *Decision:* `config.admin_enabled` defaults to `false`. The write routes are not drawn while it is false, and every admin controller re-checks the flag on each request rather than trusting boot time. +* *Rationale:* Whatever a host judged acceptable for a read surface, they did not judge it for a write surface, and the two are different threat models: reading exposes what happened, writing changes what routes where, for every future approval, silently. A gem upgrade must never make that call on someone's behalf. The per-request re-check exists because routes are drawn once at boot and a host may point their own routes at these controllers. **Turning the flag on is not authentication** — the engine ships none — so the mount stays wrapped in the host's own constraint either way. The admin also needs a full-stack host (sessions for CSRF, and flash), so it is unavailable under `config.api_only`. + + + + +--- + +## 5. The Blueprint/Ledger Boundary + +Every table in the engine sits on one side of a line, and the line is the reason +a flow can be rewritten at any time without anyone auditing a paused deploy. + +| Side | Tables | Nature | +| --- | --- | --- | +| **Blueprint** | `TrackTemplate`, `TemplateStep`, `TriggerRule` | Mutable configuration. Says what *should* happen. Owned by a `define_flow` block, or by an admin in the UI. | +| **Ledger** | `Approval`, `Track`, `Step`, `AuditLog`, `OutboxEvent` | Append-only history. Says what *did* happen. Owned by nobody; written only by transitions. | + +The blueprint is read exactly once per approval, at build time, and never again. +What crosses the line then: + +* `TemplateStep#name`, `#layer`, `#approvals_required` and `#timeout_after` are **copied** onto each `Step` row. +* `TemplateStep#assigned_group` is **resolved** — handed to the host's `resolve_approval_group(group_name, target)` and expanded into one `Step` per actor it returns, with a concrete `assigned_actor`. +* `TrackTemplate#name` is copied onto the `Track`, and `#tenant_id` onto every ledger row the build creates. +* The `TriggerRule` that matched is recorded on the `Approval` as provenance, on a `nullify` foreign key — the one deliberate pointer back across the line, and it is allowed to become `NULL` without taking anything with it. + +What does not cross: no ledger row holds a foreign key to a `TemplateStep`. A +rework iteration is cloned by `IterationBuilder` from the previous iteration's +`Step` rows, so even the second attempt reads the ledger rather than the +blueprint. + +Four consequences follow, and they are the reason to write this down: + +1. **Redefining a flow cannot disturb an approval in flight.** Re-run `define_flow` mid-morning with a step removed and a threshold moved, and every approval already running finishes with the steps it started with. That is what makes ADR 6's reconciliation safe to put in `db/seeds.rb` on every deploy. +2. **Rework replays the ledger, not the template.** Requesting changes on an approval built yesterday appends an iteration containing the step you deleted from the flow this morning. That is correct: the approver is finishing the process they were shown. +3. **Reconciliation stays cheap.** Because nothing outside the template points at its steps, there is no template versioning scheme, no soft-delete tombstone, and no "which blueprint version was this built from?" column to keep honest. +4. **The cost, honestly:** a fix to a blueprint does not reach running approvals. If a template step named the wrong group, every approval already built carries the wrong assignee, and the remedy is `step.reassign!` on the ledger — not an edit to the template. The engine prefers this to the alternative, which is an audit trail that changes shape after the fact. --- -## 5. Use Cases & Solutions +## 6. Use Cases & Solutions ### The "I Want To" User Stories @@ -136,13 +210,29 @@ 17. **Admin Monitoring:** "As a Support Tech, I want to see a dashboard of stuck approvals without writing custom queries." -* *Solution:* Mountable Rails Engine provides a pre-built Rack admin dashboard. +* *Solution:* The mountable engine provides a read-only ops dashboard — every approval, filterable by status, drilling into tracks, steps, the audit trail, and a time-in-step column. It has no authentication of its own; the host wraps the mount. + + +18. **The Flow as One Block:** "As a Rails Dev, I don't want four `create!` calls and a hand-written AST to say 'manager, then CFO, over $10k'." +* *Solution:* `ApprovalEngine.define_flow` writes the template, its ordered steps and its rules in one block, and reconciles them on every re-run (ADR 6). It writes ordinary rows, so nothing downstream can tell a defined flow from a hand-built one. + + +19. **Runtime Rule Editing:** "As a SaaS Admin, I want to move the threshold from $10k to $25k myself, today." +* *Solution:* The opt-in admin (`config.admin_enabled`, ADR 10) offers a field/operator/value builder over `Condition` (ADR 5), with the field list drawn from `exposes_for_approval` and a raw JSON Logic fallback for anything the simple form cannot represent. + + +20. **Telling the Approver:** "As a Submitter, I want the manager to actually find out there is something waiting on them." +* *Solution:* Six built-in messages dispatched by `Notifier` from inside the outbox relay (ADR 8), off until the host enables them (ADR 9). Views are overridden by dropping files into the host's own `app/views/approval_engine/notification_mailer/`. + + +21. **The Quiet Approver:** "As an Ops lead, I want a nudge to go out after two days — once, not every hour." +* *Solution:* `ReminderSweepJob` / `Step.sweep_reminders!` over the `remindable` scope, stamping `reminded_at` so a step is nudged at most once. A nudge is not a verdict: **ApprovalEngine never auto-approves.** --- -## 6. The End Picture: Installation & Usage +## 7. The End Picture: Installation & Usage **1. Installation** @@ -157,8 +247,17 @@ rails db:migrate ```ruby ApprovalEngine.configure do |config| - config.outbox_queue = :high_priority + config.outbox_queue = :high_priority config.current_tenant_method = -> { Current.account } + + # Both 1.1 surfaces are off until asked for — see ADR 9 and ADR 10. + config.notifications_enabled = true + config.mailer_from = "approvals@acme.com" + # Url helpers are not in scope in an initializer — go through the routes proxy. + config.approval_url_builder = ->(approval) { Rails.application.routes.url_helpers.invoice_url(approval.target) } + config.reminder_after = 2.days.to_i + + config.admin_enabled = true # routes are drawn at boot; restart to apply end ``` @@ -187,7 +286,28 @@ end ``` -**4. Controller Execution (Rich Models)** +**4. Flow Definition (`db/seeds.rb`)** + +```ruby +ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + + parallel(approvals_required: :all) do + step "Legal review", group: "legal" + step "IT review", group: "it" + end + + step "CFO sign-off", group: "cfo", timeout_after: 2.days +end + +``` + +Idempotent, so it belongs on the deploy path (ADR 6). It needs a live +connection, so it does not belong in an initializer. + +**5. Controller Execution (Rich Models)** ```ruby class ApprovalsController < ApplicationController @@ -210,7 +330,7 @@ end ``` -**5. System Admin UI** +**6. Mounting the Dashboard (and, optionally, the Admin)** ```ruby # config/routes.rb @@ -220,4 +340,9 @@ Rails.application.routes.draw do end end -``` \ No newline at end of file +``` + +The mount is read-only until `config.admin_enabled` is set, at which point the +template and rule editor appears at `/admin` beneath it. The constraint above is +the host's, not the engine's, and it is required either way: the flag decides +whether the write routes exist, never who may reach them (ADR 10). \ No newline at end of file diff --git a/docs/COOKBOOK.md b/docs/COOKBOOK.md index 95da5f9..896aa49 100644 --- a/docs/COOKBOOK.md +++ b/docs/COOKBOOK.md @@ -13,7 +13,9 @@ Assumes you've armed a model and configured an actor class — see the - [Consensus](#consensus) - [Parallel review](#parallel-review) - [Delegation](#delegation) +- [Withdrawing & escalating](#withdrawing--escalating) - [Side-effects & chaining](#side-effects--chaining) +- [Notifications](#notifications) - [Integrating with your model's status](#integrating-with-your-models-status) - [Multi-tenancy](#multi-tenancy) - [Safety](#safety) @@ -24,6 +26,123 @@ Assumes you've armed a model and configured an actor class — see the ## Routing & templates +### "One flow shouldn't take four `create!` calls" (defining a flow in code) + +`ApprovalEngine.define_flow` writes the template, its ordered steps, and the +rules that route to it in one block: + +```ruby +# db/seeds.rb +ApprovalEngine.define_flow "High-value invoice", tenant: "acme", model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" +end +``` + +Each `step` is the next layer down, so that reads "Manager, then CFO". `on` +takes a lifecycle symbol — resolved through `model:` into `"invoice.created"`, +so the rule and the engine read from one source — or any event-name string you +fire yourself, and it accepts `priority:` and `active:` as well as `when:`. +`when:` is sugar over JSON Logic: `{ amount: { gt: 10_000 } }`, +`{ department: "IT" }` for equality, `{ region: { in: %w[EU UK] } }` for a set. +Several keys AND together, and raw JSON Logic passes straight through for +anything the sugar can't say (an `or`, arithmetic). + +Same-layer review is a `parallel` block. Consensus is declared on the block +rather than the steps inside it, because a layer carries one policy for every +step in it — and it defaults to `:all`, since "Legal and IT review at the same +time" means both: + +```ruby +ApprovalEngine.define_flow "Major capital spend", tenant: "acme", model: Invoice do + on :create, when: { amount: { gt: 100_000 } }, priority: 10 + + step "Manager sign-off", group: "manager" + parallel(approvals_required: :all) do + step "Legal review", group: "legal" + step "IT review", group: "it" + end + step "CFO sign-off", group: "cfo", timeout_after: 2.days +end +``` + +A plain `step` also takes `approvals_required:` and `timeout_after:`; a +flow-wide `define_flow(..., timeout_after:)` sets the default for every step +that doesn't name its own. + +It writes ordinary `TrackTemplate` / `TemplateStep` / `TriggerRule` rows — +nothing downstream can tell a defined flow from a hand-built one, and the +[longhand below](#i-want-invoices-over-10k-to-need-extra-approval-conditional-routing) +is the same thing spelled out. `define_flow` returns the template, and +`ApprovalEngine.flow(name, tenant:)` looks one up later, which is how a defined +flow reaches `run_approval!(templates: [...])`: + +```ruby +contract.run_approval!(templates: [ ApprovalEngine.flow("Legal", tenant: "acme"), + ApprovalEngine.flow("IT", tenant: "acme") ]) +``` + +This is setup code, so it fails loudly and early — the opposite of runtime rule +evaluation, which deliberately fails closed and silent. A rule that reads an +attribute the model doesn't expose, a consensus spec that isn't one, two steps +with the same name, a group that isn't in `config.approval_groups`, a +non-positive `timeout_after`: each stops the seed with a sentence explaining +itself, instead of resolving into a flow that silently never fires. + +> **Put it in `db/seeds.rb`, not an initializer.** It opens a transaction and +> takes an advisory lock, so it needs a live database connection — +> `rails db:create` and `assets:precompile` don't have one. + +> **Never hand it user input.** `when:` accepts raw JSON Logic and stores it +> verbatim. The vetted path for rules an admin authors is +> [the rule editor](#an-admin-should-change-the-10k-threshold-without-a-deploy-the-rule-editor), +> which is constrained on purpose. + +One flow is one template is one track. Scatter-gather *across* tracks can't be +expressed in a block — a trigger rule structurally routes to exactly one +template — so that stays `run_approval!(templates: [...])`, as above. + +Covered by [`test/services/approval_engine/flow_definition_test.rb`](https://github.com/Harry-kp/approval_engine/blob/main/test/services/approval_engine/flow_definition_test.rb). + +### "My seeds run on every deploy — won't that duplicate the flow?" (reconciling a definition) + +No. A flow is keyed on `(tenant, name)`, and `define_flow` reconciles what it +finds rather than inserting: the block is the desired state, and re-running it +with nothing changed changes nothing. What reconciles, exactly: + +| Declaration | On a re-run | +| --- | --- | +| The template | Found by `(tenant, name)`; its `status` is re-asserted, so a flow an admin archived comes back `active` | +| A step | Matched **by name** and updated in place, so the ids an admin UI links to survive | +| A renamed step | Reads as a remove plus an add — the name is the identity | +| A step you stopped declaring | Destroyed | +| A rule | Matched by `(event_name, priority)` and updated in place | +| A rule you stopped declaring | **Deactivated, never destroyed** — an approval keeps its `trigger_rule` as provenance, so deleting it would erase "which rule started this?" from every approval it ever spawned | +| A block with no `on` at all | Leaves existing rules alone, so routing can stay the admin's | + +Two deploys seeding at once take an advisory lock scoped to that one flow, so +they can't race into two templates with the same name. If a tenant somehow +already holds two templates with the name, `define_flow` refuses rather than +guessing which one it owns. + +**An approval already in flight is untouched by any of it.** The ledger holds +copies, not references: `ApprovalBuilder` stamps each template step's name, +layer, consensus and timeout *onto* the `Step` rows when it builds an approval, +and a rework iteration is cloned from those ledger rows rather than from the +template. A running approval therefore holds no live reference to anything a +redefinition rewrites, and finishes with the steps it started with — including +a step you deleted from the flow this morning. + +The flip side: code and the UI can't half-own one flow, because the next run of +the block re-asserts everything it declares. Pick an owner per flow. A block +with no `on` is the useful middle — steps in code, routing in the admin. + +Covered by [`test/services/approval_engine/flow_definition_test.rb`](https://github.com/Harry-kp/approval_engine/blob/main/test/services/approval_engine/flow_definition_test.rb) +("re-running the same flow changes nothing", "rewriting a flow leaves an +approval already in flight untouched"). + ### "I want invoices over $10k to need extra approval" (conditional routing) ```ruby @@ -484,9 +603,10 @@ end ``` Other hooks: `after_rejected(reason)`, `after_cancelled(reason)`, -`after_step_approved(step)`, `after_step_rejected(step)`, -`after_step_changes_requested(step)`, `after_step_expired(step)`, -`after_step_reassigned(step)`, `on_step_timeout(step)`, `on_quarantined(reason)`. +`after_step_activated(step)`, `after_step_approved(step)`, +`after_step_rejected(step)`, `after_step_changes_requested(step)`, +`after_step_expired(step)`, `after_step_reassigned(step)`, `on_step_timeout(step)`, +`on_step_reminder(step)`, `on_quarantined(reason)`. Callbacks fire through the outbox: **at-least-once and unordered**. Make them idempotent, and don't assume one fires before another (e.g. `after_step_approved` @@ -513,11 +633,13 @@ The full channel list: | `approval_engine.approval.rejected` | an approval is rejected | | `approval_engine.approval.cancelled` | an approval is withdrawn via `cancel!` | | `approval_engine.approval.quarantined` | a malformed rule quarantined the approval | +| `approval_engine.step.activated` | a step became actionable | | `approval_engine.step.approved` | a step is approved | | `approval_engine.step.rejected` | a step is rejected | | `approval_engine.step.changes_requested` | a step is sent back for rework | | `approval_engine.step.expired` | a step is expired (timed-out denial) | | `approval_engine.step.timed_out` | a step's SLA elapsed (signal only) | +| `approval_engine.step.reminded` | the reminder sweep nudged a quiet step | | `approval_engine.step.reassigned` | a step was handed to another actor | ### "If Stripe is down, the approve click shouldn't 500" (async safety) @@ -537,6 +659,182 @@ never bubbling into the approval. --- +## Notifications + +The engine ships six emails and sends none of them until you say so. Everything +in this section is inert while `config.notifications_enabled` is false, which is +the default: upgrading the gem must never start mailing an adopter's real users. + +### "Tell the approver something is waiting on them" (turning notifications on) + +Three settings, and all three matter: + +```ruby +# config/initializers/approval_engine.rb +ApprovalEngine.configure do |config| + config.notifications_enabled = true # 1. the master switch + config.mailer_from = "approvals@acme.com" # 2. a sender + config.approval_url_builder = lambda do |approval| # 3. somewhere to go + Rails.application.routes.url_helpers.invoice_url(approval.target) + end +end +``` + +1. **`notifications_enabled`** — nothing below it has any effect while this is + false. +2. **A From address** — either `mailer_from`, or point `config.parent_mailer` at + your `ApplicationMailer` and inherit its `default from:` (and its layout, + which is usually the real reason to). Without one, Action Mailer has nothing + to hand the SMTP server, and the failure surfaces deep inside the delivery + job, far from the setting that caused it. +3. **`approval_url_builder`** — the engine cannot know your routes. Without it + the mail still sends and arrives with nowhere to go. + +Who receives what: + +| Notification | Fires when | Goes to | +| --- | --- | --- | +| `step_activated` | a step becomes actionable | the assigned actor | +| `step_reminder` | the sweep nudges a quiet step | the assigned actor | +| `step_reassigned` | a step is handed to someone else | the new assignee | +| `changes_requested` | a step is sent back for rework | `config.approval_recipients` | +| `approval_approved` | an approval gathers to approved | `config.approval_recipients` | +| `approval_rejected` | an approval is rejected | `config.approval_recipients` | + +The engine knows who approves; only you know who asked. So the bottom three stay +silent — configured, enabled, and silent — until you say where they go: + +```ruby +config.approval_recipients = ->(approval) { [ approval.target.submitter ] } +``` + +Addresses are read off an actor with `config.actor_email_method` (`:email` by +default). An actor who has no address, or doesn't respond to the method at all, +is skipped rather than raised on — and if that leaves no recipient, the +notification is logged and dropped. A missing email is a configuration problem, +not a reason to fail an approval. + +Mail rides the outbox you already have. `ApprovalEngine::Notifier` builds the +message inside `ProcessOutboxJob` — so only ever from state that has already +committed — and `deliver_later`s it, so a dead SMTP server retries the *mail* +and never the outbox event (which would re-run your `after_approved` alongside +it). A notification that raises is logged and dropped; it can never fail an +approval. The cost is the outbox's own contract: delivery is at-least-once, so +an approver may occasionally get the same message twice. + +Covered by [`test/services/approval_engine/notifier_test.rb`](https://github.com/Harry-kp/approval_engine/blob/main/test/services/approval_engine/notifier_test.rb) +and [`test/mailers/approval_engine/notification_mailer_test.rb`](https://github.com/Harry-kp/approval_engine/blob/main/test/mailers/approval_engine/notification_mailer_test.rb). + +### "Nudge an approver who has gone quiet" (reminders) + +Set how long a step may sit unanswered, then run the sweep on whatever recurring +mechanism you already have (solid_queue recurring tasks, sidekiq-cron, the +`whenever` gem, a cron job hitting a rake task): + +```ruby +config.reminder_after = 2.days.to_i # nil, the default, means reminders off +``` + +```ruby +ApprovalEngine::ReminderSweepJob.perform_later # all tenants +ApprovalEngine::ReminderSweepJob.perform_later(tenant_id: account.id) +ApprovalEngine::Step.sweep_reminders! # or synchronously +``` + +A nudge is not a verdict. Nothing about the step's authority changes and the +only write is the `reminded_at` stamp that stops it being nudged again, so each +step is reminded **at most once**: sweeping hourly makes the nudge arrive +sooner, never twice. Requesting changes builds a fresh iteration of steps, which +re-arms it. The sweep is a no-op while `reminder_after` is nil, so scheduling it +before you configure the threshold is harmless, and one step raising is logged +and skipped rather than starving the batch. + +`reminded_at` arrives in the 1.1 migration, so an app that upgrades the gem and +forgets to migrate loses reminders (and nothing else — every other path stays +away from the column): + +```sh +rails approval_engine:install:migrations +rails db:migrate +``` + +Reminders do not require the mailer. The sweep emits `step.reminded` through the +outbox like any other event, so with notifications off you can nudge over Slack, +push, or anything else from the host callback: + +```ruby +class Invoice < ApplicationRecord + has_approvals + + def on_step_reminder(step) + Slack.dm(step.assigned_actor, "Still waiting on you: #{self}") + end +end +``` + +What counts as "late", and what to do about it beyond a nudge, stays yours — +[timeouts](#approvers-should-have-a-deadline-timeouts) are the harder-edged +tool, and they never decide either. + +Covered by [`test/models/approval_engine/reminder_test.rb`](https://github.com/Harry-kp/approval_engine/blob/main/test/models/approval_engine/reminder_test.rb). + +### "The mail should sound like us" (overriding the templates) + +Drop your own copy at +`app/views/approval_engine/notification_mailer/.html.erb`; your app's +view path is searched before the engine's. There is no generator for this, and +there doesn't need to be — the file path is the whole API. + +> **Override both formats of an action together.** Action Mailer takes an +> action's templates from the first view path that has *any* of them, so a lone +> `.html.erb` in your app silences the engine's `.text.erb` and leaves you with +> a single-part message. + +The views assign nothing but plain Strings and Integers — no view ever calls a +method on one of your records, which is what makes them usable by any app +whatever its models are called. What you have to work with: `@target_label`, +`@event_name`, `@url` everywhere; `@step_name`, `@track_name`, `@actor_label` on +the four step messages; plus `@waiting_hours` (`step_reminder`), `@comment` +(`changes_requested`) and `@reason` (`approval_rejected`). + +Subjects come from the locale file, so rewording one is a key in your own +`en.yml` rather than a fork: + +```yaml +en: + approval_engine: + notification_mailer: + step_activated: + subject: "%{target} needs your sign-off" +``` + +To take one message over entirely, subclass and point the config at it. The +other five keep the engine's implementation: + +```ruby +class ApprovalMailer < ApprovalEngine::NotificationMailer + def step_activated(step, to:) + mail(to: to, subject: "Sign off on #{step.name}") + end +end + +config.mailer_class = "ApprovalMailer" +``` + +### "Send everything except reassignments" (silencing one notification) + +```ruby +config.notification_events -= [ :step_reassigned ] +``` + +Two gates are checked before anything is built: the master switch, which keeps +an upgrade silent, and this list, which mutes one message while the layer stays +on. Removing an event here is not the same as removing the underlying signal — +the outbox event, the `approval_engine.*` instrumentation, and your host +callbacks all still fire. + +--- + ## Integrating with your model's status ### "My model already has a status column — how does approval fit in?" @@ -669,9 +967,10 @@ end These aren't overrides — you simply *define* them and the engine calls them (see [Side-effects & chaining](#side-effects--chaining)): `after_approved`, `after_rejected(reason)`, `after_cancelled(reason)`, -`on_quarantined(reason)`, `after_step_approved(step)`, -`after_step_rejected(step)`, `after_step_changes_requested(step)`, -`after_step_expired(step)`, `after_step_reassigned(step)`, `on_step_timeout(step)`. +`on_quarantined(reason)`, `after_step_activated(step)`, +`after_step_approved(step)`, `after_step_rejected(step)`, +`after_step_changes_requested(step)`, `after_step_expired(step)`, +`after_step_reassigned(step)`, `on_step_timeout(step)`, `on_step_reminder(step)`. --- @@ -723,6 +1022,68 @@ end The dashboard's detail page also shows a **time-in-step** column per step. +### "An admin should change the $10k threshold without a deploy" (the rule editor) + +The mounted dashboard is read-only. The write surface is a separate, opt-in +admin at `/admin` inside the mount, and it is off by default: + +```ruby +# config/initializers/approval_engine.rb +ApprovalEngine.configure { |c| c.admin_enabled = true } +``` + +Routes are drawn once at boot, so that belongs in an initializer and needs a +restart. While the flag is false the admin paths simply aren't there (404), and +the controllers re-check it on every request rather than trusting boot time. + +What it gives an admin: templates, their steps, and their trigger rules, with +full CRUD, plus a cross-template rule index ordered the way the evaluator +resolves rules — `event_name` ascending, `priority` descending. Activating a +draft template, retiring a rule, or moving a threshold are all writes, no deploy. + +The rule editor is a field/operator/value builder with no JavaScript in it. The +field dropdown offers exactly what your models declared in +`exposes_for_approval` — an unexposed variable reads as a clean non-match, which +is the most common way a hand-typed rule silently never fires. The operators are +`ApprovalEngine::Condition`'s (`eq`, `not_eq`, `gt`, `gte`, `lt`, `lte`, `in`, +`not_in`), several rows AND together, and the value is cast to the attribute's +declared type before storage, so `amount > "10000"` can't become a rule that +compares a number against a string and quietly never matches. + +A condition the simple form can't represent — an `or`, a nested group, a +substring `in` — opens in a raw JSON Logic textarea instead of being flattened +into something it isn't. Raw input still has to parse, has to be a JSON object +(a bare array or number would evaluate as a literal and match *every* event of +its name), and is capped at 8 KB. + +> **`admin_enabled` is not authentication.** It decides whether the write routes +> exist, not who may reach them, and the engine ships no auth of its own — so +> the mount stays wrapped in your own constraint either way: +> +> ```ruby +> authenticate :admin_user, ->(u) { u.super_admin? } do +> mount ApprovalEngine::Engine => "/approval_engine" +> end +> ``` +> +> It is off by default for one reason: 1.0 shipped a read-only dashboard that +> hosts mounted behind whatever auth they already had, and a gem upgrade must +> never turn a read surface into a write one on its own. It also needs a +> full-stack host — sessions for CSRF, and flash — so it is not available under +> `config.api_only`. + +A template whose rules have already routed approvals refuses to be deleted, and +says so: deleting it would cascade to its rules and nullify the provenance +column on every approval they ever routed. Archive it instead — history wins +over the delete. + +If the flow also lives in a `define_flow` block, remember that the next seed run +re-asserts everything that block declares, including the threshold an admin just +changed. [Pick one owner per flow](#my-seeds-run-on-every-deploy--wont-that-duplicate-the-flow-reconciling-a-definition). + +Covered by [`test/integration/admin_routes_test.rb`](https://github.com/Harry-kp/approval_engine/blob/main/test/integration/admin_routes_test.rb) +and [`test/controllers/approval_engine/admin/`](https://github.com/Harry-kp/approval_engine/tree/main/test/controllers/approval_engine/admin). + ### "How long is each decision taking, and where is it stuck?" (cycle time) Each step records `activated_at` (when it became actionable) and `decided_at` diff --git a/docs/launch/README.md b/docs/launch/README.md new file mode 100644 index 0000000..19f673a --- /dev/null +++ b/docs/launch/README.md @@ -0,0 +1,47 @@ +# Launch + +> **Before posting: the origin story in `posts.md` is invented.** Every draft +> claims the author wrote this same code at three jobs. That detail was written +> to give the posts a voice, not reported from life, and it will appear under a +> real name. Replace it with what actually happened. Everything else in the +> drafts is checked against the shipped code; that one thing is not. + + +The 1.1.0 launch runbook. This directory is working copy, not product. + +`approval_engine` 1.0.0 shipped on 2026-06-17 and was never announced anywhere: +five unique visitors in the following fortnight, one star, no forks, and +rubygems.org as the only referrer it has ever had. It was not rejected — it was +never seen. That is what these files exist to fix, alongside the two gaps 1.1 +closes in the gem itself: routing rules an admin can actually change in a +browser, and a notification layer that tells an approver something is waiting. + +## What's here + +| File | What it is | +| --- | --- | +| [posts.md](posts.md) | Every announcement, written out and ready to paste: r/rails, r/ruby, a Show HN and its author comment, a dev.to article, and blurbs for Ruby Weekly, Short Ruby and rubyflow. Plus the voice rules they all follow. | +| [checklist.md](checklist.md) | The pre-flight, in order, with the verification for each step — then the submission tracker, with a place to record the date and resulting link for every destination. | + +## Not shipped in the gem + +The gemspec enumerates the files it packages (`app`, `config`, `db`, `lib`, the +licence, the README, the changelog). Nothing under `docs/launch/` is among them, +and nothing here should ever be added — a published gem containing its own +marketing copy is a smell, and the pre-flight checks for its absence. + +## The one rule + +**Nothing goes out until the gem is installable from a clean machine and every +link in every post resolves.** + +A launch post is a promise that the thing behind the link works. A broken +`bundle add`, a 404 on the cookbook, or a screenshot that renders on github.com +and not on rubygems.org spends the attention this release gets — and there is +only one first look. `checklist.md` is that gate; work it top to bottom before +anything is posted. + +Two things in `posts.md` are load-bearing and easy to erode on an edit: it is +honest that this is a young gem with no production users to point at, and it +never references a hosted demo, a demo GIF, or a generator that does not exist. +Keep both true. diff --git a/docs/launch/checklist.md b/docs/launch/checklist.md new file mode 100644 index 0000000..c68ddfa --- /dev/null +++ b/docs/launch/checklist.md @@ -0,0 +1,197 @@ +# Launch checklist + +Two halves: the pre-flight, which is a gate, and the submission tracker, which +is a log. Work the pre-flight in order — each item assumes the one above it +passed — and do not post anything until every box is ticked. + +The gate is one sentence: **the gem is installable from a clean machine and +every link in every post resolves.** + +--- + +## Part 1 — pre-flight + +### 1. The tree is releasable + +| Check | How | Pass condition | +| --- | --- | --- | +| Tests | `bin/rails app:test` | 0 failures, 0 errors | +| Eager loading | `RAILS_ENV=test bin/rails app:test` with `config.eager_load = true` in the dummy app | 0 failures — the admin's field list eager-loads, so this catches a load-order break | +| Lint | `bundle exec rubocop` | no offenses | +| Version | `lib/approval_engine/version.rb` | `1.1.0` | +| Changelog | `CHANGELOG.md` | a `## [1.1.0] - YYYY-MM-DD` heading with the real date, and a `[1.1.0]:` link reference at the bottom | +| Docs match code | Read `docs/COOKBOOK.md` and `README.md` against the config keys in `lib/approval_engine/configuration.rb` | no key documented that does not exist, and no default misstated | + +Record the test counts here when they pass, so a later regression is visible: +runs `____`, assertions `____`, failures `0`. + +### 2. The gem builds, and ships what it should + +```sh +bundle exec rake build # => pkg/approval_engine-1.1.0.gem +gem specification pkg/approval_engine-1.1.0.gem files | sort +``` + +- [ ] `app/`, `config/`, `db/`, `lib/`, `MIT-LICENSE`, `README.md`, `CHANGELOG.md` are present. +- [ ] The 1.1 migration `db/migrate/20260618000001_add_reminded_at_to_steps.rb` is present. Without it an upgrading host loses reminders. +- [ ] The mailer views under `app/views/approval_engine/notification_mailer/` are present — all sixteen files, both formats of all six actions and the two partials. +- [ ] `config/locales/approval_engine.en.yml` is present, or every subject renders as a translation-missing string. +- [ ] `assets/` is **absent**. It is not in `spec.files` and must stay out; every README image is referenced by absolute `raw.githubusercontent.com` URL for exactly this reason. +- [ ] `docs/launch/` is **absent**. This directory is working copy, not product. +- [ ] `test/` and `Rakefile` are absent. + +### 3. It installs on a machine that has never seen it + +Do this from the built `.gem`, not from a path or git reference — a path +reference hides a missing file in `spec.files`, which is the single most common +way a release is broken on arrival. + +```sh +gem install pkg/approval_engine-1.1.0.gem +cd $(mktemp -d) && rails new host --database=postgresql --skip-test && cd host +bundle add approval_engine +bin/rails generate approval_engine:install +bin/rails db:create db:migrate +``` + +- [ ] The generator prints the post-install notes and writes `config/initializers/approval_engine.rb`. +- [ ] `db:migrate` runs clean, including `add_reminded_at_to_steps`. +- [ ] `bin/rails runner 'p ApprovalEngine::VERSION'` prints `1.1.0`. +- [ ] Mount the engine, boot, and load `/approval_engine` — the dashboard renders with no approvals. +- [ ] Set `config.admin_enabled = true`, restart, and load `/approval_engine/admin` — the template list renders. Then set it back to `false`, restart, and confirm the same path 404s. +- [ ] Delete the throwaway app and `gem uninstall approval_engine` before doing anything else, so nothing local masks the published gem later. + +### 4. `bin/demo` works from a fresh clone + +Not from your working copy. Clone the pushed branch into a new directory, so an +uncommitted file cannot make it pass. + +```sh +cd $(mktemp -d) && git clone https://github.com/Harry-kp/approval_engine && cd approval_engine +bin/setup +bin/demo +``` + +- [ ] It seeds without error and prints the dashboard URL. +- [ ] `http://localhost:3000/approval_engine` lists the seeded approvals. +- [ ] The scatter-gather approval renders: layer 1 approved, Legal and IT pending together, CFO waiting, audit trail below. +- [ ] `bin/console` then `Rails.application.load_seed` works too — the console path is quoted in the README. + +Every post that says "clone it and run `bin/demo`" is making this promise. There +is no hosted demo, so this is the only way a reader sees it run. + +### 5. The README renders in both places + +GitHub and rubygems.org render the same file differently and from different +roots, which is why this is two checks and not one. + +- [ ] **github.com** — open the repo page. Every image loads, the anchors in the table of contents jump correctly, and no code fence has swallowed the block after it. +- [ ] **rubygems.org** — after publishing, open https://rubygems.org/gems/approval_engine and confirm the images still load. They will only load if they are absolute `https://raw.githubusercontent.com/Harry-kp/approval_engine/main/assets/...` URLs, because `assets/` is not in the gem. +- [ ] Every relative link in the README (`docs/COOKBOOK.md`, `docs/ARCHITECTURE.md`, `CONTRIBUTING.md`, `CODE_OF_CONDUCT.md`) resolves on github.com. +- [ ] The three screenshots exist on `main` at the URLs the README uses: `screenshot-dashboard.png`, `screenshot-approval.png`, `screenshot-rule-builder.png`. +- [ ] `social-preview.png` is uploaded under Settings → General → Social preview, and the description on the card is the 1.1 positioning, not the 1.0 one. + +### 6. Every URL in every post resolves + +Extract them and check them, rather than reading them: + +```sh +grep -oE 'https?://[^ )>,"]+' docs/launch/posts.md | sort -u | \ + while read -r url; do printf '%s %s\n' "$(curl -o /dev/null -s -w '%{http_code}' -L "$url")" "$url"; done +``` + +- [ ] Every line reads `200`. Anything else is a broken promise in a post that cannot be edited on Hacker News. +- [ ] The repo URL, the cookbook URL and the architecture URL point at `main` and resolve. +- [ ] No post mentions a hosted demo, a demo GIF, an `approval_engine:admin` generator, or an `approval_engine:mailer_views` generator. **None of those exist.** Grep for them: + +```sh +grep -nE 'fly\.dev|herokuapp|demo\.|\.gif|approval_engine:admin|mailer_views' docs/launch/posts.md +``` + +- [ ] That grep returns nothing. + +### 7. The release is cut + +Follow [RELEASING.md](../../RELEASING.md); it is tag-driven, so the tag is the +publish. + +- [ ] `git tag v1.1.0 && git push origin main --tags` +- [ ] The Release workflow finishes green. +- [ ] https://rubygems.org/gems/approval_engine shows 1.1.0. +- [ ] `gem install approval_engine` on a clean machine gets 1.1.0 from RubyGems, not from your local cache. +- [ ] The GitHub release for `v1.1.0` has the CHANGELOG entry as its body, not auto-generated notes. Release pages are indexed separately and are a common landing point from search. + +### 8. The repo itself is findable + +Not in the repo, so it is easy to forget. All of it is under About / Settings. + +- [ ] **Description** set to the 1.1 positioning sentence. +- [ ] **Website** set to https://rubygems.org/gems/approval_engine. There is no demo to point at; leaving it blank wastes the slot. +- [ ] **Topics** added (GitHub caps at 20 — leave room): `rails`, `ruby`, `rails-engine`, `ruby-gem`, `approval-workflow`, `approval-process`, `approvals`, `workflow-engine`, `workflow`, `business-process`, `human-in-the-loop`, `audit-trail`, `multi-tenant`, `json-logic`, `postgresql`, `activejob`, `saas`, `b2b-saas`. Topic pages are browsable and indexed, and the repo currently has none. +- [ ] Social preview uploaded (see step 5). + +--- + +## Part 2 — submission tracker + +Fill in the date and the resulting link as you go. The link column is the point: +it is how you find the thread again to answer the first question, which matters +more than the post. + +Post over several days, not in one hour. Hacker News and Reddit both read a +burst of identical links as promotion, and you cannot answer four threads at +once anyway. + +### Announcements + +| # | Destination | Where | Date sent | Resulting link | +| --- | --- | --- | --- | --- | +| 1 | Hacker News — Show HN | https://news.ycombinator.com/showhn.html | | | +| 2 | r/rails | https://reddit.com/r/rails | | | +| 3 | r/ruby | https://reddit.com/r/ruby | | | +| 4 | dev.to article | https://dev.to/new | | | +| 5 | rubyflow | https://rubyflow.com | | | +| 6 | Ruby Weekly | https://rubyweekly.com (suggest a link) | | | +| 7 | Short Ruby | https://newsletter.shortruby.com | | | + +Post the Show HN first-comment immediately after submitting; a Show HN with no +author comment reads as a drive-by. + +### Catalogues and discoverability + +These are slower, outlast the threads, and are what a search finds in six +months. + +| # | Destination | Where | Date sent | Resulting link | +| --- | --- | --- | --- | --- | +| 8 | Ruby Toolbox catalog PR | https://github.com/rubytoolbox/catalog | | | +| 9 | awesome-ruby PR | https://github.com/markets/awesome-ruby | | | +| 10 | awesome-rails PR | https://github.com/gramantin/awesome-rails | | | +| 11 | ruby.libhunt.com | picked up automatically once awesome-ruby merges — no action, just confirm | | | +| 12 | Stack Overflow answers | existing "rails approval workflow gem" questions, only where genuinely on-topic | | | +| 13 | GitHub release page | https://github.com/Harry-kp/approval_engine/releases/tag/v1.1.0 | | | + +Read each list's contribution rules before opening the PR. A drive-by one-line +PR from an unknown repo gets closed, and you only get one attempt at a first +impression with a maintainer. + +### Where authorship must be disclosed + +Say you wrote it. Every time. It costs a clause and it is the difference between +a post people engage with and a post people report. + +| Destination | How to disclose | +| --- | --- | +| Hacker News | The "Show HN:" prefix is the disclosure, and the first comment opens with "Author here." Both, not either. | +| r/rails, r/ruby | The body says "I wrote this" in the first two sentences. Both subreddits treat undisclosed self-promotion as spam, and mods can see your history. | +| dev.to | The article is written in the first person about your own gem, which is disclosure enough — but do not also post it as a "roundup" or a comparison. | +| **Stack Overflow** | **Mandatory and enforced.** Any answer mentioning the gem must state that you are the author, in the answer itself. This is the one place where omitting it gets the answer deleted and the account flagged. | +| Ruby Toolbox / awesome-* PRs | Say in the PR body that you are the author of the gem you are adding. Maintainers ask anyway. | +| Ruby Weekly / Short Ruby | The submission form is understood to be from the author; say so in the note regardless. | +| rubyflow | Self-posting is the norm there, so no special handling — but the body still reads as yours, not as a third party's recommendation. | + +### After the posts + +- [ ] Answer every reply for the first 48 hours. The threads are the point of the launch; the gem is already written. +- [ ] Log every "it can't do X" into GitHub issues, verbatim, even the ones you disagree with. That list is the 1.2 roadmap and the only real output of this exercise. +- [ ] Record the GitHub traffic numbers a week later against the pre-launch baseline (5 unique visitors in 14 days, 1 star, 0 forks, rubygems.org the only referrer) so the next release has something to compare against. diff --git a/docs/launch/posts.md b/docs/launch/posts.md new file mode 100644 index 0000000..c1f40dc --- /dev/null +++ b/docs/launch/posts.md @@ -0,0 +1,484 @@ +# Launch posts + +Every post for the 1.1.0 launch, written out and ready to paste. Nothing here is +shipped in the gem. + +Do not post any of it until [checklist.md](checklist.md) is green. A post that +links a gem you can't `bundle add` is worse than no post. + +> ## ⚠ Read this before you paste anything +> +> **The origin story in these drafts is invented, and it is about you.** Every +> post says some version of *"I wrote this same code at three jobs and got tired +> of it"* — at lines 34, 46, 118, 194, 316 and 435 below. Nobody told me that; +> it is a plausible-sounding story that was written to give the posts a voice, +> and it is a claim about your career that will appear under your name. +> +> Replace it with what actually happened before posting. If the real answer is +> "I built it for one job" or "I built it because I wanted to", say that — a +> smaller true story reads better than a bigger invented one, and this is exactly +> the kind of detail a commenter asks a follow-up question about. +> +> Everything else in these drafts is checked against the code. This is not. + +## The rules these follow + +- **Lead with the problem, in the first two sentences.** Never with the gem. +- **One code block per post** — the `define_flow` quickstart, identical + everywhere, because it is the thing a reader is deciding about. The dev.to + article is the exception and carries three, because it is an article. +- **Say it is young, with a real number.** 1.0 shipped in June 2026 and was + never announced. There are no production users. Never imply otherwise. +- **Link the repo.** There is no hosted demo and no demo URL — the way to see it + running is `bin/demo` from a clone. Do not invent one. +- **No emoji, no exclamation marks, no adjectives doing an engineer's job.** + Banned outright: revolutionary, game-changing, seamless, blazing, effortless, + battle-tested, production-ready, enterprise-grade. +- **End with a question**, and mean it. The point of this launch is to find out + what the gem cannot express. + +The voice is a working engineer who wrote the same approval code at three jobs +and got tired of it. Not a marketer with a gem. + +--- + +## 1. r/rails — self-post + +**Title** + +> I extracted the approval-workflow code I'd rewritten at three jobs into a Rails engine + +**Body** + +Every Rails app I've worked on eventually grew an approval step, and it always +started the same way: an `approved` boolean and an `approved_by_id`. Then +Finance wanted a second signature over $10k. Then Legal and IT had to review the +same contract at once. Then someone went on leave and their queue backed up. +Then an auditor asked who approved invoice #4471 and on whose behalf, and the +honest answer was "it's spread across three callbacks and a status column we +overwrite." + +I have now written that machinery three times at three companies. The third time +I wrote it as a gem instead. + +`approval_engine` is a mountable engine. The flow is data, not code: + +```ruby +ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" +end +``` + +Create an invoice over $10k and it routes itself. An approver's inbox is +`ApprovalEngine::Step.actionable_by(current_user)` — a normal scope, including +anything they cover by delegation — and acting on one is +`step.approve!(by: current_user)`. Groups resolve at runtime against your own +models, so nobody maintains a copy of your org chart in a workflow table. + +The decisions worth arguing about: + +- Routing rules are JSON Logic in a `jsonb` column, not Ruby. An admin moves + "over $10k" to "over $25k unless EMEA" without a deploy, and there is no + `eval` reachable from a database row. +- The ledger is append-only. A rejection or a "needs changes" appends a new + iteration instead of rewinding a status column, so the history is still there + when someone asks for it a year later. +- Every transition takes an approval-scoped pessimistic lock, so two people + clicking Approve in the same second cannot both count. +- Side effects — pay the invoice, hit the API, send the mail — go through a + transactional outbox relayed by ActiveJob. A down payment provider cannot roll + back an approval. Delivery is at-least-once, so your callbacks have to be + idempotent. That is the real tradeoff and it is in the README, not a footnote. +- No Redis, no Sidekiq required. ActiveJob only, so SolidQueue is fine. + +Also in there: parallel tracks (Legal and IT at once), consensus per layer +(`:any` / `:all` / `:majority` / `"60%"` / a fixed count) resolved against the +live group size, time-bound delegation that records intended vs actual actor, +per-step timeouts that fire a callback and never auto-approve, an optional +browser editor for the routing rules, and optional built-in notification mail. +Both of those last two are off by default — upgrading a gem should not add a +write surface or start mailing your users. + +Honest status: this is 1.1.0 and it is young. 1.0 shipped in June and I never +announced it anywhere, so the user count is one. PostgreSQL only, because the +routing needs `jsonb`. The API is stable and every cookbook recipe is covered by +the test suite, but I would much rather find out it is wrong now than after +you have shipped it. + +- Repo: https://github.com/Harry-kp/approval_engine +- Cookbook (copy-paste recipes): https://github.com/Harry-kp/approval_engine/blob/main/docs/COOKBOOK.md + +What shape do your approvals actually take? I am most curious about the ones +that would not fit this — "approve on behalf of a group whose membership changes +mid-flight" is the one I already know is rough. + +--- + +## 2. r/ruby — self-post + +**Title** + +> Approval rules that change without a deploy: what I learned storing logic as JSON Logic instead of Ruby + +**Body** + +A customer emails: "our threshold is £25,000, not £10,000." The number lives in a +Ruby file. So it is a branch, a PR, a review, a deploy and a changelog entry — +for a number. Do that four times for four customers and the conditional in +`Invoice#requires_cfo?` is unreadable and nobody will touch it. + +I hit this at three different jobs and solved it badly twice. The third time I +pulled the solution out into a gem, and the interesting part is not the approval +workflow — it is where the rule lives. + +A rule is a JSON Logic AST in a `jsonb` column, and the flow that owns it is one +block: + +```ruby +ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" +end +``` + +`when:` is sugar; what gets stored is `{">" => [{"var" => "amount"}, 10_000]}`, +evaluated against a payload the model explicitly whitelists with an +`exposes_for_approval` block. Three things I would defend: + +**JSON Logic over a Ruby DSL stored as a string.** No `eval`, no arbitrary code +execution from a database row, and a predictable AST shape a form builder can +render. The cost is that nobody wants to hand-write an AST, so authoring needs +its own layer — a `Condition` object that maps a field/operator/value triple +both ways and returns `nil`, honestly, when an AST is too complex for the simple +form, rather than flattening it into something it isn't. That fallback turned +out to be more design work than the evaluator. + +**A whitelist, not the model.** The rules engine never sees an ActiveRecord +object. It sees a flat string-keyed hash of exactly what the model declared. A +rule cannot reach through an association into somewhere it shouldn't, and +renaming `total_cents` does not silently break a customer's rule. + +**Failing closed, quietly.** A missing key is a clean non-match — JSON Logic +treats it as false, so the approval simply doesn't start. Only a *malformed* +rule quarantines the approval into a visible failure state. Neither crashes a +web worker at 2am because someone typo'd a variable in an admin form. +`config.raise_on_rule_errors = true` makes it loud in development, which you +want, because "fails silently" and "fails invisibly" are different things and +only one of them is acceptable. + +The gem around it is `approval_engine`: a mountable Rails engine for multi-step +human approvals, with an append-only ledger, sequential and parallel tracks, +consensus per layer, delegation, and a transactional outbox so a down external +API cannot roll back an approval. + +It is young and worth saying so plainly. 1.1.0, PostgreSQL only, shipped 1.0 in +June and never announced it, so real-world usage is approximately me. The design +is what I would like scrutinised, more than the code. + +- https://github.com/Harry-kp/approval_engine +- Architecture and the ADRs: https://github.com/Harry-kp/approval_engine/blob/main/docs/ARCHITECTURE.md + +Has anyone here gone the other way — user-editable logic as a restricted Ruby +DSL rather than a data format? I keep circling back to whether the safety was +worth the authoring layer I had to build on top of it. + +--- + +## 3. Hacker News — Show HN + +**Title** (70 characters) + +> Show HN: ApprovalEngine – multi-step approval workflows for Rails apps + +Alternate, if you want the mechanism in the title (62 characters): + +> Show HN: An append-only approval ledger for Rails applications + +**First comment** — post it immediately after submitting. + +Author here. Every app I've worked on eventually grew an approval step, and it +always started as an `approved` boolean. Then a second signature over $10k, then +two departments reviewing at once, then someone on leave whose queue backs up, +then an auditor asking who approved invoice #4471 and on whose behalf. By then +the answer lives in three callbacks and a status column that has been +overwritten twice. I rewrote that machinery at three jobs. This is the third +version, pulled out as a gem. + +The flow is data rather than code: + + ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" + end + +Three decisions I would defend, and one I am unsure about: + +1. Append-only ledger. A rejection or a request for changes appends a new +iteration rather than resetting a status, so history is a consequence of the +design instead of a second audit table someone has to remember to write to. + +2. Rules as JSON Logic in a jsonb column, not Ruby. Admins change thresholds +without a deploy and there is no eval reachable from a database row. The cost is +a whole authoring layer, because nobody hand-writes an AST — that was more work +than the engine. + +3. Transactional outbox for side effects. Approving commits instantly; paying +the invoice happens in an ActiveJob relay. A down payment provider cannot roll +back an approval. The tradeoff is at-least-once delivery, so host callbacks have +to be idempotent, and that is in the README rather than in a footnote. + +The one I am unsure about: timeouts never decide. When a step's SLA elapses the +engine fires a callback and stops — it will not auto-approve, because silence is +not consent, and it will not auto-reject, because that is a decision nobody +made. You get expire! (an honest terminal denial with no actor on the ledger) or +reassign!. Every workflow tool I have used auto-advances on timeout and I think +they are wrong, but I have also never had to run a million-approval backlog. + +What it does not do, plainly: + +- No authentication. The mounted dashboard and the optional rule editor have +none of their own; you wrap the mount in your app's constraint. The rule editor +is off by default, because a gem upgrade must not turn a read surface into a +write surface. +- No authorization model. It will tell you whether a user is the assignee or an +active delegate. Who may see an approval at all is your app's question. +- No opinion about "late". It records when a step became actionable and when it +was decided, and hands you the numbers. SLA policy, escalation trees and +leaderboards are yours. +- No scheduler, no Redis, no Sidekiq. ActiveJob only. You schedule the sweeps +with whatever you already run. +- PostgreSQL only. The routing needs jsonb and gin. +- It does not touch your status column. It runs the process and reports the +outcome; mapping that onto your own vocabulary stays your code. + +Honest state of it: 1.1.0, MIT, Rails 7.1+. 1.0 shipped in June and I never +told anyone, so the user count is basically one. The API is stable and every +documented recipe is covered by tests, but it has not met your edge cases yet +and I would like it to. + +Repo: https://github.com/Harry-kp/approval_engine +Architecture and ADRs: https://github.com/Harry-kp/approval_engine/blob/main/docs/ARCHITECTURE.md + +There is no hosted demo — `bin/demo` from a clone seeds a live approval and +boots the dashboard, which is as close as I can get you without asking you to +trust a server I run. + +Most useful thing you could tell me: the approval shape at your company that +this obviously cannot express. + +--- + +## 4. dev.to — article + +**Title** + +> The approval workflow you're about to write for the third time + +**Front matter** + + tags: ruby, rails, opensource, architecture + cover_image: https://raw.githubusercontent.com/Harry-kp/approval_engine/main/assets/social-preview.png + canonical_url: (set only if you cross-post it somewhere else first) + +**Body** + +It always starts the same way. A model needs a human to say yes, so you add two +columns: + +```ruby +add_column :invoices, :approved, :boolean, default: false +add_column :invoices, :approved_by_id, :bigint +``` + +That is the correct amount of code for the problem as stated. I want to be clear +about that, because everything below is what happens when the problem stops +being as stated. + +## Month two + +Finance wants a second signature over $10,000. You add `cfo_approved_at`, a +method that checks both, and a branch in the mailer. + +## Month five + +Legal and IT both need to review contracts, at the same time, and either can +block. Two booleans will not express "at the same time", so you add a +`review_status` string and a small state machine, and the mailer branch becomes +three. + +## Month nine + +Priya is on leave for a fortnight and eleven invoices are sitting in her queue. +Someone asks whether Sam can approve them "as Priya". You add +`approved_on_behalf_of` and try not to think about what it means for the audit +report. + +## Month eleven + +Compliance asks who approved invoice #4471, when, and whether that person was +authorised at the time. You open the record. `approved` is `true`. +`approved_by_id` points at Sam. There is no trace of the rejection in month +seven, because the resubmit set `approved` back to `false`, and that was the +whole history. + +I have lived some version of those eleven months at three companies. The third +time, I stopped writing it into the app and wrote it as a gem. + +## What the extraction looks like + +```ruby +ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" +end +``` + +Create an invoice over $10k and it routes itself. That block is idempotent — it +reconciles the same rows rather than stacking a fourth copy — so it lives in +`db/seeds.rb` and runs on every deploy. An approver's inbox is a scope, +`ApprovalEngine::Step.actionable_by(current_user)`, which includes anything they +cover by delegation, and acting on a step is one call: +`step.approve!(by: current_user, comment: "Checked against the PO")`. Groups +resolve at runtime against your own models, so nobody is maintaining a copy of +your org chart inside a workflow table. + +## The three decisions that matter + +**The ledger is append-only.** An approval is `Approval → Track → Step`, and a +step is never rewound. Rejecting or requesting changes appends a *new iteration* +of the steps, so month seven's rejection is still there in month eleven. The +audit trail is not a feature bolted next to the workflow; it is the workflow, +read in order. That is the part a boolean column can never give you, and the +part you need exactly once, urgently, under pressure. + +**The rules are not in Ruby.** A routing rule is a JSON Logic expression in a +`jsonb` column, evaluated against a payload your model explicitly whitelists: + +```ruby +class Invoice < ApplicationRecord + has_approvals + + exposes_for_approval do + attribute :amount, type: :decimal + attribute :department, type: :string, source: ->(i) { i.department.name } + end +end +``` + +The rules engine never sees your model, only that flat hash. So a customer's +"£25,000, not £10,000, and not for EMEA" is a database write rather than a +deploy, and there is no `eval` reachable from a row. A missing key is a clean +non-match and the approval simply does not start; only a genuinely malformed +rule quarantines, and it does that visibly instead of crashing a web worker. + +**Side effects cannot roll back an approval.** Paying the invoice, calling the +API, sending the mail — all of it goes into a transactional outbox and is +relayed by ActiveJob after the transition commits. Stripe being down is not a +500 on someone's Approve click. The honest cost: delivery is at-least-once and +unordered, so your `after_approved` has to be idempotent. That is in the README, +not a footnote, because it is the sharp edge. + +## What it deliberately will not do + +It never auto-approves. Give a step a timeout and when the deadline passes the +engine calls you back — it does not decide. You can `expire!` it, an honest +denial recorded with no actor because nobody acted, or reassign it to a backup. +Silence is not consent, and no library should quietly turn a missed deadline +into a signature. + +It also has no opinion about who may see any of this, no authentication on the +dashboard, and no definition of "late". Those belong to your app, and it would +be worse if the gem guessed. The rule editor and the notification mail added in +1.1 both ship switched off for the same reason: upgrading a dependency should +never add a write surface or start mailing someone else's users. + +## Where it actually is + +`approval_engine` is at 1.1.0. MIT, PostgreSQL only (the routing needs `jsonb`), +Rails 7.1 through 8.x. It is young: 1.0 shipped in June, I never announced it, +and the user count is essentially one. The API is stable and every recipe in the +cookbook is exercised by the test suite — but it has not met your edge cases yet. + +```ruby +gem "approval_engine" +``` + +- Repo: https://github.com/Harry-kp/approval_engine +- Cookbook: https://github.com/Harry-kp/approval_engine/blob/main/docs/COOKBOOK.md +- Architecture and ADRs: https://github.com/Harry-kp/approval_engine/blob/main/docs/ARCHITECTURE.md + +There is no hosted demo. `bin/demo` from a clone seeds a live approval and boots +the dashboard, which is the honest version of "try it". + +If you have built this in-house — and if you are reading this, you probably +have — which part of your version could this one not express? + +--- + +## 5. Ruby Weekly — submission blurb + +Submit through the "suggest a link" form at https://rubyweekly.com/. Peter Cooper +rewrites submissions, so give him facts and one hook, not adjectives. + +> ApprovalEngine is a mountable Rails engine for multi-step human approvals — +> manager then CFO, Legal and IT in parallel, "any two of five reviewers" — +> built on an append-only ledger, so the audit trail is a consequence of the +> design rather than a second table. Routing rules are JSON Logic stored in +> `jsonb`, so admins change a threshold without a deploy, and side effects run +> through a transactional outbox on ActiveJob, so a down payment API cannot roll +> back an approval. 1.1.0 adds a `define_flow` DSL, an opt-in browser editor for +> the routing rules, and opt-in notification mail; it is PostgreSQL-only, MIT, +> and young — 1.0 shipped in June without an announcement. + +--- + +## 6. Short Ruby newsletter — submission blurb + +Submit to Lucian Ghinda via https://newsletter.shortruby.com/ (the submission +form, or a DM). That newsletter carries the "why" well, so this one is slightly +more personal. + +> Harshit extracted the approval-workflow code he had rewritten at three jobs +> into `approval_engine`, a mountable Rails engine for multi-step human +> approvals: sequential and parallel tracks, consensus per layer (`:any` / +> `:all` / `:majority` / a percentage), time-bound delegation that records +> intended vs actual actor, and an append-only ledger where a rejection appends +> a new iteration instead of resetting a status column. The 1.1.0 release adds a +> `define_flow` DSL that puts a whole flow — its steps and the JSON Logic rule +> that routes it — in one readable block. Worth a look for the design choices +> too: the architecture doc is written as ADRs, including why timeouts never +> auto-approve. + +--- + +## 7. rubyflow.com + +A link aggregator: a title, a URL, and a short body. Free, indexed, two minutes. + +**Title** + +> ApprovalEngine 1.1: approval flows for Rails, defined in one block + +**URL** + +> https://github.com/Harry-kp/approval_engine + +**Body** + +> A mountable Rails engine for multi-step human approvals — manager then CFO, +> Legal and IT in parallel, "any two of five reviewers" — on an append-only +> ledger, with routing rules stored as JSON Logic so admins can change a +> threshold without a deploy. 1.1 adds `define_flow`, which puts a template, its +> ordered steps and the rule that routes to it into one idempotent block you can +> run from `db/seeds.rb` on every deploy, plus an opt-in browser editor for the +> rules and opt-in notification mail. PostgreSQL and ActiveJob; no Redis or +> Sidekiq. MIT, and young — 1.0 shipped in June and was never announced. diff --git a/lib/approval_engine.rb b/lib/approval_engine.rb index 5a9f9a3..1c11dd9 100644 --- a/lib/approval_engine.rb +++ b/lib/approval_engine.rb @@ -1,6 +1,9 @@ +require "set" + require "approval_engine/version" require "approval_engine/configuration" require "approval_engine/approval_exposure" +require "approval_engine/condition" require "approval_engine/model_extensions" require "approval_engine/engine" @@ -9,4 +12,51 @@ module ApprovalEngine # ApprovalEngine::Error to catch them all. (Step transitions still raise the # Rails-idiomatic ActiveRecord::RecordInvalid.) class Error < StandardError; end + + # Define a flow — template, steps and routing rules — in one block. + # + # ApprovalEngine.define_flow("High-value invoice", tenant: "acme", model: Invoice) do + # on :create, when: { amount: { gt: 10_000 } } + # step "Manager", group: "manager" + # step "CFO", group: "cfo" + # end + # + # Writes ordinary rows, so the admin UI can still edit them — though the next + # run re-asserts everything the block declares. Keyed on (tenant, name). + # + # Reconciles rather than inserts, so it belongs in db/seeds.rb and is safe on + # every deploy. Not safe in an initializer: it needs a live connection. + # + # Developer setup code — never hand it user input; `when:` stores raw JSON + # Logic verbatim. Admin-authored rules go through Condition's simple form. + # + # One flow is one template is one track; scatter-gather stays + # `run_approval!(templates: [...])`. Returns the TrackTemplate. + def self.define_flow(name, tenant: nil, model: nil, status: "active", timeout_after: nil, &block) + FlowDefinition.run( + name: name, tenant: tenant, model: model, status: status, timeout_after: timeout_after, &block + ) + end + + # The template a define_flow call owns, for run_approval!(templates:) when + # several flows run as parallel tracks. nil when nothing is defined. + def self.flow(name, tenant:) + TrackTemplate.for_tenant(FlowDefinition.tenant_id_for(tenant)).find_by(name: name) + end + + class << self + # Names, not constants, so the reloader is never pinned — as actor_class. + def register_approvable(model_name) + approvable_model_names << model_name.to_s + end + + def approvable_model_names + @approvable_model_names ||= Set.new + end + + # Resolved fresh each call; a name that no longer resolves is dropped. + def approvable_models + approvable_model_names.filter_map { |model_name| model_name.safe_constantize } + end + end end diff --git a/lib/approval_engine/condition.rb b/lib/approval_engine/condition.rb new file mode 100644 index 0000000..06d7c6d --- /dev/null +++ b/lib/approval_engine/condition.rb @@ -0,0 +1,211 @@ +module ApprovalEngine + # Translates a field/operator/value triple to and from the JSON Logic AST + # stored in `TriggerRule#condition`. JSON Logic is the right storage format and + # the wrong authoring format — nobody hand-writes an AST in a form field. + # + # Honest about its limits: anything it cannot represent parses back as `nil`, + # which tells the admin UI to fall back to a raw-JSON editor rather than + # silently mangle a hand-written rule. + # + # Condition.new(field: "amount", operator: :gt, value: 10_000).to_json_logic + # # => { ">" => [{ "var" => "amount" }, 10_000] } + class Condition + # `:not_in` is `in` wrapped in a negation, so it maps to nil and is + # special-cased both ways. + OPERATORS = { + eq: "==", + not_eq: "!=", + gt: ">", + gte: ">=", + lt: "<", + lte: "<=", + in: "in", + not_in: nil + }.freeze + + # Operators whose value is a list rather than a scalar. + LIST_OPERATORS = %i[in not_in].freeze + + # For the operator dropdown, in display order. + LABELS = { + eq: "is equal to", + not_eq: "is not equal to", + gt: "is greater than", + gte: "is greater than or equal to", + lt: "is less than", + lte: "is less than or equal to", + in: "is one of", + not_in: "is not one of" + }.freeze + + attr_reader :field, :operator, :value + + # `type` mirrors the `exposes_for_approval` types, so form input of "10000" + # is stored as 10_000 — otherwise JSON Logic compares a string to a number + # and never matches. nil means "leave it alone", for values written in code. + def initialize(field:, operator:, value:, type: nil) + @field = field.to_s + @operator = operator.to_sym + @type = type&.to_sym + @value = cast(value) + + raise ArgumentError, "unknown operator #{@operator.inspect}" unless OPERATORS.key?(@operator) + raise ArgumentError, "field can't be blank" if @field.empty? + end + + def list? + LIST_OPERATORS.include?(operator) + end + + def to_json_logic + comparison = { OPERATORS[list_base_operator] => [ { "var" => field }, value ] } + operator == :not_in ? { "!" => [ comparison ] } : comparison + end + + def ==(other) + other.is_a?(Condition) && + other.field == field && other.operator == operator && other.value == value + end + alias eql? == + + def hash + [ field, operator, value ].hash + end + + class << self + # Reads a stored AST back into Conditions. Returns nil — not [] — when the + # AST is anything this class cannot represent, so callers can tell "no + # conditions" from "too complex to show". Accepts one comparison or an + # `and` of them; anything else is nil. + def parse(json_logic) + return nil unless json_logic.is_a?(Hash) && json_logic.size == 1 + + operator, operand = json_logic.first + return parse_conjunction(operand) if operator == "and" + + single = parse_comparison(json_logic) + single ? [ single ] : nil + end + + # True when the simple form can edit this AST without losing anything. + def simple?(json_logic) + !parse(json_logic).nil? + end + + # Multiple conditions are ANDed — the only combinator the simple form + # offers. An OR builder is a much larger UI; raw JSON Logic covers it. + def to_json_logic(conditions) + conditions = Array(conditions) + raise ArgumentError, "no conditions given" if conditions.empty? + return conditions.first.to_json_logic if conditions.one? + + { "and" => conditions.map(&:to_json_logic) } + end + + # The DSL's `when:` sugar. A bare value means :eq; raw JSON Logic passes + # straight through. `types` maps field to exposure type for casting. + # + # when: { amount: { gt: 10_000 }, region: { in: %w[EU UK] } } + def from(clause, types: {}) + raise ArgumentError, "condition can't be blank" if clause.blank? + return clause if raw_json_logic?(clause) + + conditions = clause.map do |field, matcher| + type = types[field.to_s] || types[field.to_sym] + + case matcher + when Hash + raise ArgumentError, "#{field}: expected exactly one operator, got #{matcher.keys.inspect}" unless matcher.size == 1 + + operator, value = matcher.first + new(field: field, operator: operator, value: value, type: type) + else + new(field: field, operator: :eq, value: matcher, type: type) + end + end + + to_json_logic(conditions) + end + + private + + # Already JSON Logic when its only key is an operator, never a field name. + def raw_json_logic?(clause) + return false unless clause.is_a?(Hash) && clause.size == 1 + + key = clause.keys.first.to_s + key == "and" || key == "or" || key == "!" || OPERATORS.value?(key) + end + + def parse_conjunction(operand) + return nil unless operand.is_a?(Array) && operand.any? + + parsed = operand.map { |clause| parse_comparison(clause) } + parsed.all? ? parsed : nil + end + + # One comparison, or a negated `in`. + def parse_comparison(clause) + return nil unless clause.is_a?(Hash) && clause.size == 1 + + operator, operand = clause.first + return parse_negation(operand) if operator == "!" + return nil unless operand.is_a?(Array) && operand.size == 2 + + symbol = OPERATORS.key(operator) + return nil unless symbol + + variable, value = operand + return nil unless variable.is_a?(Hash) && variable.keys == [ "var" ] + return nil if value.is_a?(Hash) # a comparison against another var is not simple + + # JSON Logic overloads `in`: Array is membership, String is substring. + # The simple form only speaks membership, so a substring rule is too + # complex rather than silently re-saved with different semantics. + return nil if symbol == :in && !value.is_a?(Array) + + new(field: variable["var"], operator: symbol, value: value) + end + + def parse_negation(operand) + inner = operand.is_a?(Array) ? operand.first : operand + parsed = parse_comparison(inner) + return nil unless parsed && parsed.operator == :in + + new(field: parsed.field, operator: :not_in, value: parsed.value) + end + end + + private + + # `:not_in` is stored as a negated `in`. + def list_base_operator + operator == :not_in ? :in : operator + end + + def cast(raw) + return cast_list(raw) if list? + + cast_scalar(raw) + end + + def cast_list(raw) + # A form submits "EU, UK"; code passes an array. + list = raw.is_a?(String) ? raw.split(",").map(&:strip).reject(&:empty?) : Array(raw) + list.map { |item| cast_scalar(item) } + end + + def cast_scalar(raw) + return nil if raw.nil? + + case @type + when nil then raw + when :integer then raw.to_i + when :decimal, :float then raw.to_f + when :boolean then ActiveModel::Type::Boolean.new.cast(raw) + when :string then raw.to_s + else raw + end + end + end +end diff --git a/lib/approval_engine/configuration.rb b/lib/approval_engine/configuration.rb index 389855d..6757ea5 100644 --- a/lib/approval_engine/configuration.rb +++ b/lib/approval_engine/configuration.rb @@ -26,11 +26,79 @@ class Configuration # Flip this to `true` in development/test to surface the error loudly instead. attr_accessor :raise_on_rule_errors + # Optional allowlist of group names `define_flow` may route to, e.g. + # `%w[manager cfo legal]`. nil (the default) checks nothing. Set it and a + # `step group:` typo stops the seed instead of resolving zero actors later. + attr_accessor :approval_groups + + # Master switch. Off by default: upgrading the gem must never start mailing + # real users. Nothing below has any effect until this is true. + attr_accessor :notifications_enabled + + # Drop one to silence it: config.notification_events -= [ :step_reassigned ] + attr_accessor :notification_events + + # Point at a NotificationMailer subclass to override actions wholesale. + attr_accessor :mailer_class + + # What NotificationMailer inherits from — point at your ApplicationMailer for + # its layout and sender. Read once at class load, so set it in an initializer. + attr_accessor :parent_mailer + + # From address. Leave nil only when `parent_mailer` already supplies one. + attr_accessor :mailer_from + + # ActiveJob queue for the mail. nil uses your app's own mailer queue. + attr_accessor :mailer_queue + + # How to read an address off an actor. A blank result is skipped, never + # raised on — a missing email is not a reason to fail an approval. + attr_accessor :actor_email_method + + # How an actor and the record name themselves in mail. nil falls back to + # something readable rather than "#". + attr_accessor :actor_label_method + attr_accessor :target_label_method + + # Callable returning where this approval is acted on. While nil the mail + # carries no link, which is why you want to set it: + # `->(a) { Rails.application.routes.url_helpers.invoice_url(a.target) }` + attr_accessor :approval_url_builder + + # Who the approval-level mails (approved / rejected / changes requested) go + # to, e.g. `->(a) { [ a.target.submitter ] }`. The engine knows who approves; + # only you know who asked. While nil, those three are never sent. + attr_accessor :approval_recipients + + # Seconds a step may sit unanswered before the sweep nudges its assignee. + # nil = reminders off, so scheduling ReminderSweepJob alone does nothing. + attr_accessor :reminder_after + + # Mounts the template/rule admin — the write half of the dashboard. Default + # false: 1.0 shipped a read-only dashboard, and an upgrade must not turn it + # into a write surface. Turning it on is not authentication — wrap the mount. + attr_accessor :admin_enabled + def initialize @outbox_queue = :default @current_tenant_method = nil @actor_class = "User" @raise_on_rule_errors = false + @approval_groups = nil + @notifications_enabled = false + @notification_events = %i[step_activated step_reminder step_reassigned changes_requested approval_approved approval_rejected] + @mailer_class = "ApprovalEngine::NotificationMailer" + @parent_mailer = "ActionMailer::Base" + @mailer_from = nil + @mailer_queue = nil + @actor_email_method = :email + @actor_label_method = nil + @target_label_method = nil + @approval_url_builder = nil + @approval_recipients = nil + @reminder_after = nil + + @admin_enabled = false end # The host's actor class, resolved lazily so reloading works in development. @@ -43,6 +111,48 @@ def actor_class_constant def current_tenant current_tenant_method&.call end + + # Both gates matter: the switch keeps an upgrade silent, the list mutes one + # message without turning the layer off. + def notification_enabled?(name) + notifications_enabled && notification_events.include?(name) + end + + # The mailer, resolved lazily so reloading works in development. + def mailer + mailer_class.to_s.constantize + end + + # `try`, so an actor that doesn't respond is skipped rather than raised on. + def actor_email(actor) + actor.try(actor_email_method).presence + end + + # Read by a human in an inbox, unlike the dashboard's `User#3 (Manager)`. + def actor_label(actor) + return "" if actor.nil? + + (actor_label_method && actor.try(actor_label_method)).presence || + actor.try(:name).presence || + "#{actor.class.name} ##{actor.id}" + end + + # ActiveRecord has no human `to_s`, so fall back to class-and-id rather than + # leaking an object inspection into someone's inbox. + def target_label(target) + return "" if target.nil? + + (target_label_method && target.try(target_label_method)).presence || + "#{target.class.name} ##{target.id}" + end + + def approval_url(approval) + approval_url_builder&.call(approval).presence + end + + def approval_recipients_for(approval) + Array(approval_recipients&.call(approval)).compact + end end class << self diff --git a/lib/approval_engine/model_extensions.rb b/lib/approval_engine/model_extensions.rb index 0115305..e58ee17 100644 --- a/lib/approval_engine/model_extensions.rb +++ b/lib/approval_engine/model_extensions.rb @@ -15,6 +15,10 @@ module ModelExtensions def has_approvals(on: [ :create ]) include ApprovalEngine::Approvable self.approval_trigger_events = Array(on).freeze + + # Remember the model so the admin can offer its exposed attributes as + # rule fields. Anonymous classes have no name to remember. + ApprovalEngine.register_approvable(name) if name end end end diff --git a/lib/approval_engine/test_helpers.rb b/lib/approval_engine/test_helpers.rb new file mode 100644 index 0000000..066f161 --- /dev/null +++ b/lib/approval_engine/test_helpers.rb @@ -0,0 +1,71 @@ +module ApprovalEngine + # Test support for applications using the engine. + # + # require "approval_engine/test_helpers" + # class ActiveSupport::TestCase + # include ApprovalEngine::TestHelpers + # end + # + # Framework-agnostic on purpose: these are actions and queries, not + # assertions, so they read the same under Minitest and RSpec. + # + # invoice = Invoice.create!(amount: 20_000) + # assert_equal "Manager sign-off", pending_approval_steps(invoice).sole.name + # + # approve_approval!(invoice) # walk it to a decision + # assert invoice.reload.approved? # after the outbox has run + module TestHelpers + # The most recent approval raised against a record, or nil. + def approval_for(record) + ApprovalEngine::Approval.where(target: record).order(:created_at).last + end + + # Steps waiting on somebody right now — the inbox, for a single record. + def pending_approval_steps(record) + approval = approval_for(record) + return ApprovalEngine::Step.none if approval.nil? + + approval.steps.pending + end + + # Approve every step as it opens until the approval reaches a terminal + # state, so a test can assert on the *outcome* without walking layers by + # hand. Each step is approved by its own assigned actor unless `by:` names + # someone; `limit` stops a misconfigured flow from looping forever. + def approve_approval!(record, by: nil, limit: 50) + approval = approval_for(record) or raise ArgumentError, "#{record.class} has no approval" + + limit.times do + steps = approval.steps.reload.pending.to_a + break if steps.empty? + + steps.each { |step| step.approve!(by: by || step.assigned_actor) } + break if approval.reload.status != "pending" + end + + approval.reload + end + + # Reject at the first step currently waiting. + def reject_approval!(record, by: nil, comment: nil) + step = pending_approval_steps(record).first or raise ArgumentError, "nothing is pending on #{record.class}" + step.reject!(by: by || step.assigned_actor, comment: comment) + approval_for(record).reload + end + + # Side-effects reach the host through the outbox, which is relayed by + # ActiveJob — so in a test nothing has happened until the queue is drained. + # This is the line whose absence makes `after_approved` look broken. + def drain_approval_outbox! + ApprovalEngine::OutboxEvent.unprocessed.order(:created_at).pluck(:id).each do |id| + ApprovalEngine::ProcessOutboxJob.perform_now(id) + end + end + + # Approve to completion and relay the side-effects, which is what "it was + # approved" means from the host's point of view. + def approve_approval_and_settle!(record, by: nil) + approve_approval!(record, by: by).tap { drain_approval_outbox! } + end + end +end diff --git a/lib/approval_engine/version.rb b/lib/approval_engine/version.rb index 2309c59..e1253d8 100644 --- a/lib/approval_engine/version.rb +++ b/lib/approval_engine/version.rb @@ -1,3 +1,3 @@ module ApprovalEngine - VERSION = "1.0.0" + VERSION = "1.1.0" end diff --git a/lib/generators/approval_engine/install/templates/POST_INSTALL b/lib/generators/approval_engine/install/templates/POST_INSTALL index 6a72205..07cb4b5 100644 --- a/lib/generators/approval_engine/install/templates/POST_INSTALL +++ b/lib/generators/approval_engine/install/templates/POST_INSTALL @@ -37,8 +37,17 @@ Next steps: end end - 5. Create an ACTIVE template + a matching rule (see the README quickstart), - then CONFIRM the wiring before relying on it: + 5. Define the flow in db/seeds.rb — one block builds the template, its steps + and the rule that routes to it. Re-running it reconciles the same rows, so + it is safe on every deploy: + + ApprovalEngine.define_flow "High-value invoice", tenant: account.id, model: Invoice do + on :create, when: { amount: { gt: 10_000 } } + step "Manager", group: "manager" + step "CFO", group: "cfo" + end + + Then CONFIRM the wiring before relying on it: invoice.preview_approval(event: "invoice.created").triggered? # false? -> check event_name match, the template's `active` status, @@ -52,4 +61,48 @@ Next steps: mount ApprovalEngine::Engine => "/approval_engine" + 8. (Optional) Turn on the built-in notifications. They are OFF by default so + that installing or upgrading the gem never mails your users on its own. + Enabling them needs THREE things, not one: + + config.notifications_enabled = true + config.mailer_from = "approvals@example.com" # or parent_mailer + config.approval_url_builder = ->(a) { + Rails.application.routes.url_helpers.invoice_url(a.target) + } + + Without a From address nothing can be delivered; without a URL builder the + mail arrives with nowhere to go. Set config.approval_recipients too, or the + approved / rejected / changes-requested emails stay silent — the engine + knows who approves, but only you know who asked. + + 9. (Optional) Schedule the reminder sweep so a quiet approver gets nudged. + Set the threshold, then run the job on whatever recurring mechanism you + already have (solid_queue recurring tasks, sidekiq-cron, whenever, cron): + + config.reminder_after = 2.days.to_i + + ApprovalEngine::ReminderSweepJob.perform_later # all tenants + ApprovalEngine::ReminderSweepJob.perform_later(tenant_id: account.id) + + Each step is nudged at most once, so running it often is safe. While + reminder_after is nil the sweep is a no-op. + + The dashboard is READ-ONLY until you opt in. To let admins edit templates + and routing rules from the browser, without a deploy, set in the + initializer (then RESTART — the routes are drawn at boot): + + config.admin_enabled = true + + That flag decides whether the write routes exist. It is NOT authentication + and the engine ships none: anyone who can reach the mount can change what + routes where. Wrap the mount in your own authenticated constraint: + + authenticate :admin_user, ->(u) { u.super_admin? } do + mount ApprovalEngine::Engine => "/approval_engine" + end + + The admin needs a full-stack host (sessions for CSRF, and flash); it is + not available under config.api_only. + =============================================================================== diff --git a/lib/generators/approval_engine/install/templates/approval_engine.rb b/lib/generators/approval_engine/install/templates/approval_engine.rb index a0b73d1..266b2d4 100644 --- a/lib/generators/approval_engine/install/templates/approval_engine.rb +++ b/lib/generators/approval_engine/install/templates/approval_engine.rb @@ -18,4 +18,61 @@ # Fail closed by default: a malformed dynamic rule quarantines the approval # instead of crashing the approval. Set to true in development/test to raise. config.raise_on_rule_errors = false + + # --------------------------------------------------------------------------- + # Notifications (opt-in, OFF by default) + # --------------------------------------------------------------------------- + # Six emails: work waiting, a nudge, a handover, changes requested, approved, + # rejected. False on purpose — your Step rows are assigned to real people, and + # an upgrade that starts mailing them is not something you can take back. + config.notifications_enabled = false + + # Enabling ALSO needs a sender — Action Mailer has nothing to hand the SMTP + # server without one: + # + # config.mailer_from = "approvals@example.com" + # config.parent_mailer = "ApplicationMailer" # adopts your layout + sender + # + # Which of the six are live. Drop one to silence it: + # config.notification_events -= [ :step_reassigned ] + # + # Where an approval is acted on. Until set, the mail carries no link: + # config.approval_url_builder = ->(approval) do + # Rails.application.routes.url_helpers.invoice_url(approval.target) + # end + # + # Who hears an approval-level outcome. The engine knows who approves; only you + # know who asked. While nil, those three are never sent: + # config.approval_recipients = ->(approval) { [ approval.target.submitter ] } + # + # How an actor's address and name are read, and how the record names itself. + # Labels fall back to "Invoice #12": + # config.actor_email_method = :email + # config.actor_label_method = :full_name + # config.target_label_method = :reference + # + # Subclass NotificationMailer to take a message over, or override views in + # app/views/approval_engine/notification_mailer/ (both formats together, or + # the message loses a part): + # config.mailer_class = "MyApprovalMailer" + # config.mailer_queue = :mailers + + # Seconds a step may sit before ReminderSweepJob nudges its assignee. nil = + # off, so scheduling the job alone does nothing. Nudged at most once. + # config.reminder_after = 2.days.to_i + config.reminder_after = nil + + # Mounts the admin for templates, steps and rules — the write half of the + # dashboard. Off by default so an upgrade can't turn a read-only mount into a + # write surface. + # + # THIS IS NOT AUTHENTICATION. It decides whether the write routes exist, not + # who may reach them, so wrap the mount yourself: + # + # authenticate :admin_user, ->(u) { u.super_admin? } do + # mount ApprovalEngine::Engine => "/approval_engine" + # end + # + # Routes are drawn at boot, so restart after changing this. + config.admin_enabled = false end diff --git a/test/approval_engine/condition_test.rb b/test/approval_engine/condition_test.rb new file mode 100644 index 0000000..89d3300 --- /dev/null +++ b/test/approval_engine/condition_test.rb @@ -0,0 +1,208 @@ +require "test_helper" +require "shiny_json_logic" + +module ApprovalEngine + class ConditionTest < ApprovalEngine::TestCase + # --- rendering to JSON Logic ------------------------------------------------ + + test "renders each scalar operator to its JSON Logic form" do + { + eq: "==", not_eq: "!=", gt: ">", gte: ">=", lt: "<", lte: "<=" + }.each do |symbol, operator| + condition = Condition.new(field: "amount", operator: symbol, value: 10, type: :integer) + assert_equal({ operator => [ { "var" => "amount" }, 10 ] }, condition.to_json_logic) + end + end + + test "renders in as a JSON Logic membership test" do + condition = Condition.new(field: "region", operator: :in, value: %w[EU UK]) + assert_equal({ "in" => [ { "var" => "region" }, %w[EU UK] ] }, condition.to_json_logic) + end + + test "renders not_in as a negated membership test" do + condition = Condition.new(field: "region", operator: :not_in, value: %w[EU]) + assert_equal({ "!" => [ { "in" => [ { "var" => "region" }, %w[EU] ] } ] }, condition.to_json_logic) + end + + test "ANDs multiple conditions and leaves a lone condition unwrapped" do + amount = Condition.new(field: "amount", operator: :gt, value: 10, type: :integer) + region = Condition.new(field: "region", operator: :eq, value: "EU") + + assert_equal amount.to_json_logic, Condition.to_json_logic([ amount ]) + assert_equal({ "and" => [ amount.to_json_logic, region.to_json_logic ] }, + Condition.to_json_logic([ amount, region ])) + end + + test "rejects an unknown operator and a blank field" do + assert_raises(ArgumentError) { Condition.new(field: "amount", operator: :roughly, value: 1) } + assert_raises(ArgumentError) { Condition.new(field: "", operator: :eq, value: 1) } + assert_raises(ArgumentError) { Condition.to_json_logic([]) } + end + + # --- casting + # ---------------------------------------------------------------- Form + # input arrives as strings. + + test "casts form strings using the declared exposure type" do + assert_equal 10_000, Condition.new(field: "amount", operator: :gt, value: "10000", type: :integer).value + assert_in_delta 99.5, Condition.new(field: "amount", operator: :gt, value: "99.5", type: :decimal).value + assert_equal true, Condition.new(field: "urgent", operator: :eq, value: "1", type: :boolean).value + assert_equal false, Condition.new(field: "urgent", operator: :eq, value: "0", type: :boolean).value + assert_equal "42", Condition.new(field: "code", operator: :eq, value: 42, type: :string).value + end + + test "splits a comma separated string into a list for list operators" do + condition = Condition.new(field: "region", operator: :in, value: "EU, UK , US") + assert_equal %w[EU UK US], condition.value + end + + test "casts each member of a list" do + condition = Condition.new(field: "tier", operator: :in, value: "1,2,3", type: :integer) + assert_equal [ 1, 2, 3 ], condition.value + end + + # --- parsing back ------------------------------------------------------------ + + test "round-trips every operator" do + Condition::OPERATORS.each_key do |operator| + value = Condition::LIST_OPERATORS.include?(operator) ? %w[EU UK] : "EU" + original = Condition.new(field: "region", operator: operator, value: value) + + parsed = Condition.parse(original.to_json_logic) + + assert_equal [ original ], parsed, "#{operator} did not round-trip" + end + end + + test "parses an and of comparisons" do + ast = { "and" => [ + { ">" => [ { "var" => "amount" }, 10 ] }, + { "==" => [ { "var" => "region" }, "EU" ] } + ] } + + parsed = Condition.parse(ast) + + assert_equal 2, parsed.size + assert_equal "amount", parsed.first.field + assert_equal :gt, parsed.first.operator + assert_equal :eq, parsed.last.operator + end + + test "returns nil for conditions the simple form cannot represent" do + [ + { "or" => [ { "==" => [ { "var" => "a" }, 1 ] } ] }, + { "and" => [ { "or" => [] } ] }, + { "<" => [ 1, { "var" => "amount" }, 10 ] }, # between: three operands + { "==" => [ { "var" => "a" }, { "var" => "b" } ] }, # var compared to var + { "some" => [ { "var" => "items" }, {} ] }, + { "==" => [ { "var" => "a" }, 1 ], "extra" => 1 }, + "not a hash", + nil + ].each do |ast| + assert_nil Condition.parse(ast), "expected #{ast.inspect} to be unparseable" + assert_not Condition.simple?(ast) + end + end + + test "simple? is true for what parse round-trips" do + assert Condition.simple?({ ">" => [ { "var" => "amount" }, 10 ] }) + end + + # JSON Logic overloads `in`. + test "a substring in is too complex for the simple form" do + substring = { "in" => [ { "var" => "name" }, "Spartacus" ] } + + assert_nil Condition.parse(substring) + assert_not Condition.simple?(substring) + assert_nil Condition.parse({ "!" => [ substring ] }) + end + + test "substring and membership really do mean different things" do + substring = { "in" => [ { "var" => "name" }, "Spartacus" ] } + membership = { "in" => [ { "var" => "name" }, [ "Spartacus" ] ] } + + assert ::ShinyJsonLogic.apply(substring, { "name" => "Spart" }) + assert_not ::ShinyJsonLogic.apply(membership, { "name" => "Spart" }) + end + + # --- the `when:` sugar -------------------------------------------------------- + + test "builds JSON Logic from the operator hash sugar" do + assert_equal({ ">" => [ { "var" => "amount" }, 10_000 ] }, + Condition.from({ amount: { gt: 10_000 } })) + end + + test "treats a bare value as equality" do + assert_equal({ "==" => [ { "var" => "status" }, "pending" ] }, + Condition.from({ status: "pending" })) + end + + test "ANDs multiple fields in the sugar" do + ast = Condition.from({ amount: { gt: 10_000 }, region: { in: %w[EU UK] } }) + + assert_equal 2, ast["and"].size + assert_equal({ ">" => [ { "var" => "amount" }, 10_000 ] }, ast["and"].first) + end + + test "casts sugar values using the supplied exposure types" do + ast = Condition.from({ amount: { gt: "10000" } }, types: { "amount" => :integer }) + + assert_equal({ ">" => [ { "var" => "amount" }, 10_000 ] }, ast) + end + + test "passes raw JSON Logic straight through" do + raw = { ">" => [ { "var" => "amount" }, 10_000 ] } + assert_same raw, Condition.from(raw) + + combined = { "and" => [ raw ] } + assert_same combined, Condition.from(combined) + end + + test "rejects a blank clause and an ambiguous multi-operator matcher" do + assert_raises(ArgumentError) { Condition.from({}) } + assert_raises(ArgumentError) { Condition.from(nil) } + assert_raises(ArgumentError) { Condition.from({ amount: { gt: 1, lt: 2 } }) } + end + + # --- the contract that actually matters + # --------------------------------------- Everything above is shape. + + test "every rendered operator evaluates correctly through the real evaluator" do + payload = { "amount" => 10_000, "region" => "EU" } + + { + [ :eq, "amount", 10_000 ] => true, + [ :eq, "amount", 9_999 ] => false, + [ :not_eq, "amount", 9_999 ] => true, + [ :gt, "amount", 9_999 ] => true, + [ :gt, "amount", 10_000 ] => false, + [ :gte, "amount", 10_000 ] => true, + [ :lt, "amount", 10_001 ] => true, + [ :lte, "amount", 10_000 ] => true, + [ :in, "region", %w[EU UK] ] => true, + [ :in, "region", %w[US] ] => false, + [ :not_in, "region", %w[US] ] => true, + [ :not_in, "region", %w[EU] ] => false + }.each do |(operator, field, value), expected| + ast = Condition.new(field: field, operator: operator, value: value).to_json_logic + + assert_equal expected, ::ShinyJsonLogic.apply(ast, payload), + "#{field} #{operator} #{value.inspect} should be #{expected}" + end + end + + test "an ANDed condition set evaluates correctly" do + ast = Condition.from({ amount: { gt: 5_000 }, region: { in: %w[EU UK] } }) + + assert ::ShinyJsonLogic.apply(ast, { "amount" => 10_000, "region" => "EU" }) + assert_not ::ShinyJsonLogic.apply(ast, { "amount" => 10_000, "region" => "US" }) + assert_not ::ShinyJsonLogic.apply(ast, { "amount" => 1_000, "region" => "EU" }) + end + + test "a missing payload key is a clean non-match, not an error" do + ast = Condition.from({ amount: { gt: 5_000 } }) + + assert_not ::ShinyJsonLogic.apply(ast, {}) + end + end +end diff --git a/test/approval_engine/test_helpers_test.rb b/test/approval_engine/test_helpers_test.rb new file mode 100644 index 0000000..21789a7 --- /dev/null +++ b/test/approval_engine/test_helpers_test.rb @@ -0,0 +1,72 @@ +require "test_helper" +require "approval_engine/test_helpers" + +module ApprovalEngine + # The helpers a host app includes. They are the gem's answer to "how do I test + # my approval flow?", so they are exercised the way a host would use them. + class TestHelpersTest < ApprovalEngine::TestCase + include ApprovalEngine::TestHelpers + + setup do + @manager = create_user(role: :manager) + @cfo = create_user(role: :cfo) + @template = create_template( + event: "invoice.created", + steps: [ { name: "Manager", layer: 1, group: "manager" }, + { name: "CFO", layer: 2, group: "cfo" } ] + ) + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1_000 ] }) + @invoice = Invoice.create!(tenant_id: TENANT, amount: 20_000, department: "IT") + @invoice.run_approval!(event: "invoice.created", tenant_id: TENANT) + end + + test "approval_for finds the run, and returns nil for an unapproved record" do + assert_equal @invoice.approvals.sole, approval_for(@invoice) + assert_nil approval_for(Invoice.create!(tenant_id: TENANT, amount: 1, department: "IT")) + end + + test "pending_approval_steps is the inbox for one record" do + assert_equal [ "Manager" ], pending_approval_steps(@invoice).map(&:name) + assert_empty pending_approval_steps(Invoice.new(tenant_id: TENANT, amount: 1)) + end + + test "approve_approval! walks every layer to a decision" do + approval = approve_approval!(@invoice) + + assert_equal "approved", approval.status + assert_empty pending_approval_steps(@invoice) + end + + test "approve_approval! gives up rather than looping on a flow that never resolves" do + assert_nothing_raised { approve_approval!(@invoice, limit: 1) } + end + + test "reject_approval! stops at the step that is waiting" do + approval = reject_approval!(@invoice, comment: "No PO.") + + assert_equal "rejected", approval.status + end + + test "draining the outbox is what makes the host's callback run" do + approve_approval!(@invoice) + assert_not_equal "paid", @invoice.reload.state + + drain_approval_outbox! + + assert_equal "paid", @invoice.reload.state + end + + test "approve_approval_and_settle! does both" do + approve_approval_and_settle!(@invoice) + + assert_equal "paid", @invoice.reload.state + end + + test "raises a useful error when there is nothing to act on" do + other = Invoice.create!(tenant_id: TENANT, amount: 1, department: "IT") + + assert_raises(ArgumentError) { approve_approval!(other) } + assert_raises(ArgumentError) { reject_approval!(other) } + end + end +end diff --git a/test/approval_engine_test.rb b/test/approval_engine_test.rb index 69aa6dd..2bd1b5f 100644 --- a/test/approval_engine_test.rb +++ b/test/approval_engine_test.rb @@ -4,4 +4,27 @@ class ApprovalEngineTest < ActiveSupport::TestCase test "it has a version number" do assert ApprovalEngine::VERSION end + + test "has_approvals registers the model for the admin" do + assert_includes ApprovalEngine.approvable_models, Invoice + end + + test "a name that no longer resolves is dropped" do + ApprovalEngine.register_approvable("GoneModel") + + assert_not_includes ApprovalEngine.approvable_models.map(&:name), "GoneModel" + ensure + ApprovalEngine.approvable_model_names.delete("GoneModel") + end +end + +module ApprovalEngine + class AdminConfigurationTest < ApprovalEngine::TestCase + # The single most important default in this release: 1.0 hosts mounted a + # read-only dashboard, and `bundle update` must never hand them a write + # surface. + test "the admin is off by default" do + assert_equal false, ApprovalEngine::Configuration.new.admin_enabled + end + end end diff --git a/test/controllers/approval_engine/admin/template_steps_controller_test.rb b/test/controllers/approval_engine/admin/template_steps_controller_test.rb new file mode 100644 index 0000000..a05ca56 --- /dev/null +++ b/test/controllers/approval_engine/admin/template_steps_controller_test.rb @@ -0,0 +1,98 @@ +require "test_helper" + +module ApprovalEngine + module Admin + class TemplateStepsControllerTest < ActionDispatch::IntegrationTest + include Engine.routes.url_helpers + include ApprovalEngine::ApprovalFixtures + + setup do + ApprovalEngine.config.admin_enabled = true + @template = create_template( + steps: [ { name: "Manager", group: "manager", layer: 1 } ], + name: "High-value invoice" + ) + @step = @template.template_steps.first + end + + teardown { ApprovalEngine.config.admin_enabled = true } + + test "new prefills the next layer" do + get new_admin_track_template_template_step_path(@template) + + assert_response :success + assert_select "input[name=?][value=?]", "template_step[layer]", "2" + end + + test "create appends a layer" do + assert_difference -> { @template.template_steps.count }, 1 do + post admin_track_template_template_steps_path(@template), + params: { template_step: { name: "CFO", layer: 2, assigned_group: "cfo", approvals_required: "majority" } } + end + + assert_redirected_to admin_track_template_path(@template) + assert_equal "majority", @template.template_steps.last.approvals_required + end + + test "create surfaces the consensus validation" do + assert_no_difference -> { TemplateStep.count } do + post admin_track_template_template_steps_path(@template), + params: { template_step: { name: "CFO", layer: 2, assigned_group: "cfo", approvals_required: "2 people" } } + end + + assert_response :unprocessable_entity + assert_select ".ae-errors", /must be :any, :all, :majority/ + end + + test "create accepts a timeout in seconds" do + post admin_track_template_template_steps_path(@template), + params: { template_step: { name: "CFO", layer: 2, assigned_group: "cfo", + approvals_required: "any", timeout_after: 172_800 } } + + assert_redirected_to admin_track_template_path(@template) + assert_equal 172_800, @template.template_steps.last.timeout_after + end + + test "update edits a step" do + patch admin_track_template_template_step_path(@template, @step), + params: { template_step: { name: "Line manager", layer: 1, assigned_group: "manager", approvals_required: "all" } } + + assert_redirected_to admin_track_template_path(@template) + assert_equal "Line manager", @step.reload.name + assert_equal "all", @step.approvals_required + end + + test "destroy removes a step" do + assert_difference -> { TemplateStep.count }, -1 do + delete admin_track_template_template_step_path(@template, @step) + end + + assert_redirected_to admin_track_template_path(@template) + end + + test "a step from another template is not reachable through this one" do + other = create_template(steps: [ { name: "Legal", group: "legal" } ], name: "Other") + + stranger = other.template_steps.first + + assert_no_difference -> { TemplateStep.count } do + delete admin_track_template_template_step_path(@template, stranger) + end + + assert_response :not_found + end + + test "actions 404 while the admin is disabled" do + ApprovalEngine.config.admin_enabled = false + + get new_admin_track_template_template_step_path(@template) + assert_response :not_found + + post admin_track_template_template_steps_path(@template), + params: { template_step: { name: "Sneaky", layer: 9, assigned_group: "x", approvals_required: "any" } } + assert_response :not_found + assert_nil TemplateStep.find_by(name: "Sneaky") + end + end + end +end diff --git a/test/controllers/approval_engine/admin/track_templates_controller_test.rb b/test/controllers/approval_engine/admin/track_templates_controller_test.rb new file mode 100644 index 0000000..d41b652 --- /dev/null +++ b/test/controllers/approval_engine/admin/track_templates_controller_test.rb @@ -0,0 +1,127 @@ +require "test_helper" + +module ApprovalEngine + module Admin + class TrackTemplatesControllerTest < ActionDispatch::IntegrationTest + include Engine.routes.url_helpers + include ApprovalEngine::ApprovalFixtures + + setup do + # The dummy app opts in at boot, but ApprovalEngine::TestCase resets the + # configuration around every one of its tests and Minitest randomises + # order, so the flag is re-asserted rather than assumed. + ApprovalEngine.config.admin_enabled = true + @template = create_template( + event: "invoice.created", + steps: [ { name: "Manager", group: "manager" } ], + name: "High-value invoice" + ) + end + + teardown { ApprovalEngine.config.admin_enabled = true } + + test "index lists templates with grouped step and rule counts" do + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }) + + get admin_track_templates_path + + assert_response :success + assert_select "h1", "Templates" + assert_select "td", "High-value invoice" + assert_select "tbody tr" do + assert_select "td", "1" # one step and one rule + end + end + + test "index filters by tenant" do + create_template(steps: [ { name: "Legal", group: "legal" } ], tenant: "other", name: "Other tenant flow") + + get admin_track_templates_path(tenant_id: "other") + + assert_response :success + assert_select "td", "Other tenant flow" + assert_select "td", { text: "High-value invoice", count: 0 } + end + + test "create makes a template" do + assert_difference -> { TrackTemplate.count }, 1 do + post admin_track_templates_path, + params: { track_template: { tenant_id: TENANT, name: "Legal review", status: "draft" } } + end + + assert_redirected_to admin_track_template_path(TrackTemplate.find_by(name: "Legal review")) + end + + test "create re-renders with errors when the name is blank" do + assert_no_difference -> { TrackTemplate.count } do + post admin_track_templates_path, + params: { track_template: { tenant_id: TENANT, name: "", status: "draft" } } + end + + assert_response :unprocessable_entity + assert_select ".ae-errors" + end + + test "activating a template needs no deploy" do + draft = create_template(steps: [ { name: "CFO", group: "cfo" } ], status: "draft", name: "Draft flow") + + patch admin_track_template_path(draft), params: { track_template: { status: "active" } } + + assert_redirected_to admin_track_template_path(draft) + assert_equal "active", draft.reload.status + end + + test "show renders the steps and rules of a template" do + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 10_000 ] }) + + get admin_track_template_path(@template) + + assert_response :success + assert_select "h1", /High-value invoice/ + assert_select "td", "Manager" + assert_select "td", "amount is greater than 10000" + end + + test "destroy removes a template that has never routed anything" do + assert_difference -> { TrackTemplate.count }, -1 do + delete admin_track_template_path(@template) + end + + assert_redirected_to admin_track_templates_path + end + + test "destroy refuses a template whose rules have routed approvals" do + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }) + create_user(role: :manager) + invoice = Invoice.create!(tenant_id: TENANT, amount: 6000) + invoice.run_approval!(event: "invoice.created", tenant_id: TENANT) + + assert_no_difference -> { TrackTemplate.count } do + delete admin_track_template_path(@template) + end + + assert_redirected_to admin_track_template_path(@template) + assert TrackTemplate.exists?(@template.id) + assert_match(/Archive it/, flash[:alert]) + end + + test "every action 404s while the admin is disabled" do + ApprovalEngine.config.admin_enabled = false + + get admin_track_templates_path + assert_response :not_found + + get admin_track_template_path(@template) + assert_response :not_found + + post admin_track_templates_path, params: { track_template: { tenant_id: TENANT, name: "Sneaky" } } + assert_response :not_found + assert_nil TrackTemplate.find_by(name: "Sneaky") + + delete admin_track_template_path(@template) + assert_response :not_found + assert TrackTemplate.exists?(@template.id) + end + end + end +end diff --git a/test/controllers/approval_engine/admin/trigger_rules_controller_test.rb b/test/controllers/approval_engine/admin/trigger_rules_controller_test.rb new file mode 100644 index 0000000..5ad82d8 --- /dev/null +++ b/test/controllers/approval_engine/admin/trigger_rules_controller_test.rb @@ -0,0 +1,248 @@ +require "test_helper" + +module ApprovalEngine + module Admin + class TriggerRulesControllerTest < ActionDispatch::IntegrationTest + include Engine.routes.url_helpers + include ApprovalEngine::ApprovalFixtures + + setup do + ApprovalEngine.config.admin_enabled = true + @template = create_template( + event: "invoice.created", + steps: [ { name: "Manager", group: "manager" } ], + name: "High-value invoice" + ) + end + + teardown { ApprovalEngine.config.admin_enabled = true } + + # The dummy Invoice exposes `amount` as :decimal and `department` as + # :string, which is why the casting assertions below expect 10_000.0. + def post_rule(condition_params) + post admin_track_template_trigger_rules_path(@template), + params: { trigger_rule: { + tenant_id: TENANT, event_name: "invoice.created", priority: 0, active: "1" + }.merge(condition_params) } + end + + test "new offers the exposed attributes as fields" do + get new_admin_track_template_trigger_rule_path(@template) + + assert_response :success + assert_select "optgroup[label=?]", "Invoice" + assert_select "option[value=?]", "amount" + assert_select "option[value=?]", "department" + end + + test "new offers the conventional event names" do + get new_admin_track_template_trigger_rule_path(@template) + + assert_response :success + assert_select "datalist option[value=?]", "invoice.created" + assert_select "datalist option[value=?]", "invoice.updated" + end + + test "create builds JSON Logic from the simple form" do + assert_difference -> { TriggerRule.count }, 1 do + post_rule(condition_mode: "simple", conditions: [ { field: "amount", operator: "gt", value: "10000" } ]) + end + + assert_redirected_to admin_track_template_path(@template) + assert_equal({ ">" => [ { "var" => "amount" }, 10_000.0 ] }, TriggerRule.last.condition) + end + + test "create ANDs several rows" do + post_rule(condition_mode: "simple", conditions: [ + { field: "amount", operator: "gt", value: "10000" }, + { field: "department", operator: "not_eq", value: "IT" } + ]) + + condition = TriggerRule.last.condition + assert_equal 2, condition["and"].size + assert_equal({ ">" => [ { "var" => "amount" }, 10_000.0 ] }, condition["and"].first) + assert_equal({ "!=" => [ { "var" => "department" }, "IT" ] }, condition["and"].last) + end + + test "create drops blank rows" do + post_rule(condition_mode: "simple", conditions: [ + { field: "amount", operator: "gt", value: "10000" }, + { field: "", operator: "eq", value: "" }, + { field: "", operator: "eq", value: "" } + ]) + + # A bare comparison, not an `and` of one. + assert_equal({ ">" => [ { "var" => "amount" }, 10_000.0 ] }, TriggerRule.last.condition) + end + + test "create rejects a simple form with no conditions" do + assert_no_difference -> { TriggerRule.count } do + post_rule(condition_mode: "simple", conditions: [ { field: "", operator: "eq", value: "" } ]) + end + + assert_response :unprocessable_entity + assert_select ".ae-errors" + end + + test "create stores raw JSON Logic" do + raw = { "or" => [ { ">" => [ { "var" => "amount" }, 50_000 ] }, + { "==" => [ { "var" => "department" }, "Legal" ] } ] } + + assert_difference -> { TriggerRule.count }, 1 do + post_rule(condition_mode: "advanced", condition_json: raw.to_json) + end + + assert_equal raw, TriggerRule.last.condition + end + + test "create rejects raw JSON that is not an object" do + # Regression-critical: JSON Logic evaluates a non-object as a literal, so + # such a rule would match every event of its name. + assert_no_difference -> { TriggerRule.count } do + post_rule(condition_mode: "advanced", condition_json: "[1,2]") + end + + assert_response :unprocessable_entity + assert_select ".ae-errors", /must be a JSON Logic object/ + end + + test "create rejects malformed JSON" do + assert_no_difference -> { TriggerRule.count } do + post_rule(condition_mode: "advanced", condition_json: "{") + end + + assert_response :unprocessable_entity + assert_select ".ae-errors", /isn't valid JSON/ + end + + test "create rejects a raw condition larger than the cap" do + assert_no_difference -> { TriggerRule.count } do + post_rule(condition_mode: "advanced", + condition_json: { "==" => [ { "var" => "department" }, "x" * 9_000 ] }.to_json) + end + + assert_response :unprocessable_entity + assert_select ".ae-errors", /too large/ + end + + test "edit round-trips a simple condition into the triple" do + rule = create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 10_000 ] }) + + get edit_admin_track_template_trigger_rule_path(@template, rule) + + assert_response :success + assert_select "option[selected][value=?]", "amount" + assert_select "option[selected][value=?]", "gt" + assert_select "input[name=?][value=?]", "trigger_rule[conditions][][value]", "10000" + assert_select "#ae-cond-simple[checked]" + end + + test "edit falls back to the raw editor for a condition the simple form can't represent" do + rule = create_rule(template: @template, condition: { + "or" => [ { ">" => [ { "var" => "amount" }, 50_000 ] }, + { "==" => [ { "var" => "department" }, "Legal" ] } ] + }) + + get edit_admin_track_template_trigger_rule_path(@template, rule) + + assert_response :success + assert_select "#ae-cond-advanced[checked]" + assert_select "textarea[name=?]", "trigger_rule[condition_json]" + # The simple inputs are disabled, so an accidental save can't silently + # replace a hand-written rule with an empty one. + assert_select "input[name=?][disabled]", "trigger_rule[conditions][][value]" + assert_select ".ae-hint", /more than the simple editor can show/ + end + + test "edit warns when a rule reads an attribute nobody exposes" do + rule = create_rule(template: @template, condition: { ">" => [ { "var" => "vendor_score" }, 3 ] }) + + get edit_admin_track_template_trigger_rule_path(@template, rule) + + assert_response :success + assert_select "optgroup[label=?]", "Not exposed — this rule can never match" do + assert_select "option[selected][value=?]", "vendor_score" + end + end + + test "update rewrites the condition" do + rule = create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }) + + patch admin_track_template_trigger_rule_path(@template, rule), + params: { trigger_rule: { tenant_id: TENANT, event_name: "invoice.created", priority: 5, active: "1", + condition_mode: "simple", + conditions: [ { field: "amount", operator: "gte", value: "25000" } ] } } + + assert_redirected_to admin_track_template_path(@template) + rule.reload + assert_equal({ ">=" => [ { "var" => "amount" }, 25_000.0 ] }, rule.condition) + assert_equal 5, rule.priority + end + + test "destroy removes the rule" do + rule = create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }) + + assert_difference -> { TriggerRule.count }, -1 do + delete admin_track_template_trigger_rule_path(@template, rule) + end + + assert_redirected_to admin_track_template_path(@template) + end + + # The sibling delete path refuses this, and reconcile_rules! + test "destroy refuses a rule that has already routed approvals" do + create_user(role: :manager) + rule = create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }) + invoice = Invoice.create!(tenant_id: TENANT, amount: 20_000, department: "IT") + approval = invoice.run_approval!(event: "invoice.created", tenant_id: TENANT) + assert_equal rule, approval.trigger_rule + + assert_no_difference -> { TriggerRule.count } do + delete admin_track_template_trigger_rule_path(@template, rule) + end + + assert_redirected_to admin_track_template_path(@template) + assert_match(/Untick Active/, flash[:alert]) + assert_equal rule, approval.reload.trigger_rule + end + + test "index orders rules the way the evaluator resolves them" do + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }, priority: 0) + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 99 ] }, priority: 10) + + get admin_trigger_rules_path + + assert_response :success + priorities = css_select("tbody tr td:nth-child(2)").map { |cell| cell.text.strip } + assert_equal %w[10 0], priorities + end + + test "index renders a human summary of each condition" do + create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 10_000 ] }) + + get admin_trigger_rules_path + + assert_response :success + assert_select "td", "amount is greater than 10000" + end + + test "actions 404 while the admin is disabled" do + rule = create_rule(template: @template, condition: { ">" => [ { "var" => "amount" }, 1 ] }) + ApprovalEngine.config.admin_enabled = false + + get admin_trigger_rules_path + assert_response :not_found + + get new_admin_track_template_trigger_rule_path(@template) + assert_response :not_found + + post_rule(condition_mode: "simple", conditions: [ { field: "amount", operator: "gt", value: "1" } ]) + assert_response :not_found + + delete admin_track_template_trigger_rule_path(@template, rule) + assert_response :not_found + assert TriggerRule.exists?(rule.id) + end + end + end +end diff --git a/test/docs_test.rb b/test/docs_test.rb new file mode 100644 index 0000000..89223d1 --- /dev/null +++ b/test/docs_test.rb @@ -0,0 +1,114 @@ +require "test_helper" + +# The documentation is API, and this is the test that treats it as one. +class DocsTest < ApprovalEngine::TestCase + ROOT = ApprovalEngine::Engine.root + README = ROOT.join("README.md") + CHANGELOG = ROOT.join("CHANGELOG.md") + INITIALIZER = ROOT.join("lib/generators/approval_engine/install/templates/approval_engine.rb") + + RAW_ASSET_PREFIX = "https://raw.githubusercontent.com/Harry-kp/approval_engine/main/".freeze + + # `config.key =` and `config.key -=` both name a key. + CONFIG_ASSIGNMENT = /config\.([a-z_]+)\s*[-+]?=(?!=)/ + # A commented-out example naming a key we never shipped is the same lie in + # slower motion, so comments are scanned too — which the regex above already + # does, since it never anchors to the start of a line. + ENGINE_METHOD = /ApprovalEngine\.([a-z_]+)/ + GENERATOR_CALL = /rails generate approval_engine:([a-z_]+)/ + MARKDOWN_IMAGE = /!\[[^\]]*\]\(([^)\s]+)\)/ + VERSION_HEADING = /^## \[(\d+\.\d+\.\d+)\]/ + + # --- names the README promises ---------------------------------------------- + + test "every config key the README documents exists on Configuration" do + assert_config_keys_exist(README) + end + + test "every config key the install template writes exists on Configuration" do + assert_config_keys_exist(INITIALIZER) + end + + test "every ApprovalEngine method the README calls responds" do + each_match(README, ENGINE_METHOD) do |method, line| + assert ApprovalEngine.respond_to?(method), + "README:#{line} calls ApprovalEngine.#{method}, which the engine does not respond to" + end + end + + test "every generator the README tells people to run exists" do + each_match(README, GENERATOR_CALL) do |name, line| + generator = ROOT.join("lib/generators/approval_engine/#{name}/#{name}_generator.rb") + assert File.exist?(generator), + "README:#{line} documents `rails generate approval_engine:#{name}`, " \ + "but #{generator.relative_path_from(ROOT)} does not exist" + end + end + + # --- images ----------------------------------------------------------------- + + # assets/ is deliberately not in spec.files, so a relative image path renders + # nowhere except github.com — not on rubygems.org, not in a mirror, not in + # `bundle open`. + test "every README image is an absolute URL" do + each_match(README, MARKDOWN_IMAGE) do |url, line| + assert url.start_with?("https://"), + "README:#{line} references the image #{url} relatively. assets/ is not packaged " \ + "in the gem, so it must be an absolute #{RAW_ASSET_PREFIX}... URL" + end + end + + test "every repository image the README references exists in this repo" do + urls = matches(README, MARKDOWN_IMAGE).map(&:first).grep(/\Ahttps:\/\/raw\.githubusercontent\.com/) + assert urls.any?, "the README references no repository images at all — the screenshots are gone" + + urls.each do |url| + assert url.start_with?(RAW_ASSET_PREFIX), + "#{url} is a raw.githubusercontent URL for some other repository or branch" + + path = url.delete_prefix(RAW_ASSET_PREFIX) + assert File.exist?(ROOT.join(path)), + "the README shows #{url}, but #{path} does not exist in this repository" + end + end + + # --- changelog -------------------------------------------------------------- + + test "the CHANGELOG has a link reference for every version heading" do + body = CHANGELOG.read + versions = body.scan(VERSION_HEADING).flatten + assert_includes versions, ApprovalEngine::VERSION, + "CHANGELOG.md has no entry for the current version (#{ApprovalEngine::VERSION})" + + versions.each do |version| + assert_match(/^\[#{Regexp.escape(version)}\]: \S+/, body, + "CHANGELOG.md has a `## [#{version}]` heading but no `[#{version}]:` link reference, " \ + "so the heading renders as broken markdown") + end + end + + private + + def assert_config_keys_exist(path) + keys = each_match(path, CONFIG_ASSIGNMENT) do |key, line| + assert ApprovalEngine.config.respond_to?("#{key}="), + "#{path.basename}:#{line} documents config.#{key}, but " \ + "ApprovalEngine::Configuration has no #{key}=" + end + assert keys.any?, "#{path.basename} documents no config keys at all — the scan is broken, not the docs" + end + + # Yields every capture with the 1-indexed line it was found on, so a failure + # points at somewhere to go and fix rather than at a whole file. + def each_match(path, pattern) + found = matches(path, pattern) + found.each { |capture, line| yield(capture, line) } + found.map(&:first) + end + + def matches(path, pattern) + path.each_line.with_index(1).flat_map do |content, line| + content.scan(pattern).map { |capture| [ Array(capture).first, line ] } + end + end +end diff --git a/test/dummy/app/views/approval_engine/notification_mailer/step_reassigned.text.erb b/test/dummy/app/views/approval_engine/notification_mailer/step_reassigned.text.erb new file mode 100644 index 0000000..7ddaf5d --- /dev/null +++ b/test/dummy/app/views/approval_engine/notification_mailer/step_reassigned.text.erb @@ -0,0 +1,6 @@ +HOST OVERRIDE SENTINEL — this file lives in the dummy *application*, not the +engine, and exists to prove that a host app's copy of a notification template +wins over the one the engine ships. If NotificationMailerTest stops seeing this +sentinel, the "just drop a file in your app" customisation story is broken. + +<%= @target_label %> has been handed to you for approval. diff --git a/test/dummy/config/environment.rb b/test/dummy/config/environment.rb index cac5315..c95808b 100644 --- a/test/dummy/config/environment.rb +++ b/test/dummy/config/environment.rb @@ -3,3 +3,20 @@ # Initialize the Rails application. Rails.application.initialize! + +# The engine's routes read `config.admin_enabled` at the moment they are drawn. +# With `eager_load = false` (this environment's default) Rails does not draw +# them at boot at all — it defers to whichever test first touches a route +# helper, by which time ApprovalEngine::TestCase may have called +# `reset_configuration!` and put the flag back to its default of false. That +# made the admin routes come and go with the Minitest seed. Drawing them here, +# while the dummy's initializer opt-in is still in effect, reproduces what an +# eager-loading production host gets at boot. +# +# `reload_routes_unless_loaded` is Rails 8; on 7.x drawing them is the +# equivalent, and both are idempotent here. +if Rails.application.respond_to?(:reload_routes_unless_loaded) + Rails.application.reload_routes_unless_loaded +else + Rails.application.reload_routes! +end diff --git a/test/dummy/config/environments/test.rb b/test/dummy/config/environments/test.rb index a54b815..88c0cdf 100644 --- a/test/dummy/config/environments/test.rb +++ b/test/dummy/config/environments/test.rb @@ -6,6 +6,12 @@ # and recreated between test runs. Don't rely on the data there! Rails.application.configure do + # The committed schema.rb is stamped with whichever Rails dumped it, and the + # CI matrix runs several. Migrate instead of loading it, so an older Rails + # never chokes on a newer schema version — and so the migrations themselves + # are exercised on every Rails we claim to support. + config.active_record.maintain_test_schema = false + # Settings specified here will take precedence over those in config/application.rb. # Turn false under Spring and add config.action_view.cache_template_loading = true. @@ -32,7 +38,11 @@ config.active_job.queue_adapter = :test # Raise exceptions instead of rendering exception templates. - config.action_dispatch.show_exceptions = false + # `:rescuable` renders a 404 for RecordNotFound while still raising real + # errors, which is what the admin's scoping tests assert. Rails 7.0 predates + # the symbol and spells the same intent `true`. + config.action_dispatch.show_exceptions = + Rails.gem_version >= Gem::Version.new("7.1") ? :rescuable : true # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false diff --git a/test/dummy/config/initializers/approval_engine.rb b/test/dummy/config/initializers/approval_engine.rb new file mode 100644 index 0000000..9cc5941 --- /dev/null +++ b/test/dummy/config/initializers/approval_engine.rb @@ -0,0 +1,4 @@ +# The dummy app opts into the admin so the engine's own tests can exercise it. +# Routes are drawn once at boot, so this can't be flipped per test — the +# request-time guard is what the "disabled" tests toggle. +ApprovalEngine.configure { |config| config.admin_enabled = true } diff --git a/test/dummy/db/migrate/20260614110904_create_users.rb b/test/dummy/db/migrate/20260614110904_create_users.rb index c632053..89f4859 100644 --- a/test/dummy/db/migrate/20260614110904_create_users.rb +++ b/test/dummy/db/migrate/20260614110904_create_users.rb @@ -1,4 +1,4 @@ -class CreateUsers < ActiveRecord::Migration[8.1] +class CreateUsers < ActiveRecord::Migration[7.0] def change create_table :users do |t| t.string :name diff --git a/test/dummy/db/migrate/20260614120000_create_invoices.rb b/test/dummy/db/migrate/20260614120000_create_invoices.rb index e407741..0f0724d 100644 --- a/test/dummy/db/migrate/20260614120000_create_invoices.rb +++ b/test/dummy/db/migrate/20260614120000_create_invoices.rb @@ -1,4 +1,4 @@ -class CreateInvoices < ActiveRecord::Migration[8.1] +class CreateInvoices < ActiveRecord::Migration[7.0] def change create_table :invoices do |t| t.string :tenant_id diff --git a/test/dummy/db/migrate/20260618000002_add_email_to_users.rb b/test/dummy/db/migrate/20260618000002_add_email_to_users.rb new file mode 100644 index 0000000..c042d0e --- /dev/null +++ b/test/dummy/db/migrate/20260618000002_add_email_to_users.rb @@ -0,0 +1,7 @@ +class AddEmailToUsers < ActiveRecord::Migration[7.0] + # The dummy actor needs a real address for the notification tests to resolve + # one — `config.actor_email_method` defaults to :email. + def change + add_column :users, :email, :string + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb deleted file mode 100644 index f792cda..0000000 --- a/test/dummy/db/schema.rb +++ /dev/null @@ -1,185 +0,0 @@ -# This file is auto-generated from the current state of the database. Instead -# of editing this file, please use the migrations feature of Active Record to -# incrementally modify your database, and then regenerate this schema definition. -# -# This file is the source Rails uses to define your schema when running `bin/rails -# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to -# be faster and is potentially less error prone than running all of your -# migrations from scratch. Old migrations may fail to apply correctly if those -# migrations use external dependencies or application code. -# -# It's strongly recommended that you check this file into your version control system. - -ActiveRecord::Schema[8.1].define(version: 2026_06_17_000002) do - # These are extensions that must be enabled in order to support this database - enable_extension "pg_catalog.plpgsql" - - create_table "approval_engine_approvals", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "approval_engine_trigger_rule_id" - t.string "approvals_required", default: "all", null: false - t.datetime "created_at", null: false - t.string "event_name" - t.string "status", default: "pending", null: false - t.bigint "target_id", null: false - t.string "target_type", null: false - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["approval_engine_trigger_rule_id"], name: "idx_ae_approvals_on_trigger_rule" - t.index ["status"], name: "index_approval_engine_approvals_on_status" - t.index ["target_type", "target_id", "created_at"], name: "idx_ae_approvals_target_recency" - t.index ["target_type", "target_id"], name: "index_approval_engine_approvals_on_target" - t.index ["tenant_id"], name: "index_approval_engine_approvals_on_tenant_id" - t.check_constraint "approvals_required::text ~ '^([1-9][0-9]*%?|any|all|majority)$'::text", name: "chk_approval_engine_approval_approvals_required" - t.check_constraint "status::text = ANY (ARRAY['pending'::character varying, 'approved'::character varying, 'rejected'::character varying, 'quarantined'::character varying, 'cancelled'::character varying]::text[])", name: "chk_approval_engine_approval_status" - end - - create_table "approval_engine_audit_logs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.bigint "actual_actor_id" - t.string "actual_actor_type" - t.uuid "approval_engine_step_id", null: false - t.text "comment" - t.datetime "created_at", null: false - t.string "event", null: false - t.bigint "intended_actor_id" - t.string "intended_actor_type" - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["actual_actor_type", "actual_actor_id"], name: "index_approval_engine_audit_logs_on_actual_actor" - t.index ["approval_engine_step_id"], name: "index_approval_engine_audit_logs_on_approval_engine_step_id" - t.index ["intended_actor_type", "intended_actor_id"], name: "index_approval_engine_audit_logs_on_intended_actor" - t.index ["tenant_id", "created_at"], name: "index_approval_engine_audit_logs_on_tenant_id_and_created_at" - end - - create_table "approval_engine_delegations", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.boolean "active", default: true, null: false - t.datetime "created_at", null: false - t.bigint "delegatee_id", null: false - t.string "delegatee_type", null: false - t.bigint "delegator_id", null: false - t.string "delegator_type", null: false - t.datetime "ends_at", null: false - t.datetime "starts_at", null: false - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["delegatee_type", "delegatee_id", "active", "starts_at", "ends_at"], name: "idx_ae_delegations_lookup" - t.index ["delegatee_type", "delegatee_id"], name: "index_approval_engine_delegations_on_delegatee" - t.index ["delegator_type", "delegator_id"], name: "index_approval_engine_delegations_on_delegator" - end - - create_table "approval_engine_outbox_events", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "created_at", null: false - t.text "delivery_error" - t.text "error_payload" - t.string "event_name", null: false - t.datetime "failed_at" - t.boolean "processed", default: false, null: false - t.datetime "processed_at" - t.uuid "record_id", null: false - t.string "record_type", null: false - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["created_at"], name: "idx_ae_outbox_unprocessed", where: "(processed = false)" - t.index ["processed"], name: "index_approval_engine_outbox_events_on_processed" - t.index ["record_type", "record_id"], name: "index_approval_engine_outbox_events_on_record" - end - - create_table "approval_engine_steps", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "activated_at" - t.uuid "approval_engine_track_id", null: false - t.string "approvals_required", default: "any", null: false - t.bigint "assigned_actor_id", null: false - t.string "assigned_actor_type", null: false - t.datetime "created_at", null: false - t.datetime "decided_at" - t.integer "iteration", default: 1, null: false - t.integer "layer", default: 1, null: false - t.string "name" - t.string "status", default: "pending", null: false - t.string "tenant_id", null: false - t.datetime "timed_out_at" - t.integer "timeout_after" - t.datetime "timeout_at" - t.datetime "updated_at", null: false - t.index ["approval_engine_track_id", "iteration", "layer", "status"], name: "idx_ae_steps_layer_consensus" - t.index ["approval_engine_track_id"], name: "index_approval_engine_steps_on_approval_engine_track_id" - t.index ["assigned_actor_type", "assigned_actor_id"], name: "index_approval_engine_steps_on_assigned_actor" - t.index ["status"], name: "index_approval_engine_steps_on_status" - t.index ["tenant_id", "assigned_actor_type", "assigned_actor_id", "status"], name: "idx_approval_engine_pending_tasks" - t.index ["timeout_at"], name: "idx_ae_steps_overdue", where: "((timeout_at IS NOT NULL) AND (timed_out_at IS NULL))" - t.check_constraint "approvals_required::text ~ '^([1-9][0-9]*%?|any|all|majority)$'::text", name: "chk_approval_engine_step_approvals_required" - t.check_constraint "status::text = ANY (ARRAY['waiting'::character varying, 'pending'::character varying, 'approved'::character varying, 'rejected'::character varying, 'changes_requested'::character varying, 'expired'::character varying, 'cancelled'::character varying]::text[])", name: "chk_approval_engine_step_status" - end - - create_table "approval_engine_template_steps", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "approval_engine_track_template_id", null: false - t.string "approvals_required", default: "any", null: false - t.string "assigned_group", null: false - t.datetime "created_at", null: false - t.integer "layer", default: 1, null: false - t.string "name", null: false - t.integer "timeout_after" - t.datetime "updated_at", null: false - t.index ["approval_engine_track_template_id"], name: "idx_ae_tpl_steps_on_tpl_id" - t.check_constraint "approvals_required::text ~ '^([1-9][0-9]*%?|any|all|majority)$'::text", name: "chk_approval_engine_template_step_approvals_required" - end - - create_table "approval_engine_track_templates", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "created_at", null: false - t.string "name", null: false - t.string "status", default: "draft", null: false - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["tenant_id"], name: "index_approval_engine_track_templates_on_tenant_id" - t.check_constraint "status::text = ANY (ARRAY['draft'::character varying, 'active'::character varying, 'archived'::character varying]::text[])", name: "chk_approval_engine_template_status" - end - - create_table "approval_engine_tracks", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "approval_engine_approval_id", null: false - t.datetime "created_at", null: false - t.string "name", null: false - t.string "status", default: "pending", null: false - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["approval_engine_approval_id", "status"], name: "idx_ae_tracks_approval_status" - t.index ["approval_engine_approval_id"], name: "index_approval_engine_tracks_on_approval_engine_approval_id" - t.check_constraint "status::text = ANY (ARRAY['pending'::character varying, 'approved'::character varying, 'rejected'::character varying, 'cancelled'::character varying]::text[])", name: "chk_approval_engine_track_status" - end - - create_table "approval_engine_trigger_rules", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.boolean "active", default: true, null: false - t.uuid "approval_engine_track_template_id", null: false - t.jsonb "condition", default: {}, null: false - t.datetime "created_at", null: false - t.string "event_name", null: false - t.integer "priority", default: 0, null: false - t.string "tenant_id", null: false - t.datetime "updated_at", null: false - t.index ["approval_engine_track_template_id"], name: "idx_ae_trigger_rules_on_tpl_id" - t.index ["condition"], name: "index_approval_engine_trigger_rules_on_condition", using: :gin - t.index ["tenant_id", "event_name", "active", "priority"], name: "idx_ae_trigger_rules_resolution" - t.index ["tenant_id"], name: "index_approval_engine_trigger_rules_on_tenant_id" - end - - create_table "invoices", force: :cascade do |t| - t.decimal "amount", precision: 12, scale: 2, default: "0.0", null: false - t.datetime "created_at", null: false - t.string "department" - t.string "state", default: "submitted", null: false - t.string "tenant_id" - t.datetime "updated_at", null: false - end - - create_table "users", force: :cascade do |t| - t.datetime "created_at", null: false - t.string "name" - t.string "role" - t.datetime "updated_at", null: false - end - - add_foreign_key "approval_engine_approvals", "approval_engine_trigger_rules", on_delete: :nullify - add_foreign_key "approval_engine_audit_logs", "approval_engine_steps" - add_foreign_key "approval_engine_steps", "approval_engine_tracks" - add_foreign_key "approval_engine_template_steps", "approval_engine_track_templates" - add_foreign_key "approval_engine_tracks", "approval_engine_approvals" - add_foreign_key "approval_engine_trigger_rules", "approval_engine_track_templates" -end diff --git a/test/dummy/db/seeds.rb b/test/dummy/db/seeds.rb index ee18d87..d51ced7 100644 --- a/test/dummy/db/seeds.rb +++ b/test/dummy/db/seeds.rb @@ -1,8 +1,12 @@ -# Demo seed for the dummy app — builds a small, live approval track you can +# Demo seed for the dummy app — a small but *lived-in* set of approvals you can # explore in the console or watch in the dashboard. Safe to re-run. # # bin/rails app:db:seed # from the engine root # bin/console -> Rails.application.load_seed +# +# It deliberately seeds finished work as well as pending work. A single freshly +# created approval shows an empty audit trail, which is the least interesting +# thing this engine does; the point of the ledger is what it remembers. TENANT = "demo".freeze puts "Resetting demo data…" @@ -13,33 +17,80 @@ ApprovalEngine.configure { |c| c.actor_class = "User" } -manager = User.create!(name: "Maria (Manager)", role: "manager") -cfo = User.create!(name: "Carl (CFO)", role: "cfo") +manager = User.create!(name: "Maria Okonkwo", role: "manager", email: "maria@example.com") +cfo = User.create!(name: "Carl Jensen", role: "cfo", email: "carl@example.com") +legal = User.create!(name: "Lena Petrova", role: "legal", email: "lena@example.com") +it_lead = User.create!(name: "Ivan Torres", role: "it", email: "ivan@example.com") + +# One block instead of a template, two steps and a hand-written JSON Logic rule. +ApprovalEngine.define_flow "High-value invoice", tenant: TENANT, model: Invoice do + on :create, when: { amount: { gt: 1_000 } } + step "Manager sign-off", group: "manager" + step "CFO sign-off", group: "cfo" +end + +# The same idea one size up: Legal and IT review at the same time, and both must +# sign off before it reaches the CFO. +ApprovalEngine.define_flow "Major capital spend", tenant: TENANT, model: Invoice do + on :create, when: { amount: { gt: 100_000 } }, priority: 10 + step "Manager sign-off", group: "manager" + parallel(approvals_required: :all) do + step "Legal review", group: "legal" + step "IT review", group: "it" + end + step "CFO sign-off", group: "cfo" +end + +def raise_invoice(amount, department) + invoice = Invoice.create!(tenant_id: TENANT, amount: amount, department: department) + [ invoice, invoice.run_approval!(event: "invoice.created", tenant_id: TENANT) ] +end + +def pending_step(approval, name) + approval.steps.reload.pending.find { |step| step.name == name } +end -template = ApprovalEngine::TrackTemplate.create!( - tenant_id: TENANT, name: "High-value invoice", status: "active" -) -template.template_steps.create!(name: "Manager sign-off", layer: 1, assigned_group: "manager") -template.template_steps.create!(name: "CFO sign-off", layer: 2, assigned_group: "cfo") +# 1. Untouched — sitting in Maria's inbox right now. +raise_invoice(6_000, "IT") -template.trigger_rules.create!( - tenant_id: TENANT, event_name: "invoice.created", - condition: { ">" => [ { "var" => "amount" }, 1000 ] } -) +# 2. Half-done — Maria signed, so it is Carl's turn and the trail has history. +_, waiting_on_cfo = raise_invoice(12_000, "Marketing") +pending_step(waiting_on_cfo, "Manager sign-off").approve!(by: manager, comment: "Budgeted for Q3.") -invoice = Invoice.create!(tenant_id: TENANT, amount: 6000, department: "IT") -approval = invoice.run_approval!(event: "invoice.created", tenant_id: TENANT) +# 3. Finished — the full trail, end to end, and the invoice marked paid by the +# host's own after_approved callback. +_, approved = raise_invoice(8_500, "Sales") +pending_step(approved, "Manager sign-off").approve!(by: manager) +pending_step(approved, "CFO sign-off").approve!(by: cfo, comment: "Approved against the annual plan.") + +# 4. Rejected at the last step, which is the case people most want to audit. +_, rejected = raise_invoice(25_000, "Operations") +pending_step(rejected, "Manager sign-off").approve!(by: manager) +pending_step(rejected, "CFO sign-off").reject!(by: cfo, comment: "No PO on file — resubmit with one.") + +# 5. Sent back for rework. Requesting changes appends a second iteration rather +# than editing the first, so the ledger still shows what happened the first +# time round. +_, reworked = raise_invoice(40_000, "Engineering") +pending_step(reworked, "Manager sign-off").request_changes!(by: manager, comment: "Split this across two POs.") + +# 6. Mid-scatter — Legal and IT are both holding it, neither has moved. +_, capital = raise_invoice(150_000, "Facilities") +pending_step(capital, "Manager sign-off").approve!(by: manager, comment: "Board approved the capex line.") + +counts = ApprovalEngine::Approval.group(:status).count puts <<~SUMMARY - Seeded a live approval: - Invoice ##{invoice.id} ($#{invoice.amount}) → Approval ##{approval.id} (#{approval.status}) - Pending now: #{approval.steps.pending.map(&:name).join(", ")} + Seeded #{ApprovalEngine::Approval.count} approvals across #{counts.size} states: + #{counts.map { |status, n| "#{n} #{status}" }.join(", ")} + + Waiting on someone right now: + #{ApprovalEngine::Step.pending.map { |s| "#{s.name} → #{s.assigned_actor&.name}" }.uniq.join("\n ")} Try it in the console: - step = ApprovalEngine::Step.pending.first - step.approve!(by: User.find_by(role: "manager")) # advances to the CFO - ApprovalEngine::Step.pending.first.approve!(by: User.find_by(role: "cfo")) + step = ApprovalEngine::Step.actionable_by(User.find_by(role: "legal")).first + step.approve!(by: User.find_by(role: "legal")) # Legal signs; IT still holds it Or watch it in the dashboard: cd test/dummy && bin/rails server → http://localhost:3000/approval_engine diff --git a/test/gemspec_test.rb b/test/gemspec_test.rb new file mode 100644 index 0000000..95944c6 --- /dev/null +++ b/test/gemspec_test.rb @@ -0,0 +1,70 @@ +require "test_helper" + +# The packaging contract. +class GemspecTest < ActiveSupport::TestCase + REPO_URL = "https://github.com/Harry-kp/approval_engine".freeze + + def spec + @spec ||= Dir.chdir(ApprovalEngine::Engine.root) do + Gem::Specification.load(ApprovalEngine::Engine.root.join("approval_engine.gemspec").to_s) + end + end + + test "declares the metadata rubygems.org renders" do + %w[source_code_uri documentation_uri changelog_uri bug_tracker_uri + rubygems_mfa_required].each do |key| + assert spec.metadata[key].present?, "the gemspec declares no #{key}" + end + + # The Homepage link comes from `spec.homepage`; a `homepage_uri` alongside it + # is a duplicate rubygems.org ignores and `gem build` warns about. + assert_equal REPO_URL, spec.homepage + assert_nil spec.metadata["homepage_uri"] + end + + # Derived from `homepage` it would follow the homepage anywhere — including to + # a marketing page some day, which would silently stop "Source code" pointing + # at source. + test "source_code_uri is pinned to the repository, not derived from homepage" do + assert_equal REPO_URL, spec.metadata["source_code_uri"] + end + + test "documentation_uri points at a file that exists in this repository" do + uri = spec.metadata["documentation_uri"] + path = uri[%r{/blob/main/(.+)\z}, 1] + assert path, "documentation_uri (#{uri}) is not a /blob/main/ link into this repository" + assert File.exist?(ApprovalEngine::Engine.root.join(path)), + "documentation_uri points at #{path}, which does not exist — the link is already a 404" + end + + test "the summary leads with what the gem does" do + assert_match(/\Amulti-step approval workflows for rails/i, spec.summary) + assert_operator spec.summary.length, :<=, 160, + "rubygems.org truncates a long summary in search results" + end + + test "packages the two reference docs" do + assert_includes spec.files, "docs/COOKBOOK.md" + assert_includes spec.files, "docs/ARCHITECTURE.md" + end + + # Named explicitly rather than globbed for exactly this reason: a `docs/**/*` + # would ship whatever lands in docs/ next, and assets/ is megabytes of + # screenshots the README already loads over HTTP. + test "packages neither the screenshots nor anything else under docs" do + assert_empty spec.files.grep(%r{\Aassets/}), "assets/ is being packaged into the gem" + + extra = spec.files.grep(%r{\Adocs/}) - %w[docs/ARCHITECTURE.md docs/COOKBOOK.md] + assert_empty extra, "docs/ is being globbed rather than enumerated" + end + + test "the packaged version matches the library" do + assert_equal ApprovalEngine::VERSION, spec.version.to_s + end + + # Host apps `require "approval_engine/test_helpers"`, so it has to be in the + # package — it lives under lib/ but is not loaded by the engine itself. + test "ships the test helpers host apps are told to require" do + assert_includes spec.files, "lib/approval_engine/test_helpers.rb" + end +end diff --git a/test/generators/approval_engine/install_generator_test.rb b/test/generators/approval_engine/install_generator_test.rb index 5dca81f..33299fd 100644 --- a/test/generators/approval_engine/install_generator_test.rb +++ b/test/generators/approval_engine/install_generator_test.rb @@ -15,6 +15,12 @@ class InstallGeneratorTest < Rails::Generators::TestCase assert_match(/ApprovalEngine\.configure do \|config\|/, content) assert_match(/config\.actor_class = "User"/, content) assert_match(/config\.raise_on_rule_errors = false/, content) + # The opt-in is written out rather than implied: an adopter reading the + # generated file must see that notifications are off, not infer it. + assert_match(/config\.notifications_enabled = false/, content) + assert_match(/config\.reminder_after = nil/, content) + + assert_match(/config\.admin_enabled = false/, content) end end end diff --git a/test/integration/admin_routes_test.rb b/test/integration/admin_routes_test.rb new file mode 100644 index 0000000..22b136a --- /dev/null +++ b/test/integration/admin_routes_test.rb @@ -0,0 +1,56 @@ +require "test_helper" + +module ApprovalEngine + # The only test that touches global routing state. + class AdminRoutesTest < ActionDispatch::IntegrationTest + include Engine.routes.url_helpers + + # Redraws the engine's routes with the admin back on, and — the part that + # matters — leaves the reloader in a *loaded* state. + def restore_admin_routes! + ApprovalEngine.config.admin_enabled = true + Rails.application.reload_routes! + # Rails 8 only; on 7.x reload_routes! above already leaves them loaded. + if Rails.application.respond_to?(:reload_routes_unless_loaded) + Rails.application.reload_routes_unless_loaded + end + admin_track_templates_path + end + + test "admin routes are not drawn when the admin is disabled" do + ApprovalEngine.config.admin_enabled = false + Rails.application.reload_routes! + + # Not "the controller refused" — the route does not exist. + get "/approval_engine/admin/track_templates" + assert_response :not_found + assert_not Engine.routes.url_helpers.respond_to?(:admin_track_templates_path) + ensure + restore_admin_routes! + end + + test "the read-only dashboard is unchanged for a host that never opts in" do + ApprovalEngine.config.admin_enabled = false + Rails.application.reload_routes! + + get "/approval_engine/approvals" + + assert_response :success + # No nav and no flash region — exactly the 1.0 page. + assert_select "nav.ae-nav", false + assert_select ".ae-flash", false + assert_select "header.ae-top .ae-tag", "ops dashboard" + ensure + restore_admin_routes! + end + + test "the admin routes are drawn again once the host opts in" do + restore_admin_routes! + + get "/approval_engine/admin/track_templates" + + assert_response :success + assert_select "h1", "Templates" + end + end +end diff --git a/test/jobs/approval_engine/process_outbox_job_test.rb b/test/jobs/approval_engine/process_outbox_job_test.rb index 3dca5f4..40880d4 100644 --- a/test/jobs/approval_engine/process_outbox_job_test.rb +++ b/test/jobs/approval_engine/process_outbox_job_test.rb @@ -116,6 +116,45 @@ def emit(event_name, record: @approval, reason: nil) end end + test "step.activated invokes the host's after_step_activated callback" do + step = pending_step + seen = [] + Invoice.define_method(:after_step_activated) { |s| seen << s } + + ProcessOutboxJob.perform_now(OutboxEvent.find_by!(event_name: "step.activated", record: step).id) + + assert_equal [ step ], seen + ensure + Invoice.remove_method(:after_step_activated) if Invoice.method_defined?(:after_step_activated) + end + + test "step.reminded invokes the host's on_step_reminder callback" do + step = pending_step + seen = [] + Invoice.define_method(:on_step_reminder) { |s| seen << s } + + ProcessOutboxJob.perform_now(emit("step.reminded", record: step).id) + + assert_equal [ step ], seen + ensure + Invoice.remove_method(:on_step_reminder) if Invoice.method_defined?(:on_step_reminder) + end + + test "instruments approval_engine.step.activated for subscribers" do + step = pending_step + event = OutboxEvent.find_by!(event_name: "step.activated", record: step) + received = [] + callback = ->(*args) { received << ActiveSupport::Notifications::Event.new(*args) } + + ActiveSupport::Notifications.subscribed(callback, "approval_engine.step.activated") do + ProcessOutboxJob.perform_now(event.id) + end + + assert_equal 1, received.size + assert_equal step, received.first.payload[:record] + assert_equal @invoice, received.first.payload[:target] + end + test "drain! re-enqueues stale unprocessed events, skipping fresh and processed ones" do stale = emit("approval.approved") stale.update_column(:created_at, 2.minutes.ago) @@ -158,6 +197,14 @@ def emit(event_name, record: @approval, reason: nil) private + # A real, actionable step under @approval — the shape the step-level events + # carry. Creating it also writes its own step.activated event. + def pending_step + track = @approval.tracks.create!(tenant_id: TENANT, name: "Main") + track.steps.create!(tenant_id: TENANT, layer: 1, status: "pending", + assigned_actor: User.create!(name: "M", role: "manager")) + end + # Temporarily make the host callback blow up, then restore it — exercises the # real job's failure path without any mocking library. def with_failing_callback diff --git a/test/mailers/approval_engine/notification_mailer_test.rb b/test/mailers/approval_engine/notification_mailer_test.rb new file mode 100644 index 0000000..383c6b1 --- /dev/null +++ b/test/mailers/approval_engine/notification_mailer_test.rb @@ -0,0 +1,158 @@ +require "test_helper" + +module ApprovalEngine + # The notifications themselves. + class NotificationMailerTest < ApprovalEngine::TestCase + setup do + @invoice = Invoice.create!(tenant_id: TENANT, amount: 6000) + @manager = create_user(role: :manager, email: "manager@example.test") + @approval = Approval.create!(tenant_id: TENANT, target: @invoice, status: "pending", event_name: "invoice.created") + @track = @approval.tracks.create!(tenant_id: TENANT, name: "Finance") + @step = @track.steps.create!(tenant_id: TENANT, name: "Manager sign-off", layer: 1, + status: "pending", assigned_actor: @manager) + end + + def activated + NotificationMailer.step_activated(@step, to: "manager@example.test") + end + + def body_of(mail, format) + mail.parts.find { |part| part.content_type.start_with?("text/#{format}") }.body.to_s + end + + test "step_activated names the target and the approver without touching the host model" do + mail = activated + + assert_equal "Approval needed: Invoice ##{@invoice.id}", mail.subject + assert_equal [ "manager@example.test" ], mail.to + assert_match(/Manager/, body_of(mail, :plain), "the approver is greeted by name") + end + + test "it renders both a text and an html part" do + mail = activated + + assert mail.multipart? + assert_equal 2, mail.parts.size + assert_match(/Invoice ##{@invoice.id}/, body_of(mail, :plain)) + assert_match(/Invoice ##{@invoice.id}/, body_of(mail, :html)) + end + + test "the label falls back to class and id when the host configures nothing" do + mail = activated + + assert_match(/Invoice #/, mail.subject) + mail.parts.each do |part| + assert_no_match(/#(approval) { "https://example.test/approvals/#{approval.id}" } + mail = activated + + assert_match(%r{https://example\.test/approvals/#{@approval.id}}, body_of(mail, :plain)) + assert_match(%r{https://example\.test/approvals/#{@approval.id}}, body_of(mail, :html)) + end + + test "mailer_from is applied only when configured" do + assert_nil activated.from, "an unconfigured engine never invents a sender" + + ApprovalEngine.config.mailer_from = "approvals@example.test" + + assert_equal [ "approvals@example.test" ], activated.from + end + + test "step_reminder says how long it has been waiting" do + @step.update_column(:activated_at, 26.hours.ago) + + mail = NotificationMailer.step_reminder(@step.reload, to: "manager@example.test") + + assert_equal "Still waiting on you: Invoice ##{@invoice.id}", mail.subject + assert_match(/26 hours/, body_of(mail, :plain)) + end + + test "changes_requested carries the approver's comment" do + @step.request_changes!(by: @manager, comment: "Missing PO number") + + mail = NotificationMailer.changes_requested(@step.reload, to: "submitter@example.test") + + assert_equal "Changes requested: Invoice ##{@invoice.id}", mail.subject + assert_match(/Missing PO number/, body_of(mail, :plain)) + assert_match(/Missing PO number/, body_of(mail, :html)) + end + + test "approval_rejected carries the reason when the ledger recorded one" do + mail = NotificationMailer.approval_rejected(@approval, to: "submitter@example.test", reason: "over budget") + + assert_equal "Rejected: Invoice ##{@invoice.id}", mail.subject + assert_match(/over budget/, body_of(mail, :plain)) + end + + test "subjects come from the locale file so a host can rewrite them" do + subjects = [ + NotificationMailer.step_activated(@step, to: "a@example.test"), + NotificationMailer.step_reminder(@step, to: "a@example.test"), + NotificationMailer.step_reassigned(@step, to: "a@example.test"), + NotificationMailer.changes_requested(@step, to: "a@example.test"), + NotificationMailer.approval_approved(@approval, to: "a@example.test"), + NotificationMailer.approval_rejected(@approval, to: "a@example.test") + ].map(&:subject) + + assert_equal 6, subjects.uniq.size, "each notification says something different" + subjects.each do |subject| + assert_no_match(/translation missing/i, subject) + assert_match(/Invoice ##{@invoice.id}/, subject) + end + end + + # "Drop a file in your app" is the whole answer to "how do I customise one + # email", so prove it rather than assuming it. + test "a host's own copy of a template wins over the engine's" do + mail = NotificationMailer.step_reassigned(@step, to: "manager@example.test") + + assert_match(/HOST OVERRIDE SENTINEL/, mail.body.to_s) + end + + # Action Mailer takes an action's templates from the first view path holding + # any of them, so a lone .text.erb silences the engine's .html.erb. + test "overriding one format of an action shadows the other" do + mail = NotificationMailer.step_reassigned(@step, to: "manager@example.test") + + assert_not mail.multipart? + assert_equal "text/plain", mail.mime_type + end + # The ledger records who actually acted. When a delegate asks for changes, + # naming the assignee tells the reader the wrong person. + test "changes_requested names whoever actually asked" do + cover = create_user(role: :manager, name: "Cover Person") + Delegation.create!(tenant_id: TENANT, delegator: @step.assigned_actor, delegatee: cover, + starts_at: 1.day.ago, ends_at: 1.day.from_now) + @step.request_changes!(by: cover, comment: "Split this across two POs.") + + mail = NotificationMailer.changes_requested(@step.reload, to: "requester@example.test") + + assert_match "Cover Person", body_of(mail, :plain) + assert_match "Split this across two POs.", body_of(mail, :plain) + end + end +end diff --git a/test/models/approval_engine/reminder_test.rb b/test/models/approval_engine/reminder_test.rb new file mode 100644 index 0000000..2536b92 --- /dev/null +++ b/test/models/approval_engine/reminder_test.rb @@ -0,0 +1,150 @@ +require "test_helper" + +module ApprovalEngine + # Reminders. + class ReminderTest < ApprovalEngine::TestCase + setup do + @invoice = Invoice.create!(tenant_id: TENANT, amount: 6000) + @manager = create_user(role: :manager) + @approval = Approval.create!(tenant_id: TENANT, target: @invoice, status: "pending") + @track = @approval.tracks.create!(tenant_id: TENANT, name: "Main") + end + + def pending_step(waiting_for: nil) + step = @track.steps.create!(tenant_id: TENANT, layer: 1, status: "pending", assigned_actor: @manager) + step.update_column(:activated_at, waiting_for.ago) if waiting_for + step + end + + test "remindable selects only pending, un-nudged steps past the threshold" do + pending_step(waiting_for: 10.minutes) + stale = pending_step(waiting_for: 3.days) + already = pending_step(waiting_for: 3.days) + already.update_column(:reminded_at, 1.hour.ago) + + assert_equal [ stale ], Step.remindable(after: 2.days).to_a + end + + test "remind! nudges once and never decides the step" do + step = pending_step(waiting_for: 3.days) + + assert_difference -> { OutboxEvent.where(event_name: "step.reminded").count }, 1 do + step.remind! + end + + assert step.reload.pending?, "a nudge does NOT decide the step" + assert_not_nil step.reminded_at + assert_nil step.decided_at, "a nudge is not a decision" + + assert_no_difference -> { OutboxEvent.count } do + step.remind! # already nudged — idempotent + end + assert_empty Step.remindable(after: 2.days).to_a + end + + test "a reminder is a system event with no human actor on the ledger" do + step = pending_step(waiting_for: 3.days) + + step.remind! + + log = step.audit_logs.find_by(event: "reminded") + assert_not_nil log, "the nudge is durable compliance evidence, not just a column" + assert_nil log.actual_actor, "nobody acted, and the ledger says so" + assert_not log.by_proxy? + end + + test "the sweep is a no-op until reminder_after is configured" do + pending_step(waiting_for: 30.days) + + assert_no_difference -> { OutboxEvent.count } do + assert_equal 0, Step.sweep_reminders! + end + end + + test "sweep_reminders! nudges every stale step and returns the count" do + first = pending_step(waiting_for: 3.days) + second = pending_step(waiting_for: 3.days) + pending_step(waiting_for: 1.hour) # still inside the window + ApprovalEngine.config.reminder_after = 2.days.to_i + + assert_equal 2, Step.sweep_reminders! + + assert_not_nil first.reload.reminded_at + assert_not_nil second.reload.reminded_at + end + + test "sweep_reminders! scopes to one tenant" do + mine = pending_step(waiting_for: 3.days) + theirs = other_tenant_step(waiting_for: 3.days) + ApprovalEngine.config.reminder_after = 2.days.to_i + + assert_equal 1, Step.sweep_reminders!(tenant_id: TENANT) + + assert_not_nil mine.reload.reminded_at + assert_nil theirs.reload.reminded_at, "another tenant's approvers are not ours to nudge" + end + + test "ReminderSweepJob sweeps stale steps" do + step = pending_step(waiting_for: 3.days) + ApprovalEngine.config.reminder_after = 2.days.to_i + + ReminderSweepJob.perform_now + + assert_not_nil step.reload.reminded_at + end + + test "the sweep fires through the host's on_step_reminder callback" do + step = pending_step(waiting_for: 3.days) + ApprovalEngine.config.reminder_after = 2.days.to_i + seen = [] + Invoice.define_method(:on_step_reminder) { |s| seen << s } + + assert_equal 1, Step.sweep_reminders! + ProcessOutboxJob.perform_now(OutboxEvent.find_by!(event_name: "step.reminded").id) + + assert_equal [ step ], seen + ensure + Invoice.remove_method(:on_step_reminder) if Invoice.method_defined?(:on_step_reminder) + end + + test "a fresh iteration re-arms the reminder" do + step = pending_step(waiting_for: 3.days) + step.remind! + + step.request_changes!(by: @manager, comment: "Missing PO number") + + redone = @track.steps.for_iteration(2).sole + assert redone.pending? + assert_nil redone.reminded_at, "rework restarts the clock without special-casing" + redone.update_column(:activated_at, 3.days.ago) + assert_equal [ redone ], Step.remindable(after: 2.days).to_a + end + + private + + # A whole second approval under another tenant — the sweep must not reach + # across the boundary even though the steps look identical. + def other_tenant_step(waiting_for:) + invoice = Invoice.create!(tenant_id: "tenant-2", amount: 100) + approval = Approval.create!(tenant_id: "tenant-2", target: invoice, status: "pending") + track = approval.tracks.create!(tenant_id: "tenant-2", name: "Main") + step = track.steps.create!(tenant_id: "tenant-2", layer: 1, status: "pending", + assigned_actor: create_user(role: :cfo)) + step.update_column(:activated_at, waiting_for.ago) + step + end + # Whoever now holds a stalled step is exactly the person who needs nudging, + # so handing it over restarts the clock rather than inheriting the previous + # assignee's stamp. + test "reassigning a step clears its reminder stamp" do + step = pending_step + step.update!(reminded_at: 1.hour.ago) + successor = create_user(role: :cfo, name: "Successor") + + step.reassign!(to: successor) + + assert_nil step.reload.reminded_at + assert_equal successor, step.assigned_actor + end + end +end diff --git a/test/models/approval_engine/step_test.rb b/test/models/approval_engine/step_test.rb index 76eedce..4cdab19 100644 --- a/test/models/approval_engine/step_test.rb +++ b/test/models/approval_engine/step_test.rb @@ -13,7 +13,10 @@ class StepTest < ApprovalEngine::TestCase end test "approval writes an audit row and an outbox event" do - assert_difference -> { AuditLog.count } => 1, -> { OutboxEvent.count } => 1 do + # Counted by name, not in total: approving layer 1 also activates layer 2, + # and that activation is an outbox event of its own (step.activated). + assert_difference -> { AuditLog.count } => 1, + -> { OutboxEvent.where(event_name: "step.approved").count } => 1 do @step.approve!(by: @manager, comment: "Looks good") end @@ -94,6 +97,39 @@ class StepTest < ApprovalEngine::TestCase assert_nil @step.reload.decided_at, "a cancellation is not a decision" end + # step.activated is the event that finally lets something downstream tell an + # approver they have work. It has to fire exactly once per step: a missed one + # is silence, a duplicate one is a second email for the same task. + test "a step becoming actionable emits exactly one activation event" do + activations = -> { OutboxEvent.where(event_name: "step.activated").count } + + assert_equal 1, activations.call, "one for the pending step, none for the waiting one" + + assert_difference activations, 1 do + @step.approve!(by: @manager) # opens layer 2 + end + end + + test "reassigning does not re-announce the step as newly activated" do + backup = create_user(role: :backup) + + assert_no_difference -> { OutboxEvent.where(event_name: "step.activated").count } do + @step.reassign!(to: backup) + end + end + + test "a decision never emits an activation event for the step that was decided" do + assert_no_difference -> { OutboxEvent.where(event_name: "step.activated", record: @step).count } do + @step.approve!(by: @manager) + end + end + + test "a fresh iteration announces its first layer" do + assert_difference -> { OutboxEvent.where(event_name: "step.activated").count }, 1 do + @step.request_changes!(by: @manager) + end + end + test "a waiting step has no activation time until its layer opens" do late = @track.steps.create!(tenant_id: TENANT, layer: 3, status: "waiting", assigned_actor: create_user(role: :late)) assert_nil late.activated_at diff --git a/test/services/approval_engine/flow_definition_test.rb b/test/services/approval_engine/flow_definition_test.rb new file mode 100644 index 0000000..5051c83 --- /dev/null +++ b/test/services/approval_engine/flow_definition_test.rb @@ -0,0 +1,678 @@ +require "test_helper" + +module ApprovalEngine + class FlowDefinitionTest < ApprovalEngine::TestCase + FLOW = "High-value invoice".freeze + + setup do + @invoice = Invoice.create!(tenant_id: TENANT, amount: 20_000, department: "IT") + create_user(role: :manager) + create_user(role: :cfo) + end + + def define_two_step_flow(name: FLOW) + ApprovalEngine.define_flow(name, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 10_000 } } + step "Manager", group: "manager" + step "CFO", group: "cfo" + end + end + + def route!(target = @invoice) + target.run_approval!(event: "invoice.created", tenant_id: TENANT) + end + + def row_counts + [ -> { TrackTemplate.count }, -> { TemplateStep.count }, -> { TriggerRule.count } ] + end + + # --- shape ----------------------------------------------------------- + + test "builds the template, its steps and its rule in one block" do + template = define_two_step_flow + + assert_equal 1, TrackTemplate.count + assert_equal "active", template.status + assert_equal [ [ "Manager", 1 ], [ "CFO", 2 ] ], template.template_steps.pluck(:name, :layer) + assert_equal "invoice.created", template.trigger_rules.sole.event_name + end + + test "sequential steps get incrementing layers" do + create_user(role: :ceo) + + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + step "Manager", group: "manager" + step "CFO", group: "cfo" + step "CEO", group: "ceo" + end + + assert_equal [ 1, 2, 3 ], template.template_steps.pluck(:layer) + end + + test "a parallel block puts its steps on one layer" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + parallel do + step "Legal", group: "legal" + step "IT", group: "it" + end + step "CEO", group: "ceo" + end + + assert_equal [ 1, 1, 2 ], template.template_steps.pluck(:layer) + assert_equal 2, template.template_steps.where(layer: 1).count + end + + test "a parallel block's consensus is stamped on every step in the layer" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + parallel(approvals_required: :all) do + step "Legal", group: "legal" + step "IT", group: "it" + end + end + + # The engine reads one spec per layer, so both rows have to carry it — + # whichever one Track#tally_for happens to read must say the same thing. + assert_equal %w[all all], template.template_steps.where(layer: 1).pluck(:approvals_required) + end + + test "a parallel block defaults to needing every reviewer in the layer" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + parallel do + step "Legal", group: "legal" + step "IT", group: "it" + end + end + + assert_equal %w[all all], template.template_steps.pluck(:approvals_required) + end + + test "steps default to the engine's any consensus" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "manager" } + + assert_equal "any", template.template_steps.sole.approvals_required + end + + test "a consensus count is stored as the string the ledger expects" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + step "Board", group: "manager", approvals_required: 2 + end + + assert_equal "2", template.template_steps.sole.approvals_required + end + + test "timeout_after accepts a duration and a flow-wide default" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT, timeout_after: 1.day) do + step "Manager", group: "manager" + step "CFO", group: "cfo", timeout_after: 2.days + end + + assert_equal 86_400, template.template_steps.find_by(name: "Manager").timeout_after + assert_equal 172_800, template.template_steps.find_by(name: "CFO").timeout_after + end + + test "defaults the template to active so the flow actually fires" do + assert_equal "active", define_two_step_flow.status + + draft = ApprovalEngine.define_flow("Draft flow", tenant: TENANT, status: "draft") do + step "Manager", group: "manager" + end + + assert_equal "draft", draft.status + end + + test "on(:create) uses the model's conventional event name" do + template = define_two_step_flow + + assert_equal Invoice.approval_event_name(:create), template.trigger_rules.sole.event_name + end + + test "on without when: routes the event unconditionally" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create + step "Manager", group: "manager" + end + + assert_equal({ "==" => [ 1, 1 ] }, template.trigger_rules.sole.condition) + assert_not_nil route!, "the tautology has to survive TriggerRule's presence validation and evaluate true" + end + + # --- when: sugar all the way through to a real route ----------------- + + test "the when: sugar compiles to JSON Logic the evaluator matches" do + define_two_step_flow + + approval = route! + + assert_equal "pending", approval.status + assert_equal %w[Manager CFO], approval.steps.order(:layer).pluck(:name) + assert_equal 1, approval.steps.pending.count + assert_equal 1, approval.steps.waiting.count + end + + test "a numeric when: value stays numeric" do + define_two_step_flow + + assert_equal({ ">" => [ { "var" => "amount" }, 10_000.0 ] }, TriggerRule.sole.condition) + + small = Invoice.create!(tenant_id: TENANT, amount: 100, department: "IT") + assert_nil route!(small), "a string-cast threshold would compare \"100\" > \"10000.0\" and match" + end + + test "raw JSON Logic passes through untouched" do + raw = { "or" => [ { ">" => [ { "var" => "amount" }, 10_000 ] }, { "==" => [ { "var" => "department" }, "IT" ] } ] } + + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on "invoice.created", when: raw + step "Manager", group: "manager" + end + + assert_equal raw, template.trigger_rules.sole.condition + end + + test "an in: list compiles to the shape the cookbook documents" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on "invoice.created", when: { department: { in: %w[Legal Finance] } } + step "Manager", group: "manager" + end + + assert_equal({ "in" => [ { "var" => "department" }, %w[Legal Finance] ] }, template.trigger_rules.sole.condition) + end + + # --- idempotency ----------------------------------------------------- + + test "re-running the same flow changes nothing" do + template = define_two_step_flow + touched = template.template_steps.pluck(:updated_at) << template.updated_at + + assert_no_difference row_counts do + define_two_step_flow + end + + assert_equal touched, template.reload.template_steps.pluck(:updated_at) << template.updated_at, + "an update! over unchanged attributes must not even touch updated_at" + end + + test "re-running keeps step ids so admin links survive" do + before = define_two_step_flow.template_steps.order(:layer).pluck(:id) + + after = define_two_step_flow.template_steps.order(:layer).pluck(:id) + + assert_equal before, after + end + + test "a changed group is updated in place" do + step_id = define_two_step_flow.template_steps.find_by(name: "Manager").id + + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + step "Manager", group: "director" + step "CFO", group: "cfo" + end + + manager = TemplateStep.find(step_id) + assert_equal "director", manager.assigned_group + end + + test "a removed step is deleted" do + define_two_step_flow + + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + step "Manager", group: "manager" + end + + assert_equal [ "Manager" ], template.template_steps.pluck(:name) + assert_equal 1, TemplateStep.count + end + + test "a changed condition is updated, not duplicated" do + define_two_step_flow + + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 50_000 } } + step "Manager", group: "manager" + end + + assert_equal 1, TriggerRule.count + assert_equal({ ">" => [ { "var" => "amount" }, 50_000.0 ] }, TriggerRule.sole.condition) + end + + test "a removed on deactivates the rule instead of destroying it, so provenance survives" do + define_two_step_flow + approval = route! + rule = approval.trigger_rule + + # The block still declares routing, so dropping this event really means + # "retire that rule" — a block with no `on` at all means something else + # entirely (the admin owns routing) and is asserted separately below. + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :update, when: { amount: { gt: 10_000 } } + step "Manager", group: "manager" + end + + assert_not rule.reload.active? + assert_equal rule, approval.reload.trigger_rule, "the approval still knows which rule routed it" + assert_nil route!(Invoice.create!(tenant_id: TENANT, amount: 30_000, department: "IT")), + "a deactivated rule routes nothing new" + end + + test "a block with no on leaves existing rules alone" do + define_two_step_flow + rule = TriggerRule.sole + + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + step "Manager", group: "manager" + end + + assert rule.reload.active? + assert_equal 1, TriggerRule.count + end + + test "raises when two templates already share the tenant and name" do + first = TrackTemplate.create!(tenant_id: TENANT, name: FLOW, status: "active") + second = TrackTemplate.create!(tenant_id: TENANT, name: FLOW, status: "active") + + error = assert_raises(FlowDefinition::DefinitionError) { define_two_step_flow } + + assert_includes error.message, first.id + assert_includes error.message, second.id + end + + test "an admin's archive is re-asserted back to the declared status" do + template = define_two_step_flow + template.update!(status: "archived") + + assert_equal "active", define_two_step_flow.status + end + + # --- in-flight approvals --------------------------------------------- + + test "rewriting a flow leaves an approval already in flight untouched" do + define_two_step_flow + approval = route! + + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 10_000 } } + step "Director", group: "director" + end + + assert_equal %w[Manager CFO], approval.reload.steps.order(:layer).pluck(:name) + + approval.steps.pending.sole.approve!(by: User.find_by(role: "manager")) + + assert_equal "pending", approval.steps.find_by(name: "CFO").reload.status + end + + test "requesting changes after a rewrite replays the ledger, not the new template" do + define_two_step_flow + approval = route! + + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 10_000 } } + step "Director", group: "director" + end + approval.steps.pending.sole.request_changes!(by: User.find_by(role: "manager")) + + second = approval.reload.steps.for_iteration(2).order(:layer) + assert_equal %w[Manager CFO], second.pluck(:name), + "IterationBuilder clones from track.steps, so a rework replays the audited steps" + end + + test "a step removed from a flow doesn't strand a running approval" do + define_two_step_flow + approval = route! + + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 10_000 } } + step "CFO", group: "cfo" + end + + approval.steps.pending.sole.approve!(by: User.find_by(role: "manager")) + approval.reload.steps.pending.sole.approve!(by: User.find_by(role: "cfo")) + + assert_equal "approved", approval.reload.status + end + + # --- validation ------------------------------------------------------ + + test "raises when a flow declares no steps" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { on "invoice.created" } + end + + assert_includes error.message, "sit pending forever" + end + + test "raises on a duplicate step name" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + step "Manager", group: "manager" + step "Manager", group: "cfo" + end + end + + assert_includes error.message, "declares two steps named \"Manager\"" + end + + test "raises on an invalid consensus spec" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + step "Board", group: "manager", approvals_required: "two" + end + end + + assert_includes error.message, "step \"Board\" has approvals_required \"two\"" + assert error.message.end_with?("must be :any, :all, :majority, a percentage like \"60%\", or a positive integer.") + end + + test "raises when a step inside a parallel block sets its own approvals_required" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + parallel do + step "Legal", group: "legal", approvals_required: 2 + end + end + end + + assert_includes error.message, "Every step in a layer shares one consensus policy" + end + + test "raises on a nested parallel block" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + parallel do + parallel { step "Legal", group: "legal" } + end + end + end + + assert_equal "parallel blocks can't be nested.", error.message + end + + test "raises on a parallel block with no steps" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { parallel { } } + end + + assert_includes error.message, "declares no steps" + end + + test "raises when a rule reads an attribute the model doesn't expose" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { total: { gt: 1 } } + step "Manager", group: "manager" + end + end + + assert_includes error.message, "reads total" + assert_includes error.message, "it exposes amount, department" + end + + test "finds an unexposed attribute nested inside raw JSON Logic too" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on "invoice.created", when: { "or" => [ { ">" => [ { "var" => "amount" }, 1 ] }, + { "==" => [ { "var" => "totl" }, "IT" ] } ] } + step "Manager", group: "manager" + end + end + + assert_includes error.message, "reads totl" + end + + test "skips the exposure check when no model is given" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + on "invoice.created", when: { total: { gt: 1 } } + step "Manager", group: "manager" + end + + assert_equal({ ">" => [ { "var" => "total" }, 1 ] }, template.trigger_rules.sole.condition) + end + + test "raises on an unknown group when config.approval_groups is set" do + ApprovalEngine.configure { |c| c.approval_groups = %w[manager cfo] } + + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "managr" } + end + + assert_includes error.message, "isn't in config.approval_groups (manager, cfo)" + end + + test "allows any group when config.approval_groups is nil" do + assert_nil ApprovalEngine.config.approval_groups + + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "whatever" } + + assert_equal "whatever", template.template_steps.sole.assigned_group + end + + test "raises when a step has no group" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "" } + end + + assert_includes error.message, "needs a group:" + end + + test "raises when the actor class can't resolve approval groups" do + ApprovalEngine.configure { |c| c.actor_class = "Invoice" } + + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "manager" } + end + + assert_equal "Invoice must define `self.resolve_approval_group(group_name, target)`.", error.message + end + + test "raises when the actor class doesn't resolve to a loaded class" do + ApprovalEngine.configure { |c| c.actor_class = "NoSuchActor" } + + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "manager" } + end + + assert_includes error.message, "which doesn't resolve to a loaded class" + end + + test "raises when no tenant can be resolved" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW) { step "Manager", group: "manager" } + end + + assert_equal "define_flow needs a tenant: — pass one, or set config.current_tenant_method.", error.message + end + + test "falls back to the configured current tenant" do + ApprovalEngine.configure { |c| c.current_tenant_method = -> { TENANT } } + + template = ApprovalEngine.define_flow(FLOW) { step "Manager", group: "manager" } + + assert_equal TENANT, template.tenant_id + end + + test "raises on an unknown on option" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + on "invoice.created", condition: {} + step "Manager", group: "manager" + end + end + + assert_equal "on: unknown option(s) [:condition]. It takes when:, priority: and active:.", error.message + end + + test "raises when on(:create) is used without model:" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + on :create + step "Manager", group: "manager" + end + end + + assert_includes error.message, "needs the model: option" + end + + test "raises when model: hasn't called has_approvals" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: User) { step "Manager", group: "manager" } + end + + assert_includes error.message, "hasn't called has_approvals" + end + + test "raises on a duplicate event and priority pair" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 1 } } + on :create, when: { amount: { gt: 2 } } + step "Manager", group: "manager" + end + end + + assert_includes error.message, "declares two rules for \"invoice.created\" at priority 0" + end + + test "two rules on the same event are fine at different priorities" do + template = ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice) do + on :create, when: { amount: { gt: 100_000 } }, priority: 10 + on :create, when: { amount: { gt: 10_000 } } + step "Manager", group: "manager" + end + + assert_equal [ 10, 0 ], template.trigger_rules.by_priority.pluck(:priority) + end + + test "raises on a non-positive timeout_after" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT, timeout_after: 0) { step "Manager", group: "manager" } + end + + assert_includes error.message, "must be a positive number of seconds" + + assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "manager", timeout_after: -1 } + end + end + + test "raises on a malformed when: with one error class" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) do + on "invoice.created", when: { amount: { bigger_than: 1 } } + step "Manager", group: "manager" + end + end + + assert_includes error.message, "the when: for \"invoice.created\" is invalid" + end + + test "nothing is written when a validation fails" do + assert_no_difference row_counts do + assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT) { step "Manager", group: "" } + end + end + end + + test "a failed redefine rolls back everything it had already rewritten" do + template = define_two_step_flow + before = template.template_steps.pluck(:name, :layer, :assigned_group).sort + + # A blank event name is caught by TriggerRule's own presence validation, + # i.e. + assert_no_difference row_counts do + assert_raises(ActiveRecord::RecordInvalid) do + ApprovalEngine.define_flow(FLOW, tenant: TENANT, model: Invoice, status: "archived") do + on "" + step "Director", group: "manager" + end + end + end + + assert_equal "active", template.reload.status + assert_equal before, template.template_steps.pluck(:name, :layer, :assigned_group).sort + end + + # --- return value ---------------------------------------------------- + + test "returns the template so it can be handed to run_approval!(templates:)" do + template = define_two_step_flow + + assert_instance_of TrackTemplate, template + + approval = @invoice.run_approval!(templates: [ template ]) + + assert_equal 1, approval.tracks.count + assert_equal %w[Manager CFO], approval.steps.order(:layer).pluck(:name) + end + + test "ApprovalEngine.flow looks a defined flow up by name" do + template = define_two_step_flow + + assert_equal template, ApprovalEngine.flow(FLOW, tenant: TENANT) + assert_nil ApprovalEngine.flow("Nothing defined", tenant: TENANT) + end + # JSON Logic writes `var` three ways. + test "accepts the var forms JSON Logic actually supports" do + [ + { ">" => [ { "var" => [ "amount", 0 ] }, 10_000 ] }, + { ">" => [ { "var" => "amount.cents" }, 10_000 ] } + ].each do |condition| + assert_nothing_raised do + ApprovalEngine.define_flow("Var form #{condition.hash}", tenant: TENANT, model: Invoice) do + on :create, when: condition + step "Manager", group: "manager" + end + end + end + end + + test "still rejects a var that names an attribute nobody exposes" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow("Typo", tenant: TENANT, model: Invoice) do + on :create, when: { ">" => [ { "var" => [ "amonut", 0 ] }, 1 ] } + step "Manager", group: "manager" + end + end + + assert_includes error.message, "amonut" + end + + # A plain count is validated per group when the approval is built, so this + # would write a template that raises BuilderError on every run instead. + test "refuses an absolute count spread across several groups" do + error = assert_raises(FlowDefinition::DefinitionError) do + ApprovalEngine.define_flow("Board sign-off", tenant: TENANT, model: Invoice) do + on :create + parallel(approvals_required: 2) do + step "Legal", group: "legal" + step "IT", group: "it" + end + end + end + + assert_includes error.message, "checked against each group's own members" + end + + test "allows a relative spec across several groups, and an absolute count on one" do + create_user(role: :legal) + create_user(role: :it) + + assert_nothing_raised do + ApprovalEngine.define_flow("Relative", tenant: TENANT, model: Invoice) do + on :create + parallel(approvals_required: :all) do + step "Legal", group: "legal" + step "IT", group: "it" + end + end + + ApprovalEngine.define_flow("Single group", tenant: TENANT, model: Invoice) do + on :create, priority: 5 + parallel(approvals_required: 2) do + step "Legal", group: "legal" + end + end + end + end + end +end diff --git a/test/services/approval_engine/host_primary_key_test.rb b/test/services/approval_engine/host_primary_key_test.rb new file mode 100644 index 0000000..ffe85fd --- /dev/null +++ b/test/services/approval_engine/host_primary_key_test.rb @@ -0,0 +1,73 @@ +require "test_helper" + +# A real constant, not an anonymous class: reading a polymorphic association +# back constantizes the stored type name. +class UuidWidget < ActiveRecord::Base + self.table_name = "uuid_widgets" +end + +module ApprovalEngine + # Approvals reference host records polymorphically, so those columns have to + # hold whatever the approved models use as a primary key. `t.references` + # defaults to bigint, which casts a UUID to 0 — the row saves, and the ledger + # points at nothing. They are strings instead, which fit every key shape. + class HostPrimaryKeyTest < ApprovalEngine::TestCase + setup do + create_user(role: :manager) + @template = create_template(event: "widget.created", steps: [ { name: "Manager", group: "manager" } ]) + connection.execute("DROP TABLE IF EXISTS uuid_widgets") + connection.execute(<<~SQL) + CREATE TABLE uuid_widgets (id uuid PRIMARY KEY DEFAULT gen_random_uuid(), name varchar) + SQL + end + + teardown { connection.execute("DROP TABLE IF EXISTS uuid_widgets") } + + test "a host record with a UUID primary key keeps its target" do + widget = uuid_widget_class.create!(name: "Thing") + + approval = ApprovalBuilder.build!(template: @template, target: widget, event_name: "widget.created") + + assert_equal widget.id, approval.reload.target_id + assert_equal widget, approval.target + end + + test "a host record with a bigint primary key keeps its target" do + invoice = Invoice.create!(tenant_id: TENANT, amount: 20_000, department: "IT") + + approval = ApprovalBuilder.build!(template: @template, target: invoice, event_name: "invoice.created") + + assert_equal invoice, approval.reload.target + end + + test "a UUID actor is assigned and readable back off the step" do + widget = uuid_widget_class.create!(name: "Thing") + approval = ApprovalBuilder.build!(template: @template, target: widget, event_name: "widget.created") + + step = approval.steps.sole + assert_equal User.sole, step.assigned_actor + assert_equal widget, step.target + end + + test "every polymorphic reference to a host record is a string column" do + { Approval => %w[target_id], Step => %w[assigned_actor_id], + AuditLog => %w[actual_actor_id intended_actor_id], + Delegation => %w[delegator_id delegatee_id] }.each do |model, columns| + columns.each do |column| + assert_equal :string, model.type_for_attribute(column).type, + "#{model.table_name}.#{column} must fit any host primary key" + end + end + end + + private + + def connection + ActiveRecord::Base.connection + end + + def uuid_widget_class + UuidWidget + end + end +end diff --git a/test/services/approval_engine/notifier_test.rb b/test/services/approval_engine/notifier_test.rb new file mode 100644 index 0000000..708a83e --- /dev/null +++ b/test/services/approval_engine/notifier_test.rb @@ -0,0 +1,206 @@ +require "test_helper" + +module ApprovalEngine + # A mailer a host might plug in via `config.mailer_class` to take one + # notification over completely, leaving the other five alone. + class HostNotificationMailer < ApprovalEngine::NotificationMailer + def step_activated(step, to:) + mail(to: to, from: "ops@example.test", subject: "Custom: #{step.name}") do |format| + format.text { render plain: "our own words" } + end + end + end + + # Dispatch: which outbox event becomes which email, who receives it, and — the + # question this whole layer is shaped around — when nothing is sent at all. + class NotifierTest < ApprovalEngine::TestCase + include ActiveJob::TestHelper + + setup do + ActionMailer::Base.deliveries.clear + @invoice = Invoice.create!(tenant_id: TENANT, amount: 6000) + @manager = create_user(role: :manager, email: "manager@example.test") + @requester = create_user(role: :requester, email: "requester@example.test") + @approval = Approval.create!(tenant_id: TENANT, target: @invoice, status: "pending") + @track = @approval.tracks.create!(tenant_id: TENANT, name: "Finance") + @step = @track.steps.create!(tenant_id: TENANT, name: "Sign-off", layer: 1, + status: "pending", assigned_actor: @manager) + end + + def enable! + ApprovalEngine.config.notifications_enabled = true + ApprovalEngine.config.mailer_from = "approvals@example.test" + end + + def emit(event_name, record: @approval, reason: nil) + OutboxEvent.create!(tenant_id: TENANT, event_name: event_name, record: record, error_payload: reason) + end + + # The step.activated event the engine wrote when the step above was built. + def activation + OutboxEvent.find_by!(event_name: "step.activated", record: @step) + end + + def relay(event) + perform_enqueued_jobs { ProcessOutboxJob.perform_now(event.id) } + end + + # Multipart mail keeps its text in the parts, not in the container body. + def body_of(mail) + mail.multipart? ? mail.parts.map { |part| part.body.to_s }.join("\n") : mail.body.to_s + end + + # The regression guard for the risk this whole design is shaped around: a + # v1.0.0 install that upgrades the gem and runs its outbox must stay silent. + test "notifications are off by default, so upgrading never starts sending mail" do + perform_enqueued_jobs do + @step.approve!(by: @manager) + OutboxEvent.unprocessed.order(:created_at).each { |e| ProcessOutboxJob.perform_now(e.id) } + end + + assert_equal "approved", @approval.reload.status, "the approval itself still ran" + assert_empty ActionMailer::Base.deliveries, "nobody was mailed without opting in" + end + + test "an activated step notifies its assignee" do + enable! + + relay(activation) + + assert_equal 1, ActionMailer::Base.deliveries.size + mail = ActionMailer::Base.deliveries.first + assert_equal [ "manager@example.test" ], mail.to + assert_equal "Approval needed: Invoice ##{@invoice.id}", mail.subject + end + + test "removing an event from notification_events silences it" do + enable! + ApprovalEngine.config.notification_events -= [ :step_activated ] + + assert_no_enqueued_jobs only: ActionMailer::Base.delivery_job do + ProcessOutboxJob.perform_now(activation.id) + end + end + + test "an actor with no address is skipped, not raised on" do + enable! + @manager.update!(email: nil) + event = activation + + assert_no_enqueued_jobs only: ActionMailer::Base.delivery_job do + ProcessOutboxJob.perform_now(event.id) + end + assert event.reload.processed, "a missing address is a config problem, not a failed event" + end + + test "actor_email_method reads whatever the host calls its address" do + enable! + ApprovalEngine.config.actor_email_method = :work_email + User.define_method(:work_email) { "manager@work.example.test" } + + relay(activation) + + assert_equal [ "manager@work.example.test" ], ActionMailer::Base.deliveries.first.to + ensure + User.remove_method(:work_email) if User.method_defined?(:work_email) + end + + test "approval-level notifications stay silent until approval_recipients is configured" do + enable! + + relay(emit("approval.approved")) + assert_empty ActionMailer::Base.deliveries, "the engine knows who approves, not who asked" + + ApprovalEngine.config.approval_recipients = ->(_approval) { [ @requester ] } + relay(emit("approval.approved")) + + assert_equal [ "requester@example.test" ], ActionMailer::Base.deliveries.first.to + end + + test "approval.rejected carries the outbox reason into the mail" do + enable! + ApprovalEngine.config.approval_recipients = ->(_approval) { [ @requester ] } + + relay(emit("approval.rejected", reason: "over budget")) + + assert_match(/over budget/, body_of(ActionMailer::Base.deliveries.first)) + end + + test "step_reassigned goes to the new assignee, not the old one" do + enable! + backup = create_user(role: :backup, email: "backup@example.test") + @step.reassign!(to: backup) + + relay(OutboxEvent.find_by!(event_name: "step.reassigned", record: @step)) + + assert_equal [ "backup@example.test" ], ActionMailer::Base.deliveries.first.to + end + + test "mailer_class lets a host replace a notification wholesale" do + enable! + ApprovalEngine.config.mailer_class = "ApprovalEngine::HostNotificationMailer" + + relay(activation) + + assert_equal "Custom: Sign-off", ActionMailer::Base.deliveries.first.subject + end + + test "mailer_queue is honoured, and left to the host's default when unset" do + enable! + # What this mail would land on if the host enqueued it themselves — the + # engine must not quietly move it off that queue. + clear_enqueued_jobs + NotificationMailer.step_activated(@step, to: "probe@example.test").deliver_later + host_default = enqueued_jobs.last[:queue] + + clear_enqueued_jobs + ProcessOutboxJob.perform_now(activation.id) + assert_equal host_default, enqueued_jobs.last[:queue], "the engine picks no queue of its own" + + clear_enqueued_jobs + ApprovalEngine.config.mailer_queue = :approvals_mail + ProcessOutboxJob.perform_now(emit("step.reassigned", record: @step).id) + + assert_equal "approvals_mail", enqueued_jobs.last[:queue] + end + + # A broken notification must not fail the event: the retry would re-run the + # host's callbacks, turning a mailer problem into a business one. + test "a broken notification is logged and never fails the outbox event" do + enable! + ApprovalEngine.config.mailer_class = "ApprovalEngine::NoSuchMailer" + event = activation + clear_enqueued_jobs + + ProcessOutboxJob.perform_now(event.id) + + assert event.reload.processed, "the ledger event was delivered; only the courtesy mail failed" + assert_no_enqueued_jobs only: ProcessOutboxJob # a mailer problem must not retry the event + assert_no_enqueued_jobs only: ActionMailer::Base.delivery_job + end + # A delegate can act on the step, so notifying only the assignee tells the + # person who is away and not the person who can do something about it. + test "a step notification also reaches an active delegate" do + enable! + cover = create_user(role: :manager, email: "cover@example.test") + Delegation.create!(tenant_id: TENANT, delegator: @manager, delegatee: cover, + starts_at: 1.day.ago, ends_at: 1.day.from_now) + + relay(activation) + + assert_equal [ "cover@example.test", "manager@example.test" ], + ActionMailer::Base.deliveries.flat_map(&:to).sort + end + + test "an expired delegation is not notified" do + enable! + cover = create_user(role: :manager, email: "cover@example.test") + Delegation.create!(tenant_id: TENANT, delegator: @manager, delegatee: cover, + starts_at: 10.days.ago, ends_at: 5.days.ago) + + relay(activation) + + assert_equal [ "manager@example.test" ], ActionMailer::Base.deliveries.flat_map(&:to) + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 741034c..60a4d19 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,8 +14,8 @@ module ApprovalEngine module ApprovalFixtures TENANT = "tenant-1".freeze - def create_user(role:, name: role.to_s.titleize) - User.create!(name: name, role: role.to_s) + def create_user(role:, name: role.to_s.titleize, email: "#{role}@example.com") + User.create!(name: name, role: role.to_s, email: email) end # steps: array of hashes — { name:, layer:, group:, approvals_required: }