Save region correctly to save sales address from admin [backport] #11234
Conversation
…ales address from admin
|
Hi @raumatbel I just sent you an invitation to magento/magento2 repo. Could you please accept it |
|
@okorshenko Accepted |
|
@okorshenko I have a problem with travis. The error shown is the next. |
|
|
||
| /** | ||
| * @SuppressWarnings(PHPMD.CouplingBetweenObjects) | ||
| * @SuppressWarnings(PHPMD.ExcessiveParameterList) |
There was a problem hiding this comment.
@SuppressWarnings(PHPMD.ExcessiveParameterList) should be used for the constructor.
| OrderManagementInterface $orderManagement, | ||
| OrderRepositoryInterface $orderRepository, | ||
| LoggerInterface $logger, | ||
| RegionFactory $regionFactory |
There was a problem hiding this comment.
Please use approach for extending constructor arguments in backward compatible way
| * @param array $attributeValues | ||
| * @return void | ||
| */ | ||
| protected function updateRegionData(&$attributeValues) |
There was a problem hiding this comment.
Passing arguments by reference is not recommended. Return modified data instead (possible memory usage optimization is not so important here).
…d change SuppressWarnings
| * @param array $attributeValues | ||
| * @return array | ||
| */ | ||
| protected function updateRegionData($attributeValues) |
There was a problem hiding this comment.
Why this method protected? Is it referenced from the children classes?
There was a problem hiding this comment.
@vrann this function is only used in this class. I have updated the visibility to private.
raumatbel
changed the title
* 'develop' of github.com:magento/magento2: (4059 commits) MAGETWO-81324: Fix functional test fails randomly MAGETWO-80916: Save region correctly to save sales address from admin magento#11234 Improve constructor arguments, arguments by reference is not valid and change SuppressWarnings MQE-428: Update Magento2 Acceptance Readme File MQE-352: Review and Update SampleCest.xml for added functionality MQE-415: Change required-entity's persistedKey in test schema to createDataKey MQE-335: Headless Browser Spike - Updating all @env tags for tests. Only listing the ones that the test works in currently. - Updating README. Replacing "Acceptance" with "Functional". - Adding Headless command to robo. MAGETWO-80319: Remove array indexing by entityId magento#11086 Error CBO coupling between object. Error CBO coupling between object. Error CBO coupling between object. Error CBO coupling between object. Fix Code Style and Codacy Variable too long Error CBO coupling between object. Add Test to cover cancel Invoice MQE-424: Fix static code issues in MFTF tests and MFTF Solve error Field declared dynamically Solve error PHP Code Sniffer FR#StateNotDisplayedSalesAdminAddress Save region correctly to save sales address from admin MQE-419: README.MD should not reference the magento-pangolin org ...
Related with PR#11381
Description
Save region correctly to save sales address from admin
Fixed Issues (if relevant)
Steps to reproduce
Note: The same issue happen with the Shipping Address.
Manual testing scenarios
Contribution checklist