diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2dac85424cc658bc70ca0857fe2de994e0f6a40c..d3d6230536498168004e057eb19f9f67c5f717a0 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -35,7 +35,6 @@ build_branch:
DOCKER_DRIVER: overlay2
image: ${CI_DEPENDENCY_PROXY_DIRECT_GROUP_IMAGE_PREFIX}/docker:18.09
stage: build
- when: manual
only:
- merge_requests
script:
diff --git a/docker-compose.yml b/docker-compose.yml
index 629f0d153f8bfd1da935ed0763c1da96baf344e4..789f1e441ec6e334c071915157cb819bcf4bef7f 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -28,6 +28,9 @@ services:
ELASTICSEARCH_NODE: ${ELASTICSEARCH_NODE}
ELASTICSEARCH_USERNAME: ${ELASTICSEARCH_USERNAME}
ELASTICSEARCH_PASSWORD: ${ELASTICSEARCH_PASSWORD}
+ MC_API_KEY: ${MC_API_KEY}
+ MC_SERVER: ${MC_SERVER}
+ MC_LIST_ID: ${MC_LIST_ID}
restart: unless-stopped
networks:
- backend
diff --git a/src/admin/admin.controller.spec.ts b/src/admin/admin.controller.spec.ts
index 97ba73ce36e4fb5f24927bfea36ac140d918162d..675158f30b5261dc42d2ace11d146b28c37b5981 100644
--- a/src/admin/admin.controller.spec.ts
+++ b/src/admin/admin.controller.spec.ts
@@ -328,26 +328,6 @@ describe('AdminController', () => {
});
});
- describe('Search user newsletter subscription', () => {
- it('should return all subscribed users, empty string', async () => {
- expect((await controller.getNewsletterSubscriptions({ searchString: '' })).length).toBe(3);
- });
- it('should return all subscribed users, null input', async () => {
- expect((await controller.getNewsletterSubscriptions({ searchString: null })).length).toBe(3);
- });
- it('should find one user', async () => {
- expect((await controller.getNewsletterSubscriptions({ searchString: 'a@a.com' })).length).toBe(1);
- expect((await controller.getNewsletterSubscriptions({ searchString: 'a@a.com' }))[0].email).toBe('a@a.com');
- });
- it('should find no user', async () => {
- expect((await controller.getNewsletterSubscriptions({ searchString: 'adgdgsdg@a.com' })).length).toBe(0);
- });
- });
-
- it('should count user subscribed to newsletter', async () => {
- expect(await controller.countNewsletterSubscriptions()).toBe(246);
- });
-
describe('Search delete a user subscription', () => {
it('should return a deleted object', async () => {
expect((await controller.unsubscribeUserFromNewsletter('a@a.com')).email).toBe('a@a.com');
diff --git a/src/admin/admin.controller.ts b/src/admin/admin.controller.ts
index 0a9bf0cd9e6ea35381780f2e67de77cc8f77b256..415bad28c53aa1b8f35abd9301e1a5ea692041f0 100644
--- a/src/admin/admin.controller.ts
+++ b/src/admin/admin.controller.ts
@@ -251,24 +251,6 @@ export class AdminController {
});
}
- @UseGuards(JwtAuthGuard, RolesGuard)
- @Roles('admin')
- @ApiBearerAuth('JWT')
- @Post('searchNewsletterSubscriptions')
- public async getNewsletterSubscriptions(@Body() searchString: { searchString: string }) {
- if (searchString && searchString.searchString && searchString.searchString.length > 0)
- return this.newsletterService.searchNewsletterSubscription(searchString.searchString);
- else return this.newsletterService.findAll();
- }
-
- @UseGuards(JwtAuthGuard, RolesGuard)
- @Roles('admin')
- @ApiBearerAuth('JWT')
- @Get('countNewsletterSubscriptions')
- public async countNewsletterSubscriptions(): Promise<number> {
- return this.newsletterService.countNewsletterSubscriptions();
- }
-
@UseGuards(JwtAuthGuard, RolesGuard)
@Roles('admin')
@ApiBearerAuth('JWT')
diff --git a/src/newsletter/newsletter.service.spec.ts b/src/newsletter/newsletter.service.spec.ts
index c79529572d1af9aeadf4533f1c9166876d25f457..60c91fd90203b6e8ecb7172ef76e38c8dc179a7b 100644
--- a/src/newsletter/newsletter.service.spec.ts
+++ b/src/newsletter/newsletter.service.spec.ts
@@ -51,24 +51,21 @@ describe('NewsletterService', () => {
});
describe('newsletterSubscribe', () => {
- it('it should not add subscription for email test2@test.com : already exist', async () => {
- const result = { email: 'test2@test.com' } as INewsletterSubscription;
+ it('it should add subscription for email test2@test.com even if it exists', async () => {
+ const _doc = { _id: 'a1aaaaa1a1', email: 'test2@test.com' } as INewsletterSubscription;
+ mailchimp.lists.setListMember.mockResolvedValueOnce({ email_address: 'test2@test.com' });
jest
.spyOn(service, 'findOne')
- .mockImplementationOnce(async (): Promise<INewsletterSubscription | undefined> => result);
- try {
- await service.newsletterSubscribe('test2@test.com');
- // Fail test if above expression doesn't throw anything.
- expect(true).toBe(false);
- } catch (e) {
- expect(e.message).toEqual('Email already exists');
- expect(e.status).toEqual(HttpStatus.BAD_REQUEST);
- }
+ .mockImplementationOnce(async (): Promise<INewsletterSubscription | undefined> => _doc);
+ mockNewsletterModel.create.mockResolvedValueOnce(_doc);
+
+ const subscription = await service.newsletterSubscribe('test2@test.com');
+ expect(subscription).toEqual(_doc);
});
it('it should add a subscription for email test2@test.com', async () => {
- const result: INewsletterSubscription = { email: 'test2@test.com' } as INewsletterSubscription;
+ const result = { email: 'test2@test.com' } as INewsletterSubscription;
const _doc = { _id: 'a1aaaaa1a1', email: 'test2@test.com' };
- mailchimp.lists.addListMember.mockResolvedValueOnce({ email_address: 'test2@test.com' });
+ mailchimp.lists.setListMember.mockResolvedValueOnce({ email_address: 'test2@test.com' });
jest
.spyOn(service, 'findOne')
.mockImplementationOnce(async (): Promise<INewsletterSubscription | undefined> => undefined)
@@ -76,12 +73,12 @@ describe('NewsletterService', () => {
mockNewsletterModel.create.mockResolvedValueOnce(_doc);
const subscription = await service.newsletterSubscribe('test2@test.com');
- expect(subscription).toEqual({ email: 'test2@test.com' });
+ expect(subscription).toEqual(_doc);
});
- it('it should return mailchimp 400 issue', async () => {
- const result: INewsletterSubscription = { email: 'test2@test.com' } as INewsletterSubscription;
+ it('it should return error if mailchimp 400 issue', async () => {
+ const result = { email: 'test2@test.com' } as INewsletterSubscription;
const _doc = { _id: 'a1aaaaa1a1', email: 'test2@test.com' };
- mailchimp.lists.addListMember.mockRejectedValueOnce({ status: 400 });
+ mailchimp.lists.setListMember.mockRejectedValueOnce({ status: 400 });
jest
.spyOn(service, 'findOne')
.mockImplementationOnce(async (): Promise<INewsletterSubscription | undefined> => undefined)
@@ -92,14 +89,14 @@ describe('NewsletterService', () => {
await service.newsletterSubscribe('test2@test.com');
expect(true).toBe(false);
} catch (e) {
- expect(e.message).toBe('Email already exists');
- expect(e.status).toEqual(HttpStatus.BAD_REQUEST);
+ expect(e.message).toBe('Subscribe error');
+ expect(e.status).toEqual(HttpStatus.INTERNAL_SERVER_ERROR);
}
});
- it('it should return mailchimp 500 issue', async () => {
- const result: INewsletterSubscription = { email: 'test2@test.com' } as INewsletterSubscription;
+ it('it should return error if mailchimp 500 issue', async () => {
+ const result = { email: 'test2@test.com' } as INewsletterSubscription;
const _doc = { _id: 'a1aaaaa1a1', email: 'test2@test.com' };
- mailchimp.lists.addListMember.mockRejectedValueOnce({ status: 500 });
+ mailchimp.lists.setListMember.mockRejectedValueOnce({ status: 500 });
jest
.spyOn(service, 'findOne')
.mockImplementationOnce(async (): Promise<INewsletterSubscription | undefined> => undefined)
@@ -110,28 +107,27 @@ describe('NewsletterService', () => {
await service.newsletterSubscribe('test2@test.com');
expect(true).toBe(false);
} catch (e) {
- expect(e.message).toBe('Server error');
+ expect(e.message).toBe('Subscribe error');
expect(e.status).toEqual(HttpStatus.INTERNAL_SERVER_ERROR);
}
});
});
describe('newsletterUnsubscribe', () => {
it('it should not remove subscription for email test@test.com : does not exist', async () => {
- const result: INewsletterSubscription = undefined;
- jest
- .spyOn(service, 'findOne')
- .mockImplementationOnce(async (): Promise<INewsletterSubscription | undefined> => result);
+ mailchimp.lists.getListMember.mockRejectedValueOnce({ status: 404 });
try {
await service.newsletterUnsubscribe('test@test.com');
// Fail test if above expression doesn't throw anything.
expect(true).toBe(false);
} catch (e) {
- expect(e.message).toEqual('Invalid email');
- expect(e.status).toEqual(HttpStatus.BAD_REQUEST);
+ expect(e.message).toEqual('Email not found');
+ expect(e.status).toEqual(HttpStatus.NOT_FOUND);
}
});
it('it should remove a subscription for email test2@test.com', async () => {
const _doc = { _id: 'a1aaaaa1a1', email: 'test2@test.com' };
+ mailchimp.lists.getListMember.mockResolvedValueOnce({ email_address: 'test2@test.com' });
+ mailchimp.lists.setListMember.mockResolvedValueOnce({ email_address: 'test2@test.com' });
const result = {
email: 'test2@test.com',
deleteOne: async () => _doc,
@@ -145,15 +141,6 @@ describe('NewsletterService', () => {
});
});
- describe('countNewsletterSubscriptions', () => {
- it('it should count subscriptions', async () => {
- mockNewsletterModel.countDocuments.mockResolvedValueOnce(69);
- const count = await service.countNewsletterSubscriptions();
-
- expect(count).toEqual(69);
- });
- });
-
describe('findOne', () => {
it('it should not find a subscription with email test@test.com', async () => {
mockNewsletterModel.findOne.mockResolvedValueOnce(undefined);
@@ -175,18 +162,6 @@ describe('NewsletterService', () => {
expect(findOneEmail).toEqual(_docs);
});
});
- describe('searchNewsletterSubscription', () => {
- it('it should find 2 search result', async () => {
- const _docs = [
- { _id: 'a1aaaaa1a1', email: 'test2@test.com' } as INewsletterSubscription,
- { _id: 'bbbbb', email: 'test@test.com' } as INewsletterSubscription,
- ];
- mockNewsletterModel.find.mockResolvedValueOnce(_docs);
- const findOneEmail = await service.searchNewsletterSubscription('test');
- expect(findOneEmail.length).toBe(2);
- });
- });
-
describe('updateNewsletterSubscription', () => {
it('should update existing user subscription', () => {
mailchimp.lists.getListMembersInfo.mockResolvedValueOnce({ total_items: 10 }).mockResolvedValueOnce({
@@ -198,10 +173,9 @@ describe('NewsletterService', () => {
});
const result = { email: 'test2@test.com' } as INewsletterSubscription;
const spyer = jest.spyOn(mockNewsletterModel, 'findOne');
- // jest.spyOn(service, 'findOne').mockResolvedValueOnce(result).mockResolvedValueOnce(result);
mockNewsletterModel.findOne.mockResolvedValueOnce(result).mockResolvedValueOnce(null);
service.updateNewsletterSubscription();
- expect(spyer).toBeCalledTimes(2);
+ expect(spyer).toBeCalledTimes(3);
// expect(spyerDelete).toBeCalledTimes(1);
});
});
diff --git a/src/newsletter/newsletter.service.ts b/src/newsletter/newsletter.service.ts
index 7efffd693731a043e1b0566dde4f9c2e3fc25b48..984de7a3926703550808b158ed2b84ef27d37914 100644
--- a/src/newsletter/newsletter.service.ts
+++ b/src/newsletter/newsletter.service.ts
@@ -2,11 +2,12 @@ import { HttpException, HttpStatus, Injectable, Logger } from '@nestjs/common';
import { InjectModel } from '@nestjs/mongoose';
import { Cron, CronExpression } from '@nestjs/schedule';
import { Model } from 'mongoose';
+import { md5 } from '../shared/utils';
import { IMailchimpSubscription } from './interface/mailchimp-subscription';
import { INewsletterSubscription } from './interface/newsletter-subscription.interface';
-import { NewsletterSubscription, NewsletterSubscriptionDocument } from './newsletter-subscription.schema';
-// eslint-disable-next-line @typescript-eslint/no-var-requires
-const mailchimp = require('@mailchimp/mailchimp_marketing');
+import { NewsletterSubscription } from './newsletter-subscription.schema';
+import mailchimp = require('@mailchimp/mailchimp_marketing');
+
@Injectable()
export class NewsletterService {
private readonly logger = new Logger(NewsletterService.name);
@@ -41,40 +42,60 @@ export class NewsletterService {
}
public async newsletterSubscribe(email: string): Promise<NewsletterSubscription> {
- this.logger.debug('newsletterSubscribe');
- const existingEmail = await this.findOne(email);
- if (existingEmail) {
- throw new HttpException('Email already exists', HttpStatus.BAD_REQUEST);
- }
+ this.logger.debug(`newsletterSubscribe: ${email}`);
+ email = email.toLocaleLowerCase();
try {
- const member = await mailchimp.lists.addListMember(this.LIST_ID, {
+ // Add or update list member (to be able to subscribe again a member who had already subscribed then unsubscribed)
+ // (the second parameter is the MD5 hash of the lowercase email, we actually don't need to maintain a mapping in newsletterSubscription : cf. https://mailchimp.com/developer/marketing/docs/methods-parameters/#path-parameters )
+ const member = await mailchimp.lists.setListMember(this.LIST_ID, md5(email), {
email_address: email,
+ status_if_new: 'subscribed', // cf. https://mailchimp.com/developer/marketing/api/list-members/add-or-update-list-member/
status: 'subscribed',
});
- await this.newsletterSubscriptionModel.create({ email: email, mailchimpId: member.id });
- return this.findOne(email);
- } catch (e) {
- if (e.status === 400) {
- this.logger.error(`Error ${e.status}, user might already exist in mailchimplist`);
- throw new HttpException('Email already exists', HttpStatus.BAD_REQUEST);
- } else {
- this.logger.error(`Mailchimp configuration error`);
- throw new HttpException('Server error', HttpStatus.INTERNAL_SERVER_ERROR);
+
+ // We may not be aware that the user has unsubscribed from the newsletter, so it is ok if it already exists in newsletterSubscription
+ let newsletterSubscription = await this.findOne(email);
+ if (!newsletterSubscription) {
+ newsletterSubscription = await this.newsletterSubscriptionModel.create({
+ email: email,
+ mailchimpId: member.id,
+ });
}
+ return newsletterSubscription;
+ } catch (e) {
+ this.logger.error(`newsletterSubscribe ${email}: ${JSON.stringify(e)}`);
+ throw new HttpException('Subscribe error', HttpStatus.INTERNAL_SERVER_ERROR);
}
}
public async newsletterUnsubscribe(email: string): Promise<NewsletterSubscription> {
- this.logger.debug('newsletterUnsubscribe');
- const subscription = await this.findOne(email);
- if (!subscription) {
- throw new HttpException('Invalid email', HttpStatus.BAD_REQUEST);
+ this.logger.debug(`newsletterUnsubscribe: ${email}`);
+ email = email.toLocaleLowerCase();
+ const emailMd5Hashed = md5(email);
+
+ let newsletterSubscription = await this.findOne(email);
+ if (newsletterSubscription) {
+ newsletterSubscription = newsletterSubscription.deleteOne();
}
- await mailchimp.lists.setListMember(this.LIST_ID, subscription.mailchimpId, {
- email_address: email,
- status: 'unsubscribed',
- });
- return subscription.deleteOne();
+
+ try {
+ const response = await mailchimp.lists.getListMember(this.LIST_ID, emailMd5Hashed);
+ if (response.status === 'unsubscribed') {
+ throw new HttpException('Email not found', HttpStatus.NOT_FOUND);
+ }
+ await mailchimp.lists.setListMember(this.LIST_ID, emailMd5Hashed, {
+ status: 'unsubscribed',
+ });
+ } catch (e) {
+ if (e.status === 404) {
+ throw new HttpException('Email not found', HttpStatus.NOT_FOUND);
+ } else {
+ this.logger.error(`newsletterUnsubscribe ${email}: ${JSON.stringify(e)}`);
+ throw new HttpException('Unsubscribe error', HttpStatus.INTERNAL_SERVER_ERROR);
+ }
+ }
+
+ return newsletterSubscription;
}
public async findOne(mail: string): Promise<INewsletterSubscription | undefined> {
@@ -82,16 +103,6 @@ export class NewsletterService {
return this.newsletterSubscriptionModel.findOne({ email: mail });
}
- public async searchNewsletterSubscription(searchString: string): Promise<NewsletterSubscriptionDocument[]> {
- this.logger.debug('searchNewsletterSubscription');
- return this.newsletterSubscriptionModel.find({ email: new RegExp(searchString, 'i') });
- }
-
- public async countNewsletterSubscriptions(): Promise<number> {
- this.logger.debug('countNewsletterSubscriptions');
- return this.newsletterSubscriptionModel.countDocuments({});
- }
-
public async findAll(): Promise<NewsletterSubscription[]> {
this.logger.debug('findAll');
return this.newsletterSubscriptionModel.find();
diff --git a/src/shared/utils.ts b/src/shared/utils.ts
index 0283a1a3a31d32c1b4dca17fa8abce7b7d5ac955..625df75b54351cdb931f234bcd6ed1c385147f24 100644
--- a/src/shared/utils.ts
+++ b/src/shared/utils.ts
@@ -4,6 +4,9 @@ import { Page } from '../pages/schemas/page.schema';
import { Post } from '../posts/schemas/post.schema';
import { UserRole } from '../users/enum/user-role.enum';
import { User } from '../users/schemas/user.schema';
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const crypto = require('crypto');
+export const md5 = (data: string): string => crypto.createHash('md5').update(data).digest('hex');
export function rewriteGhostImgUrl(configService: ConfigurationService, itemData: Page | Post): Page | Post {
// Handle image display. Rewrite image URL to fit ghost infra issue.