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

Add missing setResponse() setter for FilterUserResponseEvent #2606

Merged
merged 2 commits into from Oct 4, 2017

Conversation

Projects
None yet
2 participants
@shakaran

shakaran commented Sep 13, 2017

Add the missing setter setResponse for Response object in FilterUserResponseEvent class.

Other filters like FormEvent, FilterGroupResponseEvent contains this setResponse when the object Response is present as attribute in the class.

Probably a good idea is add in GetResponseGroupEvent and GetResponseUserEvent which is missing too. I could make another separate PR for that if you desire.

I am using the setResponse() method in FilterUserResponseEvent in a listener after capture the event FOSUserEvents::RESETTING_RESET_COMPLETED creating a callback that sent to the admin an email notifing which user has changed his password (with option of know the new password or not). After all this, I redirect the response to a custom url, which is where I use the setResponse() method.

Without the method I have to fetch the Response object and set the target url:

    $response = $event->getResponse();
    $response->setTargetUrl($url);

If I have this PR accepted, then I could write:

    $response = $event->setResponse(new RedirectResponse($url));
Missing setter for FilterUserResponseEvent
Add missing setter setResponse for Response object in FilterUserResponseEvent class
@XWB

This comment has been minimized.

Show comment
Hide comment
@XWB

XWB Oct 3, 2017

Member

Looks good, just the CS test fails:

  1. Event/FilterUserResponseEvent.php (whitespacy_lines)
Member

XWB commented Oct 3, 2017

Looks good, just the CS test fails:

  1. Event/FilterUserResponseEvent.php (whitespacy_lines)
@shakaran

This comment has been minimized.

Show comment
Hide comment
@shakaran

shakaran Oct 3, 2017

@XWB fixed, and ready to merge if you want, thanks!

shakaran commented Oct 3, 2017

@XWB fixed, and ready to merge if you want, thanks!

@XWB XWB merged commit 3fc7e36 into FriendsOfSymfony:master Oct 4, 2017

2 checks passed

Scrutinizer 2 new issues, 1 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@XWB

This comment has been minimized.

Show comment
Hide comment
@XWB

XWB Oct 4, 2017

Member

Thanks :)

Member

XWB commented Oct 4, 2017

Thanks :)

shakaran added a commit to shakaran/FOSUserBundle that referenced this pull request Oct 4, 2017

Add missing setResponse() setter for GetResponseGroupEvent
Add missing setResponse() setter for GetResponseGroupEvent. Follows the mentioned in PR FriendsOfSymfony#2606

shakaran added a commit to shakaran/FOSUserBundle that referenced this pull request Oct 4, 2017

Add missing setResponse() setter for GetResponseUserEvent
Add missing setResponse() setter for GetResponseUserEvent. Follows the mentioned in PR FriendsOfSymfony#2606

@shakaran shakaran deleted the shakaran:patch-1 branch Oct 4, 2017

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