Skip to content

Refactor Code for Mass Order Unhold #14629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 14, 2018

Conversation

AnshuMishra17
Copy link
Contributor

Description

I have observed that MassAction Unhold is using the collection to release order from hold, whereas MassAction Hold is using Interface to put the order on hold.
So, I have refactor code in app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php same as in app/code/Magento/Sales/Controller/Adminhtml/Order/MassHold.php

Manual testing scenarios

  1. Go to Sales -> Order
  2. Select any number of orders (orders which are on hold.)
  3. Select Unhold Action from the Action dropdown.

I have observed that MassAction Unhold is using the collection to release order from hold, whereas MassAction Hold is using Interface to put the order on hold.
So, I have refactor code in app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php same as in app/code/Magento/Sales/Controller/Adminhtml/Order/MassHold.php
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Apr 11, 2018

CLA assistant check
All committers have signed the CLA.

@sidolov sidolov self-assigned this Apr 11, 2018
Copy link
Contributor

@sidolov sidolov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AnshuMishra17 , please, fix all issue according to review and take a look to failed tests

/**
* @var OrderManagementInterface
*/
protected $orderManagement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, make this property private

*/
public function __construct(Context $context, Filter $filter, CollectionFactory $collectionFactory)
public function __construct(Context $context, Filter $filter, CollectionFactory $collectionFactory, OrderManagementInterface $orderManagement)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add new dependencies according to our Backward Compatible Development Guide

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added backward compatibility.

AnshuMishra17 and others added 2 commits April 11, 2018 15:03
Added backward compatibility for the new orderManagementInterface dependency and made the variable $orderManagement private.
@sidolov
Copy link
Contributor

sidolov commented Apr 12, 2018

Hi @AnshuMishra17 , I fixed test failures

@magento-engcom-team
Copy link
Contributor

Hi @AnshuMishra17. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.2.5 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants