Bugfix > Adding Success Message > Recent Ordered #9852
Conversation
|
@rafaelstz I'll be reviewing this, will get back to you in case we have any questions or feedback. Thanks for the contribution! |
|
@rafaelstz I checked the code and all the tests are passing. The only thing I saw is that Magento escapes the name of the product. Can you add that to the PR so we maintain the same code style? |
|
Done @miguelbalparda 👍 |
| @@ -53,13 +59,15 @@ public function __construct( | |||
| \Magento\Checkout\Model\Session $checkoutSession, | |||
| \Magento\Store\Model\StoreManagerInterface $storeManager, | |||
| \Magento\Framework\Data\Form\FormKey\Validator $formKeyValidator, | |||
| CustomerCart $cart | |||
| CustomerCart $cart, | |||
| Escaper $escaper | |||
There was a problem hiding this comment.
Your change is not backward compatible. Could you make it optional with fallback to get this object from object manager?
There was a problem hiding this comment.
I'm not really sure using the OM is the right approach, @rafaelstz thoughts?
There was a problem hiding this comment.
Did you have some time to work on this @rafaelstz? Thanks!
There was a problem hiding this comment.
Not yet @miguelbalparda 😞
There was a problem hiding this comment.
Hi @rafaelstz Please, review this document: http://devdocs.magento.com/guides/v2.0/contributor-guide/backward-compatible-development/#adding-a-constructor-parameter
There is an example how to add new constructor argument in a backward compatible way (mentioned by @ihor-sviziev)
| @@ -53,13 +59,15 @@ public function __construct( | |||
| \Magento\Checkout\Model\Session $checkoutSession, | |||
| \Magento\Store\Model\StoreManagerInterface $storeManager, | |||
| \Magento\Framework\Data\Form\FormKey\Validator $formKeyValidator, | |||
| CustomerCart $cart | |||
| CustomerCart $cart, | |||
| Escaper $escaper | |||
There was a problem hiding this comment.
Hi @rafaelstz Please, review this document: http://devdocs.magento.com/guides/v2.0/contributor-guide/backward-compatible-development/#adding-a-constructor-parameter
There is an example how to add new constructor argument in a backward compatible way (mentioned by @ihor-sviziev)
|
@rafaelstz please let me know if you need some help |
|
@okorshenko I did right? |
|
There are some code style tests failing now, more details here. |
| ) { | ||
| $this->_formKeyValidator = $formKeyValidator; | ||
| $this->_scopeConfig = $scopeConfig; | ||
| $this->_checkoutSession = $checkoutSession; | ||
| $this->_storeManager = $storeManager; | ||
| $this->cart = $cart; | ||
| $this->cart = $cart; | ||
| $this->escaper = $escaper ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Framework\Escaper::class); |
| @@ -31,6 +32,7 @@ public function __construct( | |||
| \Magento\Store\Model\StoreManagerInterface $storeManager, | |||
| \Magento\Framework\Data\Form\FormKey\Validator $formKeyValidator, | |||
| \Magento\Checkout\Model\Cart $cart, | |||
| \Magento\Framework\Escaper $escaper, | |||
There was a problem hiding this comment.
this argument must be optional according to parent class.
Please update all other child classes of \Magento\Checkout\Controller\Cart with new optional argument
|
Hi @rafaelstz you fixed the constructor correctly. Please look at my review comments |
- fixed backward compatibility issue
Adding a Success message
Description
I've fixed the issue with success message when the customer adds a product to the cart using the sidebar "Recent Ordered" like that image:
Manual testing scenarios
Contribution checklist