Skip to content
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

[Taxation] Add validation of negative tax rate #13731

Merged
merged 7 commits into from
Mar 10, 2022

Conversation

coldic3
Copy link
Contributor

@coldic3 coldic3 commented Mar 7, 2022

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

@coldic3 coldic3 requested a review from a team as a code owner March 7, 2022 12:36
@coldic3 coldic3 changed the title [Taxation] Add validation of negative tax rate [WIP][Taxation] Add validation of negative tax rate Mar 7, 2022
@coldic3 coldic3 changed the title [WIP][Taxation] Add validation of negative tax rate [Taxation] Add validation of negative tax rate Mar 7, 2022
@Zales0123 Zales0123 added the Bug Confirmed bugs or bugfixes. label Mar 7, 2022
@coldic3 coldic3 force-pushed the feature/tax-rate-without-negative-amount branch from e868b40 to 64d7919 Compare March 7, 2022 18:05
@GSadee GSadee changed the base branch from master to 1.10 March 7, 2022 19:51
@GSadee
Copy link
Member

GSadee commented Mar 7, 2022

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "feature/tax-rate-without-negative-amount" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@GSadee GSadee force-pushed the feature/tax-rate-without-negative-amount branch from 64d7919 to dc727c5 Compare March 7, 2022 19:51
@coldic3 coldic3 force-pushed the feature/tax-rate-without-negative-amount branch from 2231002 to 92d3561 Compare March 10, 2022 13:11
@GSadee GSadee force-pushed the feature/tax-rate-without-negative-amount branch from 8c6d34d to 523a4ea Compare March 10, 2022 20:37
@GSadee GSadee merged commit 0ae681c into Sylius:1.10 Mar 10, 2022
@GSadee
Copy link
Member

GSadee commented Mar 10, 2022

Thank you, Kevin! 🥇

@coldic3 coldic3 deleted the feature/tax-rate-without-negative-amount branch March 11, 2022 13:47
lchrusciel added a commit that referenced this pull request Mar 14, 2022
…ic3)

This PR was merged into the 1.10 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.10
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | mentioned in #13731 (comment)
| License         | MIT

As far as "I want to..." statement is a kind of user action the step should be 'When'.

Commits
-------

984488d [Behat] Use "When" for user actions where possible
@bashilbers
Copy link
Contributor

Guys, why not use https://symfony.com/doc/current/reference/constraints/PositiveOrZero.html. In many cases 0% is also a perfectly fine tax percentage. This is now no longer possible because the Positive constraint does not allow 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bugs or bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants