-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
HSB-439 feature: invite link with SMTP optional #4078
base: next
Are you sure you want to change the base?
Conversation
@@ -42,6 +46,8 @@ export class MailerService { | |||
to: string, | |||
mailDesc: MailDescription | UserMagicLinkMailDescription, | |||
) { | |||
if (this.configService.get('INFRA.MAILER_SMTP_ENABLE') !== 'true') return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can throw an error here right saying "SMTP_NOT_ENABLED" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we using event emit, throwing errors is not effective.
@@ -64,6 +70,8 @@ export class MailerService { | |||
to: string, | |||
mailDesc: AdminUserInvitationMailDescription, | |||
) { | |||
if (this.configService.get('INFRA.MAILER_SMTP_ENABLE') !== 'true') return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here too.
// If mailer SMTP is DISABLED, return the module without any configuration (service, listener, etc.) | ||
if (env.INFRA.MAILER_SMTP_ENABLE !== 'true') { | ||
console.log('Mailer SMTP is disabled'); | ||
return { module: MailerModule }; | ||
} | ||
|
||
// If mailer is ENABLED, return the module with configuration (service, listener, etc.) | ||
let transportOption: TransportType; | ||
|
||
const mailerAddressFrom = | ||
env.INFRA.MAILER_ADDRESS_FROM ?? | ||
config.get('MAILER_ADDRESS_FROM') ?? | ||
throwErr(MAILER_SMTP_URL_UNDEFINED); | ||
|
||
if ( | ||
(env.INFRA.MAILER_USE_ADVANCE_CONFIGS ?? | ||
config.get('MAILER_USE_ADVANCE_CONFIGS')) === 'false' | ||
) { | ||
console.log('Using simple mailer configuration'); | ||
|
||
transportOption = | ||
env.INFRA.MAILER_SMTP_URL ?? | ||
config.get('MAILER_SMTP_URL') ?? | ||
throwErr(MAILER_SMTP_URL_UNDEFINED); | ||
} else if ( | ||
(env.INFRA.MAILER_USE_ADVANCE_CONFIGS ?? | ||
config.get('MAILER_USE_ADVANCE_CONFIGS')) === 'true' | ||
) { | ||
console.log('Using advance mailer configuration'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you simplify this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll split this into different functions.
.env.example
Outdated
MAILER_SMTP_HOST="smtp.domain.com" # if MAILER_USE_ADVANCE_CONFIGS is false | ||
MAILER_SMTP_PORT="587" # if MAILER_USE_ADVANCE_CONFIGS is false | ||
MAILER_SMTP_SECURE="true" # if MAILER_USE_ADVANCE_CONFIGS is false | ||
MAILER_SMTP_USER="user@domain.com" # if MAILER_USE_ADVANCE_CONFIGS is false | ||
MAILER_SMTP_PASSWORD="pass" # if MAILER_USE_ADVANCE_CONFIGS is false | ||
MAILER_TLS_REJECT_UNAUTHORIZED="true" # if MAILER_USE_ADVANCE_CONFIGS is false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do all these have to be strings, can some of them normal booleans and numbers and we do a toString()
conversion in the infra-config module anyway??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default behavior is process.env always reads the env variable as string regarding of string/boolean.
No matter if you add an inverted comma or not, process.env reads it as a string.
|
Closes HSB-439
Description
This PR
mailer-module
optionalmailer-module
more configurableChecks
Additional Information
Nil