search

camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.05k stars 1.53k forks source link

Fixed the problem that setting variables does not take effect #4236

Closed yzz6619 closed 3 months ago

yzz6619 commented 4 months ago

image image

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

danielkelemen commented 3 months ago

Hi @yzz6619, Thank you for opening a PR.

Can you describe the problem this solves in more details?

-Daniel

yzz6619 commented 3 months ago

@danielkelemen Setting variables in external tasks has no effect. Because the property names are not mapped correctly. image image image

danielkelemen commented 3 months ago

Hi @yzz6619,

I looked into it and it looks like modifications is the correct parameter indeed. This the API this calls: https://docs.camunda.org/rest/camunda-bpm-platform/7.20/#tag/Process-Instance/operation/modifyProcessInstanceVariables

I'm wondering why the IT test isn't failing in this case. I'll debug it a little bit.

-Daniel

danielkelemen commented 3 months ago

I found the root cause. The test is the ExternalTaskHandlerIT#shouldSetVariablesByProcessInstanceId and the reason it's not failing is because it also calls the complete with the same variable which can set it correctly, so the test succeeds.

Would you be interested in fixing the test as well? I'd not pass the variables in the complete call. This would only pass with your recent change. I'd also add this to the shouldSetVariablesByExternalTask test. That essentially uses the same logic.

-Daniel

danielkelemen commented 3 months ago

Hi @yzz6619, your change was merged with https://github.com/camunda/camunda-bpm-platform/commit/622a6b071832cac62fd55f336216f629b7bd3728. I added the additional test change.

Thanks for your contribution!