[Product] Add a crossed price #3373

TristanPerchec · 2015-09-30T08:01:11Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?
Fixed tickets
License	MIT
Doc PR

Added a crossed price attribute on every variant.

It works as amazon system price :

the crossed one is overpriced
the real one is normal
Add Field in DB
Add Backend display and fonctions
Add Frontend display and fonctions
Add Behat test
Add Phpspec test

This development was sponsorised by Store Factory

peteward · 2015-10-01T03:46:08Z

Does 'crossed price' mean 'original price'? The naming feels a bit confusing

TristanPerchec · 2015-10-01T07:29:28Z

It means "overpriced" as amazon, banggood and others brands do. It's commonly used to convince customers it's a good deal. Sorry for the confusion I edited the description.

jimbocoder · 2015-10-01T17:17:51Z

I think the most common term, at least in my experience, is MSRP.

pjedrzejewski · 2015-10-01T18:05:03Z

Nice work Tristan! I'd like to hear more opinions about the naming. :)

peteward · 2015-10-03T17:12:22Z

src/Sylius/Bundle/WebBundle/Resources/views/Frontend/Product/_singleBox.html.twig

@@ -2,6 +2,9 @@
    <a href="{{ path(product) }}"><h4>{{ product.name|truncate(18) }}</h4></a>
    <a href="{{ path(product) }}">
        <img class="img-rounded img-responsive" src="{{ product.image ? product.image.path|imagine_filter('sylius_medium') : 'http://placehold.it/262x255' }}" alt="{{ product.name }}" />
+        {% if (product.masterVariant.crossedprice != 0) %}


I think the check here should be if crossedPrice > price, also I think this could be done in a macro as it's like you would want to display the price in this way in lots of places

I would put it inside the main span so that it could be used as a macro something like:

 {{ sylius_customer_product_price(variant) }} 

Again not sure about naming, could be formatted_product_price()?
Then your markup might look something like:

 €40€25 

Then you have something reusable and don't have to repeat the formatting that you've got below - do it once in the macro.

peteward · 2015-10-03T17:33:17Z

We have done something similar very similar in our project. We take this further with multiple PriceLists per product per channel. In this structure we have originalPrice and retailPrice. We have a very long discussion about names and we felt this was the best description.

From a native English-speakers perspective, 'crossed price' isn't a known term. I actually like it because it's not committing to anything specific, it's just describing that it's the crossed-out price on the page, but still not really known and obvious.

msrp isn't clear enough to read and manufacturers_suggested_retail_price is too long and too specific. Also rrp (recommended retal price) is another more common term that I'm aware of as well as list price. But what if you are the manufacturer in your application? It doesn't really make sense in every application.

So my vote would go for originalPrice over anything else.

aramalipoor · 2015-10-03T17:42:19Z

+1 for originalPrice

peteward · 2015-10-03T18:14:14Z

I would also look at logic on the ProductVariant such as isReduced() isDiscounted() or isMarkedDown() to handle the logic. This could also go into an Interface, although I'm not sure it necessarily belongs in the PriceableInterface...

TristanPerchec · 2015-10-05T10:33:18Z

Tank you all for your feedbacks. I updated my work and now crossed_price has been replaced by original_price as suggested.

michalmarcinkowski · 2015-10-05T11:27:28Z

src/Sylius/Bundle/CoreBundle/Resources/translations/messages.fr.yml

@@ -88,6 +88,7 @@ sylius:
      images: Images
      on_hand: Stock actuel
      price: Prix
+      original_price: Prix original


