Skip to content

840: Support a form flow as start form of a building block - #886

Draft
maarten-ritense wants to merge 10 commits into
next-minorfrom
bugfix/840-form-flow-start-form-building-block
Draft

840: Support a form flow as start form of a building block#886
maarten-ritense wants to merge 10 commits into
next-minorfrom
bugfix/840-form-flow-start-form-building-block

Conversation

@maarten-ritense

@maarten-ritense maarten-ritense commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Describe the changes

Link to the related Github issue: generiekzaakafhandelcomponent/gzac-issues#840

Important

Depends on #879 — please review and merge that one first. This branch is built on top of bugfix/819-actions-respect-building-block-versions because it needs StartProcessForDocumentRequest.withProcessDefinitionId from it.

Until #879 merges, this PR's diff against next-minor also contains its commits. Only the three 840: commits (11 files) belong to this change; the diff collapses to those once #879 is merged. Everything below concerns issue 840 only.

A form flow configured as the start form of a building block's main process never worked. Starting that building block from the actions of a case failed before the form even opened. The same setup with a form.io start form did work, which is why this went unnoticed.

Three defects sit on that one path, each hidden behind the previous one.

1. Resolving the start form threw a NullPointerException. FormFlowProcessLinkActivityHandler.getStartEventObject resolved the form flow definition through the case-definition link, and ProcessDefinitionCaseDefinitionService.findByProcessDefinitionId ends in !!. A building block's main process has no process_definition_case_definition row: it is owned by a building block, and OperatonProcessService.deploy only creates link rows for CD: blueprints — there is an explicit TODO about that at OperatonProcessService.java:622. So GET /api/v1/process-definition/{processDefinitionId}/start-form threw and no start form opened.

Resolution now falls back to the blueprint encoded in the process definition's version tag. OperatonProcessDefinition.getBlueprintId() already parses both BB: and CD: tags, and FormFlowService already has the findDefinitionOrNull(key, BuildingBlockDefinitionId) overload, so the fallback reuses what is there rather than adding a lookup.

2. The building block version was lost on submit. The handler put only the process definition key into the form flow instance's additional properties, and ValtimoFormFlow.startCase / startSupportingProcess built a key-only StartProcessForDocumentRequest, so the form flow submission path never carried the version. Every building block version redeploys the same process definition key under a new engine version, so a key cannot identify a version. The instance now carries processDefinitionId and passes it on with withProcessDefinitionId(...), so the version the start form was opened for is the version that starts.

3. The process document association clashed. Not in the issue — found while writing the regression test, with the first two defects already fixed. Submitting still failed with IllegalStateException: Process was already associated with another document. startProcessForDocument unconditionally created a process document association, but BuildingBlockStartEventListener has already created the building block document, pointed the association at it and rewritten the business key while the process starts. Its sibling modifyDocumentAndStartProcess already guarded this with findProcessDocumentInstance(...).isEmpty() — which is exactly why a form.io start form worked and a form flow did not. The guard is now on both.

