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 support for friendly admin emails #336

Merged
merged 6 commits into from
May 24, 2024
Merged

Conversation

fritzmg
Copy link
Collaborator

@fritzmg fritzmg commented May 15, 2024

Currently the notification center will not send an email notification if you are using a friendly email for the administrator email address.

Similiar to #298 this PR fixes that.

Notes:

  • I wasn't sure whether translations are managed on Transifex or not. I have only adjusted the English translation.
  • @Toflar I changed the implementation of how the token values are added to the token collection. Is there a practical difference between
    $tokenCollection->addToken(new Token(…))
    and
    $tokenCollection->add($this->tokenDefinitionFactory->create(EmailTokenDefinition::class, …)->createToken())
    ? For simplicity in this implementation I only used the former to add the token to the collection.

@fritzmg fritzmg added the bug label May 15, 2024
@fritzmg fritzmg requested a review from Toflar May 15, 2024 15:42
@fritzmg fritzmg self-assigned this May 15, 2024
@fritzmg fritzmg mentioned this pull request May 15, 2024
@Toflar
Copy link
Member

Toflar commented May 22, 2024

I wasn't sure whether translations are managed on Transifex or not. I have only adjusted the English translation.

They are not, see README.

  • @Toflar I changed the implementation of how the token values are added to the token collection. Is there a practical difference between
    $tokenCollection->addToken(new Token(…))

Yes, please change back. The reason for this is that you can replace/decorate the TokenDefnitionFactory and provide your own logic for the EmailTokenDefinition and do whatever you want in createToken() then.

@fritzmg
Copy link
Collaborator Author

fritzmg commented May 22, 2024

I see :). Will adjust both.

@fritzmg
Copy link
Collaborator Author

fritzmg commented May 23, 2024

Done 👍

@Toflar Toflar merged commit 6f3cfbc into main May 24, 2024
8 checks passed
@Toflar Toflar deleted the support-friendly-admin-email branch May 24, 2024 06:41
@Toflar
Copy link
Member

Toflar commented May 24, 2024

Perfect, thanks @fritzmg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants