Skip to content

Eliminate the need for inheritance for action controllers. #16268

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

Closed
wants to merge 25 commits into from
Closed

Eliminate the need for inheritance for action controllers. #16268

wants to merge 25 commits into from

Conversation

Vinai
Copy link
Contributor

@Vinai Vinai commented Jun 20, 2018

Description

Kindly see issue magento/community-features#9 for detailed information.

Fixed Issues (if relevant)

All the CE unit and integration tests pass. The big question is how the EE will do.
When this PR is merged, frontend action controllers don't have to extend AbstractAction, they only have to implement the ActionInterface.

This PR mainly moves the dispatching of the controller_predispatch events into a plugin.
It also changes the plugins for Action::dispatch() method to ActionInterface::execute().
Finally, it preserves the behavior in regards to the NO_DISPATCH and NO_POST_DISPATCH action flags.

Backend action controllers still have to extend the backend module abstract Action for authentication - moving that to a plugin will be a separate PR.

NOTE:

This PR is a port of #13045 to the 2.3-develop branch as discussed with @sidolov
The PR to the 2.2-develop branch will be closed in favor of this one. Please refer to the original PR for details on the discussion.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

Vinai added 25 commits June 20, 2018 06:32
@magento-engcom-team
Copy link
Contributor

Hi @Vinai. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on Pull Request changes
  • @magento-engcom-team give me new test instance - deploy NEW test instance based on Pull Request changes
  • @magento-engcom-team give me {$VERSION} instance - deploy Vanilla Magento instance for Issue or Pull Request

For more details, please, review the Magento Contributor Assistant documentation

@orlangur
Copy link
Contributor

orlangur commented Apr 4, 2019

@Vinai not sure what happened with this one, but we cannot process PR from an unknown repository. Please restore branch and recreate PR.

@orlangur orlangur closed this Apr 4, 2019
@m2-assistant
Copy link

m2-assistant bot commented Apr 4, 2019

Hi @Vinai, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@Vinai
Copy link
Contributor Author

Vinai commented Apr 5, 2019

Interesting, no idea either. I hope I can do that.

@Vinai
Copy link
Contributor Author

Vinai commented May 24, 2019

Just to wrap this up, I'll won't be picking this PR up again. I think it's futile without access to EE and B2B repos.
Having to depend on rare core team developers to complete a change that isn't high on any product managers list of priorities is kind of a blocker.
Should anybody who has access to the non-public repos take a real personal interest in this topic, I'm happy to recreate the PR for CE, but I hope the commitment from the team member would be strong enough to see it through.

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.

5 participants