Cleanup for object manager references and depricated method #12061
Conversation
There was a problem hiding this comment.
- We can't remove optional parameters in constructor due to backward compatibility
- Replacing addSuccess to addSuccessMessage - looks good
- Remove getDataPersistor and use property instead - looks good
- Re-formatting code just to reformat - I think it looked better
- Using constant for frontend area - good
| $this->dataPersistor = $dataPersistor; | ||
| $this->logger = $logger ?: \Magento\Framework\App\ObjectManager::getInstance()->get(LoggerInterface::class); | ||
| $this->logger = $logger; |
There was a problem hiding this comment.
This change is not backward compatible, so please revert it.
| $this->inlineTranslation = $inlineTranslation; | ||
| $this->storeManager = $storeManager ?: | ||
| ObjectManager::getInstance()->get(StoreManagerInterface::class); | ||
| $this->storeManager = $storeManager; |
There was a problem hiding this comment.
This change is not backward compatible, so please revert it.
| @@ -44,20 +44,19 @@ public function __construct( | |||
| ConfigInterface $contactsConfig, | |||
| TransportBuilder $transportBuilder, | |||
| StateInterface $inlineTranslation, | |||
| StoreManagerInterface $storeManager = null | |||
| StoreManagerInterface $storeManager | |||
There was a problem hiding this comment.
This change is not backward compatible, so please revert it.
|
Hello @ihor-sviziev, |
|
@atishgoswami integration tests failed. Could you check it? |
|
@ihor-sviziev Yes working on it |
…o contact-module-cleanups
- Updated coding style to match the one used in core magento - Updated integration test to verify for the escaped string
| @@ -24,7 +24,7 @@ public function testPostAction() | |||
| $this->assertRedirect($this->stringContains('contact/index')); | |||
| $this->assertSessionMessages( | |||
| $this->contains( | |||
| "Thanks for contacting us with your comments and questions. We'll respond to you very soon." | |||
| "Thanks for contacting us with your comments and questions. We'll respond to you very soon." | |||
There was a problem hiding this comment.
Not sure that it will be good idea to make it escaped there. Have you checked this change?
There was a problem hiding this comment.
Yes, I have checked this manually. On Contact Form It works as expected:
Since this test is testing the controller I assume that it is valid to check for the exact html we expect.
|
@ishakhsuvarov Thanks for fixing the tests 👍 |
Contact Module Cleanups
Description
This PR will:
frontendarea string toMagento\Framework\App\Area::AREA_FRONTENDconstantFixed Issues (if relevant)
None, code improvement
Contribution checklist