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

feat(ARC-2267): add language prop to send email function #484

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

RobbeBierebeeck
Copy link

No description provided.

@bertyhell bertyhell changed the base branch from release/sprint-3 to develop July 1, 2024 08:42
@@ -1,17 +1,33 @@
import { Template } from './campaign-monitor.types';

export const getTemplateId = (template: string): string => {
export const getTemplateId = (template: string, language: 'nl' | 'en'): string => {
console.log(template, language);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the console.log

@@ -1,17 +1,33 @@
import { Template } from './campaign-monitor.types';

export const getTemplateId = (template: string): string => {
export const getTemplateId = (template: string, language: 'nl' | 'en'): string => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const getTemplateId = (template: string, language: 'nl' | 'en'): string => {
export const getTemplateId = (template: string, language: Locale): string => {

.CAMPAIGN_MONITOR_TEMPLATE_MATERIAL_REQUEST_MAINTAINER as string,
[Template.EMAIL_CONFIRMATION]: process.env.CAMPAIGN_MONITOR_TEMPLATE_CONFIRMATION as string,
[Template.SHARE_FOLDER]: process.env[
`CAMPAIGN_MONITOR_EMAIL_TEMPLATE_SHARE_FOLDER__${language.toUpperCase()}`
Copy link
Contributor

Choose a reason for hiding this comment

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

if you change environment variables here, you should also update them in the .env.template file

CAMPAIGN_MONITOR_TEMPLATE_VISIT_DENIED=

@@ -13,6 +13,7 @@ export enum Template {
MATERIAL_REQUEST_REQUESTER = 'materialRequestRequester',
MATERIAL_REQUEST_MAINTAINER = 'materialRequestMaintainer',
EMAIL_CONFIRMATION = 'emailConfirmation',
CREATE_VISIT_REQUEST = 'createVisitRequest',
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you added this one?
Was it missing before?
Because CAMPAIGN_MONITOR_EMAIL_TEMPLATE_CREATE_VISIT_REQUEST_FOR_USER__ sounds like a feature that is out of scope:

Start nieuwe toegangsaanvraag voor gebruiker (als een CP-admin)

await this.campaignMonitorService.sendTransactionalMail(emailInfo);
await this.campaignMonitorService.sendTransactionalMail(
emailInfo,
user?.getLanguage() || 'nl'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
user?.getLanguage() || 'nl'
user?.getLanguage() || Locale.Nl

data,
},
language
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

public async sendTransactionalMail(emailInfo: CampaignMonitorSendMailDto): Promise<void> {
public async sendTransactionalMail(
emailInfo: CampaignMonitorSendMailDto,
lang: 'nl' | 'en'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
lang: 'nl' | 'en'
lang: Locale,

@@ -281,7 +297,8 @@ export class CampaignMonitorService implements OnApplicationBootstrap {

let cmTemplateId: string;
if (Object.values(Template).includes(emailInfo.template as any)) {
cmTemplateId = getTemplateId(emailInfo.template);
cmTemplateId = getTemplateId(emailInfo.template, lang);
console.log(cmTemplateId);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove console.log

template: Template.VISIT_REQUEST_CP,
visitRequest: visitRequest,
},
user.getLanguage()
Copy link
Contributor

Choose a reason for hiding this comment

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

language is already passed for each recipient. There should be no need to pass this language param

@@ -236,7 +239,8 @@ export class NotificationsService {
*/
public async onApproveVisitRequest(
visitRequest: VisitRequest,
space: VisitorSpace
space: VisitorSpace,
language: 'en' | 'nl'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
language: 'en' | 'nl'
language: Locale,

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.

None yet

2 participants