[Currency] Refactoring of display currency feature #6820

lchrusciel · 2016-11-22T16:44:01Z

Q	A
Bug fix?	yes
New feature?	yes
BC breaks?	yes
Related tickets	partially #6641
License	MIT

michalmarcinkowski · 2016-11-24T06:42:46Z

.../customer_account/viewing_orders_history/viewing_order_recalculated_by_exchange_rate.feature

@@ -5,17 +5,16 @@ Feature: Viewing details of an order
    I want to be able to view details of my placed order


This feature isn't good, we should have a feature "Order is always placed in a base currency of a channel" and then in the scenario:

Browse shop in a different than base currency

Place order through UI

Verify than order amounts are in a base currency

We have such a scenario in checkout

michalmarcinkowski · 2016-11-24T06:45:06Z

features/cart/shopping_cart/currency_rounding_edge_case.feature

+        Given I have added 2 products "The Pug Mug" to the cart
+        When I switch to the "EUR" currency
+        Then grand total value should be "€13.63"
+        And grand total value in base currency should be "$14.00"


We should also assert the value of a single item, to show the rounding issue.

michalmarcinkowski · 2016-11-24T06:46:40Z

...ng_shipping_address/modifying_shipping_address_on_order_with_different_exchange_rate.feature

@@ -14,13 +14,12 @@ Feature: Modifying a customer's shipping address of an order with a different cu
        And the store has "DHL" shipping method with "$20.00" fee within the "EN" zone


This feature can be removed. We don't have exchange rate on the order anymore, and we always display the amounts from database in admin, so there is no possibility to break something when changing exchange rate.

michalmarcinkowski · 2016-11-24T06:49:21Z

features/order/managing_orders/seeing_orders_currency_on_index.feature

-        Then I should see the order "#00000666" with total "£40.00"
-
-    @ui
-    Scenario: Seeing multiple orders in different currencies


We should have such scenario, but use 2 channels with different base currencies.

michalmarcinkowski · 2016-11-24T06:55:37Z

src/Sylius/Bundle/ShopBundle/Resources/views/Account/Order/Show/_payments.html.twig

@@ -9,7 +10,7 @@
                    {{ payment.method }}
                </div>
                <p>
-                    {{ payment.amount|sylius_price(payment.order.currencyCode) }}
+                    {{ money.convertAndFormat(payment.amount) }}


Not sure if we should convert the value here, the customer will be charged in a base currency anyway /cc @pjedrzejewski

Of course not.

michalmarcinkowski · 2016-11-24T06:58:10Z

Looks good! 👍 Rebase needed.

NoResponseMate · 2016-11-24T09:44:57Z

features/cart/shopping_cart/changing_dispalay_currency_of_cart.feature

    As a Visitor
-    I want to see every cash amount in my chosen currency
+    I want to see rounded every cash amount in my chosen currency


Changing display currency of the cart
and
I want to see every cash amount rounded to my chosen currency
or something alike

NoResponseMate · 2016-11-24T09:46:21Z

features/cart/shopping_cart/currency_rounding_edge_case.feature

+    Scenario: Changing the currency of my cart
+        Given I have product "The Pug Mug" in the cart
+        When I switch to the "EUR" currency
+        Then grand total value should be "€6.82"


the grand total... same below

NoResponseMate · 2016-11-24T09:51:38Z

src/Sylius/Behat/Context/Ui/Shop/CartContext.php

+     */
+    public function myBaseCartTotalShouldBe($total)
+    {
+        $this->summaryPage->open();


add an empty line below

michalmarcinkowski · 2016-11-24T17:26:22Z

Rebase needed :)

lchrusciel · 2016-11-24T21:08:35Z

@michalmarcinkowski done

michalmarcinkowski · 2016-11-25T07:08:47Z

...ccount/viewing_orders_history/order_is_always_placed_in_a_base_currency_of_a_channel.feature

+        And that channel allows to shop using the "CAD" currency with exchange rate 3.0
+        And the store ships to "United States"
+        And the store has a zone "United States" with code "US"
+        And this zone has the "United States" country member


We should use DefaultUnitedStatesChannelFactory and then the 3 steps above won't be necessary.

This test is turned of, it can be adjusted later on.

michalmarcinkowski · 2016-11-25T07:09:10Z

...ccount/viewing_orders_history/order_is_always_placed_in_a_base_currency_of_a_channel.feature

+        And the store has a product "Angel T-Shirt" priced at "$20.00"
+        And I am a logged in customer
+
+#   This test should pass, but refactoring of context would be needed to fulfill requirements. Because refactoring is postponed this test will be turned on later


What is the issue?

I need to process whole checkout and then access a show page of an order. Both of them have specifyEmail method. There is no point in creating order programmatically.

michalmarcinkowski · 2016-11-25T07:18:12Z

src/Sylius/Bundle/ShopBundle/Resources/views/Common/Macro/money.html.twig

+{% macro convertAndFormat(amount) %}
+    {% import _self as macro %}
+
+    {{ macro.format(amount|sylius_convert_money(shopperContext.currencyCode), shopperContext.currencyCode) }}


