From d63a3cbeb9c4fa9a96ae8ecd9c5271bc024e97df Mon Sep 17 00:00:00 2001 From: Ayoub Ait Lachgar <44379029+theaubmov@users.noreply.github.com> Date: Fri, 21 Jun 2024 19:02:01 +0100 Subject: [PATCH] Fix/messages bugs 1764 (#104) * Fix : Unable to add cProperties and handle empty formal expression * Fix : Unable to modify retrieval expression * Fix Message variable and payload are deleted on change * MessagePayload disappears on message update * Remove comment code and log * Retrigger workflow * removed some commented out code w/ burnettk * removed console.log w/ burnettk * removed useles node version declaration w/ burnettk --------- Co-authored-by: jasquat --- .github/workflows/tests.yml | 3 - .tool-versions | 2 +- app/app.js | 128 +++++------ app/spiffworkflow/constants.js | 2 + app/spiffworkflow/messages/MessageHelpers.js | 59 +++-- .../MessagesPropertiesProvider.js | 16 +- .../elementLevelProvider/MessageSelect.jsx | 209 ++++++++++-------- 7 files changed, 226 insertions(+), 193 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9ba4779..1800dc0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -21,10 +21,7 @@ jobs: - uses: actions/checkout@v4 #Checkout Repo - uses: actions/setup-node@v4 #Setup Node - uses: nanasess/setup-chromedriver@v2 #Setup ChromeDriver - with: - node-version: '18' - name: Run Karma Tests run: | npm ci npm run test - diff --git a/.tool-versions b/.tool-versions index 61b6cd0..a5a47f9 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1 +1 @@ -nodejs 18.3.0 +nodejs 22.3.0 diff --git a/app/app.js b/app/app.js index dacfb0e..63dac60 100644 --- a/app/app.js +++ b/app/app.js @@ -6,6 +6,10 @@ import { import diagramXML from '../test/spec/bpmn/user_form.bpmn'; import spiffworkflow from './spiffworkflow'; import setupFileOperations from './fileOperations'; +import { + SPIFF_ADD_MESSAGE_REQUESTED_EVENT, + SPIFF_ADD_MESSAGE_RETURNED_EVENT, +} from './spiffworkflow/constants'; const modelerEl = document.getElementById('modeler'); const panelEl = document.getElementById('panel'); @@ -187,8 +191,13 @@ bpmnModeler.on('spiff.dmn_files.requested', (event) => { bpmnModeler.on('spiff.data_stores.requested', (event) => { event.eventBus.fire('spiff.data_stores.returned', { options: [ - { id: 'countriesID', type: 'json', name: 'Countries', clz: 'JSONDataStore' }, - { id: 'foodsID', type: 'kkv', name: 'Foods', clz: 'JSONDataStore' } + { + id: 'countriesID', + type: 'json', + name: 'Countries', + clz: 'JSONDataStore', + }, + { id: 'foodsID', type: 'kkv', name: 'Foods', clz: 'JSONDataStore' }, ], }); }); @@ -196,87 +205,74 @@ bpmnModeler.on('spiff.data_stores.requested', (event) => { bpmnModeler.on('spiff.messages.requested', (event) => { event.eventBus.fire('spiff.messages.returned', { configuration: { - "messages": [ + messages: [ { - "identifier": "basic_message", - "location": "examples/1-basic-concepts", - "schema": {}, - "correlation_properties": [] + identifier: 'basic_message', + location: 'examples/1-basic-concepts', + schema: {}, + correlation_properties: [], }, { - "identifier": "end_of_day_receipts", - "location": "examples", - "schema": {}, - "correlation_properties": [] + identifier: 'end_of_day_receipts', + location: 'examples', + schema: {}, + correlation_properties: [], }, { - "identifier": "order_ready", - "location": "examples", - "schema": {}, - "correlation_properties": [ + identifier: 'order_ready', + location: 'examples', + schema: {}, + correlation_properties: [ { - "identifier": "table_number", - "retrieval_expression": "table_number" + identifier: 'table_number', + retrieval_expression: 'table_number', }, { - "identifier": "franchise_id", - "retrieval_expression": "franchise_id" - } - ] + identifier: 'franchise_id', + retrieval_expression: 'franchise_id', + }, + ], }, { - "identifier": "table_seated", - "location": "examples", - "schema": {}, - "correlation_properties": [ + identifier: 'table_seated', + location: 'examples', + schema: {}, + correlation_properties: [ { - "identifier": "table_number", - "retrieval_expression": "table_number-v2" + identifier: 'table_number', + retrieval_expression: 'table_number-v2', }, { - "identifier": "franchise_id", - "retrieval_expression": "franchise_id-v2" - } - ] - } - ] - } + identifier: 'franchise_id', + retrieval_expression: 'franchise_id-v2', + }, + ], + }, + ], + }, }); }); -bpmnModeler.on('spiff.add_message.requested', (event) => { - // event.eventBus.fire('spiff.add_message.returned', { - // name: 'test_message', - // correlation_properties: { - // "c1": { - // "retrieval_expressions": "c1_expression" - // }, - // "c2": { - // "retrieval_expressions": "c2_expression" - // } - // } - // element: {} - // }); - event.eventBus.fire('spiff.add_message.returned', { - name: 'test_message', +bpmnModeler.on(SPIFF_ADD_MESSAGE_REQUESTED_EVENT, (event) => { + event.eventBus.fire(SPIFF_ADD_MESSAGE_RETURNED_EVENT, { + name: 'test_message1', correlation_properties: { - "c1": { - "retrieval_expressions": ["c1_expression"] + c1: { + retrieval_expression: 'c1_expression', + }, + c2: { + retrieval_expression: 'c2_expression', }, - "c2": { - "retrieval_expressions": ["c2_expression"] - } }, element: { - name: 'elementName' - } + id: 'my_user_task', + }, }); }); // As call activites might refernce processes across the system // it should be possible to search for a paticular call activity. bpmnModeler.on('spiff.callactivity.search', (event) => { - console.log("Firing call activity update", event.element) event.eventBus.fire('spiff.callactivity.update', { value: 'searched_bpmn_id', element: event.element, @@ -285,13 +281,19 @@ bpmnModeler.on('spiff.callactivity.search', (event) => { /* This restores unresolved references that camunda removes */ -bpmnModeler.on('import.parse.complete', event => { - const refs = event.references.filter(r => r.property === 'bpmn:loopDataInputRef' || r.property === 'bpmn:loopDataOutputRef'); - const desc = bpmnModeler._moddle.registry.getEffectiveDescriptor('bpmn:ItemAwareElement'); - refs.forEach(ref => { +bpmnModeler.on('import.parse.complete', (event) => { + const refs = event.references.filter( + (r) => + r.property === 'bpmn:loopDataInputRef' || + r.property === 'bpmn:loopDataOutputRef' + ); + const desc = bpmnModeler._moddle.registry.getEffectiveDescriptor( + 'bpmn:ItemAwareElement' + ); + refs.forEach((ref) => { const props = { id: ref.id, - name: ref.id ? typeof (ref.name) === 'undefined' : ref.name, + name: ref.id ? typeof ref.name === 'undefined' : ref.name, }; let elem = bpmnModeler._moddle.create(desc, props); elem.$parent = ref.element; @@ -299,7 +301,7 @@ bpmnModeler.on('import.parse.complete', event => { }); }); -bpmnModeler.importXML(diagramXML).then(() => { }); +bpmnModeler.importXML(diagramXML).then(() => {}); // This handles the download and upload buttons - it isn't specific to // the BPMN modeler or these extensions, just a quick way to allow you to diff --git a/app/spiffworkflow/constants.js b/app/spiffworkflow/constants.js index 1dc04c7..70f3d7f 100644 --- a/app/spiffworkflow/constants.js +++ b/app/spiffworkflow/constants.js @@ -1 +1,3 @@ export const SPIFFWORKFLOW_XML_NAMESPACE = 'spiffworkflow'; +export const SPIFF_ADD_MESSAGE_REQUESTED_EVENT = 'spiff.add_message.requested'; +export const SPIFF_ADD_MESSAGE_RETURNED_EVENT = 'spiff.add_message.returned'; diff --git a/app/spiffworkflow/messages/MessageHelpers.js b/app/spiffworkflow/messages/MessageHelpers.js index ad7e7ee..0877ebf 100755 --- a/app/spiffworkflow/messages/MessageHelpers.js +++ b/app/spiffworkflow/messages/MessageHelpers.js @@ -153,7 +153,7 @@ function getRetrievalExpressionFromCorrelationProperty( for (const retrievalExpression of correlationProperty.correlationPropertyRetrievalExpression) { if ( retrievalExpression.$type === - 'bpmn:CorrelationPropertyRetrievalExpression' && + 'bpmn:CorrelationPropertyRetrievalExpression' && retrievalExpression.messageRef && retrievalExpression.messageRef.id === message.id ) { @@ -416,6 +416,16 @@ export function createOrUpdateCorrelationPropertiesV2( correlationProperty.correlationPropertyRetrievalExpression.push( retrievalExpression ); + } else { + const existingRetrievalExpression = + correlationProperty.correlationPropertyRetrievalExpression[ + existingExpressionIndex + ]; + const existingFormalExpression = + existingRetrievalExpression.messagePath; + existingFormalExpression.body = propConfig.retrieval_expression + ? propConfig.retrieval_expression + : ''; } const existingIndex = definitions.rootElements.findIndex( @@ -496,8 +506,8 @@ function isMessageRefInCorrelationPropertiesRetrivalExpression( ) { return correlationProperty.correlationPropertyRetrievalExpression ? correlationProperty.correlationPropertyRetrievalExpression.some( - (expr) => expr.messageRef === messageRef - ) + (expr) => expr.messageRef === messageRef + ) : false; } @@ -617,11 +627,11 @@ export function setMessageRefToListofCorrelationProperties( retrievalExpression.messageRef = messageRef; rootElement.correlationPropertyRetrievalExpression ? rootElement.correlationPropertyRetrievalExpression.push( - retrievalExpression - ) + retrievalExpression + ) : (rootElement.correlationPropertyRetrievalExpression = [ - retrievalExpression, - ]); + retrievalExpression, + ]); } else if ( rootElement.$type === 'bpmn:CorrelationProperty' && !correlationPropertyIDs.includes(rootElement.id) && @@ -684,7 +694,6 @@ export function setParentCorrelationKeys( element, moddle ) { - // Retrieve all correlation properties let correlationProperties = findCorrelationProperties( element.businessObject, @@ -716,7 +725,6 @@ export function setParentCorrelationKeys( .find((element) => element.$type === 'bpmn:Collaboration'); if (collaboration) { - // Remove existing correlation keys other than the main correlation key collaboration.get('correlationKeys').forEach((key, index) => { if (key.name !== 'MainCorrelationKey') { @@ -734,7 +742,9 @@ export function setParentCorrelationKeys( // Replace the existing key with mainCorrelationKey const index = collaboration.get('correlationKeys').indexOf(existingKey); if (index !== -1) { - collaboration.get('correlationKeys').splice(index, 1, mainCorrelationKey); + collaboration + .get('correlationKeys') + .splice(index, 1, mainCorrelationKey); } } } else { @@ -788,10 +798,16 @@ function findOrCreateMainCorrelationKey(definitions, bpmnFactory, moddle) { return mainCorrelationKey; } -export function synCorrleationProperties(element, definitions, moddle, msgObject) { +export function synCorrleationProperties( + element, + definitions, + moddle, + msgObject +) { const { businessObject } = element; const correlationProps = findCorrelationProperties(businessObject, moddle); const expressionsToDelete = []; + for (let cProperty of correlationProps) { let isUsed = false; for (const cpExpression of cProperty.correlationPropertyRetrievalExpression) { @@ -800,16 +816,16 @@ export function synCorrleationProperties(element, definitions, moddle, msgObject cpExpression.messageRef.id, definitions ); - isUsed = (msgRef && msgObject && cpExpression.messageRef.id !== msgObject.identifier) ? true : isUsed; + isUsed = + msgRef && + msgObject && + cpExpression.messageRef.id !== msgObject.identifier + ? true + : isUsed; // if unused false, delete retrival expression if (!msgRef) { - console.log('Delete expression', cpExpression); - expressionsToDelete.push(cpExpression); - } else if (msgObject && !msgObject.correlation_properties.some(obj => obj.identifier === cProperty.id)) { - console.log('Delete expression', cpExpression); expressionsToDelete.push(cpExpression); } - } // Delete the retrieval expressions that are not used @@ -826,11 +842,16 @@ export function synCorrleationProperties(element, definitions, moddle, msgObject } // If Unused, delete the correlation property - const propertyToBeDeleted = (isUsed || msgObject && msgObject.correlation_properties && msgObject.correlation_properties.some(obj => obj.identifier === cProperty.id)); + const propertyToBeDeleted = + isUsed || + (msgObject && + msgObject.correlation_properties && + msgObject.correlation_properties.some( + (obj) => obj.identifier === cProperty.id + )); if (!propertyToBeDeleted) { const index = definitions.get('rootElements').indexOf(cProperty); if (index > -1) { - console.log('Delete property', cProperty); definitions.rootElements.splice(index, 1); } } diff --git a/app/spiffworkflow/messages/propertiesPanel/MessagesPropertiesProvider.js b/app/spiffworkflow/messages/propertiesPanel/MessagesPropertiesProvider.js index 1fb92e9..ba76b21 100755 --- a/app/spiffworkflow/messages/propertiesPanel/MessagesPropertiesProvider.js +++ b/app/spiffworkflow/messages/propertiesPanel/MessagesPropertiesProvider.js @@ -1,7 +1,5 @@ -import { is } from 'bpmn-js/lib/util/ModelUtil'; import { isMessageElement } from '../MessageHelpers'; -import { createCollaborationGroup } from './processLevelProvider/CollaborationPropertiesProvider'; import { createMessageGroup } from './elementLevelProvider/TaskEventMessageProvider'; const LOW_PRIORITY = 500; @@ -15,17 +13,6 @@ export default function MessagesPropertiesProvider( ) { this.getGroups = function getGroupsCallback(element) { return function pushGroup(groups) { - // if (is(element, 'bpmn:Collaboration') || is(element, 'bpmn:Process')) { - // groups.push( - // ...createCollaborationGroup( - // element, - // translate, - // moddle, - // commandStack, - // elementRegistry - // ) - // ); - // } else if (isMessageElement(element)) { const messageIndex = findEntry(groups, 'message'); if (messageIndex) { @@ -64,4 +51,5 @@ MessagesPropertiesProvider.$inject = [ 'moddle', 'commandStack', 'elementRegistry', -]; \ No newline at end of file +]; + diff --git a/app/spiffworkflow/messages/propertiesPanel/elementLevelProvider/MessageSelect.jsx b/app/spiffworkflow/messages/propertiesPanel/elementLevelProvider/MessageSelect.jsx index 5bf852d..e5ad3b6 100755 --- a/app/spiffworkflow/messages/propertiesPanel/elementLevelProvider/MessageSelect.jsx +++ b/app/spiffworkflow/messages/propertiesPanel/elementLevelProvider/MessageSelect.jsx @@ -2,7 +2,6 @@ import React from 'react'; import { useService } from 'bpmn-js-properties-panel'; import { SelectEntry } from '@bpmn-io/properties-panel'; import { - createOrUpdateCorrelationProperties, createOrUpdateCorrelationPropertiesV2, findMessageModdleElements, getMessageRefElement, @@ -13,6 +12,7 @@ import { setParentCorrelationKeys, synCorrleationProperties, } from '../../MessageHelpers'; +import { SPIFF_ADD_MESSAGE_RETURNED_EVENT } from '../../../constants'; export const spiffExtensionOptions = {}; @@ -38,135 +38,84 @@ export function MessageSelect(props) { }; const setValue = async (value) => { - - if (ELEMENT_ID) { - // ⚠️⚠️ Not sure if we can keep this - // This condition verify if Setvalue trigger is about the same element triggered from spifarena - const nwElement = elementRegistry.get(ELEMENT_ID); - shapeElement = (nwElement) ? nwElement : shapeElement; - element = (nwElement) ? nwElement : element; - } - - // Define variables const messageId = value; const { businessObject } = element; - let oldMessageRef = (businessObject.eventDefinitions) ? businessObject.eventDefinitions[0].messageRef : businessObject.messageRef; - let definitions = getRoot(businessObject); - if (!definitions || !definitions.get('rootElements')) { - definitions.set('rootElements', []); + const oldMessageRef = + businessObject.eventDefinitions?.[0].messageRef || + businessObject.messageRef; + const definitions = getRoot(businessObject); + + if (ELEMENT_ID) { + // This condition verify if Setvalue trigger is about the same element triggered from spifarena + const nwElement = elementRegistry.get(ELEMENT_ID); + shapeElement = nwElement ? nwElement : shapeElement; + element = nwElement ? nwElement : element; } - // Retrieve Message - let bpmnMessage = definitions - .get('rootElements') - .find( - (element) => - element.$type === 'bpmn:Message' && - (element.id === messageId || element.name === messageId), - ); + if (!definitions?.rootElements) { + definitions.rootElements = []; + } + + let bpmnMessage = findMessageById(definitions, messageId); - // If the Message doesn't exist, create new one if (!bpmnMessage) { - bpmnMessage = bpmnFactory.create('bpmn:Message', { - id: messageId, - name: messageId, - }); - definitions.get('rootElements').push(bpmnMessage); + bpmnMessage = createMessage(bpmnFactory, messageId); + definitions.rootElements.push(bpmnMessage); } else if (bpmnMessage.id !== bpmnMessage.name) { bpmnMessage.id = bpmnMessage.name; } - // Update messageRef of current Element - if (isMessageEvent(shapeElement)) { - element.businessObject.eventDefinitions[0].set( - 'extensionElements', - moddle.create('bpmn:ExtensionElements'), - ); // Clear extension element - const messageEventDefinition = element.businessObject.eventDefinitions[0]; - messageEventDefinition.messageRef = bpmnMessage; - // call this to update the other elements in the props panel like payload - commandStack.execute('element.updateModdleProperties', { - element: element, - moddleElement: element.businessObject, - properties: {}, - }); - } else if (isMessageElement(shapeElement)) { - element.businessObject.set( - 'extensionElements', - moddle.create('bpmn:ExtensionElements'), - ); // Clear extension element - element.businessObject.messageRef = bpmnMessage; - commandStack.execute('element.updateProperties', { - element: element, - properties: {}, - }); - } + updateElementMessageRef(element, bpmnMessage, moddle, commandStack); - // Add Correlation Properties of for the new message - const msgObject = spiffExtensionOptions['spiff.messages'] - ? spiffExtensionOptions['spiff.messages'].find( - (msg) => msg.identifier === messageId, - ) - : undefined; + const messageObject = findMessageObject(messageId); - if (msgObject) { + if (messageObject) { createOrUpdateCorrelationPropertiesV2( bpmnFactory, commandStack, element, - msgObject['correlation_properties'], - messageId, + messageObject.correlation_properties, + messageId ); } if (oldMessageRef) { - - // Remove previous message in case it's not used anymore - const isOldMessageUsed = isMessageRefUsed(definitions, oldMessageRef.id); - - if (!isOldMessageUsed) { - const rootElements = definitions.get('rootElements'); - const oldMessageIndex = rootElements.findIndex( - (element) => - element.$type === 'bpmn:Message' && element.id === oldMessageRef.id, - ); - if (oldMessageIndex !== -1) { - rootElements.splice(oldMessageIndex, 1); - definitions.rootElements = rootElements; - } - } - - // Automatic deletion of previous message correlation properties - synCorrleationProperties(element, definitions, moddle, msgObject); + cleanupOldMessage(definitions, oldMessageRef.id); + synCorrleationProperties(element, definitions, moddle, messageObject); } - // Update Correlation key if Process has collaboration try { setParentCorrelationKeys(definitions, bpmnFactory, element, moddle); } catch (error) { - console.error('Error Caught while synchronizing Correlation key', error); + console.error('Error caught while synchronizing Correlation key', error); } - }; - eventBus.on('spiff.add_message.returned', async (event) => { - + eventBus.on(SPIFF_ADD_MESSAGE_RETURNED_EVENT, async (event) => { // Check if the received element matches the current element if (event.elementId !== element.id) { ELEMENT_ID = event.elementId; } - const cProperties = Object.entries(event.correlation_properties).map(([identifier, properties]) => ({ - identifier, - retrieval_expression: Array.isArray(properties.retrieval_expressions) ? properties.retrieval_expressions[0] : properties.retrieval_expressions - })); + const cProperties = Object.entries(event.correlation_properties).map( + ([identifier, properties]) => ({ + identifier, + retrieval_expression: Array.isArray(properties.retrieval_expression) + ? properties.retrieval_expression[0] + : properties.retrieval_expression, + }) + ); let newMsg = { identifier: event.name, - correlation_properties: cProperties + correlation_properties: cProperties, }; - spiffExtensionOptions['spiff.messages'] = (Array.isArray(spiffExtensionOptions['spiff.messages']) && spiffExtensionOptions['spiff.messages']) ? spiffExtensionOptions['spiff.messages'] : []; + spiffExtensionOptions['spiff.messages'] = + Array.isArray(spiffExtensionOptions['spiff.messages']) && + spiffExtensionOptions['spiff.messages'] + ? spiffExtensionOptions['spiff.messages'] + : []; const messageIndex = spiffExtensionOptions['spiff.messages'].findIndex( (msg) => msg.identifier === newMsg.identifier ); @@ -175,7 +124,6 @@ export function MessageSelect(props) { } else { spiffExtensionOptions['spiff.messages'].push(newMsg); } - setValue(event.name); }); @@ -234,3 +182,78 @@ function removeDuplicatesByLabel(array) { return seen.has(item.label) ? false : seen.set(item.label, true); }); } + +function findMessageById(definitions, messageId) { + return definitions.rootElements?.find( + (element) => + element.$type === 'bpmn:Message' && + (element.id === messageId || element.name === messageId) + ); +} + +function createMessage(bpmnFactory, messageId) { + return bpmnFactory.create('bpmn:Message', { + id: messageId, + name: messageId, + }); +} + +function updateElementMessageRef(element, bpmnMessage, moddle, commandStack) { + if (isMessageEvent(element) && element.businessObject) { + const messageEventDefinition = element.businessObject.eventDefinitions[0]; + messageEventDefinition.extensionElements = + messageEventDefinition.extensionElements + ? messageEventDefinition.extensionElements + : moddle.create('bpmn:ExtensionElements'); + messageEventDefinition.messageRef = bpmnMessage; + commandStack.execute('element.updateModdleProperties', { + element: element, + moddleElement: element.businessObject, + properties: {}, + }); + } else if (isMessageElement(element) && element.businessObject) { + element.businessObject.extensionElements = element.businessObject + .extensionElements + ? element.businessObject.extensionElements + : moddle.create('bpmn:ExtensionElements'); + element.businessObject.messageRef = bpmnMessage; + commandStack.execute('element.updateProperties', { + element: element, + properties: {}, + }); + } +} + +function findMessageObject(messageId) { + const messageObject = spiffExtensionOptions['spiff.messages']?.find( + (msg) => msg.identifier === messageId + ); + + if (messageObject) { + return { + identifier: messageObject.identifier, + correlation_properties: messageObject.correlation_properties.map( + (prop) => ({ + identifier: prop.identifier, + retrieval_expression: prop.retrieval_expression, + }) + ), + }; + } else { + return null; + } +} + +function cleanupOldMessage(definitions, oldMessageId) { + if (!isMessageRefUsed(definitions, oldMessageId)) { + const rootElements = definitions.rootElements; + const oldMessageIndex = rootElements.findIndex( + (element) => + element.$type === 'bpmn:Message' && element.id === oldMessageId + ); + if (rootElements && oldMessageIndex !== -1) { + rootElements.splice(oldMessageIndex, 1); + definitions.rootElements = rootElements; + } + } +}