[Shop] Different variant prices on product details page #6161

Zales0123 · 2016-09-20T08:50:01Z

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

On product details page, we have always one price displayed (from the first/default/etc variant). It's obviously confusing, as we could have different price for each variant - therefore it should change after changing variant/option. In this PR, I will implement some lovely JS script, to handle this logic and make Sylius product details page more and more perfect, to reach the sky and more with its perfectness 🌠 🎉

pamil · 2016-09-20T13:27:37Z

...ures/product/viewing_products/viewing_different_price_for_different_product_variants.feature

+        Given the store operates on a single channel in "United States"
+        And the store has a "Wyborowa Vodka" configurable product
+        And the product "Wyborowa Vodka" has "Wyborowa Vodka Exquisite" variant priced at "$40.00"
+        And the product "Wyborowa Vodka" has "Wyborowa Apple" variant priced at "$4.00"


Their prices differ by just one zero, can we use some other numbers than 4 and 0 to make the difference more clear?

pamil · 2016-09-20T13:41:13Z

src/Sylius/Bundle/CoreBundle/Controller/ProductController.php

+     *
+     * @return View
+     */
+    protected function prepareHtmlRequestView(ProductInterface $resource, RequestConfiguration $configuration, View $view)


$resource -> $product

pamil · 2016-09-20T13:44:31Z

src/Sylius/Bundle/CoreBundle/Controller/ProductController.php

