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

Add subject_email parameter #187

Closed
wants to merge 1 commit into from
Closed

Add subject_email parameter #187

wants to merge 1 commit into from

Conversation

sdespont
Copy link
Contributor

This PR brings the possibility to define the subject of the mail sent with the authentication code.
The parameter subject_email have been added, with a default value equals to the actual text "Authentication Code"

Psalm auto target php version 8 (inferred from composer.json)
@sdespont
Copy link
Contributor Author

@scheb what do you think about having a configurable prefix text before the code in the email? I don't want to spend time to push a PR if you don't agree with this approach

@scheb
Copy link
Owner

scheb commented May 19, 2023

Hmm, wouldn't want to add more configuration options. I'm not having the intention to provide the ultimate configurable mailer with the bundle.

Way to go would rather be to implement a custom mailer, which generates the email exactly as you want it. The on-board mailer is just doing the bare minimum to have something working.

@sdespont
Copy link
Contributor Author

@scheb you are right, implement a custom mailer is easy https://symfony.com/bundles/SchebTwoFactorBundle/current/providers/email.html#custom-mailer

After reflection, suffix or prefix is not a good idea. But permitting to set the email subject of the default email still makes sense for me.

@scheb
Copy link
Owner

scheb commented Jun 28, 2023

There seem a whole lot of changes in the diff, that aren't related to this merge request. In its current form I cannot merge it, as it will mess things up.

If you want to proceed, please review the diff and provide a cleaned-up changeset that just contains your changes.

@scheb
Copy link
Owner

scheb commented Aug 5, 2023

Closing due to inactivity. If you feel like follow-up on this, please create another PR. Thank you!

@scheb scheb closed this Aug 5, 2023
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.

3 participants