Skip to content
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

MassAction abstract class refactoring #4380

Closed
ldusan84 opened this issue May 2, 2016 · 3 comments
Closed

MassAction abstract class refactoring #4380

ldusan84 opened this issue May 2, 2016 · 3 comments
Assignees
Labels
improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@ldusan84
Copy link
Contributor

ldusan84 commented May 2, 2016

Steps to reproduce

  1. Install Magento from develop branch.
  2. Create a class that extends Magento\Sales\Controller\Adminhtml\Order\AbstractMassAction.
  3. Use that mass action from dropdown.

Expected result

  1. No errors.

Actual result

  1. PHP Fatal error: Call to a member function create() on null in /var/www/html/magento2/app/code/Magento/Sales/Controller/Adminhtml/Order/AbstractMassAction.php on line 54

There is a hidden dependency in https://github.com/magento/magento2/blob/develop/app/code/Magento/Sales/Controller/Adminhtml/Order/AbstractMassAction.php#L54

It is not obvious that the child class requires collectionFactory to be injected so it's not transparent which objects are needed for this class to work.

I tried to create a PR for this, by moving the collection factory to constructor, but the problem is that for example this class:

https://github.com/magento/magento2/blob/develop/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfinvoices.php

Requires Magento\Sales\Model\ResourceModel\Order\Collection and this one:

https://github.com/magento/magento2/blob/develop/app/code/Magento/Sales/Controller/Adminhtml/Shipment/AbstractShipment/Pdfshipments.php

Requires Magento\Sales\Model\ResourceModel\Order\Shipment\CollectionFactory

These two don't have the same parent class or interface, so it seems that it's not possible to type hint.

Any thoughts?

@piotrekkaminski
Copy link
Contributor

@maksek please redirect to proper person

@NadiyaS
Copy link
Contributor

NadiyaS commented May 18, 2016

Hi @ldusan84 ,
thank you for reporting this issue. Internal ticket was created MAGETWO-52955 to consider this problem.

@NadiyaS NadiyaS added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label May 18, 2016
@vkorotun vkorotun removed the CS label Aug 4, 2016
@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

magento-engcom-team pushed a commit that referenced this issue Jun 19, 2019
[tango] MAGETWO-99930: [Magento Cloud] Configuarable product stock status stays 'In Stock'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

7 participants