bpmn-js-differ
bpmn-js-differ copied to clipboard
feat: Add support for diffing changes to timer events. Added tests to verify
Proposed Changes
closes #15
The library has missed support for diffing changes made to bpmn:TimerEventDefinition. This PR adds support for this, by including bpmn:TimerEventDefinition in the list of tracked definitions. The solution is in whole taken from this comment.
Steps to test
- Pull down repository
- Run
npm install - Run
npm run all - See that all tests passes and that the results object from the timer event looks something like this:
ChangeHandler {
_layoutChanged: {},
_changed: { TimerEventDefinition_0fgktse: { model: [Base], attrs: {} } },
_removed: {},
_added: {}
}
Checklist
To ensure you provided everything we need to look at your PR:
- [x] Brief textual description of the changes present
- [x] Visual demo attached (hopefully the code sample is enough 😅 )
- [x] Steps to try out present, i.e. using the
@bpmn-io/srtool - [x] Related issue linked via
Closes {LINK_TO_ISSUE}orRelated to {LINK_TO_ISSUE}
Hi,
Thanks for your contribution. I see that it's not ready for review yet given your comment https://github.com/bpmn-io/bpmn-js-differ/pull/19#discussion_r1643397380
Do you want to finish this pull request? If so, please tag me in a comment. Otherwise we will close it if there are no updates soon.
Hi @barmac, been on holiday the last weeks. Will look into the regression this week :)
@Gawdfrey Thanks for your contribution.
This looks very tailored towards "timer event definition". What about any kind of change, affecting any kind of event definition? How would such a solution look like? Is there a good reason to only support timer events?
@Gawdfrey I thank you again for your contribution! As commented previously (https://github.com/bpmn-io/bpmn-js-differ/pull/19#issuecomment-2296502852) this, unfortunately, is a fix tailored to timers (only). We cannot generalize it easily to be applicable to all sorts of events (and, in the context of technical execution, extension elements). Hence I'm closing your PR.
We hope to get to the linked issue back at some point, and implement a fix.