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

fix(engine-rest): /messages. Return result based on resultType #4321

Merged
merged 3 commits into from May 21, 2024

Conversation

punitdarira
Copy link
Contributor

related to #4310

@punitdarira
Copy link
Contributor Author

Testing scenarios performed on this bpmn file

@tasso94
Copy link
Member

tasso94 commented May 13, 2024

Hi @punitdarira,

Thank you for your contribution. We will have a look and get back to you.

Stay tuned!

Best,
Tassilo

Copy link
Member

@yanavasileva yanavasileva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like the made changes in the implementation.
❌ One test case needs to be adjusted to cover MessageCorrelationResultWithVariables use case. In
https://github.com/punitdarira/camunda-bpm-platform/blob/4310-fix/engine-rest/engine-rest/src/test/java/org/camunda/bpm/engine/rest/MessageRestServiceTest.java#L1263
please add a check for the execution like this: checkExecutionResult(content, 0);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good catch to adjust this DTO as well and extract the #setExecutionAndProcessInstance() method.

Copy link
Member

@yanavasileva yanavasileva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thank you for considering my feedback. I will merge the changes now.

@yanavasileva yanavasileva merged commit 3bb88cf into camunda:master May 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants