Skip to content

Conversation

@osrecio
Copy link
Member

@osrecio osrecio commented Sep 25, 2017

Delete CallExit to allow Around/After plugin work correctly

Description

If you call a exit, you break the lifecycle of Magento, Post/Dispatch Events, Profiler, etc

Fixed Issues

  1. M2.1.2 : Impossible to create an after interceptor when files are being downloaded by controllers #7356

Manual testing scenarios

Create a plugin that executes code after \Magento\Sales\Controller\Adminhtml\Invoice\AbstractInvoice\PrintAction::execute(). e.g:

#File: di.xml
<type name="Magento\Sales\Controller\Adminhtml\Invoice\PrintAction">
    <plugin name="after_print_shipment"
            type="VendorModule\Plugin\Magento\Sales\Controller\Adminhtml\Invoice\PrintAction\Plugin"
            sortOrder='10'
    />
</type>
#File: Plugin.php
function afterExecute($subject, $result) {
    echo 'After Plugin';
    die;
}

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)

@osrecio osrecio changed the title Delete CallExit function for After/Around plugin logic execution Delete CallExit function for After/Around plugin logic execution in FileFactory Sep 25, 2017
@miguelbalparda
Copy link
Contributor

From here, it is recommended to mark the unused private methods with the @deprecated tag instead of removing it.

@osrecio
Copy link
Member Author

osrecio commented Sep 26, 2017

Added @deprecated annotation to functioncallExit with description @miguelbalparda

@miguelbalparda
Copy link
Contributor

This is going to be merged soon, can you please backport this fix to 2.2 and 2.1? Thanks!

magento-team pushed a commit that referenced this pull request Sep 27, 2017
@vrann
Copy link
Contributor

vrann commented Sep 28, 2017

@osrecio thank you for the contribution. Your code was successfully merged, the status of the PR does not reflect that because of the additional commit with the deprected tag which was not in the accepted code. Closing this ticket.

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