diff --git a/test/api/unit/libs/email.test.js b/test/api/unit/libs/email.test.js index 8673e03b62..1b34f8d03f 100644 --- a/test/api/unit/libs/email.test.js +++ b/test/api/unit/libs/email.test.js @@ -171,23 +171,23 @@ describe('emails', () => { expect(got.post).not.to.be.called; }); - it('throws error when mail target is only a string', () => { + it('throws error when mail target is only a string', async () => { const emailType = 'an email type'; const mailingInfo = 'my email'; - expect(sendTxn(mailingInfo, emailType)).to.throw; + await expect(sendTxn(mailingInfo, emailType)).to.be.rejectedWith('Argument Error mailingInfoArray: does not contain email or _id'); }); - it('throws error when mail target has no _id or email', () => { + it('throws error when mail target has no _id or email', async () => { const emailType = 'an email type'; const mailingInfo = { }; - expect(sendTxn(mailingInfo, emailType)).to.throw; + await expect(sendTxn(mailingInfo, emailType)).to.be.rejectedWith('Argument Error mailingInfoArray: does not contain email or _id'); }); - it('throws error when variables not an array', () => { + it('throws error when variables not an array', async () => { const emailType = 'an email type'; const mailingInfo = { name: 'my name', @@ -195,9 +195,10 @@ describe('emails', () => { }; const variables = {}; - expect(sendTxn(mailingInfo, emailType, variables)).to.throw; + await expect(sendTxn(mailingInfo, emailType, variables)).to.be.rejectedWith('Argument Error variables: is not an array'); }); - it('throws error when variables array not contain name/content', () => { + + it('throws error when variables array not contain name/content', async () => { const emailType = 'an email type'; const mailingInfo = { name: 'my name', @@ -209,8 +210,9 @@ describe('emails', () => { }, ]; - expect(sendTxn(mailingInfo, emailType, variables)).to.throw; + await expect(sendTxn(mailingInfo, emailType, variables)).to.be.rejectedWith('Argument Error variables: does not contain name or content'); }); + it('throws no error when variables array contain name but no content', () => { const emailType = 'an email type'; const mailingInfo = { diff --git a/website/server/controllers/api-v3/quests.js b/website/server/controllers/api-v3/quests.js index 1c2e94b457..f9152bc5bc 100644 --- a/website/server/controllers/api-v3/quests.js +++ b/website/server/controllers/api-v3/quests.js @@ -120,9 +120,12 @@ api.inviteToQuest = { // send out invites const inviterVars = getUserInfo(user, ['name', 'email']); - const membersToEmail = members.filter(async member => { + const membersToEmail = []; + + for (const member of members) { // send push notifications while filtering members before sending emails if (member.preferences.pushNotifications.invitedQuest !== false) { + // eslint-disable-next-line no-await-in-loop await sendPushNotification( member, { @@ -141,8 +144,11 @@ api.inviteToQuest = { quest, }); - return member.preferences.emailNotifications.invitedQuest !== false; - }); + if (member.preferences.emailNotifications.invitedQuest !== false) { + membersToEmail.push(member); + } + } + sendTxnEmail(membersToEmail, `invite-${quest.boss ? 'boss' : 'collection'}-quest`, [ { name: 'QUEST_NAME', content: quest.text() }, { name: 'INVITER', content: inviterVars.name }, diff --git a/website/server/libs/email.js b/website/server/libs/email.js index f9e66e9e0b..bca8b4b294 100644 --- a/website/server/libs/email.js +++ b/website/server/libs/email.js @@ -65,7 +65,13 @@ export function getGroupUrl (group) { return groupUrl; } -// Send a transactional email using Mandrill through the external email server +/** + * Send a transactional email using Mandrill through the external email server + * + * Individual "canSend" per type needs to be done before, + * internally it checks by `getUserInfo` if the + * `unsubscribeFromAll` is set to true, if so it won't send the email. + */ export async function sendTxn (mailingInfoArray, emailType, variables, personalVariables) { if (!Array.isArray(mailingInfoArray)) { mailingInfoArray = [mailingInfoArray]; // eslint-disable-line no-param-reassign @@ -73,7 +79,7 @@ export async function sendTxn (mailingInfoArray, emailType, variables, personalV for (const entry of mailingInfoArray) { if (typeof entry === 'string' - && (typeof entry._id === 'undefined' && typeof entry.email === 'undefined') + || (typeof entry._id === 'undefined' && typeof entry.email === 'undefined') ) { throw new Error('Argument Error mailingInfoArray: does not contain email or _id'); } @@ -100,7 +106,7 @@ export async function sendTxn (mailingInfoArray, emailType, variables, personalV // Always send reset-password emails // Don't check canSend for non registered users as already checked before .filter(mailingInfo => mailingInfo.email - && (!mailingInfo._id || mailingInfo.canSend || emailType === 'reset-password')); + && (!mailingInfo._id || mailingInfo.canSend || emailType === 'reset-password')); // Personal variables are personal to each email recipient, if they are missing // we manually create a structure for them with RECIPIENT_NAME and RECIPIENT_UNSUB_URL diff --git a/website/server/models/group.js b/website/server/models/group.js index 0061fbb25c..5bbd296714 100644 --- a/website/server/models/group.js +++ b/website/server/models/group.js @@ -770,22 +770,27 @@ schema.methods.startQuest = async function startQuest (user) { const membersToEmail = []; // send notifications and webhooks in the background without blocking - await members.forEach(async member => { - if (member._id !== user._id) { - // send push notifications and filter users that disabled emails - if (member.preferences.emailNotifications.questStarted !== false) { - membersToEmail.push(member); - } + for (const member of members) { + if (member._id === user._id) { + // early "exit", saving one indention level + // eslint-disable-next-line no-continue + continue; + } - // send push notifications and filter users that disabled emails - if (member.preferences.pushNotifications.questStarted !== false) { - const memberLang = member.preferences.language; - await sendPushNotification(member, { - title: quest.text(memberLang), - message: shared.i18n.t('questStarted', memberLang), - identifier: 'questStarted', - }); - } + // add email to send if that user did not disabled this email + if (member.preferences.emailNotifications.questStarted !== false) { + membersToEmail.push(member); + } + + // send push notifications if that user did not disabled this notifications + if (member.preferences.pushNotifications.questStarted !== false) { + const memberLang = member.preferences.language; + // eslint-disable-next-line no-await-in-loop + await sendPushNotification(member, { + title: quest.text(memberLang), + message: shared.i18n.t('questStarted', memberLang), + identifier: 'questStarted', + }); } // Send webhooks @@ -794,7 +799,7 @@ schema.methods.startQuest = async function startQuest (user) { group: this, quest, }); - }); + } // Send emails in bulk sendTxnEmail(membersToEmail, 'quest-started', [