diff --git a/src/mail-scheduler/index.ts b/src/mail-scheduler/index.ts index 43a519097..af83724d9 100644 --- a/src/mail-scheduler/index.ts +++ b/src/mail-scheduler/index.ts @@ -1,10 +1,5 @@ import { ConfigVariable, config } from '../utils/config' -import { - EmailDeliveryAttempt, - EmailFailure, - EmailSuccess, - NotificationEmailDelivery, -} from '../model' +import { EmailDeliveryAttempt, NotificationEmailDelivery } from '../model' import { EntityManager } from 'typeorm' import { globalEm } from '../utils/globalEm' import { uniqueId } from '../utils/crypto' @@ -28,40 +23,30 @@ export async function mailsToDeliver(em: EntityManager): Promise= (await getMaxAttempts(em))) { notificationDelivery.discard = true } } - await em.save(notificationDelivery) + await em.save(newAttempt) } + await em.save(newEmailDeliveries) } diff --git a/src/mail-scheduler/tests/scheduler.test.ts b/src/mail-scheduler/tests/scheduler.test.ts index 75faa52ab..41e0c2fe2 100644 --- a/src/mail-scheduler/tests/scheduler.test.ts +++ b/src/mail-scheduler/tests/scheduler.test.ts @@ -2,14 +2,33 @@ import { expect } from 'chai' import { NotificationEmailDelivery, Notification } from '../../model' import { clearDb, populateDbWithSeedData } from './testUtils' import { globalEm } from '../../utils/globalEm' -import { getMaxAttempts, sendNew } from '..' +import { getMaxAttempts, deliverEmails } from '..' import { EntityManager } from 'typeorm' import { RUNTIME_NOTIFICATION_ID_TAG } from '../../utils/notification/helpers' +const getDeliveryFromNotificationId = async (em: EntityManager, notificationId: string) => { + const res = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ + where: { notification: { id: notificationId } }, + relations: { attempts: true }, + }) + return res +} + +const correctRecipientAccountEmail = async (em: EntityManager, notificationId: string) => { + const accountId = (await em.getRepository(Notification).findOneByOrFail({ id: notificationId })) + .accountId + let account = await em.getRepository('Account').findOneByOrFail({ id: accountId }) + account.email = `correct-${accountId}@example.com` + await em.save(account) +} + describe('Scheduler', () => { - const okNotificationId = RUNTIME_NOTIFICATION_ID_TAG + '-1' - const errNotificationId = RUNTIME_NOTIFICATION_ID_TAG + '-2' - const okAtSecondNotificationId = RUNTIME_NOTIFICATION_ID_TAG + '-3' + const okNotificationId = RUNTIME_NOTIFICATION_ID_TAG + '-0' + const errNotificationId = RUNTIME_NOTIFICATION_ID_TAG + '-1' + const okAtSecondNotificationId = RUNTIME_NOTIFICATION_ID_TAG + '-2' + let successfulDelivery: NotificationEmailDelivery + let failingDelivery: NotificationEmailDelivery + let successfulAtSecondDelivery: NotificationEmailDelivery let em: EntityManager let maxAttempts: number @@ -23,142 +42,116 @@ describe('Scheduler', () => { await clearDb() }) - describe('case success at first attempt', () => { - let emailDelivery: NotificationEmailDelivery + describe('first run of deliverEmails', () => { before(async () => { - await sendNew() - - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, - }) + await correctRecipientAccountEmail(em, okNotificationId) + await deliverEmails() + successfulDelivery = await getDeliveryFromNotificationId(em, okNotificationId) + failingDelivery = await getDeliveryFromNotificationId(em, errNotificationId) + successfulAtSecondDelivery = await getDeliveryFromNotificationId(em, okAtSecondNotificationId) + }) + describe('successful delivery case: 👍', () => { it('should change email delivery to discard when success', async () => { - expect(emailDelivery.discard).to.be.true + expect(successfulDelivery.discard).to.be.true }) it('should create a successful attempt', async () => { - expect(emailDelivery.attempts).to.have.lengthOf(1) - expect(emailDelivery.attempts[0].notificationDeliveryId).to.equal(emailDelivery.id) - expect(emailDelivery.attempts[0].status.isTypeOf).to.equal('SuccessfulDelivery') + expect(successfulDelivery.attempts).to.have.lengthOf(1) + expect(successfulDelivery.attempts[0].notificationDeliveryId).to.equal( + successfulDelivery.id + ) + expect(successfulDelivery.attempts[0].status.isTypeOf).to.equal('EmailSuccess') }) }) - describe('case failure at first attempt', async () => { - let emailDelivery: NotificationEmailDelivery - before(async () => { - await sendNew() - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, - }) - }) - + describe('failing delivery case: 👎', async () => { it('should keep email delivery discard to false', () => { - expect(emailDelivery.discard).to.be.false + expect(failingDelivery.discard).to.be.false }) it('should create a failed attempt', () => { - expect(emailDelivery.attempts).to.have.lengthOf(1) - expect(emailDelivery.attempts[0].notificationDeliveryId).to.equal(emailDelivery.id) - expect(emailDelivery.attempts[0].status.isTypeOf).to.equal('FailedDelivery') + expect(failingDelivery.attempts).to.have.lengthOf(1) + expect(failingDelivery.attempts[0].notificationDeliveryId).to.equal(failingDelivery.id) + expect(failingDelivery.attempts[0].status.isTypeOf).to.equal('EmailFailure') }) }) - describe('case second attempt is again a failure', async () => { - let emailDelivery: NotificationEmailDelivery - before(async () => { - await sendNew() - await sendNew() - - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, - }) - }) + describe('successful at second attempt delivery case: 👎', async () => { it('should keep email delivery discard to false', () => { - expect(emailDelivery.discard).to.be.false + expect(successfulAtSecondDelivery.discard).to.be.false }) - it('should create another failed attempt', () => { - expect(emailDelivery.attempts).to.have.lengthOf(2) - expect(emailDelivery.attempts[0].notificationDeliveryId).to.equal(emailDelivery.id) - expect(emailDelivery.attempts[0].status.isTypeOf).to.equal('FailedDelivery') - expect(emailDelivery.attempts[1].status.isTypeOf).to.equal('FailedDelivery') + it('should create a failed attempt', () => { + expect(successfulAtSecondDelivery.attempts).to.have.lengthOf(1) + expect(successfulAtSecondDelivery.attempts[0].notificationDeliveryId).to.equal( + successfulAtSecondDelivery.id + ) + expect(successfulAtSecondDelivery.attempts[0].status.isTypeOf).to.equal('EmailFailure') }) }) - describe('case max attempts reached', () => { - let emailDelivery: NotificationEmailDelivery + describe('second run of deliverEmails', () => { before(async () => { - await sendNew() // first attempt - const maxAttempts = await getMaxAttempts(em) - - for (let i = 0; i < maxAttempts - 1; i++) { - await sendNew() - } - - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, + await correctRecipientAccountEmail(em, okAtSecondNotificationId) + await deliverEmails() + failingDelivery = await getDeliveryFromNotificationId(em, errNotificationId) + successfulAtSecondDelivery = await getDeliveryFromNotificationId( + em, + okAtSecondNotificationId + ) + }) + describe('failing delivery case: 👎', async () => { + it('should keep email delivery discard to false', () => { + expect(failingDelivery.discard).to.be.false }) - }) - - it('should set the email delivery discard to true after MAX_ATTEMPTS', async () => { - expect(emailDelivery.discard).to.be.true - }) - it('should create at most MAX_ATTEMPTS failed attempts', async () => { - const attemptsStatus = emailDelivery.attempts.map((attempt) => attempt.status.isTypeOf) - expect(emailDelivery.attempts).to.have.lengthOf(maxAttempts) - expect(attemptsStatus.every((status) => status === 'EmailFailure')).to.be.true - }) - it('calling the sending function once more should not change anything', async () => { - await sendNew() - - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, + it('should create another failed attempt', () => { + expect(failingDelivery.attempts).to.have.lengthOf(2) + expect(failingDelivery.attempts[0].notificationDeliveryId).to.equal(failingDelivery.id) + expect(failingDelivery.attempts[0].status.isTypeOf).to.equal('EmailFailure') + expect(failingDelivery.attempts[1].status.isTypeOf).to.equal('EmailFailure') }) - - const attemptsStatus = emailDelivery.attempts.map((attempt) => attempt.status.isTypeOf) - expect(emailDelivery.attempts).to.have.lengthOf(maxAttempts) - expect(attemptsStatus.every((status) => status === 'EmailFailure')).to.be.true - }) - }) - describe('case success at second attempt', () => { - let emailDelivery: NotificationEmailDelivery - before(async () => { - await sendNew() // first attempt }) - describe('should be failuer at first attempt', async () => { - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, - }) - - it('should keep email delivery discard to false', () => { - expect(emailDelivery.discard).to.be.false + describe('successful at second delivery case: 👍', () => { + it('should change email delivery to discard when success', async () => { + expect(successfulAtSecondDelivery.discard).to.be.true }) - it('should create a failed attempt', () => { - expect(emailDelivery.attempts[-1].notificationDeliveryId).to.equal(emailDelivery.id) - expect(emailDelivery.attempts[-1].status.isTypeOf).to.equal('FailedDelivery') + it('should create a new successful attempt', async () => { + expect(successfulAtSecondDelivery.attempts).to.have.lengthOf(2) + expect(successfulAtSecondDelivery.attempts[1].notificationDeliveryId).to.equal( + successfulAtSecondDelivery.id + ) + expect(successfulAtSecondDelivery.attempts[1].status.isTypeOf).to.equal('EmailSuccess') }) }) - describe('should be success at second attempt', async () => { - const accountId = ( - await em.getRepository(Notification).findOneByOrFail({ id: okAtSecondNotificationId }) - ).account.id - const account = await em.getRepository('Account').findOneByOrFail({ id: accountId }) - account.email = 'correct@example.com' - await em.save(account) - - await sendNew() // second attempt - - emailDelivery = await em.getRepository(NotificationEmailDelivery).findOneOrFail({ - where: { notification: { id: errNotificationId } }, - relations: { attempts: true }, + describe('MAX_ATTEMPTS - 2 runs of deliverEmails', () => { + before(async () => { + const maxAttempts = await getMaxAttempts(em) + expect(maxAttempts).to.be.greaterThan(2) + for (let i = 0; i < maxAttempts - 2; i++) { + await deliverEmails() + } + failingDelivery = await getDeliveryFromNotificationId(em, errNotificationId) }) - - it('should change email delivery to discard when success', async () => { - expect(emailDelivery.discard).to.be.true + describe('failing delivery case: 👎', async () => { + it('should set the email delivery discard to true after MAX_ATTEMPTS', async () => { + expect(failingDelivery.discard).to.be.true + }) + it('should create MAX_ATTEMPTS failed attempts', async () => { + const attemptsStatus = failingDelivery.attempts.map( + (attempt) => attempt.status.isTypeOf + ) + expect(failingDelivery.attempts).to.have.lengthOf(maxAttempts) + expect(attemptsStatus.every((status) => status === 'EmailFailure')).to.be.true + }) }) - - it('should create a successful attempt', async () => { - expect(emailDelivery.attempts[-1].notificationDeliveryId).to.equal(emailDelivery.id) - expect(emailDelivery.attempts[-1].status.isTypeOf).to.equal('SuccessfulDelivery') + describe('one more run of deliverEmails', () => { + before(async () => { + await deliverEmails() + failingDelivery = await getDeliveryFromNotificationId(em, errNotificationId) + }) + describe('failing delivery case: 👎', async () => { + it('should not create more MAX_ATTEMPTS failed attempts', async () => { + const attemptsStatus = failingDelivery.attempts.map( + (attempt) => attempt.status.isTypeOf + ) + expect(failingDelivery.attempts).to.have.lengthOf(maxAttempts) + expect(attemptsStatus.every((status) => status === 'EmailFailure')).to.be.true + }) + }) }) }) }) diff --git a/src/mail-scheduler/tests/seedData.test.ts b/src/mail-scheduler/tests/seedData.test.bak similarity index 89% rename from src/mail-scheduler/tests/seedData.test.ts rename to src/mail-scheduler/tests/seedData.test.bak index f40c9907a..9358deb45 100644 --- a/src/mail-scheduler/tests/seedData.test.ts +++ b/src/mail-scheduler/tests/seedData.test.bak @@ -28,12 +28,10 @@ describe('Database seed data tests', () => { expect(account?.membership.handle).to.equal('handle-1') }) it('check that notification delivery entity is correct', async () => { - const result = await em - .getRepository(NotificationEmailDelivery) - .findOne({ - where: { notification: { id: RUNTIME_NOTIFICATION_ID_TAG + '-1' } }, - relations: { notification: { account: true } }, - }) + const result = await em.getRepository(NotificationEmailDelivery).findOne({ + where: { notification: { id: RUNTIME_NOTIFICATION_ID_TAG + '-1' } }, + relations: { notification: { account: true } }, + }) expect(result).to.not.be.null expect(result?.notification.account).to.not.be.null diff --git a/src/mail-scheduler/tests/testUtils.ts b/src/mail-scheduler/tests/testUtils.ts index 64b988603..0466aaf43 100644 --- a/src/mail-scheduler/tests/testUtils.ts +++ b/src/mail-scheduler/tests/testUtils.ts @@ -8,6 +8,7 @@ import { NotificationEmailDelivery, GatewayConfig, AuctionWon, + EmailDeliveryAttempt, } from '../../model' import { defaultNotificationPreferences } from '../../utils/notification' import { globalEm } from '../../utils/globalEm' @@ -15,7 +16,7 @@ import { idStringFromNumber } from '../../utils/misc' import { RUNTIME_NOTIFICATION_ID_TAG } from '../../utils/notification/helpers' import { uniqueId } from '../../utils/crypto' -export const NUM_ENTITIES = 5 +export const NUM_ENTITIES = 3 export async function populateDbWithSeedData() { const em = await globalEm @@ -42,7 +43,7 @@ export async function populateDbWithSeedData() { const account = await em.getRepository(Account).save({ id: idStringFromNumber(i), userId: user.id, - email: `testEmail_${i}@example.com`, + email: `incorrect${i}@example.com`, isEmailConfirmed: false, isBlocked: false, registeredAt: member.createdAt, @@ -72,6 +73,7 @@ export async function populateDbWithSeedData() { export async function clearDb(): Promise { const em = await globalEm + await em.getRepository(EmailDeliveryAttempt).delete({}) await em.getRepository(NotificationEmailDelivery).delete({}) await em.getRepository(Notification).delete({}) await em.getRepository(Account).delete({}) diff --git a/src/mail-scheduler/utils.ts b/src/mail-scheduler/utils.ts index eb502e48f..e234a94ec 100644 --- a/src/mail-scheduler/utils.ts +++ b/src/mail-scheduler/utils.ts @@ -71,6 +71,9 @@ export async function sendGridSend({ content, }: SendMailArgs): Promise { const apiKey = process.env.SENDGRID_API_KEY + if (process.env.TESTING === 'true' || process.env.TESTING === '1') { + return mockSend(to) + } if (apiKey) { sgMail.setApiKey(apiKey) } @@ -106,3 +109,21 @@ export async function sendGridSend({ return sendGrideFailure as SendGridResponseFailure } } + +const mockSend = (to: string): SendGridResponse => { + if (to.match(/incorrect/gi)) { + return { + type: new Error('Test error'), + success: false, + } + } else { + return { + type: { + statusCode: 202, + headers: {}, + body: {}, + }, + success: true, + } + } +}