Skip to content

refactor(rules): decision tables live in OpenRegister's decision-tables register (decision 196) - #1100

Merged
rubenvdlinde merged 4 commits into
developmentfrom
refactor/decision-tables-onto-openregister
Oct 12, 2026
Merged

rubenvdlinde merged 4 commits into
developmentfrom
refactor/decision-tables-onto-openregister

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Decision 196 (Ruben, against the recommendation): OpenRegister owns the schema slug decision-table, and buildiq's rule engine migrates onto OpenRegister's stored decision tables.

Lands together with ConductionNL/openregister#4572 (the decision-tables register and DecisionTableStore) and ConductionNL/.github#840 (gate 106 contract: decision-table owned by openregister). Without #4572 the class OCA\OpenRegister\Service\Dmn\DecisionTableStore does not exist and the rule engine cannot be constructed. Order: .github first, then #4572 and this PR.

What changes

  • StoredDecisionTables: a rule set's table is held in OpenRegister's decision-tables register under buildiq.<organisation>.<rule-set-slug> and read through DecisionTableStore::find(). A shipped seed uses the shared key buildiq.<rule-set-slug>, read as a fallback. The store reads across organisations, so the tenant boundary (REQ-BRE-007) moves into the key: a table is only reached through a rule set the caller's organisation resolves.
  • DecisionTableShape: maps buildiq's authoring shape (input columns with expressionPath, rules as conditions and values maps) onto OpenRegister's stored shape (positional inputEntries and outputEntries, cells in the shared grammar, upper-case hit policy) and back. It keeps the expression paths, the defaults, the labels and the priorities. A value name that no output column declares becomes an output column.
  • RuleEngineService and RulesController::schema read the table through it. RuleEngineService::DECISION_TABLE_SCHEMA is removed.
  • PUT /api/rules/{ruleSetSlug}/decision-table (admin only, the same isInGroup('admin') guard as SettingsController::update). The editor saves through it. If OpenRegister's validator refuses the table, the endpoint answers 422 with the reason.
  • Repair step MoveDecisionTablesToOpenRegister (before InitializeSettings) moves every row of the old decision-table schema into OpenRegister's register under its rule set's key. It skips any key that is already held. Only when every table moved does it rename the old schema's slug to buildiq-decision-table-moved, so the import's global slug lookup finds only OpenRegister's schema. It deletes nothing and never throws. If OpenRegister's register is not there yet, it moves nothing and retries on the next upgrade.
  • The DecisionTable schema is removed from register.d/10-business-rules.json and openbuild_register.json. The loan-eligibility seed table now targets OpenRegister's register in the stored shape.
  • info.xml 0.7.16-unstable.20261011120000. openspec change decision-tables-live-in-openregister.

Verified

  • Red first, against development (logs/buildiq-red.log): 35 tests, 31 errors (the new classes did not exist), 1 failure (the fragment still declared DecisionTable).
  • Green: 65 targeted tests OK (StoredDecisionTables, the repair, RulesController, RuleEngineService, DecisionTableEvaluator, the fragment tests).
  • Cross-check against OpenRegister's real classes (DecisionTableValidator, DecisionTableEvaluator, and the decision-table schema fragment through Opis, all from #4572's tree):
    • The shipped seed and the seed mapped from development's seed are identical, Opis-valid, validator-clean, and their keys match the pattern.
    • 36 evaluations (3 tables, including the editor's ==, *, any and bare-range dialects and a COLLECT table, against 12 payloads) give the same result and the same triggered rule before and after the move.
  • phpcs, phpmd, phpstan and psalm on the changed files: 0 new findings. eslint was not run locally (this worktree has no node_modules), so CI's lint answers for the one changed .vue file.
  • composer check:strict was not run: no check slot started within 10 minutes, so targeted checks were run instead. CI is the full check.

Known consequences

  • A rule set that had two tables: the engine only ever evaluated the first. The second stays on the renamed old schema.
  • Rows stay on the renamed schema after the move. A later cleanup can delete them once the move has been checked on a live instance.

Still owed (live instance)

After #4572 and this PR are installed, run occ upgrade. Then check that GET /apps/openregister/api/objects/decision-tables/decision-table lists the moved tables (owner: buildiq) and that POST /apps/buildiq/api/rules/loan-eligibility/evaluate answers as before.

…es register (decision 196)

A rule set's decision table is stored in OpenRegister's register under
buildiq.<organisation>.<rule-set-slug> and read through its DecisionTableStore.
DecisionTableShape maps buildiq's authoring shape onto the stored shape and
back; the editor saves through PUT /api/rules/{slug}/decision-table (admin).
buildiq's DecisionTable schema and RuleEngineService::DECISION_TABLE_SCHEMA
are gone; repair step MoveDecisionTablesToOpenRegister moves existing tables
and then renames the old schema's slug. Needs openregister#4572.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/buildiq @ 7f03736

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
test-l10n ✅
check-gitignore ✅
check-nc-floor ✅
format ✅
check-l10n-js ✅
check-schema-l10n ✅
composer ✅ ✅ 109/109
npm ✅ ✅ 742/742
app:check-code ⏭️
info.xml ✅
REUSE ❌
lockfile sync ✅
PHPUnit ❌
Newman ❌
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ❌

Quality workflow — 2026-10-11 09:03 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/buildiq @ 17d3e66

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
test-l10n ✅
check-gitignore ✅
check-nc-floor ✅
format ✅
check-l10n-js ✅
check-schema-l10n ✅
composer ✅ ✅ 109/109
npm ✅ ✅ 742/742
app:check-code ⏭️
info.xml ✅
REUSE ❌
lockfile sync ✅
PHPUnit ❌
Newman ❌
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ❌

Quality workflow — 2026-10-12 10:10 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/buildiq @ c5a9f97

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
test-l10n ✅
check-gitignore ✅
check-nc-floor ✅
format ✅
check-l10n-js ✅
check-schema-l10n ✅
composer ✅ ✅ 109/109
npm ✅ ✅ 742/742
app:check-code ⏭️
info.xml ✅
REUSE ❌
lockfile sync ✅
PHPUnit ❌ 1395/1395 (no test failed, the job failed for another reason — see its log)
Newman ✅ 266/266 · 1 skipped
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ❌

Quality workflow — 2026-10-12 13:20 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 82e5f79 into development Oct 12, 2026
44 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant