In and Out Variables for Tasks #84
Conversation
WalkthroughThe recent update introduces styling adjustments for the bio-properties panel in the CSS, new import configurations, and export settings in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
test/spec/IoVariablesSpec.js
Outdated
|
|
||
| // We Select a userTask element | ||
| const shapeElement = await expectSelected('Activity_1hmit5k'); | ||
| expect(shapeElement, "I can't find ManualTask element").to.exist; |
There was a problem hiding this comment.
Fix the typo in the error message.
- "I can't find ManualTask element"
+ "I can't find User Task element"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| expect(shapeElement, "I can't find ManualTask element").to.exist; | |
| expect(shapeElement, "I can't find User Task element").to.exist; |
test/spec/IoVariablesSpec.js
Outdated
|
|
||
| // We Select a userTask element | ||
| const shapeElement = await expectSelected('Activity_1hmit5k'); | ||
| expect(shapeElement, "I can't find Uset Task element").to.exist; |
There was a problem hiding this comment.
Fix the typo in the error message.
- "I can't find Uset Task element"
+ "I can't find User Task element"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| expect(shapeElement, "I can't find Uset Task element").to.exist; | |
| expect(shapeElement, "I can't find User Task element").to.exist; |
| import { is } from 'bpmn-js/lib/util/ModelUtil'; | ||
| import { createIoGroup } from './IoGroup.js'; |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has revealed that the file IoGroup.js is correctly implemented and available as expected. However, there are issues with locating ModelUtil.js. The file could not be found in the specified directories, suggesting that either the file does not exist, or the import path provided in the code is incorrect. This needs further investigation or correction in the codebase.
Analysis chain
Ensure that the imported modules and functions (is and createIoGroup) are correctly implemented and available.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence and correct implementation of ModelUtil and IoGroup.js
fd 'ModelUtil.js' 'bpmn-js/lib/util'
fd 'IoGroup.js' 'app/spiffworkflow/InputOutput/propertiesProvider'
Length of output: 263
Script:
#!/bin/bash
# Correct the search path for ModelUtil.js and verify its existence
fd 'ModelUtil.js' --search-path 'node_modules/bpmn-js/lib/util' --exec cat {}
# Verify the implementation of IoGroup.js by displaying its contents
cat 'app/spiffworkflow/InputOutput/propertiesProvider/IoGroup.js'
Length of output: 1506
| }) : []; | ||
|
|
||
| function add(event) { | ||
| const { businessObject } = element; | ||
|
|
||
| const newOutputID = moddle.ids.nextPrefixed('DataOutput_'); | ||
|
|
||
| // Create a new DataOutput | ||
| const newOutput = bpmnFactory.create('bpmn:DataOutput', { id: newOutputID, name: newOutputID }); | ||
|
|
||
| // Check if ioSpecification already exists | ||
| createSpecification(bpmnFactory, businessObject, 'output', newOutput) | ||
|
|
||
| // Update the element | ||
| updateElementProperties(commandStack, element); | ||
|
|
||
| event.stopPropagation(); | ||
| } | ||
|
|
||
| return { items, add }; | ||
| } | ||
|
|
||
| function removeFactory(props) { | ||
| const { element, bpmnFactory, commandStack, outputEntry } = props; | ||
| return function (event) { | ||
| event.stopPropagation(); | ||
| removeElementFromSpecification(element, outputEntry, 'output'); | ||
| updateElementProperties(commandStack, element); | ||
| }; | ||
| } | ||
|
|
||
| function OutputParamGroup(props) { | ||
|
|
||
| const { id, outputEntry, element, moddle, commandStack, translate, bpmnFactory } = props; | ||
|
|
||
| return [ | ||
| { | ||
| id, | ||
| outputEntry, | ||
| component: OutputParamTextField, | ||
| isEdited: isTextFieldEntryEdited, | ||
| element, | ||
| moddle, | ||
| commandStack, | ||
| translate, | ||
| bpmnFactory | ||
| } | ||
| ]; | ||
| } | ||
|
|
||
| function OutputParamTextField(props) { | ||
|
|
||
| const { id, element, outputEntry, moddle, commandStack, translate, bpmnFactory } = props; | ||
|
|
||
| const debounce = useService('debounceInput'); | ||
|
|
||
| const setValue = (value) => { | ||
| try { | ||
| const ioSpecification = element.businessObject.ioSpecification; | ||
|
|
||
| if (!value || value == '') { | ||
| console.error('No value provided for this input.'); | ||
| return; | ||
| } | ||
|
|
||
| if (!ioSpecification) { | ||
| console.error('No ioSpecification found for this element.'); | ||
| return; | ||
| } | ||
|
|
||
| let existingInput = ioSpecification.dataOutputs.find(input => input.id === outputEntry.name || input.name === outputEntry.name); | ||
|
|
||
| if (existingInput) { | ||
| existingInput.name = value; | ||
| existingInput.id = value; | ||
| } else { | ||
| console.error(`No DataOutput found :> ${outputEntry.name}`); | ||
| return; | ||
| } | ||
|
|
||
| updateElementProperties(commandStack, element); | ||
|
|
||
| } catch (error) { | ||
| console.log('Setting Value Error : ', error); | ||
| } | ||
| }; | ||
|
|
||
| const getValue = () => { | ||
| return outputEntry.name; | ||
| }; | ||
|
|
||
| return TextFieldEntry({ | ||
| element, | ||
| id: `${id}-output`, | ||
| label: translate('Output Name'), | ||
| getValue, | ||
| setValue, | ||
| debounce, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The functions createSpecification, removeElementFromSpecification, and updateElementProperties are correctly implemented and integrated as verified by their presence and definitions in helpers.js. However, the component TextFieldEntry could not be located within the repository. It is possible that this component is sourced externally or not included in the repository. Please ensure that TextFieldEntry is correctly integrated and available for use in the project.
Analysis chain
The OutputParametersArray function is well-implemented, providing structured management for output parameters. Ensure that the components and helper functions (TextFieldEntry, createSpecification, removeElementFromSpecification, updateElementProperties) are correctly implemented and integrated.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the correct implementation and integration of TextFieldEntry, createSpecification, removeElementFromSpecification, and updateElementProperties
fd 'TextFieldEntry.js' '@bpmn-io/properties-panel'
fd 'helpers.js' 'app/spiffworkflow/InputOutput'
Length of output: 248
Script:
#!/bin/bash
# Search for TextFieldEntry.js in the entire repository
fd 'TextFieldEntry.js'
# Check for the presence of createSpecification, removeElementFromSpecification, and updateElementProperties in helpers.js
rg 'function createSpecification' app/spiffworkflow/InputOutput/helpers.js
rg 'function removeElementFromSpecification' app/spiffworkflow/InputOutput/helpers.js
rg 'function updateElementProperties' app/spiffworkflow/InputOutput/helpers.js
Length of output: 489
|
i think there are a couple nitpicky copy exits suggested by coderabbit. perhaps it would be worth demoing this one to the team? |
|
@burnettk Except for the typing mistakes, the main point raised by Coderabbit is having some challenges locating some utils functions that exist at the level of the BPMN.js library, and we import them in our extensions, such as the is method that can be used to verify the type of an element. Do you mean these two points, or are there other suggestions I've missed? |
|
i just wanted you to take a look at them and see if they were worth fixing, you did it, thanks. 👍 |
|
i guess there are some conflicts here now, @theaubmov since we stalled so long, sorry. are you worried about this change in general, or it's not too likely to break existing functionality? i just merged everything else, but this one scared me the most. i'd love to get it merged if you think it's good to go. thank you. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/css/app.css (1 hunks)
- app/spiffworkflow/index.js (3 hunks)
Additional comments not posted (4)
app/css/app.css (2)
77-77: Padding adjustment in.bio-properties-panel-descriptionThe reduction in
padding-blockfrom 8px to 5px could enhance the compactness of the UI, making more content visible without scrolling. Ensure this change aligns with the overall design principles of the application.
80-87: Styling for expanded groups in the properties panelThe modifications to
.bio-properties-panel-group-entries.open > .bio-properties-panel-groupinclude adjustments to margins, borders, and the removal of box-shadow and transition effects. This simplifies the styling and potentially improves performance by reducing repaints caused by transitions.app/spiffworkflow/index.js (2)
19-19: Addition ofIoPropertiesProviderandCallActivityInterceptorto module exportsThe integration of
IoPropertiesProviderandCallActivityInterceptoris crucial for the new functionalities described in the PR. Ensure these components are properly documented and tested, especially since they are now part of the exported API.Also applies to: 48-48
71-72: Correct registration ofIoPropertiesProviderandCallActivityInterceptorThe registration of these components in the module configuration is essential for their activation and use within the framework. Verify that these components behave as expected in the runtime environment.
Verification successful
Correct registration and likely testing of
IoPropertiesProviderandCallActivityInterceptorThe components
IoPropertiesProviderandCallActivityInterceptorare correctly registered in the module configuration. The presence of related test files such astest/spec/CallActivitySpec.jsandtest/spec/BpmnInputOutputSpec.jssuggests that these components are indirectly tested.
test/spec/CallActivitySpec.jstest/spec/BpmnInputOutputSpec.jsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the behavior of IoPropertiesProvider and CallActivityInterceptor in the runtime environment. # Test: Check logs for any runtime errors or warnings related to these components. cat logs/runtime.log | grep -E 'IoPropertiesProvider|CallActivityInterceptor'Length of output: 128
Script:
#!/bin/bash # Search for the definitions of IoPropertiesProvider and CallActivityInterceptor in the codebase echo "Definitions:" ast-grep --lang javascript --pattern 'class IoPropertiesProvider { $$$ }' ast-grep --lang javascript --pattern 'class CallActivityInterceptor { $$$ }' # Search for the usages of IoPropertiesProvider and CallActivityInterceptor in the codebase echo "Usages:" rg 'IoPropertiesProvider' rg 'CallActivityInterceptor' # Search for any existing tests related to IoPropertiesProvider and CallActivityInterceptor echo "Tests:" rg 'IoPropertiesProvider' --glob '*test*' rg 'CallActivityInterceptor' --glob '*test*'Length of output: 1864
Script:
#!/bin/bash # Search for any test suites or files that might indirectly test IoPropertiesProvider and CallActivityInterceptor echo "Indirect Tests:" rg 'IoPropertiesProvider' --glob '*spec*' rg 'CallActivityInterceptor' --glob '*spec*' # Check if there are any references to these components in the test configuration or setup files echo "Test Configuration References:" rg 'IoPropertiesProvider' --glob '*config*' rg 'CallActivityInterceptor' --glob '*config*'Length of output: 587
Script:
#!/bin/bash # Re-run the search with --debug to understand why files are being skipped echo "Debug Indirect Tests:" rg 'IoPropertiesProvider' --glob '*spec*' --debug rg 'CallActivityInterceptor' --glob '*spec*' --debug # Search for any test files in the codebase without filtering by name echo "All Test Files:" rg --files | grep -E 'test|spec'Length of output: 59962
|
Yes, I got your concerns. I believe we can merge this change. The new feature is an extension to the editor and it won't impact or alter any existing features. The point of introducing in and out variables for tasks is to allow users to define input and output elements at the task level. |
This PR is about controlling data flow through tasks, making sure that tasks receive the right data (inputs) and produce expected results (outputs) that other parts of the process can use.
Summary by CodeRabbit
New Features
Style