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.

Sign up for GitHub

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

Jump to bottom

[ChannelPricing] Ability to remove price on channel pricing when channel is disabled #11820

Merged
merged 3 commits into from
Sep 9, 2020
Merged

[ChannelPricing] Ability to remove price on channel pricing when channel is disabled #11820

merged 3 commits into from
Sep 9, 2020

Conversation

Tomanhez
Copy link
Contributor

@Tomanhez Tomanhez commented Sep 8, 2020 •
edited
Loading

Q A
Branch? 1.8
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
License MIT

@Tomanhez Tomanhez requested a review from a team as a code owner September 8, 2020 12:02
@Tomanhez Tomanhez mentioned this pull request Sep 8, 2020
Closed
@Tomanhez Tomanhez force-pushed the remove-channel-pricing branch from 529a09d to eeba2a8 Compare September 8, 2020 12:05
lchrusciel
lchrusciel reviewed Sep 8, 2020
View reviewed changes
src/Sylius/Bundle/CoreBundle/Form/Type/Product/ChannelPricingType.php
/** @var ChannelInterface $channel */
$channel = $options['channel'];

if (!$channel->isEnabled() && $channelPricing->getPrice() === null) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we should cover the original price as well in this condition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is next bug, I will open new PR with next scenario to cover it

src/Sylius/Bundle/CoreBundle/Form/Type/Product/ChannelPricingType.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Ui/Admin/ManagingProductsContext.php Outdated Show resolved Hide resolved
@Tomanhez Tomanhez force-pushed the remove-channel-pricing branch from 0add6e1 to 547dedd Compare September 9, 2020 05:33
@Tomanhez Tomanhez force-pushed the remove-channel-pricing branch 3 times, most recently from ac5a27d to 73ca063 Compare September 9, 2020 08:28
@Tomanhez Tomanhez force-pushed the remove-channel-pricing branch from 73ca063 to 56b55d5 Compare September 9, 2020 13:59
@Tomanhez Tomanhez force-pushed the remove-channel-pricing branch from 56b55d5 to b116c7c Compare September 9, 2020 17:37
GSadee
GSadee approved these changes Sep 9, 2020
View reviewed changes
...oduct/managing_product_variants/removing_product_variant_price_from_disabled_channel.feature

Background:
Given the store operates on a channel named "Web-US" in "USD" currency
And the store operates on another channel named "Web-GB" in "USD" currency
Copy link
Member

Choose a reason for hiding this comment

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

Why not?

Suggested change
And the store operates on another channel named "Web-GB" in "USD" currency
And the store operates on another channel named "Web-GB" in "GBP" currency

src/Sylius/Behat/Context/Ui/Admin/ManagingProductsContext.php
@@ -855,6 +855,17 @@ public function theProductShouldNotHaveAnAssociationWithProduct(
Assert::false($this->updateSimpleProductPage->hasAssociatedProduct($productName, $productAssociationType));
}

/**
* @Then I should be notified that original price can not be defined without price
Copy link
Member

Choose a reason for hiding this comment

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

I guess it should be removed from this PR

@GSadee GSadee merged commit 54a5840 into Sylius:1.8 Sep 9, 2020
@GSadee
Copy link
Member

GSadee commented Sep 9, 2020

Thanks, Tomasz! 🎉

@Tomanhez Tomanhez mentioned this pull request Sep 11, 2020
Merged
GSadee added a commit that referenced this pull request Sep 14, 2020
@GSadee
bug #11835 [ChannelPricing] Remove channel pricing 1.7 (AdamKasp, Tom…
cb4a0d4 
…anhez, lchrusciel)

This PR was merged into the 1.7 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.7
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| License         | MIT

Down merge for : #11820

<!--
 - Bug fixes must be submitted against the 1.7 branch (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

8b3c8f6 [Behat] removing product price for disabled channel
b376ec1 Fix channel pricing type
e465dc8 Fixes for tests and FormType
8bcc4a5 [Admin] Channel pricing removal for channels where the product is not available
6a4eba6 Fixes for tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lchrusciel lchrusciel lchrusciel left review comments

@GSadee GSadee GSadee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Tomanhez @GSadee @lchrusciel @AdamKasp