Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script task FEEL expression not recognized as typed available FEEL context #4614

Closed
nikku opened this issue Oct 16, 2024 · 5 comments
Closed
Assignees
Labels
Milestone

Comments

@nikku
Copy link
Member

nikku commented Oct 16, 2024

Describe the bug

If I use a bpmn:ScriptTask with a FEEL expression the actual expression is not added to editor intelligence, only the fact that an output variable exists:

capture up2Shw_optimized

Steps to reproduce

  1. Create a script task with implementation FEEL expression
  2. Assign a value, and resultVariable
  3. Use the resultVariable elsewhere on the diagram
  4. See that value information is not provided, though it is available (through the FEEL expression)

Expected behavior

  • Value (structure) is parsed from the FEEL expression on a best effort basis, and provided, as it is the case for other expressions:

    image

Environment

  • OS: Linux
  • Camunda Modeler Version: v5.28.0
  • Execution Platform: Camunda 8
  • Installed plug-ins: None

Additional context

No response

@nikku nikku added bug Something isn't working feel editing data handling labels Oct 16, 2024
@nikku
Copy link
Member Author

nikku commented Oct 16, 2024

CC @mschoe (we discussed and detected this issue).

@jarekdanielak jarekdanielak added the ready Ready to be worked on label Oct 22, 2024
@marstamm
Copy link
Member

@marstamm to have a time-boxed look into this, either fix or give implementation hints to be picked up by the team

@marstamm
Copy link
Member

Context

This turns out to be a deeper problem than anticipated. While it is quite simple to parse result from a script task, I uncovered some deeper problems with our Variable resolving.
image

The "simple but wrong" solution can be found on this branch https://github.com/bpmn-io/variable-resolver/tree/parse-zeebe-script.

The Edge cases are:

  • Input or Output mappings bind to the same variable name as the script
  • There are Output mappings <-- also a problem without adding Feel context

Problems

Assume the following Script task:

   inputMapping:    foo = 1
   Feel Script:     foo = 2
   outputMapping:   foo = 3

The smallest scale for scopes we currently support is Task level. In this case, we would only have 2 Variable assignments. For the local variable, it is unclear if it should evaluate the input mapping or the feel script, an which result to use in the output mapping.

Additionally, I found that our current Variable Resolver does not properly scope the result variables. In https://github.com/bpmn-io/extract-process-variables/blob/main/src/zeebe/util/ProcessVariablesUtil.js#L54-L58 , the scope is defined as the parent which has an input mapping with the same name.
This is correct on a process level, but not on task level. Any Variable that is in Task scope (such as in the example) should not be propagated to the process. So this diagram:
image
Only mappedResult should be suggested. However, scriptResult is also suggested
image

@marstamm marstamm removed their assignment Oct 25, 2024
@abdul99ahad
Copy link
Contributor

Here’s my understanding of the two main issues:

  • FEEL Expression Parsing for Script Tasks
  • Variable Scoping
    • Handling local variables within the same task.
    • Managing variables that other tasks can access.

@nikku
Copy link
Member Author

nikku commented Dec 10, 2024

@abdul99ahad is the solution already integrated into the modeler? If not, please move this issue to fixed upstream, and work towards the integration of the fixed upstream library.

@abdul99ahad abdul99ahad reopened this Dec 10, 2024
@abdul99ahad abdul99ahad added the fixed upstream Requires integration of upstream change label Dec 10, 2024
barmac pushed a commit to bpmn-io/variable-resolver that referenced this issue Dec 13, 2024
barmac pushed a commit to bpmn-io/variable-resolver that referenced this issue Dec 13, 2024
@barmac barmac closed this as completed in 5f6b59b Dec 13, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants