add config object for emails content url #1402
Conversation
|
OutOfSchool/OutOfSchool.RazorTemplatesData/Config/EmailContentConfig.cs
Outdated
Show resolved
Hide resolved
| @@ -485,7 +486,8 @@ public static void AddApplicationServices(this WebApplicationBuilder builder) | |||
| services.AddEmailSender( | |||
| builder.Environment.IsDevelopment(), | |||
| mailConfig.SendGridKey, | |||
| builder => builder.Bind(configuration.GetSection(EmailOptions.SectionName))); | |||
| builder => builder.Bind(configuration.GetSection(EmailOptions.SectionName))) | |||
| .AddEmailRendererConfiguration(new EmailContentConfig(configuration.GetSection("Hosts")["BackendUrl"])); | |||
There was a problem hiding this comment.
(Logic) configuration.GetSection("Hosts")["BackendUrl"] - here string keys are different from mentioned in AuthCommonServiceExtensions.cs. Is it intended to have same thing named differently?
There was a problem hiding this comment.
Sorry to have come back to this PR after a few months :) but had to do other email refactorings first. Yes the reason to create the config object is to unify the string keys because in their own projects they have own logic to be used that way and we wanted to reuse the keys without dublicating.
There was a problem hiding this comment.
Yes the reason to create the config object is to unify the string keys ...
If I get you correctly, some other commit to this PR should be done soon, where all string literals be done as EmailContentConfig class constants and reused in both WebApi and Auth library, right?
If so, could PR be in Draft mode until that commit is done?
| @@ -31,7 +32,8 @@ public static void AddAuthCommon(this IServiceCollection services, Configuration | |||
| services.AddEmailSender( | |||
| isDevelopment, | |||
| mailConfig.SendGridKey, | |||
| builder => builder.Bind(config.GetSection(EmailOptions.SectionName))); | |||
| builder => builder.Bind(config.GetSection(EmailOptions.SectionName))) | |||
| .AddEmailRendererConfiguration(new EmailContentConfig(config.GetSection("Identity")["Authority"])); | |||
There was a problem hiding this comment.
(Code style) Please consider storing string literals in related classes (see f.e. EmailOptions.SectionName one line above). Especially, when they are reused more than twice
DmyMi
force-pushed
the
dmin/update_email_config
branch
from
61e4cbf to
993c8b0
Compare
|
As different projects have differently named configuration fields - a way to unify them and show email data correctly