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

Action after run result #19870

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

rhertogh
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature? ✔️
Breaks BC? ✔️
Fixed issues #13484

The \yii\base\Action::afterRun() documentation states:

"You may override this method to do post-processing work for the action run"

Currently post-processing is not possible since the $result is not passed to the afterRun() function in the \yii\base\Action::runWithParams() function.

This was discussed earlier, but since Yii 2.1 is cancelled I would like to re-submit this feature.

ToDo:

  • Implementation
  • Documentation/Changelog/Upgrade steps
  • Unit tests

@what-the-diff
Copy link

what-the-diff bot commented Jun 26, 2023

PR Summary

  • Updated afterRun() method in framework/base/Action.php
    • Modified the method call to pass the $result parameter
    • Added a new parameter $result to the method
    • Included a proper return statement for the method

@rhertogh
Copy link
Contributor Author

@samdark @bizley What do you think?

@bizley
Copy link
Member

bizley commented Jun 27, 2023

This is a big BC break, not sure if we want to change it like that.

@samdark
Copy link
Member

samdark commented Jun 29, 2023

@rhertogh do you have a use-case for it or is it based mainly on the contradiction of current code with phpdoc?

@rhertogh
Copy link
Contributor Author

@samdark I've an actual use case for it where I need to run some code in the beforeRun (based on the arguments) and in the afterRun (based on the result).
I thought these functions would solve my needs but then I ran into the issue that the the functions don't behave as expected.

@rhertogh
Copy link
Contributor Author

@bizley Something for Yii 2.2?

@samdark
Copy link
Member

samdark commented Jun 30, 2023

@rhertogh maybe. I've posted there in a discussion.

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

Successfully merging this pull request may close these issues.

None yet

3 participants