Make Reset Password Optional

[Hanmac]

Subject

I am targeting this branch, because its BC break (most likely).

Closes #1491.

Changelog

### Changed
- Make Reset Password Optional

To do

• Update the tests, i probably need new App Configs without Mailer
• Update the documentation;
• Add an upgrade note.

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[Hanmac]

@VincentLanglet should i try to fix this errors even if i didn't touched this code?

[Hanmac]

getFlashBag needs symfony/symfony#46491 symfony/symfony@026c748
which only exists in Symfony 6.2+
so just do a $session instance of Session around it?

[VincentLanglet]

I made #1595 ; you just need to rebase once 5.x will be in 6.x.

Btw, the 6.x release won't be soon. Can't you do the PR on the 5.x ? I don't see a real BC-break

[VincentLanglet]

Seems ok ; but didn't you plan to write doc and update note ?

[Hanmac]

Seems ok ; but didn't you plan to write doc and update note ?

I'm not very good at documentation and update note stuff

i probably need to write something like this:

When updating, please require "symfony/mailer" and "symfony/mime" manually
Or to disable the function, remove the configuration

IMO isn't this still a BC break? and shouldn't that be marked for remove next-major?

[Hanmac]

@VincentLanglet i'm unsure, is this a BC Break?

updating with composer would remove the dependency and composer would remove the packages?
which then causes a problem that the current default config (and what the user might have configured)
are still looking for the mailer packages? (i mean it probably would cause problems with cache:clear after composer update, so the user should see the problem directly?)

[VincentLanglet]

Friendly ping @jordisala1991.
I don't remember where but I think we already did such change (which an upgrade note).

The other option would be to keep those dependencies and only remove them in 6.x branch

[jordisala1991]

Since you have to configure mailer, it should be a dep you have when installing this bundle. IMO we can try moving it to require-dev

[jordisala1991]

IMO this is missing a check to really validate the user has required deps (since they will be optional on require-dev) wdyt ?

[jordisala1991]

WDYT about adding an option to disable reset mail (default to enabled) and deprecate not defining a value for it (like symfony did for the enable_authenticator_manager). Not sure how they did it or how hard is to implement. But would make the feature easier to discover.

Also, some docs about how to disable/enable the feature, would be nice.

[VincentLanglet]

WDYT about adding an option to disable reset mail (default to enabled) and deprecate not defining a value for it (like symfony did for the enable_authenticator_manager). Not sure how they did it or how hard is to implement. But would make the feature easier to discover.

There is already

$config['resetting']['email']['address']
$config['resetting']['email']['sender_name']

which are two values required for reset mail

If you don't want reset mail, you remove those value ; and if you want reset mail, you set those values.
Isn't it enough ? It avoid handling case when you enable the reset mail but doesn't set those values...

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[VincentLanglet]

Except the need of a rebase, this PR need to update the doc in order to explain how to enable/disable this feature @Hanmac

[Hanmac]

For the Documentation i need to think about what the best way to write it

[VincentLanglet]

This part need to be updated https://github.com/sonata-project/SonataUserBundle/blob/5.x/docs/reference/installation.rst#configuration
since

resetting:
        email:
            address: sonata@localhost
            sender_name: Sonata Admin

is not mandatory.

I would say the routing too https://github.com/sonata-project/SonataUserBundle/blob/5.x/docs/reference/installation.rst#integrating-the-bundle-into-the-sonata-admin-bundle

Maybe this section too https://github.com/sonata-project/SonataUserBundle/blob/5.x/docs/reference/installation.rst#mailer-configuration

Basically we need for every reset-specific code to give an exemple with and without the feature.

[VincentLanglet]

For the Documentation i need to think about what the best way to write it

Hi @Hanmac, how are you doing with the doc update ?
This PR would be a nice improvement to add to this bundle :)

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[VincentLanglet]

Friendly ping @Hanmac ; any time for the doc update ? :)

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Hanmac]

I still need to do this an some Doc Update

Maybe next Weekend

[VincentLanglet]

I still need to do this an some Doc Update

Maybe next Weekend

Thanks.

You'll also need a rebase.