From 1cb9756f221df38a707252cd24a745ba5681d178 Mon Sep 17 00:00:00 2001 From: Nedelcho Delchev Date: Thu, 10 Sep 2026 17:27:04 +0300 Subject: [PATCH] templates: route the task form's COMPLETE catch through apiErrors, not raw e.message (#7296) #7271/#7263 routed the admin page, the my/partner list and calendar loads, the report page and the generic submit() through apiErrors.refusalMessageFor - but not the path every process user task actually takes. FormIntentGenerator's generated COMPLETE catch read `error.data.message`, and the $http compat shim in template-form-builder-harmonia's form.js.template collapsed a caught error into `{ data: { message: e.message } }` before that, discarding httpStatus/ errorMessage entirely - so a 500 from POST /services/inbox/tasks/{id} (a repository throwing inside a check gate, a constraint the 409 mapper does not know) printed the raw Hibernate/JDBC sentence on the task form. - form.js.template: the $http shim's rejection now also carries httpStatus/ errorMessage at the top level (ApiError's shape), alongside the legacy data.message field kept for .form code authored against the old shim. - FormIntentGenerator: the generated COMPLETE catch now calls App.services.apiErrors.refusalMessageFor(error, 'Submit failed.') instead of reading error.data.message. - template-bpm's trigger-new-process.form.template scaffold had the identical error.data.message pattern in its own alert() - same code shape, same fix. - IntentEmissionCoverageIT's generated-page walk now also flags `error.data.message`, the third spelling of the raw-message defect (neither of the existing two patterns matched it, which is why the walk missed this surface in the first place). - Added TaskFormApiErrorTest (mirrors FormCancelActionTest): no `forms:`/ userTask form binding exists in IntentEmissionCoverageIT's giant fixture to exercise gen/.../forms/*/form.js, so FormIntentGenerator's fix is unit-tested directly against its generated `code` field instead of growing that fixture. Verified: mvn formatter:validate (repo-wide, BUILD SUCCESS); TaskFormApiErrorTest and FormCancelActionTest green; IntentEmissionCoverageIT green. Not run: the Selenide BPMStarterTemplateIT that exercises the template-bpm scaffold end to end - the touched line is the failure-path alert() text only, the success path the test asserts on is unchanged, and the JSON/JS is valid by inspection. Fixes #7296 Co-Authored-By: Claude Sonnet 5 --- .../generator/form/FormIntentGenerator.java | 2 +- .../generator/form/TaskFormApiErrorTest.java | 69 +++++++++++++++++++ .../trigger-new-process.form.template | 2 +- .../ui/form.js.template | 15 +++- .../tests/api/IntentEmissionCoverageIT.java | 11 ++- 5 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/form/TaskFormApiErrorTest.java diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/form/FormIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/form/FormIntentGenerator.java index c535336d6be..c82fceeec70 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/form/FormIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/form/FormIntentGenerator.java @@ -475,7 +475,7 @@ function __completeTask(action) { __dialogs.closeWindow(); window.close(); }).catch((error) => { - const message = error && error.data && error.data.message ? error.data.message : 'Unknown error'; + const message = App.services.apiErrors.refusalMessageFor(error, 'Submit failed.'); __notifications.show({ type: 'negative', title: 'Submit failed', description: message }); }); } diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/form/TaskFormApiErrorTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/form/TaskFormApiErrorTest.java new file mode 100644 index 00000000000..40cae9ad38b --- /dev/null +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/form/TaskFormApiErrorTest.java @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.components.intent.generator.form; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Map; + +import org.eclipse.dirigible.components.intent.model.IntentModel; +import org.eclipse.dirigible.components.intent.parser.IntentParser; +import org.junit.jupiter.api.Test; + +/** + * Verifies that a task form's COMPLETE catch routes a failed {@code POST /services/inbox/tasks/*} + * through the shared, safe {@code apiErrors.refusalMessageFor} instead of printing the raw + * developer-facing {@code error.data.message} a 500 carries (issue #7296, a #7263 follow-up). + * + * #7151/#7062 made the shared apiErrors helper safe and wired it into the power form/dialog; #7152 + * reached the personal/partner line dialogs. The one path every process user task actually takes - + * {@code __completeTask}'s own {@code .catch} - was left printing + * {@code error && error.data && error.data.message}, a spelling the emission-coverage walk's string + * checks could not see either (neither "(e && e.message)" nor "String(e.message" matches it), which + * is why this generator-level unit test exists rather than relying on that walk alone. + */ +class TaskFormApiErrorTest { + + private static final String YAML = """ + name: sales + entities: + - name: SalesOrder + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: total, type: decimal } + processes: + - name: OrderApproval + trigger: { onCreate: SalesOrder } + steps: + - { name: confirm, kind: userTask, args: { assignee: manager, form: ConfirmOrder } } + - { name: end, kind: end } + forms: + - { name: ConfirmOrder, forEntity: SalesOrder, fields: [total], actions: [confirm] } + """; + + private static String codeOf(String form) { + IntentModel model = IntentParser.parse(YAML); + Map> forms = FormIntentGenerator.buildFormsForTest(model); + return String.valueOf(forms.get(form) + .get("code")); + } + + @Test + void completeTaskCatchRoutesThroughApiErrors() { + String code = codeOf("ConfirmOrder"); + + assertTrue(code.contains("App.services.apiErrors.refusalMessageFor(error, 'Submit failed.')"), + "the COMPLETE catch must map the failure through the shared apiErrors helper: " + code); + assertFalse(code.contains("error.data.message"), + "the COMPLETE catch must never read the developer-facing error.data.message: " + code); + assertFalse(code.contains("'Unknown error'"), "the raw 'Unknown error' fallback must be gone: " + code); + } +} diff --git a/components/template/template-bpm/src/main/resources/META-INF/dirigible/template-bpm/trigger-new-process.form.template b/components/template/template-bpm/src/main/resources/META-INF/dirigible/template-bpm/trigger-new-process.form.template index d9cabfea60b..d3465a9e83a 100644 --- a/components/template/template-bpm/src/main/resources/META-INF/dirigible/template-bpm/trigger-new-process.form.template +++ b/components/template/template-bpm/src/main/resources/META-INF/dirigible/template-bpm/trigger-new-process.form.template @@ -2,7 +2,7 @@ { "feeds": [], "scripts": [], - "code": "${dollar}scope.model.param1 = \"\";\n${dollar}scope.model.param2 = 0;\n\n${dollar}scope.onTriggerClicked = function(){\n ${dollar}http.post(\"/services/java/${projectName}/${javaPackageName}/ProcessService/processes\", JSON.stringify(${dollar}scope.model)).then(function (response) {\n alert(\"A new process instance has been triggered.\\nResponse: \" + JSON.stringify(response.data));\n }, function (error) {\n alert(`Unable to trigger a new process: '${dollar}{(error && error.data && error.data.message) || error}'`);\n });\n}\n", + "code": "${dollar}scope.model.param1 = \"\";\n${dollar}scope.model.param2 = 0;\n\n${dollar}scope.onTriggerClicked = function(){\n ${dollar}http.post(\"/services/java/${projectName}/${javaPackageName}/ProcessService/processes\", JSON.stringify(${dollar}scope.model)).then(function (response) {\n alert(\"A new process instance has been triggered.\\nResponse: \" + JSON.stringify(response.data));\n }, function (error) {\n alert('Unable to trigger a new process: ' + App.services.apiErrors.refusalMessageFor(error, 'Unable to trigger a new process.'));\n });\n}\n", "form": [ { "controlId": "header", diff --git a/components/template/template-form-builder-harmonia/src/main/resources/META-INF/dirigible/template-form-builder-harmonia/ui/form.js.template b/components/template/template-form-builder-harmonia/src/main/resources/META-INF/dirigible/template-form-builder-harmonia/ui/form.js.template index 48561572c04..cd76eb6a857 100644 --- a/components/template/template-form-builder-harmonia/src/main/resources/META-INF/dirigible/template-form-builder-harmonia/ui/form.js.template +++ b/components/template/template-form-builder-harmonia/src/main/resources/META-INF/dirigible/template-form-builder-harmonia/ui/form.js.template @@ -82,8 +82,21 @@ function formController(ctx) { // AngularJS resolves $http with { data, status, statusText, ... } and legacy .form code branches // on the status (`if (response.status != 202)`), so the success path must carry it too - not just // the rejection path. ctx.http resolves the body alone, hence the exchange() call here. + // + // The rejection carries the ApiError shape App.services.apiErrors reads (httpStatus/errorMessage) + // on the thrown object ITSELF, so a .form's own .catch can hand it straight to + // refusalMessageFor(error, '') - never read .data.message, the raw + // developer-facing sentence a 500 carries (#7296). The legacy .data.message field stays too, for + // .form code authored against the old AngularJS $http shim before this fix existed. const wrap = (p) => p.then((r) => ({ data: r.data, status: r.status })) - .catch((e) => { throw { data: { message: e && e.message }, status: e && e.httpStatus }; }); + .catch((e) => { + throw { + data: { message: e && e.message }, + status: e && e.httpStatus, + httpStatus: e && e.httpStatus, + errorMessage: e && e.errorMessage, + }; + }); const $http = { get: (url) => wrap(harmoniaHttp.exchange('GET', url, undefined)), post: (url, data) => wrap(harmoniaHttp.exchange('POST', url, data)), diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java index cd40694450a..4d224865fe6 100644 --- a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java @@ -3020,10 +3020,19 @@ private void assertEmission() { emittedPages.contains("gen/emission/admin/index.html") && emittedPages.contains("gen/emission/js/components/pages/my/ClaimMyListPage.js"), "the generated-page walk must cover the admin page and the SPA pages, else the rule below is vacuous: " + emittedPages); + // #7296: `error.data.message` (a BPM task form's old .catch, and the old template-bpm + // scaffold's alert()) is a THIRD spelling of the same defect that neither pattern below + // matched. It does not collide with the $http shim's own `message: e && e.message` field + // WRITE (form.js.template), which every generated form.js legitimately still carries for + // .form code authored against the old AngularJS $http compat shape - that is a different + // token sequence. FormIntentGenerator's own fix is unit-tested directly (no `forms:` / + // userTask form binding exists in this fixture to exercise gen/.../forms/*/form.js) - see + // TaskFormApiErrorTest. List rawMessagePages = emittedPages.stream() .filter(page -> { String content = contentOf(page); - return content.contains("(e && e.message)") || content.contains("String(e.message"); + return content.contains("(e && e.message)") || content.contains("String(e.message") + || content.contains("error.data.message"); }) .toList(); assertTrue(rawMessagePages.isEmpty(),