Skip to content

Conversation

@azine
Copy link

@azine azine commented Dec 19, 2017

Untagled some configuration dependencies between
email_confirmation (upon registration) and
confirm_email_update (when the users changes her email) to fix #2667

@azine
Copy link
Author

azine commented Dec 19, 2017

I moved the confirmEmailUpdateAction into a separate/new controller, re-grouped the configuration related to the email-update-confirmation-feature, fixed a typo and added a test for the routing.

DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!!
I didn't yet run this code in a fully installed application. I only just started updating my own application from Symfony 2.7 to 2.8 (and then later 3.x). I'll give this code a try once I have updated my app.
DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!! DISCLAIMER!!!

I you (@covex-nn or @XWB or @stof or anyone else) want to give it a try before that, you are very welcome to do so and give feedback.

Cheers,
Dominik

@azine azine force-pushed the issue/2667-configuration-dependency-problem-with-confirm-updated-email-address-and-profile-controller branch 2 times, most recently from a345ec6 to 4746b54 Compare December 19, 2017 21:37
<tag name="form.type" alias="fos_user_profile" />
</service>

<service id="fos_user.profiler.controller" class="FOS\UserBundle\Controller\ProfileController" public="true">
Copy link
Author

@azine azine Dec 19, 2017

Choose a reason for hiding this comment

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

fixed typo: fos_user.profile.controller vs. fos_user.profiler.controller

@covex-nn
Copy link

@azine without enabling profile.email_update_confirmation feature, /profile and /profile/edit work as before. And /profile/confirm-email-update/qwerty is not accessable with this error message:

Controller not found: service "fos_user.confirm.email.update.controller" does not exist.

After enabling email update confirmation with only profile.email_update_confirmation.enabled: true, and after updating email in /profile/edit there was a error in FOS\UserBundle\Mailer\Mailer::sendUpdateEmailConfirmation

Notice: Undefined index: template

This error occurred because, there is no template key in $this->parameters. But, there is email_updating.template key, and $this->parameters['email_updating.template'] === '@FOSUser/Profile/email_update_confirmation.txt.twig'

…o separate controller

Untagled some configuration dependencies between
email_confirmation (upon registration) and
confirm_email_update (when the users changes her email)

Fixed parameter access in Mailer/Mailer.php
@azine azine force-pushed the issue/2667-configuration-dependency-problem-with-confirm-updated-email-address-and-profile-controller branch from 4746b54 to f35defb Compare December 20, 2017 19:43
@azine
Copy link
Author

azine commented Dec 20, 2017

@covex-nn: Thanks for your feedback & testing.
The first part (service not found, when the feature is not enabled) sounds fine to me. If the feature is not enabled, the Controller is not available. The loading of the routes does not take the configuration into account (afaik).

The second part is a bug in the Mailer/Mailer.php. I fixed that and updated the PR.

…m-with-confirm-updated-email-address-and-profile-controller
…m-with-confirm-updated-email-address-and-profile-controller
@covex-nn
Copy link

@azine i checked: second part was fixed =)

After updating email, fos_user.email_canonical was changed to new value, fos_user.email had old value.
Then after visiting $confirmationUrl, fos_user.email was updated to new value too, fos_user.confirmation_token was updated back to null.

So, it works!

@azine azine force-pushed the issue/2667-configuration-dependency-problem-with-confirm-updated-email-address-and-profile-controller branch from 56d31c2 to cc7ab03 Compare December 20, 2017 22:44
@azine
Copy link
Author

azine commented Dec 20, 2017

@covex-nn Thanks again for testing. Leaving the canonical value of the email with the new value while waiting for confirmation, was not intentional. => I fixed this unintended behaviour.

@covex-nn
Copy link

@azine I thought, that it was a feature. I wrote about it just because is was strange =)

With last commit, fos_user.email and fos_user.email_canonical are changed only after visiting $confirmationUrl. So, it works2! Hooray! =)

@azine
Copy link
Author

azine commented Dec 21, 2017

@covex-nn cool! Thanks for the "testing service". :-)

IMO thiis PR would be ready to squash & merge. @XWB or @stof : any objecttions? Anything I need to change?

@XWB XWB merged commit 17c299b into FriendsOfSymfony:master Dec 27, 2017
@XWB
Copy link
Member

XWB commented Dec 27, 2017

Thank you guys.

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.

ProfileController is not available if registration.confirmation is not enabled

3 participants