fix: corrected the validation condition #6556
Conversation
🦋 Changeset detectedLatest commit: 83b158a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
d44f7a0 to
fd38512
Compare
|
@shwetha263 Thanks for the fix. When we fix an issue like this I would really like to see a test covering it. |
a3e83d8 to
41a379b
Compare
|
@vanpho93 @zenweasel : I added test case for create surcharges with amount 0 and undefined. |
|
@shwetha263 Great, thanks so much. Please remember to add a changeset as well to bump the package version. https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md |
apps/reaction/tests/integration/api/mutations/surcharges/createSurcharge.test.js
Show resolved
Hide resolved
apps/reaction/tests/integration/api/mutations/surcharges/createSurcharge.test.js
Outdated
Show resolved
Hide resolved
|
@shwetha263 also linting is failing now |
41a379b to
6147364
Compare
|
|
@shwetha263 Changeset being the last thing we need to bump the package https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md |
Added the changeset with the summary. Let me know if i need to add more details. |
| "reaction": major | ||
| "@reactioncommerce/api-plugin-surcharges": major |
There was a problem hiding this comment.
@zenweasel
Do you think these are correct bumps?
There was a problem hiding this comment.
No, a bug fix would just be a patch change.
bee49fb to
a515479
Compare
|
@shwetha263 Thanks so much for this. Can you correct the changeset to just be a patch change? The semver standard would define a backwards compatible bug fix as just a patch change. Thanks again |
a515479 to
0064f15
Compare
|
Hi @zenweasel |
if the amount was zero then node.amount would return false, hence changed the check to check for undefined. Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
Test case added while creating the surcharge with amount 0 and undefined Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
Signed-off-by: Shwetha Shashidhara <shwethaks263@gmail.com>
0064f15 to
83b158a
Compare
if the amount was zero then node.amount would return false, hence changed the check to check for undefined.
Resolves #6507
Impact: minor
Type: bugfix
Issue
Conditional check was incorrect for the resolver. if the amount is 0 then the condition was resolved to false. iif the amount is undefined the condition has to be evaluated to false else for "0" amount also the condition must be evaluated as true.
Solution
Checking if the amount is not undefined and not empty.
Breaking changes
None
Testing
Tested the condition.
More detail for what each of these sections should include are available in our Contributing Docs. This project uses semantic-release, please use their commit message format..