[Backport 2.2] Translate order getCreatedAtFormatted() to store locale #11422

JeroenVanLeusden · 2017-10-13T11:47:25Z

Description

Backport of version of #10491.

Add getDefaultStoreLocale() to allow fetching scoped values. Use this in getCreatedAtFormatted() so created_at date of order will be translated in emails to locale being used in that store view.

Fixed Issues (if relevant)

None that I could find.

Manual testing scenarios

Set default locale of your store to any language but English.
Create order and send confirmation email.

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

dmanners · 2017-10-13T12:53:37Z

app/code/Magento/Sales/Model/Order.php

@@ -213,6 +215,11 @@ class Order extends AbstractModel implements EntityInterface, OrderInterface
    protected $_currencyFactory;

    /**
+     * @var \Magento\Eav\Model\Config
+     */
+    private $_eavConfig;


If this is a new variable you do not need the _ at the beginning.

It isn't a new variable, just a missing phpdoc.

In version 2.2-develop of this file there is no such variable. https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Sales/Model/Order.php It seems be have been added to develop in you original PR. I wonder if it is a left over from when the file was created. I will see what I can find out.

@dmanners I remember, variable was already there but never declared as a property.

https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Sales/Model/Order.php#L338

Yup seems to be. My guess is we can remove the property and the mapping but leave the injection for BiC reasons. But I will dig a bit deeper to make sure.

Sure, ping me if you need any changes done in this PR.

Yup so this looks like a left over from the M1 approach of doing things. I cannot find a usage in this class and in the history it was last changed 4 years ago. I would suggest you can remove the new property, remove the assignment in the constructor $this->_eav and add the tag @SuppressWarnings(PHPMD.UnusedFormalParameter) to the constructor phpdocs.

dmanners · 2017-10-13T13:30:03Z

app/code/Magento/Sales/Model/Order.php

@@ -213,6 +215,11 @@ class Order extends AbstractModel implements EntityInterface, OrderInterface
    protected $_currencyFactory;

    /**
+     * @var \Magento\Eav\Model\Config
+     */
+    private $_eavConfig;


Yup so this looks like a left over from the M1 approach of doing things. I cannot find a usage in this class and in the history it was last changed 4 years ago. I would suggest you can remove the new property, remove the assignment in the constructor $this->_eav and add the tag @SuppressWarnings(PHPMD.UnusedFormalParameter) to the constructor phpdocs.

JeroenVanLeusden · 2017-10-13T13:39:35Z

@dmanners I removed the variable and added the @SuppressWarnings(PHPMD.UnusedFormalParameter)

JeroenVanLeusden · 2017-10-17T16:02:31Z

@dmanners Can you proceed merging? Would love to see this released in 2.2.2

dmanners · 2017-10-24T08:03:01Z

app/code/Magento/Sales/Model/Order.php

