Skip to content
New issue

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

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

Already on GitHub? # to your account

Migrate compensation boundary event subscriptions only by boundary event mapping #24568

Conversation

berkaycanbc
Copy link
Contributor

@berkaycanbc berkaycanbc commented Nov 8, 2024

Description

Check related issue description and commit messages for details.

Related issues

closes #24487

@github-actions github-actions bot added the component/zeebe Related to the Zeebe component/team label Nov 8, 2024
Compensation subscription migration tests are updated to only provide mapping between boundary
events.
@berkaycanbc berkaycanbc force-pushed the bcan_24487_rejections-for-invalid-compensation-boundary-event-mappings branch from d3b279c to 964fede Compare November 12, 2024 15:10
@berkaycanbc berkaycanbc changed the title Rejections for invalid compensation boundary event mappings Migrate compensation boundary event subscriptions only by boundary event mapping Nov 12, 2024
@berkaycanbc berkaycanbc requested a review from nicpuppa November 12, 2024 15:11
The code is executed in following steps:
- Retrieve all compensation subscriptions of the process instance
- Filter subscriptions that are belong tasks only and return the compensable activity task of it
- For each compensable activity, if mapped with mapping instructions, retrieve target activity.
Then, return source and target activity together inside CompensableElementMapping object.
- For each source and target activity mapping:
  - If both source and target activity is in at the root level, return them immediately
  - If they are inside another flow scope (e.g. inside subprocess), recursively call the same
  method for their flow scopes until reaching the root process scope.
  - If there is a scope difference, that means user tries to change the flow scope of the
  compensation subscription. We do not allow changing the flow scope of elements. (this rule also
  applies to all other elements as previously implemented in preconditions)
@berkaycanbc berkaycanbc force-pushed the bcan_24487_rejections-for-invalid-compensation-boundary-event-mappings branch from 964fede to 9d5c992 Compare November 13, 2024 10:11
Copy link
Contributor

@nicpuppa nicpuppa left a comment

Choose a reason for hiding this comment

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

The implementation looks good 👍

Tests are added for the following:
- If handler type is changed, the flow should still work in the target process (e.g. handler was
service task in source and became subprocess in target)
- Compensation subscription migration should work for multi instance compensation handlers
Previously, we were receiving the compensable task by calling getElementById with the expected type.
But when getElementById is called with the expected type ExecutableActivity, it will return the
inner instance of the multi instance body, which is wrong. We wanted to retrieve the multi instance
body element itself. Later, we can retrieve its boundary events correctly.

A new method `getCompensableActivity()` is implemented to do casting manually and used in related
places.
@berkaycanbc berkaycanbc requested a review from nicpuppa November 13, 2024 13:23
@berkaycanbc
Copy link
Contributor Author

@nicpuppa I've spotted a bug related to multi instance handlers. I've sent the fix. Please take another look. 👀

@berkaycanbc berkaycanbc added this pull request to the merge queue Nov 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 13, 2024
@berkaycanbc berkaycanbc added this pull request to the merge queue Nov 14, 2024
Merged via the queue into main with commit ca93bf3 Nov 14, 2024
65 checks passed
@berkaycanbc berkaycanbc deleted the bcan_24487_rejections-for-invalid-compensation-boundary-event-mappings branch November 14, 2024 09:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
component/zeebe Related to the Zeebe component/team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate compensation boundary event subscriptions only by boundary event mapping
2 participants