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

[API] add original price to product variant #12877

Merged
merged 3 commits into from Aug 12, 2021

Conversation

SirDomin
Copy link
Contributor

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

@SirDomin SirDomin requested a review from a team as a code owner August 10, 2021 22:03
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Aug 10, 2021
@AdamKasp AdamKasp merged commit 0a5872c into Sylius:master Aug 12, 2021
@AdamKasp
Copy link
Contributor

Thanks, @SirDomin! 🥇

@@ -343,6 +366,17 @@ private function hasProductWithName(array $products, string $name): bool
return false;
}

private function hasProductWithOriginalPrice(array $products, int $originalPrice): bool
{
foreach ($products as $product) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have an array of products here? Also, this loop just checks if there is any product with original_price, not the particular, which seems bad

AdamKasp added a commit that referenced this pull request Aug 13, 2021
This PR was merged into the 1.11-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         |  master 
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | fixes for #12877
| License         | MIT

! MERGE #12883 FIRST

Commits
-------

d88eed4 Change the behat implementation to be more strict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants