Skip to content

feat(forms): forms are valid against their destination; a draft saves with its findings, publishing needs none (decisions 179, 189) - #1099

Open
rubenvdlinde wants to merge 10 commits into
developmentfrom
build/d179-buildiq
Open

rubenvdlinde wants to merge 10 commits into
developmentfrom
build/d179-buildiq

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Decision 179 (no intake layer) and decision 189 (forms can be draft): buildiq's half of change forms-are-valid-against-their-destination. Spec from #1097 (merged in at a0bac45) and amended here for decision 189.

Depends on openregister#4563 (form destination validator, FormDestinationDependentsEvent). Until it lands, every destination check answers destination-unchecked: a draft still saves (with that finding), publishing is refused. Paired with ConductionNL/nextcloud-vue #1461 (CnFormPage marks a 422 findings[] per field and renders the honeypot).

What an author can now do

  • Save a half-built form as a draft and see what is wrong on each field. A registration form in status draft saves with its destination findings, which come back in the save answer and are marked on each field and preset row; findings on no row (a required property nothing fills) are listed under them. The page designer asks POST /api/forms/destination-check after every save and marks each finding on its form field.
  • Publishing needs zero findings. A registration form saved as published with findings is refused 422 with every finding. Publishing an app is refused 422 while a form page in its live manifest has findings. There is no report-only mode (decision 181).
  • External forms. Switching on external access checks the form first, and again after the public create grant; the entry and the page config carry honeypot: "_hp".
  • Only an app's editors may check its forms. POST /api/forms/destination-check takes { appSlug, manifest } and answers 403 unless the caller is an owner or editor of that app (or an admin): the findings name the destination schema's properties.
  • A schema change re-checks published forms. FormDestinationDependentsListener announces buildiq's published registration forms to OpenRegister; a form a schema change breaks is set back to draft.
  • Every form page names config.destination { register, schema }, filled from or into register/schema/submitEndpoint.

Open point: drafts (decision 189) and why not @self.status

The coordinator relayed decision 189: drafts save with findings, publish refuses. Built that way, and the spec amended in this PR (new requirement "A draft form saves with its findings"; the publish scenarios say publish, not save). buildiq keeps its forms' own status (draft/published) rather than openregister#4566's @self.status draft, because that status is left only through a form submit (FormSubmitService), and an author publishes a form, not submits it. Form pages are drafts while the app is unpublished.

How it calls OpenRegister

FormDestinationCheck resolves the schema inside the form's register (two registers may each hold a case) and hands the entity to OpenRegister's FormDestinationValidator, the service behind POST /api/forms/validate, through the container. Same rules and findings as the endpoint, without the bare-slug ambiguity. A schema that cannot be read, a register without it, or an OpenRegister without the validator is a finding (destination-unchecked), never a silent pass.

Registration form fields render their property's own enum and bounds (the registrationForm schema gives a field no options or limits), so enum-unconstrained and constraint-looser on such a field are left out; every other code stands.

New classes and their tests (coverage guard)

Class Test
Service/FormDestinationCheck tests/Unit/Service/FormDestinationCheckTest (each refusal, zero-findings control, real validator scenario)
Service/ManifestFormDestinations tests/Unit/Service/ManifestFormDestinationsTest (pages checked, mapping, public audience, real type mismatch)
Controller/FormDestinationController tests/Unit/Controller/FormDestinationControllerTest
Listener/FormDestinationDependentsListener tests/Unit/Listener/FormDestinationDependentsListenerTest
Exception/FormDestinationRefusedException tests/Unit/Exception/FormDestinationRefusedExceptionTest

Changed: RegistrationFormTargetWarnings (now the validator's caller, RegistrationFormTargetWarningsTest with the spec scenarios against the real validator), RegistrationFormAuthoringService (draft saves, publish refuses), RegistrationFormValidator (rules only), RegistrationFormController (422 with findings), ApplicationPublishController (publish refusal, testPublishIsRefusedWhileAFormPageHasFindings), ManifestPageShape (destination), frontend formDestinationCheck.js (tests/services/formDestinationCheck.spec.js), FormFieldBuilder, FormPageEditor, PageDesigner(Host), RegistrationForm{List,Editor}, FormLayoutBuilder, FormPresetsBuilder, ExternalFormAccessDialog. E2e: tests/e2e/form-destination.spec.ts (API level, skips without the validator).

The real-validator tests skip in CI until openregister#4563 is on development; locally they ran against its branch.

Spec amendments

  • Decision 189: publish refuses, a draft saves (requirement added, scenarios reworded).
  • screens.md: BqPaginaOntwerperFouten draws a request log, not findings on fields, so - Design backlog: BqFormulierBevindingen (decision 157) is added (also in the lane's design-backlog.md). The UI is built from the existing inline marks; the board refines layout only.
  • forms-per-case-type REQ-OBRF-005 "Saving SHALL warn" replaced by a pointer here; journey-designer task 3 notes the shared validator.
  • Task 1.1: a registration form's register + schema already are its destination, so no second copy is stored.

Verified

All through with-slot.sh, logs in the lane directory (build-round/d179/).

  • COMPOSER_PROCESS_TIMEOUT=0 composer check:strict, once, at f124ab0: exit 1. lint and phpstan clean, phpcs 0 errors. NEW findings, all fixed after the run: phpmd (static access, one complexity) in ManifestFormDestinations, psalm UndefinedClass on the OpenRegister event (now a guarded psalm stub, tests/stubs/openregister-form.stub.php), one PHPUnit failure (CopilotServiceTest fixture now carries the new destination key, which the canonical manifest validator accepts: tests/vitest/copilotRealPlan.spec.js green), gate-7 on the new destination-check endpoint (now an editor guard).
  • After the fixes: composer phpmd exit 0, composer psalm exit 0 ("No errors found!"), full phpunit --no-coverage exit 0 (1404 tests, 3 skipped, all inherited skips), touched vitest specs exit 0 (60 tests), earlier touched vitest runs exit 0 (73 and 113 tests).
  • npm run lint: exit 0 (0 errors, 154 warnings, none on lines this PR adds). prettier --check on changed files: clean. npm run test:l10n and check:l10n-js: exit 0.
  • Hydra gates as CI runs them (--require-full-coverage, base origin/development, this body): 84 of 84 applicable gates ran, 5 fail, all inherited: gate-22 and gate-53 (src/manifest.json, assembled manifest), gate-64 (AppHost autoload prelude), gate-104 and gate-107 (Reports menu section). This PR touches no manifest or menu file.
  • openspec validate --strict: forms-are-valid-against-their-destination, forms-per-case-type, journey-designer valid.
  • The PHPUnit tests that run OpenRegister's real validator ran locally against openregister#4563's branch (f3a6342) and passed; in CI they skip until #4563 is on development.

Still owed

  • Form pages are not announced to FormDestinationDependentsEvent (they live in app manifests; unpublishing one means unpublishing its app). Follow-up.
  • Version promotion to production does not re-check forms; only the app publish does. Follow-up.
  • Live pass (decision 139): draft registration form with a missing caseType shows the finding; publishing it answers 422; the designer marks a property-unknown on its field; external access refuses a form with findings; a breaking change to the case schema puts a published form back in draft.
  • Archive after openregister#4563 and the nextcloud-vue PR land (decision 156).

Decision 179 (Ruben, 10 Oct 2026): a form submits into its destination object, with no in-between intake layer, and is valid against that object's schema when built. Cross-app change: hydra openspec/changes/form-submits-into-its-destination-object (ADR-117, hydra#748). Spec only.
…181)

Ruben answered Q9: no report-only release. A buildiq form with findings is not saved; every finding shows on its field.
…fields; publishing needs none (decisions 179, 189)
…pmd and the copilot fixture follow the destination key
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/buildiq @ e4b845b

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-10 21:50 UTC

Download the full PDF report from the workflow artifacts.

This branch has not been deployed

No deployments
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