Skip to content

Specification and Using multiple Data filters section: Error in Workflow definition ? #630

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

Closed
radtriste opened this issue May 13, 2022 · 4 comments · Fixed by #632
Closed
Labels
change: documentation Improvements or additions to documentation. It won't impact a version change.
Milestone

Comments

@radtriste
Copy link
Contributor

https://github.com/serverlessworkflow/specification/blob/v0.8/specification.md#using-multiple-data-filters (same on main)

Event action is calling greeint function:

                "actions":[
                    {
                        "functionRef": {
                            "refName": "greetingFunction",
                            "arguments": {
                                "greeting": "${ .spanish } ",
                                "customerName": "${ .customerInfo.name } "
                            }
                        },
                        "actionDataFilter": {
                            "fromStateData": "${ .hello }",
                            "results": "${ .greetingMessageResult }",
                            "toStateData": "${ .finalCustomerGreeting }"
                        }
                    }
                ]

We have the action data filter using the "fromStateData": "${ .hello }",
which means the data for the action is

{
    "english": "Hello",
    "spanish": "Hola",
    "german": "Hallo",
    "russian": "Здравствуйте"
}

How is that possible that we use the customerInfo data into the function arguments, whereas, AFAIU, it is not in the current data for the action ?

@ricardozanini
Copy link
Member

Yeah, it sounds like a bug. In the writing, it is stated:

(3) Event state performs its actions: Before the first action is executed, its actionDataFilter is invoked. Its "fromStateData" expression filters the current state data to select from its data that should be available to action arguments. In this example it selects the "hello" property from the current state data. At this point the action is executed. We assume that for this example "greetingFunction" returns:

And the action arguments are expecting customerInfo property. So it should be corrected as:

{
   "actions":[
      {
         "functionRef":{
            "refName":"greetingFunction",
            "arguments":{
               "greeting":"${ .hello.spanish } ",
               "customerName":"${ .customerInfo.name } "
            }
         },
         "actionDataFilter":{
            "fromStateData":"${ { hello, customerInfo } }",
            "results":"${ .greetingMessageResult }",
            "toStateData":"${ .finalCustomerGreeting }"
         }
      }
   ]
}

additional brackets added for GH formmating

@radtriste
Copy link
Contributor Author

I will provide a PR then for the fix

I saw also some other small issues in the doc that I will correct in the same PR, if you are ok with that>

@tsurdilo
Copy link
Contributor

tsurdilo commented May 16, 2022

@radtriste @ricardozanini

    "fromStateData":"${ { hello, customer } }", 

I think this should be ${ { hello, customerInfo } } as the event payload would be applied to state data first

But then this would be the same thing as the current state data when action filter is applied, so think just remove fromStateData in the action filter is better in this case

@tsurdilo tsurdilo added the change: documentation Improvements or additions to documentation. It won't impact a version change. label May 16, 2022
@tsurdilo tsurdilo added this to the v0.9 milestone May 16, 2022
@ricardozanini
Copy link
Member

@tsurdilo

think just remove fromStateData in the action filter is better in this case

Not necessarily. I think the filter example is fine, and the fromStateData action filter segregates the language from the hello attribute. It's not the most complex and smart example but it will do. :)

@ricardozanini ricardozanini linked a pull request May 24, 2022 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change: documentation Improvements or additions to documentation. It won't impact a version change.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants