Skip to content

Make IEmailSender more customizable #50301

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 5 commits into from
Sep 18, 2023
Merged

Make IEmailSender more customizable #50301

merged 5 commits into from
Sep 18, 2023

Conversation

halter73
Copy link
Member

@halter73 halter73 commented Aug 23, 2023

Unlike with Identity UI, it's not simple to just replace the razor pages and rewrite the subject and htmlMessage to be whatever you want when using MapIdentityAPI. This adds DIMs that allow you to customize the emails more easily and continue to use MapIdentityAPI. It also happens to work for Identity UI if you do not want to customize the cshtml.

This is intended for RC2. It is pending API review. The API proposal that this PR fixes includes more details.

Fixes #50298

@ghost ghost added the area-identity Includes: Identity and providers label Aug 23, 2023
@halter73 halter73 changed the base branch from main to release/8.0 August 23, 2023 22:15
@Kahbazi
Copy link
Member

Kahbazi commented Aug 27, 2023

I don't know if this is an issue or not but just want to mention that no matter what lifetime the IEmailSender is added by user, It will always be created once.

var emailSender = endpoints.ServiceProvider.GetRequiredService<IEmailSender>();

@halter73
Copy link
Member Author

I don't know if this is an issue or not but just want to mention that no matter what lifetime the IEmailSender is added by user, It will always be created once.

That is a good point. That's the way I made MapIdentityApi work prior to this PR, but it is different from the Identity UI razor pages. The default no-op implementation is registered as a singleton.

Do you think it's common to need or want services from the request scope? I guess it probably wouldn't hurt. It's just another parameter to pass around.

@Kahbazi
Copy link
Member

Kahbazi commented Aug 30, 2023

Do you think it's common to need or want services from the request scope?

I don't have any scenario in mind.

@ghost
Copy link

ghost commented Sep 6, 2023

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Sep 6, 2023
Copy link
Contributor

@mkArtakMSFT mkArtakMSFT left a comment

Choose a reason for hiding this comment

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

Thanks @halter73!

Has this change been reviewed by the API Review crew?
Just noticed that the linked issue has api-approved label.

@halter73 halter73 removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Sep 18, 2023
@wtgodbe wtgodbe merged commit e68ec7f into release/8.0 Sep 18, 2023
@wtgodbe wtgodbe deleted the halter73/50298 branch September 18, 2023 23:26
@ghost ghost added this to the 8.0-rc2 milestone Sep 18, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 18, 2023
SteveSandersonMS pushed a commit that referenced this pull request Sep 20, 2023
* Revert "Remove hardcoded System.Security.Cryptography.Xml version (#48029)" (#50723)

This reverts commit 42d14c4.

* [Blazor] Prerendered state (#50742)

[Blazor] Adds support for persting prerendered state on Blazor Web applications.
* Persists state both for server and webassembly as necessary.
* Initializes the state when a given interactive runtime is initialized and renders the first set of components.
  * On WebAssembly, this is the first time the app starts.
  * On Server this happens every time a circuit starts.
* The state is available during the first render, until the components reach quiescence.

The approach we follow is different for server and webassembly:
* On Server, we support initializing the circuit with an empty set of descriptors and in that case, we delay initialization until the first `UpdateRootComponents` call is issued.
  * This is because it's hard to deal with the security constraints imposed by starting a new circuit multiple times, and its easier to handle them within UpdateRootComponents. We might switch this approach in the future to go through `StartCircuit` too.
* On WebAssembly, we query for the initial set of webassembly components when we are starting the runtime in a Blazor Web Scenario.
  * We do this because Blazor WebAssembly offers a programatic API to render root components at a given location defined by their selectors, so we need to make sure that those components can receive state at the same time the initial set of WebAssembly components added to the page.

There are a set of tests validating different behaviors with regards to enhanced navigation and streaming rendering, as well as making sure that auto mode can access the state on Server and WebAssembly, and that Server gets new state every time a circuit is opened.

* Make IEmailSender more customizable (#50301)

* Make IEmailSender more customizable

* Remove unnecessary metadata

* Add TUser parameter

* React to API review feedback

* Fix IdentitySample.DefaultUI

* Update branding to RTM (#50799)

---------

Co-authored-by: Igor Velikorossov <[email protected]>
Co-authored-by: Javier Calvarro Nelson <[email protected]>
Co-authored-by: Stephen Halter <[email protected]>
Co-authored-by: William Godbe <[email protected]>
@augustevn
Copy link

I don't know if this is an issue or not but just want to mention that no matter what lifetime the IEmailSender is added by user, It will always be created once.

That is a good point. That's the way I made MapIdentityApi work prior to this PR, but it is different from the Identity UI razor pages. The default no-op implementation is registered as a singleton.

Do you think it's common to need or want services from the request scope? I guess it probably wouldn't hurt. It's just another parameter to pass around.

Well, I tried it with FluentEmail and there were scoping issues.

@ghost
Copy link

ghost commented Oct 10, 2023

Hi @augustevn. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@augustevn
Copy link

augustevn commented Oct 10, 2023

What is this method for? SendPasswordResetLinkAsync
I'm only hitting the SendPasswordResetCodeAsync & SendConfirmationLinkAsync

Are there docs to clarify? The code comments don't clarify much.

I did create an API proposal: #50904

@ghost
Copy link

ghost commented Oct 10, 2023

Hi @augustevn. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-identity Includes: Identity and providers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make IEmailSender more easily customizable
5 participants