Specify the code branch location: bugfix/840-form-flow-start-form-building-blockbugfix/819-actions-respect-building-block-versions (#879)

Relevant comments:

Why the version tag and not the deploy TODO. The issue offers an alternative: address the TODO so building blocks get a link row of their own. ProcessDefinitionCaseDefinitionId embeds a CaseDefinitionId, so that table structurally cannot hold a building block link — it would need a new entity, a new table and a Liquibase migration that backfills every already-deployed building block process. That is not a bugfix, and it would be redundant: the version tag is already written by updateBuildingBlockDefinitionProcessesVersionTags on deploy and is already the source of truth everywhere else (it is what BuildingBlockStartEventListener derives the version from). The case-definition link is still consulted first, so nothing changes for case processes.

The openTask fallback was replaced too, not only the start-event one. findFormFlowDefinition caught the same NPE and fell back to findDefinitionByKey, which throws Multiple form flow definitions found for key '...' — specify the blueprint id as soon as two building block versions each own a form flow with that key. Creating a draft version copies the form flows, so that is the normal state of any building block with more than one version, not an edge case. Both call sites now share one resolveFormFlowDefinition; findDefinitionByKey is kept only as a last resort for process definitions deployed without a blueprint version tag.

findDefinitionOrNull rather than findDefinition. FormFlowService.findDefinition goes through getReferenceById, which hands back a lazy proxy that only fails on first access — so the trailing !! in the original line could never have caught a missing definition anyway. The null-returning variant fails at the point of resolution, with a message naming the process definition it failed for.

processDefinitionId in the additional properties cannot be set by a client. FormFlowDefinition.createInstance(additionalProperties) has exactly two callers, both in FormFlowProcessLinkActivityHandler; no REST DTO carries form flow additional properties and FormFlowResource never touches them. The value is therefore always the id the start form was opened for, and needs no guard of its own because the map never crosses the wire inbound. It is read optionally, so form flow instances that were already in progress before an upgrade still resolve by key.

Two dormant copies of the same dereference were fixed alongside. URLProcessLinkService.kt:77 and DefaultFormSubmissionService.kt:124 dereference the case-definition link the same way. Neither fires today from a case action, because both are only reached when there is neither a document nor a documentDefinitionName; they are included so the three copies of this resolution stay consistent rather than drifting. In DefaultFormSubmissionService the old helper's document != null arm was unreachable — that branch is only entered when the document is null — so it went with it, and documentDefinitionService.findByBlueprintId already accepts any BlueprintId, so no new lookup was needed. No constructor changed: both services still use ProcessDefinitionCaseDefinitionService for the primary lookup.

Each of the three defects was verified to be caught by the regression test. Reverting one file at a time and re-running BuildingBlockStartFormFlowIT:

  1. FormFlowProcessLinkActivityHandlerjava.lang.NullPointerException at ProcessDefinitionCaseDefinitionService.kt:58 — the issue as reported.
  2. ValtimoFormFlowNo process definition found with key: 'building-block-process' and blueprintId: 'bb-case:1.0.0'. Version tags deployed for this key: BB:bezwaar:1.0.0, BB:bezwaar:8.4.0 — the key-only start being rejected because the version was not carried through.
  3. OperatonProcessJsonSchemaDocumentServiceProcess was already associated with another document: <case document id>.

The test deploys the second building block version directly rather than through BuildingBlockManagementService.createDraft, and uses a deliberately distinctive 8.4.0. The non-transactional integration tests in that module commit state, so going through createDraft or reusing a plausible version number makes the test depend on what else has run.

startCase is threaded for symmetry but is not reachable for a building block. Its document creation still dereferences formFlowInstance.formFlowDefinition.id.caseDefinitionId!!, which is null for a building-block-owned form flow. Building block actions always run through startSupportingProcess, because the case document already exists, so widening that was left out of a bugfix. Worth a separate issue if a building block ever needs to create a case.

A documentation gap caused a follow-up report during review. documentation/features/case/form-flow.md only ever showed valtimoFormFlow.completeTask, with no mention of startCase or startSupportingProcess and no indication which linking context each belongs to. Copying its "complete form flow" example onto a start event fails at runtime with a bare NullPointerException on additionalProperties["processInstanceId"] as String. The page now documents all three with the context each requires. The unhelpful error itself is left alone — improving it is not part of this fix — but it would be a reasonable follow-up.

Breaking changes

  • The contribution only contains changes that are not breaking.

No signature, constructor or JSON contract changed; all new methods are private. One behavioural change worth calling out: startProcessForDocument no longer throws when the process instance is already associated with a different document, it leaves that association in place. The only thing that can create an association in that window is a listener running during the start, and modifyDocumentAndStartProcess has always behaved this way — so this removes a divergence rather than introducing one. The other four callers (UploadProcessService, VerzoekPluginEventListener, BezwaarAdHocFvmStartFormSubmissionHandler, ValtimoFormFlow) never relied on the exception.

Documentation

  • Release notes have been written for these changes.

New features or changes that have been introduced have been documented.

  • Yes

documentation/features/case/form-flow.md gained a "Completing a form flow" section covering the three valtimoFormFlow expressions, the linking context each requires, the instance.id versus step.submissionData difference, and the fact that doc: writes to the case document rather than the building block document. The complete example is now labelled as a user-task form flow.

Tests

Unit tests have been added that cover these changes

  • Yes

URLProcessLinkServiceTest and DefaultFormSubmissionServiceTest each gained a case proving the blueprint fallback resolves a document definition name when there is no case-definition link. FormFlowProcessLinkActivityHandlerIntTest additionally asserts processDefinitionId is on the instance for the existing case-scoped path.

Integration tests have been added that cover these changes

  • Yes

BuildingBlockStartFormFlowIT (new) covers the whole flow: a building-block-scoped form flow on the main process start event, the building block linked to a case, a newer building block version deployed so both share one process definition key, then getStartEventObjectcompleteStep, asserting the started instance runs the linked version and that exactly one BuildingBlockInstance exists at that version for the case document. Its ability to catch each of the three defects was verified individually — see Relevant comments.

Ran green: full backend ./gradlew test, and integrationTestingPostgresql for building-block, form-flow, process-document, process-link-url, form, case, form-view-model, zgw:verzoek and zgw:zaken-api — the last four because they are the other callers of startProcessForDocument and of the two hardened resolution paths. Checkstyle passes on the touched modules with no new warnings.

Describe the testing steps

  • Create a building block with a main process and configure a form flow as the start form of its start event, with ${valtimoFormFlow.startSupportingProcess(instance.id, {...})} on the last step
  • Mark the building block version final and link it to a case as an action
  • Open a case of that type and start the building block from the actions menu — the form flow start form opens (before: no form, NullPointerException in the log)
  • Submit the form flow — the building block starts and the case gains a building block instance at the linked version, not at a newer draft
  • Create a newer draft version of the same building block, change its process, and confirm starting the action still runs the linked version
  • A form.io start form on a building block still works, and a form flow start form on a regular case process still works
  • A form flow on a user task inside a building block still opens and completes

Security

The Secure by Design principle has been applied to these changes

  • Yes

The new processDefinitionId is populated server-side only and never bindable from a request, as described under Relevant comments — this matters because it selects which process definition gets started for a document. The association guard is narrow: it only declines to overwrite an association created moments earlier during the same start, and the association it preserves is the correct one (a building block process's business key is its own document, not the case document).

Added or changed REST API endpoints have authentication and authorization in place

  • Yes
  • Not applicable

No endpoints were added or changed, and no request or response DTO changed.

Valtimo access control checks have been implemented

  • Yes

The existing check in ProcessLinkActivityService.getStartEventObjectRelatedEntityAuthorizationRequest(OperatonExecution, CREATE, OperatonProcessDefinition, processLink.processDefinitionId) — is unchanged. The id this change carries through to the start is that same process definition id, so the definition that is started and the definition that was authorized now agree by construction, and permission rules keyed on versionTag see the linked building block version.

One pre-existing gap is left as it was and is not made worse: ValtimoFormFlow.startSupportingProcess still calls startProcessForDocument inside runWithoutAuthorization, under a //TODO: PBAC START/CREATE check that predates this change. Authorization for this flow therefore still happens when the start form is opened rather than when it is submitted.

Dependencies

Newly added dependencies do not introduce known vulnerabilities/CVE's and are in line with the Valtimo license

  • Yes
  • Not applicable

No dependencies were added or changed.

An action that starts a building block ran the newest version of that building
block instead of the version linked to the case, failing when that version
wrote to fields the linked version does not have.

The process link carries the exact, versioned process definition id, but form
submission forwarded only the process definition key. Resolving a key then fell
back to "highest engine version not linked to a case definition", which does not
exclude building-block-owned definitions, so a newer draft version won. Carry
the versioned id through to the engine for process-link starts, and derive a
document's blueprint as case or building block - caseDefinitionId() is null for
building block documents, which is what made the lookup degrade to "latest".
Resolving by key alone now refuses building-block definitions instead of
guessing a version.

processDefinitionId on the start-process requests is @JsonIgnore on purpose:
ProcessDocumentResource binds two of them from client JSON with
FAIL_ON_UNKNOWN_PROPERTIES disabled, so a bindable field would let a caller
bypass key and blueprint resolution and reach a suspended draft or another
case's definition.
13.40.0 is being released, so the entry moves to the next unreleased minor.
Adds the 13.41.0 directory and registers it in SUMMARY.md so GitBook renders it.
Resolving the start form threw a NullPointerException. The form flow definition
was looked up through the case-definition link, but a building block's main
process has no such link row: only CD: blueprints get one on deploy. Resolution
now falls back to the blueprint from the process definition's version tag, which
covers both case definitions and building blocks. This replaces the openTask
fallback on findDefinitionByKey too, which broke as soon as two building block
versions owned a form flow with the same key.

The form flow instance now also carries the process definition id, so submitting
starts the building block version linked to the case. A key cannot identify a
version, because every building block version redeploys the same key.

Finally, startProcessForDocument no longer overwrites an existing process
document association. A building block's start event listener creates the
building block document and points the association at it while the process
starts, which made the subsequent association fail. modifyDocumentAndStartProcess
already guarded against this, which is why a form.io start form did work.
Both the URL process link and the form.io submission path dereferenced the
case-definition link without a null check, so they would fail the same way as
the form flow start form for a building-block-owned process. They now fall back
to the blueprint from the process definition's version tag. Dormant until now,
because both only reach this code when neither a document nor a document
definition name is supplied.
Add the release note for the building block start form, and document the three
valtimoFormFlow expressions with the linking context each one belongs to. The
form flow page only showed completeTask, so copying its complete example onto a
start event fails at runtime with an unclear error.
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b270b3df-f426-49c8-bffc-e705c4cdbb6b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maarten-ritense
maarten-ritense changed the base branch from bugfix/819-actions-respect-building-block-versions to next-minor August 6, 2026 10:02
Klaas-Ritense and others added 2 commits August 6, 2026 16:24
…o bugfix/840-form-flow-start-form-building-block

# Conflicts:
#	documentation/release-notes/13.x.x/13.41.0/README.md
The specification that resolves a process definition from its key alone
was written out three times: in OperatonProcessService, inlined in
ProcessAuthorizationService and mirrored in the helper's integration
test. Move it to OperatonProcessDefinitionSpecificationHelper so all
three share one definition and the test covers the real specification.
…o bugfix/840-form-flow-start-form-building-block
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.

2 participants