-
-
Notifications
You must be signed in to change notification settings - Fork 1k
#3601 Always use SourceParameterCondition when checking source parameter #3620
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
Conversation
…ce parameter This is a breaking change, with this change whenever a source parameter is used as a source for a target property the condition has to apply to source parameters and not properties
These work as expected because the `isNotEmpty` presence checks is having a source parameter passed. If we change the parameter with the other the method will be invoked again
thunderhook
left a comment
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.
Just a few optional "expression" nitpicks in the changelog.
As we already said, we can definitely remove some tests, especially those you mentioned as duplicate scenarios in #3601 (comment)
Nevertheless, i will already approve, the current status could also be merged from my side.
|
Thanks @thunderhook. I've applied the proposed changes in the changelog. I also decided to remove all the fixture tests, they were not needed. The main thing that we are testing is already covered by the test that I added. Hope that's OK with you |
|
That's totally fine, thanks a lot! |
This is a breaking change, with this change whenever a source parameter is used as a source for a target property the condition has to apply to source parameters and not properties
Fixes #3601