Skip to content

Commit 5849cab

Browse files
delchevclaude
andcommitted
fix(intent): task-form button action wins over a stale model action (approve-as-reject)
In a multi-step flow (submit -> approve), the second task's form completed down the wrong branch: a prior task set an 'action' process variable, form.js preloaded it into the model on open, and the completion handler's Object.assign({ action }, $scope.model) let that stale model 'action' overwrite the clicked button -> the decision gateway (${action == 'approve'}) saw 'submit' and took the reject branch. Both Approve and Reject effectively sent 'submit', so a manager could never approve. Two coordinated fixes: - FormIntentGenerator: rebuild the COMPLETE payload explicitly - copy the model MINUS 'action' and control vars (__*, which are locators, not entity data), then set 'action' LAST so the button wins. - form.js.template: never preload the bare 'action' process variable into the model (the __* locators still load - the runtime reads them). The bug lives in emitted client JS (the browser builds the payload), so the HTTP ITs - which build the payload themselves - cannot exercise it; IntentEngineIT gains an emission assertion that the generated form code rebuilds the payload with the button action winning and no longer uses the buggy Object.assign. The runtime proof is a browser submit->approve flow (verified via the KF vacations regen, which was blocked on exactly this). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c4bdc6a commit 5849cab

3 files changed

Lines changed: 29 additions & 2 deletions

File tree

  • components
    • engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/form
    • template/template-form-builder-harmonia/src/main/resources/META-INF/dirigible/template-form-builder-harmonia/ui
  • tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api

‎components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/form/FormIntentGenerator.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,22 @@ function __completeTask(action) {
247247
__notifications.show({ type: 'negative', title: 'Cannot submit', description: 'This form was not opened from a task (no taskId).' });
248248
return;
249249
}
250+
// The clicked button's `action` MUST win. The form model can carry a STALE
251+
// `action` (a prior task in the flow set it as a process variable, preloaded
252+
// into the model on open) plus control-only vars (__*) that are not entity
253+
// data; strip both, then set `action` LAST so the gateway branches on the
254+
// button, not the stale value (fixes approve-completing-as-reject).
255+
const __data = {};
256+
const __model = $scope.model || {};
257+
Object.keys(__model).forEach((key) => {
258+
if (key !== 'action' && key.indexOf('__') !== 0) {
259+
__data[key] = __model[key];
260+
}
261+
});
262+
__data.action = action;
250263
$http.post('/services/inbox/tasks/' + __taskId, {
251264
action: 'COMPLETE',
252-
data: Object.assign({ action: action }, $scope.model || {})
265+
data: __data
253266
}).then(() => {
254267
__notifications.show({ type: 'positive', title: 'Task submitted', description: 'The task was completed (' + action + ').' });
255268
__dialogs.closeWindow();

‎components/template/template-form-builder-harmonia/src/main/resources/META-INF/dirigible/template-form-builder-harmonia/ui/form.js.template‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,12 @@ document.addEventListener('alpine:init', () => {
127127
harmoniaHttp.get('/services/inbox/tasks/' + encodeURIComponent(params.taskId) + '/variables')
128128
.then((res) => {
129129
const vars = (res && res.variables) || {};
130-
Object.keys(vars).forEach((k) => { self.model[k] = vars[k]; });
130+
// Never preload the `action` process variable into the model: it is a control value a
131+
// PRIOR task in the flow set (e.g. 'submit'), not a form field, and letting it sit in the
132+
// model made the next task's completion submit the stale action - a decision gateway then
133+
// branched wrong (approve completing as reject). The __* locators DO stay - the runtime
134+
// below reads __entityUrl / __<Fk>EntityUrl from the model.
135+
Object.keys(vars).forEach((k) => { if (k !== 'action') self.model[k] = vars[k]; });
131136
// Clear D: the process context holds only a locator (the entity's REST URL + id), so fetch the
132137
// LIVE entity now and overlay its current fields onto the model. This is what makes the task
133138
// form show up-to-date values (e.g. a document total updated after the task was created)

‎tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEngineIT.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2174,6 +2174,15 @@ private void assertForm() {
21742174
assertTrue(body.contains("closeWindow(") && body.contains("window.close("),
21752175
"on completion the form should close its host (dialog via closeWindow, standalone via window.close)");
21762176
assertFalse(body.contains("TODO: wire"), "the action handlers must no longer be TODO stubs");
2177+
// The clicked button's action MUST win over a stale `action` in the model (a prior task in a
2178+
// multi-step flow set it as a process variable, preloaded on open): the completion payload
2179+
// strips `action` + control vars (__*) from the model and sets `action` LAST. Without this,
2180+
// an Approve completes down the reject branch (the approve-as-reject bug). The .form code is
2181+
// Gson-escaped (' -> \\u0027), so match escape-free substrings.
2182+
assertTrue(body.contains("__data.action") && body.contains(".indexOf(") && body.contains("__data"),
2183+
"the completion payload must be rebuilt with the button action winning, not Object.assign with the model last");
2184+
assertFalse(body.contains("Object.assign({ action: action }, $scope.model"),
2185+
"the buggy merge (stale model action overwrites the button) must be gone");
21772186
}
21782187

21792188
private void assertReport() {

0 commit comments

Comments
 (0)