We should modify only en translations and then the translation process is handled on Crowdin (http://translate.sylius.org)

TristanPerchec · 2015-10-07T13:44:20Z

@peteward @jimbocoder @pjedrzejewski @aramalipoor @michalmarcinkowski Do you see anything to edit or improve ?

peteward · 2015-10-07T13:59:43Z

src/Sylius/Bundle/CurrencyBundle/Twig/CurrencyExtension.php

@@ -41,6 +41,7 @@ public function getFilters()
        return array(
            new \Twig_SimpleFilter('sylius_currency', array($this, 'convertAmount')),
            new \Twig_SimpleFilter('sylius_price', array($this, 'convertAndFormatAmount')),
+            new \Twig_SimpleFilter('sylius_original_price', array($this, 'convertAndFormatAmount')),


I would remove this and just use 'sylius_price' where you need to. I'm not sure that there's any point having 2 filters doing the same thing and it might even be misleading. I'm trying to think of a use-case for performing a different calculation on originalPrice compared to price and I can't think of one...

pjedrzejewski · 2015-10-19T21:13:40Z

Looks good to me, ready for merge when rebase and we get a green light from Travis! 👍 Nice work @TristanPerchec!

…d_original_price

TristanPerchec · 2015-10-20T14:42:53Z

I saw there's just one test that is not passing in travis. I'm pretty new to behat and i don't know if this error is linked to my PR. Can someone help me with this ?

pjedrzejewski · 2015-10-20T15:33:59Z

@TristanPerchec I quickly looked and not sure what's the problem, but our Travis scripts output failed scenarios to Termbin. You can have a look here - http://termbin.com/9158. Maybe it will give you some clue.

pamil · 2015-10-20T15:45:18Z

src/Sylius/Component/Core/Model/ProductVariant.php

+    }
+
+    /**
+     * {@inheritdoc}


There's no inherited doc for recently added method, these methods should be added to ProductVariantInterface.

pamil · 2015-10-20T15:58:28Z

@TristanPerchec it has something in common with channel-based pricing. If you take a look at

Sylius/features/frontend/products.feature

Line 22 in 177d653

    
                       | Super T-Shirt    | 19.99 | T-Shirts     | channel_based      | WEB-EU:15.99             |

, you will see that it returns "Super T-Shirt"'s regular price (€19.99) instead of that one returned by price calculator for WEB-EU channel which is €15.99.

Anyway, @Zales0123 it would be nicer and cleaner, if that assumption checked that "Super T-Shirt" has price "€15.99" instead of searching for "Super T-Shirt" and "€15.99" independently.

pamil · 2015-10-20T16:02:59Z

src/Sylius/Bundle/WebBundle/Resources/views/Frontend/Product/_singleBox.html.twig

 <div class="product-box">
    <a href="{{ path(product) }}"><h4>{{ product.name|truncate(18) }}</h4></a>
    <a href="{{ path(product) }}">
        <img class="img-rounded img-responsive" src="{{ product.image ? product.image.path|imagine_filter('sylius_medium') : 'http://placehold.it/262x255' }}" alt="{{ product.name }}" />
-        <span class="label label-primary">{{ sylius_calculate_price(product.masterVariant)|sylius_price }}</span>


According to your error, you do not call sylius_calculate_price() in your macro. Moreover, current isReduced method in ProductVariant isn't aware of price calculator, so those channel differences are not processed (and it may introduce some errors if for some channel price is higher than regular price).

Zales0123 · 2015-10-21T07:45:07Z

@pamil Sure, you're right! 👍 I will fix in the nearest future (at least I hope so ;)).

nakashu · 2015-11-25T11:20:23Z

@TristanPerchec any update on this ?

TristanPerchec · 2015-11-25T12:59:34Z

@nakashu Sorry got a lot of work lately. Maybe I will be able to work on it in the future but I can't right now.

pjedrzejewski · 2015-11-25T15:28:39Z

@TristanPerchec No worries, thanks for letting us know. Anyone willing to pick up this PR? :)

michalmarcinkowski · 2015-12-11T15:15:42Z

Related issue #1812

kwedrowicz · 2015-12-22T15:21:10Z

@pjedrzejewski I can pick up this PR if you want. I need this feature in a project at work so I have to write it anyways. Just tell me if I should work on core or extending model only for my own usages.

pjedrzejewski · 2015-12-22T15:47:08Z

@starspire Sounds great, please open a new PR on top of this branch. 👍 Thanks!

peteward · 2015-12-30T09:09:01Z

This one can be closed, well done @TristanPerchec on your foundation work here.

TristanPerchec · 2015-12-30T09:11:39Z

Well thanks to all of you for helping me on the work I did.

pjedrzejewski · 2015-12-30T09:25:11Z

Yes, thank you Tristan a lot! 👍

peteward reviewed Oct 3, 2015
View reviewed changes

michalmarcinkowski reviewed Oct 5, 2015
View reviewed changes

TristanPerchec force-pushed the feature/SF-186 branch 4 times, most recently from b3e2ed4 to 98340e2 Compare October 7, 2015 13:32

TristanPerchec changed the title ~~[WIP] [Product] Add a crossed price~~ [Product] Add a crossed price Oct 7, 2015

peteward reviewed Oct 7, 2015
View reviewed changes

TristanPerchec added 5 commits October 20, 2015 10:10

Added a field 'original_price'

1028a2e

Edited macro to include price and original price

cf664b4

Edited backend twigs to use the filter sylius_price instead of instea…

d5991d6

…d_original_price

Added a field 'original_price'

99eb65f

Edited macro to include price and original price

6bf861b

TristanPerchec force-pushed the feature/SF-186 branch from 059f95d to 6bf861b Compare October 20, 2015 08:14

pamil reviewed Oct 20, 2015
View reviewed changes

pjedrzejewski added the New Feature label Oct 20, 2015

pjedrzejewski added this to the v0.16.0 milestone Oct 20, 2015

pjedrzejewski removed this from the v0.16.0 milestone Dec 14, 2015

kwedrowicz mentioned this pull request Dec 28, 2015

[Product] added original price #3783

Merged

TristanPerchec closed this Dec 30, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Product] Add a crossed price #3373

[Product] Add a crossed price #3373

TristanPerchec commented Sep 30, 2015

peteward commented Oct 1, 2015

TristanPerchec commented Oct 1, 2015

jimbocoder commented Oct 1, 2015

pjedrzejewski commented Oct 1, 2015

peteward Oct 3, 2015

peteward Oct 3, 2015

peteward commented Oct 3, 2015

aramalipoor commented Oct 3, 2015

peteward commented Oct 3, 2015

TristanPerchec commented Oct 5, 2015

michalmarcinkowski Oct 5, 2015

TristanPerchec commented Oct 7, 2015

peteward Oct 7, 2015

pjedrzejewski commented Oct 19, 2015

TristanPerchec commented Oct 20, 2015

pjedrzejewski commented Oct 20, 2015

pamil Oct 20, 2015

pjedrzejewski Oct 20, 2015

pamil commented Oct 20, 2015

pamil Oct 20, 2015

Zales0123 commented Oct 21, 2015

nakashu commented Nov 25, 2015

TristanPerchec commented Nov 25, 2015

pjedrzejewski commented Nov 25, 2015

michalmarcinkowski commented Dec 11, 2015

kwedrowicz commented Dec 22, 2015

pjedrzejewski commented Dec 22, 2015

peteward commented Dec 30, 2015

TristanPerchec commented Dec 30, 2015

pjedrzejewski commented Dec 30, 2015

[Product] Add a crossed price #3373

[Product] Add a crossed price #3373

Conversation

TristanPerchec commented Sep 30, 2015

peteward commented Oct 1, 2015

TristanPerchec commented Oct 1, 2015

jimbocoder commented Oct 1, 2015

pjedrzejewski commented Oct 1, 2015

peteward Oct 3, 2015

Choose a reason for hiding this comment

peteward Oct 3, 2015

Choose a reason for hiding this comment

peteward commented Oct 3, 2015

aramalipoor commented Oct 3, 2015

peteward commented Oct 3, 2015

TristanPerchec commented Oct 5, 2015

michalmarcinkowski Oct 5, 2015

Choose a reason for hiding this comment

TristanPerchec commented Oct 7, 2015

peteward Oct 7, 2015

Choose a reason for hiding this comment

pjedrzejewski commented Oct 19, 2015

TristanPerchec commented Oct 20, 2015

pjedrzejewski commented Oct 20, 2015

pamil Oct 20, 2015

Choose a reason for hiding this comment

pjedrzejewski Oct 20, 2015

Choose a reason for hiding this comment

pamil commented Oct 20, 2015

pamil Oct 20, 2015

Choose a reason for hiding this comment

Zales0123 commented Oct 21, 2015

nakashu commented Nov 25, 2015

TristanPerchec commented Nov 25, 2015

pjedrzejewski commented Nov 25, 2015

michalmarcinkowski commented Dec 11, 2015

kwedrowicz commented Dec 22, 2015

pjedrzejewski commented Dec 22, 2015

peteward commented Dec 30, 2015

TristanPerchec commented Dec 30, 2015

pjedrzejewski commented Dec 30, 2015