+        ];
+
+        if (
+            !$resource->isSimple() &&


Creating $product->isConfigurable() method which is a shortcut for !$product->isSimple() would improve readability a lot.

lchrusciel · 2016-09-20T13:41:11Z

src/Sylius/Bundle/CoreBundle/Controller/ProductController.php

+
+        $this->isGrantedOr403($configuration, ResourceActions::SHOW);
+        /** @var ProductInterface $resource */
+        $resource = $this->findOr404($configuration);


If we typehinting $resource anyway, can we change a name of variable?

lchrusciel · 2016-09-20T13:45:05Z

src/Sylius/Bundle/ShopBundle/Resources/private/js/variants-prices.js

+        $selector = '';
+
+        $('#sylius-product-adding-to-cart select').each(function() {
+            $selector += '[data-'+$(this).attr('data-option')+'="'+$(this).find('option:selected').text()+'"]';


Missing spaces before and after each '+'

lchrusciel · 2016-09-20T13:49:17Z

src/Sylius/Behat/Context/Setup/ProductContext.php

@@ -153,6 +153,11 @@ public function storeHasAConfigurableProduct($productName)
        $product->setCode($this->convertToCode($productName));
        $product->setDescription('Awesome '.$productName);

+        if ($this->sharedStorage->has('channel')) {
+            $channel = $this->sharedStorage->get('channel');


$channel is not used anywhere else. $product->addChannel($this->sharedStorage->get('channel'));

It is still present.

Arminek · 2016-09-20T13:48:40Z

src/Sylius/Component/Core/Provider/ProductVariantsPricesProvider.php

+/**
+ * @author Mateusz Zalewski <mateusz.zalewski@lakion.com>
+ */
+class ProductVariantsPricesProvider implements ProductVariantsPricesProviderInterface


michalmarcinkowski · 2016-09-23T13:37:22Z

...ures/product/viewing_products/viewing_different_price_for_different_product_variants.feature

+
+    @ui
+    Scenario: Viewing a detailed page with default variant's price
+        When I check product "Wyborowa Vodka" details


When I view product "Wyborowa Vodka"

michalmarcinkowski · 2016-09-23T13:37:56Z

...ures/product/viewing_products/viewing_different_price_for_different_product_variants.feature

+    Scenario: Viewing a detailed page with product's price for different variant
+        When I check product "Wyborowa Vodka" details
+        And I select "Wyborowa Apple" variant
+        Then I should see the product price "$12.55"


Then the product price should be "$12.55"

michalmarcinkowski · 2016-09-23T13:54:39Z

We need one more Behat scenario:

Give the store has a "Wyborowa Vodka" configurable product
And this product has option "Volume" with values "0.5L", "0.7L" and "1L"
And this product is available in "0.5L" volume priced at "$20.00"
And this product is available in "0.7L" volume priced at "$25.00"
When I view this product
And I select "1L" option
Then the product price should be "unavailable"

But this can be implemented in a separate PR.

michalmarcinkowski · 2016-09-23T14:58:08Z

@Zales0123 one last fix required to make it green.

Zales0123 · 2016-09-23T20:09:38Z

@michalmarcinkowski ready to go ;)

michalmarcinkowski · 2016-09-25T20:30:41Z

src/Sylius/Component/Core/Provider/ProductVariantsPricesProvider.php

+
+        /** @var OptionValueInterface $option */
+        foreach ($variant->getOptions() as $option) {
+            $optionMap[$option->getOption()->getCode()] = $option->getValue();


We can use $option->getOptionCode()

pjedrzejewski

We definitely should not override an entire controller and add protected methods to obtain this prices. You already have all the info (product + variants), so I'd say that Twig function that takes product as argument and returns array of prices would be a cleaner solution. We need it for the template, so overriding entire controller and processing the data you already have is not good. What do you think?

sylius_product_variant_prices(product)

Or something like that?

Zales0123 · 2016-09-29T07:19:26Z

@pjedrzejewski seems reasonable for me 👍

lchrusciel · 2016-09-30T06:56:28Z

...ures/product/viewing_products/viewing_different_price_for_different_product_variants.feature

+Feature: Viewing different price for different product variants
+    In order to see product variant price
+    As a Visitor
+    I want to be able to see proper price for each product variant


I want to be able to see a proper price for each product variant

lchrusciel · 2016-09-30T06:57:59Z

src/Sylius/Behat/Context/Setup/ProductContext.php

@@ -153,6 +153,11 @@ public function storeHasAConfigurableProduct($productName)
        $product->setCode($this->convertToCode($productName));
        $product->setDescription('Awesome '.$productName);

+        if ($this->sharedStorage->has('channel')) {
+            $channel = $this->sharedStorage->get('channel');


It is still present.

lchrusciel · 2016-09-30T07:02:59Z

src/Sylius/Bundle/CoreBundle/Twig/ProductVariantsPricesExtension.php

+    public function getFunctions()
+    {
+        return [
+            new \Twig_SimpleFunction('sylius_product_variant_prices', [$this, 'getVariantsPrices']),


Won't it be simpler to use productVariantsPricesHelper directly?
new \Twig_SimpleFunction('sylius_product_variant_prices', [$this->productVariantsPricesHelper, 'getPrices'])

pjedrzejewski · 2016-09-30T07:15:22Z

Thanks Mateusz! :) Nice work!

Zales0123 · 2016-09-30T07:17:33Z

👍 I will apply last comments in additional PR in a minute.

…n-product-details [Shop] Different variant prices on product details page

Zales0123 changed the title ~~[WIP][Behat][Shop] Different product variant prices scenario~~ [WIP][Behat][Shop] Different variant prices on product details page Sep 20, 2016

Zales0123 changed the title ~~[WIP][Behat][Shop] Different variant prices on product details page~~ [WIP][Shop] Different variant prices on product details page Sep 20, 2016

Zales0123 force-pushed the display-variant-price-on-product-details branch from 0b0af6e to d63eac1 Compare September 20, 2016 11:52

Zales0123 changed the title ~~[WIP][Shop] Different variant prices on product details page~~ [Shop] Different variant prices on product details page Sep 20, 2016

pamil reviewed Sep 20, 2016

View reviewed changes

lchrusciel requested changes Sep 20, 2016

View reviewed changes

Arminek approved these changes Sep 20, 2016

View reviewed changes

Zales0123 mentioned this pull request Sep 20, 2016

[RFC][Resource] Make Resource actions easier to overwrite #6165

Closed

Zales0123 force-pushed the display-variant-price-on-product-details branch 2 times, most recently from 3a8bc96 to 55793eb Compare September 22, 2016 07:45

michalmarcinkowski added New Feature Shop ShopBundle related issues and PRs. labels Sep 22, 2016

Zales0123 force-pushed the display-variant-price-on-product-details branch from 55793eb to 8230542 Compare September 23, 2016 12:57

michalmarcinkowski reviewed Sep 23, 2016

View reviewed changes

Zales0123 force-pushed the display-variant-price-on-product-details branch 2 times, most recently from 3520596 to 171918a Compare September 23, 2016 13:51

Zales0123 force-pushed the display-variant-price-on-product-details branch from 171918a to eb82868 Compare September 23, 2016 14:14

Zales0123 force-pushed the display-variant-price-on-product-details branch from eb82868 to 3a3fe90 Compare September 23, 2016 16:50

michalmarcinkowski reviewed Sep 25, 2016

View reviewed changes

pjedrzejewski requested changes Sep 27, 2016

View reviewed changes

Zales0123 added 5 commits September 30, 2016 08:11

[Behat][Shop] Different product variant prices scenario

8392f53

[Core][Shop] Change product price on options change

f7baf9e

[Shop] Extract variants prices script to external file

9d15d87

[Core] Extract service for getting options and prices map

d6b60f5

[Shop][Behat] Changing price after variant selection

5f76657

Zales0123 added 3 commits September 30, 2016 08:11

PR review fixes

ab70385

[Core] Implement twig function for variants prices map

fafa09e

[Core] Remove unneeded ProductController

1fce4f9

Zales0123 force-pushed the display-variant-price-on-product-details branch from e739c30 to 1fce4f9 Compare September 30, 2016 06:20

lchrusciel requested changes Sep 30, 2016

View reviewed changes

pjedrzejewski approved these changes Sep 30, 2016

View reviewed changes

pjedrzejewski merged commit 81c7687 into Sylius:master Sep 30, 2016

Zales0123 mentioned this pull request Sep 30, 2016

[Behat] Display variant price on product details improvements #6257

Merged

Zales0123 deleted the display-variant-price-on-product-details branch October 6, 2016 20:32

michalmarcinkowski mentioned this pull request Oct 9, 2016

No price shown for products with variant selection method "options matching" #1710

Closed

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019

Merge pull request Sylius#6161 from Zales0123/display-variant-price-o…

b8626a1

…n-product-details [Shop] Different variant prices on product details page

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Shop] Different variant prices on product details page #6161

[Shop] Different variant prices on product details page #6161

Zales0123 commented Sep 20, 2016 •

edited

Loading

pamil Sep 20, 2016

pamil Sep 20, 2016

pamil Sep 20, 2016

lchrusciel Sep 20, 2016

lchrusciel Sep 20, 2016

lchrusciel Sep 20, 2016

lchrusciel Sep 30, 2016

Arminek Sep 20, 2016

michalmarcinkowski Sep 23, 2016

michalmarcinkowski Sep 23, 2016

michalmarcinkowski commented Sep 23, 2016

michalmarcinkowski commented Sep 23, 2016

Zales0123 commented Sep 23, 2016

michalmarcinkowski Sep 25, 2016

pjedrzejewski left a comment

Zales0123 commented Sep 29, 2016

lchrusciel Sep 30, 2016

lchrusciel Sep 30, 2016

lchrusciel Sep 30, 2016

pjedrzejewski commented Sep 30, 2016

Zales0123 commented Sep 30, 2016

[Shop] Different variant prices on product details page #6161

[Shop] Different variant prices on product details page #6161

Conversation

Zales0123 commented Sep 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalmarcinkowski commented Sep 23, 2016

michalmarcinkowski commented Sep 23, 2016

Zales0123 commented Sep 23, 2016

Choose a reason for hiding this comment

pjedrzejewski left a comment

Choose a reason for hiding this comment

Zales0123 commented Sep 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjedrzejewski commented Sep 30, 2016

Zales0123 commented Sep 30, 2016

Zales0123 commented Sep 20, 2016 •

edited

Loading