Skip to content

SiteUpdateManager should return $happyMessage and not the mailer service #8224

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

Merged
merged 1 commit into from
Sep 3, 2017

Conversation

revollat
Copy link
Contributor

No description provided.

@xabbuh xabbuh added this to the 2.7 milestone Jul 27, 2017
@@ -361,7 +361,7 @@ made. To do that, you create a new class::
);
$this->mailer->send($message);

return $message;
return $happyMessage;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this method should return anything. The controller should use a custom alert instead of this returned value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that this is not an ideal example. I have merged this PR nonetheless as it would make the code at least work as it was supposed to be initially. We should handle the rest in a follow up PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #8339

@xabbuh
Copy link
Member

xabbuh commented Sep 3, 2017

Thank you @revollat.

@xabbuh xabbuh merged commit e42435c into symfony:3.3 Sep 3, 2017
xabbuh added a commit that referenced this pull request Sep 3, 2017
… mailer service (revollat)

This PR was merged into the 3.3 branch.

Discussion
----------

SiteUpdateManager should return $happyMessage and not the mailer service

Commits
-------

e42435c SiteUpdateManager should return $happyMessage and not the mailer service
javiereguiluz added a commit that referenced this pull request Jan 4, 2018
This PR was merged into the 3.3 branch.

Discussion
----------

[DI] fix flash message generation

see #8224 (comment)

Commits
-------

cba0478 Refactored the code
cdcb5c8 fix flash message generation
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