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

fix: Check if sendgrid list exists before attempting to create new one #634

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion apps/api/src/campaign/campaign.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ import { NotificationService } from '../sockets/notifications/notification.servi
import { SendGridNotificationsProvider } from '../notifications/providers/notifications.sendgrid.provider'
import { EmailService } from '../email/email.service'
import { MarketingNotificationsService } from '../notifications/notifications.service'
import { SendGridParams } from '../notifications/providers/notifications.sendgrid.types'

describe('CampaignService', () => {
let service: CampaignService
let marketing: NotificationsProviderInterface<unknown>
let marketing: NotificationsProviderInterface<SendGridParams>

const mockCreateCampaign = {
slug: 'test-slug',
Expand Down Expand Up @@ -121,6 +122,14 @@ describe('CampaignService', () => {

jest.spyOn(marketing, 'createNewContactList').mockImplementation(async () => 'list-id')
jest.spyOn(marketing, 'updateContactList').mockImplementation(async () => '')
jest.spyOn(marketing, 'getContactLists').mockResolvedValue({
statusCode: 200,
headers: '',
body: {
result: [],
_metadata: {},
},
})
jest.spyOn(service, 'createCampaignNotificationList')

expect(await service.update(mockUpdateCampaign.id, updateData, mockCampaign)).toEqual(
Expand All @@ -133,6 +142,7 @@ describe('CampaignService', () => {
include: { campaignType: { select: { name: true, slug: true, category: true } } },
})
expect(service.createCampaignNotificationList).toHaveBeenCalledWith(updatedCampaign)

expect(marketing.createNewContactList).toHaveBeenCalledWith({
name: updatedCampaign.title,
})
Expand Down
14 changes: 11 additions & 3 deletions apps/api/src/campaign/campaign.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1037,9 +1037,17 @@

async createCampaignNotificationList(updated: { title: string; id: string }) {
// Generate list in the marketing platform
const listId = await this.marketingNotificationsService.provider.createNewContactList({
name: updated.title || updated.id,
})
let listId: string
const lists = await this.marketingNotificationsService.provider.getContactLists()
const campaginEmailLists = lists.body.result

Check warning on line 1042 in apps/api/src/campaign/campaign.service.ts

View workflow job for this annotation

GitHub Actions / Run API tests

'campaginEmailLists' is assigned a value but never used
const exists = campaignEmailLists.find((campaign) => campaign.name === updated.title)
if (exists) {
listId = exists.id
} else {
listId = await this.marketingNotificationsService.provider.createNewContactList({
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to silently create a list here? I would assume we should be only doing anything if a list already exists?

Copy link
Member Author

Choose a reason for hiding this comment

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

The current issue is that for some reason, email lists for some campaigns exist in Sendgrid database, but don't exist in ours NotificationList. This causes an attempt to create two email lists with the same name, which results in Bad Request whenever somebody attempts to subscribe for email notifications to particular campaign.

What this peace of code does is to check, whether there is already a Sendgrid list with the same name, before attempting to create a new one.

name: updated.title || updated.id,
})
}

const name = updated.title || ''

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type NotificationsInterfaceParams = {
RemoveFromUnsubscribedRes: unknown
AddToUnsubscribedRes: unknown
SendNotificationRes: unknown
contactListsRes: unknown
}

export abstract class NotificationsProviderInterface<
Expand All @@ -35,4 +36,5 @@ export abstract class NotificationsProviderInterface<
data: T['RemoveFromUnsubscribedParams'],
): Promise<T['RemoveFromUnsubscribedRes']>
abstract sendNotification(data: T['SendNotificationParams']): Promise<T['SendNotificationRes']>
abstract getContactLists(): Promise<T['contactListsRes']>
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Injectable, Logger } from '@nestjs/common'
import { ConfigService } from '@nestjs/config'
import sgClient from '@sendgrid/client'
import { NotificationsProviderInterface } from './notifications.interface.providers'
import { SendGridParams } from './notifications.sendgrid.types'
import { ContactListRes, SGClientResponse, SendGridParams } from './notifications.sendgrid.types'
import { ClientRequest } from '@sendgrid/client/src/request'
import { DateTime } from 'luxon'

Expand All @@ -25,6 +25,14 @@ export class SendGridNotificationsProvider
}
}

async getContactLists() {
const request = {
url: '/v3/marketing/lists',
method: 'GET',
} as ClientRequest
const [response] = await sgClient.request(request)
return response as SGClientResponse<ContactListRes>
}
async createNewContactList(data: SendGridParams['CreateListParams']) {
const request = {
url: `/v3/marketing/lists`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { ClientResponse } from '@sendgrid/mail'
import { MarketingTemplateHTMLFields } from '../marketing_templates/template.type'
import Response from '@sendgrid/helpers/classes/response'

export type SGClientResponse<T = object> = Response<T>

export type SendGridParams = {
// Parameters
Expand All @@ -22,6 +26,7 @@ export type SendGridParams = {
RemoveFromUnsubscribedRes: unknown
AddToUnsubscribedRes: unknown
SendNotificationRes: unknown
contactListsRes: SGClientResponse<ContactListRes>

// Implementation specific
ContactData: ContactData
Expand Down Expand Up @@ -81,3 +86,15 @@ type SendNotificationParams = {
type GetContactsInfoRes = {
[key: string]: { contact: { id: string; [key: string]: unknown; list_ids: string[] } }
}

type SendGridContactList = {
id: string
name: string
contact_count: number
_metadata: object
}

export type ContactListRes = {
result: SendGridContactList[]
_metadata: object
}
Loading