We should use shopperContext.channel.baseCurrency as a base currency.

Already done

michalmarcinkowski · 2016-11-25T07:24:05Z

Great work! Please apply my comments in a separate PR.

lchrusciel mentioned this pull request Nov 22, 2016

[WIP] [RFC] Rounding issue reproduction #6730

Closed

michalmarcinkowski added BC Break PRs introducing BC breaks (do not even try to merge). Bug Fix labels Nov 22, 2016

lchrusciel changed the title ~~[WIP] [Currency] Exchange rate can into shop~~ [WIP] [Currency] Refactoring of display currency feature Nov 22, 2016

lchrusciel force-pushed the exchange-rate-can-into-shop branch 2 times, most recently from ddccf91 to 5250160 Compare November 23, 2016 10:35

pjedrzejewski force-pushed the currency-party branch from 2b9c5e9 to 15e1c83 Compare November 23, 2016 15:26

lchrusciel force-pushed the exchange-rate-can-into-shop branch from 5250160 to b0189e3 Compare November 23, 2016 21:51

lchrusciel changed the title ~~[WIP] [Currency] Refactoring of display currency feature~~ [Currency] Refactoring of display currency feature Nov 23, 2016

michalmarcinkowski reviewed Nov 24, 2016

View reviewed changes

lchrusciel force-pushed the exchange-rate-can-into-shop branch 2 times, most recently from 26f1ea5 to 92f8b3b Compare November 24, 2016 09:37

NoResponseMate reviewed Nov 24, 2016

View reviewed changes

lchrusciel force-pushed the exchange-rate-can-into-shop branch 3 times, most recently from e26edcd to 6f39136 Compare November 24, 2016 13:37

lchrusciel mentioned this pull request Nov 24, 2016

[Currency] Exchange rate refactor #6846

Merged

lchrusciel added 2 commits November 24, 2016 19:41

Multi display currencies scenarios

6238103

[Behat] Implement currency scenarios

0a0c852

lchrusciel force-pushed the exchange-rate-can-into-shop branch from 6f39136 to bcb366b Compare November 24, 2016 18:43

[Twig] Refactor of money filters

7034212

lchrusciel added 3 commits November 24, 2016 19:59

[Twig] Introduce money macro

7227f53

[Twig] Adjust twig templates

6364c4a

[Admin] Add currency to order grid

00c8fce

lchrusciel force-pushed the exchange-rate-can-into-shop branch from bcb366b to 00c8fce Compare November 24, 2016 18:59

[Pricing] Channel based price helper not price aware

95acaa6

lchrusciel force-pushed the exchange-rate-can-into-shop branch from 315b038 to 95acaa6 Compare November 24, 2016 20:12

michalmarcinkowski reviewed Nov 25, 2016

View reviewed changes

michalmarcinkowski merged commit c61ae06 into Sylius:currency-party Nov 25, 2016

lchrusciel deleted the exchange-rate-can-into-shop branch November 25, 2016 07:33

lchrusciel mentioned this pull request Nov 25, 2016

[Currency] [RFC] [Critical] Rebased currency party #6868

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Currency] Refactoring of display currency feature #6820

[Currency] Refactoring of display currency feature #6820

lchrusciel commented Nov 22, 2016

michalmarcinkowski Nov 24, 2016 •

edited

Loading

lchrusciel Nov 24, 2016

michalmarcinkowski Nov 24, 2016

michalmarcinkowski Nov 24, 2016 •

edited

Loading

michalmarcinkowski Nov 24, 2016

lchrusciel Nov 24, 2016

michalmarcinkowski Nov 24, 2016

lchrusciel Nov 24, 2016

michalmarcinkowski commented Nov 24, 2016

NoResponseMate Nov 24, 2016

NoResponseMate Nov 24, 2016

NoResponseMate Nov 24, 2016

michalmarcinkowski commented Nov 24, 2016

lchrusciel commented Nov 24, 2016

michalmarcinkowski Nov 25, 2016

lchrusciel Nov 25, 2016

michalmarcinkowski Nov 25, 2016

lchrusciel Nov 25, 2016

michalmarcinkowski Nov 25, 2016

lchrusciel Nov 25, 2016

michalmarcinkowski commented Nov 25, 2016

		@@ -5,17 +5,16 @@ Feature: Viewing details of an order
		I want to be able to view details of my placed order

		@@ -14,13 +14,12 @@ Feature: Modifying a customer's shipping address of an order with a different cu
		And the store has "DHL" shipping method with "$20.00" fee within the "EN" zone

[Currency] Refactoring of display currency feature #6820

[Currency] Refactoring of display currency feature #6820

Conversation

lchrusciel commented Nov 22, 2016

michalmarcinkowski Nov 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalmarcinkowski Nov 24, 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

michalmarcinkowski commented Nov 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalmarcinkowski commented Nov 24, 2016

lchrusciel commented Nov 24, 2016

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 Nov 25, 2016

michalmarcinkowski Nov 24, 2016 •

edited

Loading

michalmarcinkowski Nov 24, 2016 •

edited

Loading