KOGITO-2651 : Allow expressions in output assignments #3363
Conversation
|
Jenkins execute full downstream build |
|
Jenkins execute compile downstream build |
|
Jenkins execute compile downstream build |
|
|
||
| assertNull(view.getModel().getExpression()); | ||
| view.setExpression("{hello}"); | ||
| assertEquals(view.getModel().getExpression(), "{hello}"); |
There was a problem hiding this comment.
New test doesn't test constant, but expression. So {hello} should be reverted back to hello.
There was a problem hiding this comment.
I don't see the issue yet @hasys here, what you mean? I understand the view has some model which contains an expression field (String). The expression has to be set using some syntax (eg: {expr}), also the view shows the raw expression (with complet syntax) on the UI field, which makes sense to me also because this way the view is "agnostic" to the expression syntax (that's maybe a presentation stuff). So for me looks fine to return the {hello} back from the view.getModel().getExpression()...
As an example, think on the view just being an HTML input element - you would write the complete text with syntax as the value, also you'll expect the same value when you obtain it's value.
WDYT?
There was a problem hiding this comment.
@inodeman what's wrong here is the order of the arguments for the assertEquals, the first one is the expected value, and second one is the actual. So please try to fix those, if any others. Thanks!
There was a problem hiding this comment.
Hi @romartin, yes it's a bit frustrating. Main idea was to allow put constants to the output value. But test was changed from test Constants to test Expression. So it was possible to set expression before as well, but wasn't possible to set Constants but after the change constants aren't tested any more.
| getModel().setExpression(expression); | ||
| } | ||
| } | ||
|
|
||
| private static boolean isConstant(String expression) { |
There was a problem hiding this comment.
Method can be removed now since it had only one invocation.
| notification.fire(new NotificationEvent(StunnerFormsClientFieldsConstants.INSTANCE.Only_expressions_allowed_for_output(), | ||
| NotificationEvent.NotificationType.ERROR)); | ||
| processVarComboBox.textBoxValueChanged(""); | ||
| } else { | ||
| getModel().setExpression(expression); |
There was a problem hiding this comment.
Some spaces can be removed here since it is now on method level.
|
Jenkins execute full downstream build |
|
Hey @inodeman, this needs rebase plz |
|
Jenkins execute full downstream build |
|
Hi @romartin rebased, tested and builds re-fired |
|
Kudos, SonarCloud Quality Gate passed!
|
Hi @romartin @handreyrc can you review this PR? Change is simple, there was a check before to show a warning if the variable is output and expression. I removed the custom message and constants and updated test code. Let me know