@@ -310,7 +318,6 @@ public function __construct(
        \Magento\Catalog\Model\Product\Visibility $productVisibility,
        \Magento\Sales\Api\InvoiceManagementInterface $invoiceManagement,
        \Magento\Directory\Model\CurrencyFactory $currencyFactory,
-        \Magento\Eav\Model\Config $eavConfig,


you cannot remove the injection via the __constuct without this breaking BC but you can remove the setting of $this->_eavConfig = $eavConfig; which you have done. If you add back this to the construct and docblock the rest seems fine to me

dmanners · 2017-11-10T16:19:56Z

@JeroenVanLeusden while testing this our team found the following.

I couldn't see what was fixed here.
Here are the steps I used:
1. Install German thought console:
composer require splendidinternet/mage2-locale-de-de
php bin/magento setup:upgrade
php bin/magento cron:run && php bin/magento indexer:reindex && php bin/magento cache:clean
2. Go to Stores->Configurations->General->Locale Otions->Locale-> German (Germany)
3. Create a product, open storefront and order it as registered customer
4. Check Email of the customer
Expected result: 
Date of order will be translated in emails to locale being used in that store view
Actual result:
There is no difference in Emails

It seems that in the testing process both the branch for this PR and the 2.2-develop branch had the same date information in the resulting emails. Are there any special test cases that should be taken into account?

JeroenVanLeusden · 2017-11-13T21:09:26Z

@dmanners Will find some time this week to check this. Will ping you with the results!

dmanners · 2017-11-15T10:44:16Z

Thanks @JeroenVanLeusden if you get stuck and need some help feel free to drop me a message here on in the engcom slack.

JeroenVanLeusden · 2017-11-18T10:39:17Z

@dmanners I've reproduced using the following steps:

Install NL language pack, composer require honl/magento2-nl-nl:^1@dev.
Set store language to Dutch (NL_nl).
Create order and send confirmation email.
Without this patch the date is printed as Geplaatst op 2017 M10 11 19:56:05 GMT+2 (http://cloud.h-o.nl/nmts).
With this patch the date is printed as Geplaatst op 11 oktober 2017 19:56:05 CEST (http://cloud.h-o.nl/nn0T).

Using the DE language pack will give me the same results. http://cloud.h-o.nl/nmZE and with patch http://cloud.h-o.nl/nnBu

This is tested from commit d2cbddd

dmanners · 2017-11-20T08:26:18Z

Thank you for the update @JeroenVanLeusden will keep you updated on the progress.

JeroenVanLeusden · 2017-11-20T22:42:25Z

@dmanners I've been able to reproduce it using the following steps:

Clean install from 2.2-develop branch, commit 2b78994.
Install NL language pack, composer require honl/magento2-nl-nl:^1@dev.
Set store language to Dutch (NL_nl).
Create order and send confirmation email (date is translated correctly).
Login the backend and open new order and press Send Email to send the email again.
Date is now formatted using US locale.

… to store locale #11422

[EngCom] Public Pull Requests - 2.2-develop - MAGETWO-83552: save invoice ID on credit memo when using API method salesRefundInvoiceV1 #11670 - MAGETWO-82577: [Backport 2.2] Translate order getCreatedAtFormatted() to store locale #11422 - MAGETWO-84474: 10128: New Orders not being saved to order grid #12241 - MAGETWO-83783: Shipping method fixtures not compatible with getShippingMethod(true) in OrderCreateTest #12227 - MAGETWO-83290: Add swatch option: Prevent loosing data and default value if data is not populated via adminhtml #12036 - MAGETWO-83741: 11740: Sending emails from Admin in Multi-Store Environment defaults to Primary Store #11992 - MAGETWO-83399: Fix for remove 'product_list_toolbar' block from layout in XML #9413 #11473

Translate order getCreatedAtFormatted() to store locale

9d0befb

JeroenVanLeusden changed the title ~~[Backport] Translate order getCreatedAtFormatted() to store locale~~ [Backport 2.2] Translate order getCreatedAtFormatted() to store locale Oct 13, 2017

dmanners reviewed Oct 13, 2017

View reviewed changes

dmanners suggested changes Oct 13, 2017

View reviewed changes

vkublytskyi added the partner contribution label Oct 19, 2017

dmanners self-assigned this Oct 24, 2017

dmanners added this to the October 2017 milestone Oct 24, 2017

dmanners added the Release Line: 2.2 label Oct 24, 2017

dmanners suggested changes Oct 24, 2017

View reviewed changes

Remove unused eavConfig in Order Model

02bc892

JeroenVanLeusden force-pushed the patch-4 branch from 2559e0c to 02bc892 Compare October 24, 2017 08:09

magento-team added the Progress: needs update label Oct 24, 2017

dmanners approved these changes Oct 24, 2017

View reviewed changes

magento-team removed the Progress: needs update label Oct 24, 2017

okorshenko modified the milestones: October 2017, November 2017 Nov 1, 2017

magento-team added the Progress: needs update label Nov 8, 2017

magento-team removed the Progress: needs update label Nov 20, 2017

magento-team merged commit 02bc892 into magento:2.2-develop Nov 25, 2017

magento-team pushed a commit that referenced this pull request Nov 25, 2017

MAGETWO-82577: [Backport 2.2] Translate order getCreatedAtFormatted()…

5966d3a

… to store locale #11422

JeroenVanLeusden deleted the patch-4 branch December 4, 2017 15:49

vkublytskyi added the Award: test coverage label Dec 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Backport 2.2] Translate order getCreatedAtFormatted() to store locale #11422

[Backport 2.2] Translate order getCreatedAtFormatted() to store locale #11422

JeroenVanLeusden commented Oct 13, 2017 •

edited by okorshenko

Loading

dmanners Oct 13, 2017

JeroenVanLeusden Oct 13, 2017

dmanners Oct 13, 2017

JeroenVanLeusden Oct 13, 2017

dmanners Oct 13, 2017

JeroenVanLeusden Oct 13, 2017

dmanners Oct 13, 2017

dmanners Oct 13, 2017

JeroenVanLeusden commented Oct 13, 2017

JeroenVanLeusden commented Oct 17, 2017

dmanners Oct 24, 2017

dmanners commented Nov 10, 2017

JeroenVanLeusden commented Nov 13, 2017

dmanners commented Nov 15, 2017

JeroenVanLeusden commented Nov 18, 2017

dmanners commented Nov 20, 2017

JeroenVanLeusden commented Nov 20, 2017

[Backport 2.2] Translate order getCreatedAtFormatted() to store locale #11422

[Backport 2.2] Translate order getCreatedAtFormatted() to store locale #11422

Conversation

JeroenVanLeusden commented Oct 13, 2017 • edited by okorshenko Loading

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

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

JeroenVanLeusden commented Oct 13, 2017

JeroenVanLeusden commented Oct 17, 2017

Choose a reason for hiding this comment

dmanners commented Nov 10, 2017

JeroenVanLeusden commented Nov 13, 2017

dmanners commented Nov 15, 2017

JeroenVanLeusden commented Nov 18, 2017

dmanners commented Nov 20, 2017

JeroenVanLeusden commented Nov 20, 2017

JeroenVanLeusden commented Oct 13, 2017 •

edited by okorshenko

Loading