-
Notifications
You must be signed in to change notification settings - Fork 3
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(Condition): use update logic for applying conditions #35
Conversation
this.preExecute([ 'propertiesPanel.zeebe.changeTemplate' ], HIGH_PRIORITY, this._applyCondtions , true, this); | ||
} | ||
|
||
_applyCondtions(context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this looks like a typo. There is _applyConditions
already.
- _applyCondtions
+ _applyConditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, let's get this fixed 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to _handleTemplateUpgrade
since it only applies for the change template handler
// Event does not have an event definition | ||
if (!bo) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's fix the _applyConditions
confusion before we merge this :)
@@ -78,8 +80,8 @@ describe('provider/cloud-element-templates - ElementTemplatesConditionChecker', | |||
// then | |||
expect(businessObject.get('customProperty')).to.exist; | |||
|
|||
expect(businessObject.get('noDefaultProperty')).to.exist; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's clearly document this change in behavior. I believe we agreed this is a bug fix?
@@ -372,8 +374,7 @@ describe('cloud-element-templates/cmd - ChangeElementTemplateHandler', function( | |||
|
|||
const taskDefinition = findExtension(task, 'zeebe:TaskDefinition'); | |||
|
|||
expect(taskDefinition).to.exist; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's clearly document this change in behavior. I believe we agreed this is a bug fix?
This seems to ship to other hidden bug fixes ⬆️. Let's ensure they are marked as such in the CHANGELOG. |
fixes #32
property
andMessage
bindings