From 8f4cdabf495fb00d380733efe80b64990c4b6e42 Mon Sep 17 00:00:00 2001 From: Mohammad Ranjbar Z Date: Tue, 22 Nov 2022 14:15:39 +0300 Subject: [PATCH 1/4] Add trackId and make creating notification idempotence related to #14 --- .../1660546929601-createNotification.ts | 6 ++ .../1660716115917-seedNotificationType.ts | 3 +- src/controllers/v1/notificationsController.ts | 10 ++- src/entities/notification.ts | 16 +++-- .../notificationRepository.test.ts | 9 ++- src/repositories/notificationRepository.ts | 50 ++++++-------- .../notificationSettingRepository.ts | 3 - src/routes/v1/notificationRouter.test.ts | 67 +++++++++++++++++-- src/types/requestResponses.ts | 16 ++--- src/utils/errorMessages.ts | 1 + src/validators/schemaValidators.ts | 1 + 11 files changed, 121 insertions(+), 61 deletions(-) diff --git a/migrations/1660546929601-createNotification.ts b/migrations/1660546929601-createNotification.ts index b1976a7..a51cbec 100644 --- a/migrations/1660546929601-createNotification.ts +++ b/migrations/1660546929601-createNotification.ts @@ -39,6 +39,12 @@ export class createNotification1660546929601 implements MigrationInterface { type: 'varchar', isNullable: true, }, + { + name: 'trackId', + type: 'varchar', + isNullable: true, + isUnique: true, + }, { name: 'isRead', type: 'boolean', diff --git a/migrations/1660716115917-seedNotificationType.ts b/migrations/1660716115917-seedNotificationType.ts index 41abb74..cfec29c 100644 --- a/migrations/1660716115917-seedNotificationType.ts +++ b/migrations/1660716115917-seedNotificationType.ts @@ -1200,7 +1200,8 @@ export const GivethNotificationTypes = { content: ' is not verified anymore', }, ], - content: 'The project that you boosted before {project name} is not verified anymore', + content: + 'The project that you boosted before {project name} is not verified anymore', }, MADE_DONATION: { name: 'Made donation', diff --git a/src/controllers/v1/notificationsController.ts b/src/controllers/v1/notificationsController.ts index 2e0368e..9bc6f8f 100644 --- a/src/controllers/v1/notificationsController.ts +++ b/src/controllers/v1/notificationsController.ts @@ -33,6 +33,7 @@ import { User } from '../../types/general'; import { countUnreadNotifications, createNotification, + findNotificationByTrackId, getNotifications, markNotificationGroupAsRead, markNotificationsAsRead, @@ -65,6 +66,12 @@ export class NotificationsController { const { userWalletAddress, projectId } = body; try { validateWithJoiSchema(body, sendNotificationValidator); + if (body.trackId && (await findNotificationByTrackId(body.trackId))) { + return { + success: true, + message: errorMessages.DUPLICATE_TRACK_ID, + }; + } const userAddress = await createNewUserAddressIfNotExists( userWalletAddress as string, ); @@ -132,9 +139,10 @@ export class NotificationsController { } await createNotification({ notificationType, - user: userAddress, + userAddress, email: body.email, emailStatus, + trackId: body?.trackId, metadata: body?.metadata, segmentData: body.segment, projectId, diff --git a/src/entities/notification.ts b/src/entities/notification.ts index b971ebf..f6ca2e3 100644 --- a/src/entities/notification.ts +++ b/src/entities/notification.ts @@ -13,12 +13,15 @@ import { NotificationType } from './notificationType'; import { UserAddress } from './userAddress'; export class NotificationMetadata { - // name of recipient - name?: string; + /** + * @example "title" + */ + projectTitle?: string; - // For donation emails it's needed - amount?: string; - currency?: string; + /** + * @example "project link" + */ + projectLink?: string; } export const EMAIL_STATUSES = { @@ -45,6 +48,9 @@ export class Notification extends BaseEntity { @Column('text', { nullable: true }) email?: string; + @Column('text', { nullable: true, unique: true }) + trackId?: string; + @Column('text', { nullable: true }) emailContent?: string; diff --git a/src/repositories/notificationRepository.test.ts b/src/repositories/notificationRepository.test.ts index 41232c0..cf31235 100644 --- a/src/repositories/notificationRepository.test.ts +++ b/src/repositories/notificationRepository.test.ts @@ -54,19 +54,18 @@ function createNotificationTestCases() { const result = await createNotification({ notificationType: notificationType!, - user: userAddress, + userAddress, email: 'test@example.com', emailStatus: EMAIL_STATUSES.NO_NEED_TO_SEND, projectId: '1', metadata: { - name: 'Carlos', - amount: 10, - currency: 'DAI', + projectTitle: 'Carlos', + projectLink: 'https://givet.io/Carlos', }, }); assert.isOk(result); - assert.equal(result.userAddressId, userAddress.id); + assert.equal(result.userAddress.id, userAddress.id); assert.equal(result.notificationTypeId, notificationType!.id); }); } diff --git a/src/repositories/notificationRepository.ts b/src/repositories/notificationRepository.ts index 22f399f..130ab77 100644 --- a/src/repositories/notificationRepository.ts +++ b/src/repositories/notificationRepository.ts @@ -101,6 +101,16 @@ export const countUnreadNotifications = async ( }; }; +export const findNotificationByTrackId = async ( + trackId: string, +): Promise => { + return Notification.createQueryBuilder('notification') + .where('notification."trackId" = :trackId', { + trackId, + }) + .getOne(); +}; + export const baseNotificationQuery = (user: UserAddress) => { return Notification.createQueryBuilder('notification') .innerJoinAndSelect('notification.notificationType', 'notificationType') @@ -147,34 +157,14 @@ export const getNotifications = async (params: { return query.take(limit).skip(skip).getManyAndCount(); }; -export const createNotification = async (params: { - notificationType: NotificationType; - user: UserAddress; - email?: string; - emailStatus?: string; - metadata?: any; - segmentData?: any; - projectId?: string; -}) => { - const { - notificationType, - user, - projectId, - email, - emailStatus, - metadata, - segmentData, - } = params; - - const notification = Notification.create({ - userAddress: user, - email, - metadata, - segmentData, - emailStatus, - projectId, - notificationType, - }); - - return notification.save(); +export const createNotification = async ( + params: Partial, +): Promise => { + try { + const notification = Notification.create(params); + return (await notification.save()) as Notification; + } catch (e) { + logger.error('createNotification error', e); + throw e; + } }; diff --git a/src/repositories/notificationSettingRepository.ts b/src/repositories/notificationSettingRepository.ts index 1521a04..c423655 100644 --- a/src/repositories/notificationSettingRepository.ts +++ b/src/repositories/notificationSettingRepository.ts @@ -99,9 +99,6 @@ export const updateUserNotificationSetting = async (params: { ) .getOne(); - console.log('user id ' + params.userAddressId); - console.log('notif id ' + params.notificationSettingId); - if (!notificationSetting) throw new Error(errorMessages.NOTIFICATION_SETTING_NOT_FOUND); diff --git a/src/routes/v1/notificationRouter.test.ts b/src/routes/v1/notificationRouter.test.ts index 3ca19e9..2c62a80 100644 --- a/src/routes/v1/notificationRouter.test.ts +++ b/src/routes/v1/notificationRouter.test.ts @@ -9,6 +9,7 @@ import { import axios from 'axios'; import { assert } from 'chai'; import { errorMessages, errorMessagesEnum } from '../../utils/errorMessages'; +import { findNotificationByTrackId } from '../../repositories/notificationRepository'; describe('/notifications POST test cases', sendNotificationTestCases); describe('/notifications GET test cases', getNotificationTestCases); @@ -54,7 +55,6 @@ function getNotificationTestCases() { authorization: getAccessTokenForMockAuthMicroService(walletAddress), }, }); - console.log('**result**', notificationResult.data); assert.equal(notificationResult.status, 200); assert.isOk(notificationResult.data); assert.equal(notificationResult.data.count, 3); @@ -84,7 +84,6 @@ function sendNotificationTestCases() { authorization: getGivethIoBasicAuth(), }, }); - console.log('**result**', result.data); assert.equal(result.status, 200); assert.isOk(result.data); assert.isTrue(result.data.success); @@ -104,7 +103,6 @@ function sendNotificationTestCases() { authorization: getGivethIoBasicAuth(), }, }); - console.log('**result**', result.data); assert.equal(result.status, 200); assert.isOk(result.data); assert.isTrue(result.data.success); @@ -1244,7 +1242,6 @@ function sendNotificationTestCases() { authorization: getGivethIoBasicAuth(), }, }); - console.log('**result**', result.data); assert.equal(result.status, 200); assert.isOk(result.data); assert.isTrue(result.data.success); @@ -1264,7 +1261,6 @@ function sendNotificationTestCases() { authorization: getGivethIoBasicAuth(), }, }); - console.log('**result**', result.data); assert.equal(result.status, 200); assert.isOk(result.data); assert.isTrue(result.data.success); @@ -2146,4 +2142,65 @@ function sendNotificationTestCases() { assert.equal(e.response.data.description, '"projectTitle" is required'); } }); + + it('should create notification successfully with passing trackId', async () => { + const trackId = generateRandomTxHash(); + const data = { + eventName: 'Draft published', + sendEmail: false, + sendSegment: false, + userWalletAddress: generateRandomEthereumAddress(), + trackId, + metadata: { + projectTitle, + projectLink, + }, + }; + + const result = await axios.post(sendNotificationUrl, data, { + headers: { + authorization: getGivethIoBasicAuth(), + }, + }); + assert.equal(result.status, 200); + assert.isOk(result.data); + assert.isTrue(result.data.success); + assert.isNotNull(await findNotificationByTrackId(trackId)); + }); + + it('should throw error for repetitive trackId', async () => { + const trackId = generateRandomTxHash(); + const data = { + eventName: 'Draft published', + sendEmail: false, + sendSegment: false, + userWalletAddress: generateRandomEthereumAddress(), + trackId, + metadata: { + projectTitle, + projectLink, + }, + }; + + const result = await axios.post(sendNotificationUrl, data, { + headers: { + authorization: getGivethIoBasicAuth(), + }, + }); + + assert.equal(result.status, 200); + assert.isOk(result.data); + assert.isTrue(result.data.success); + assert.isNotNull(await findNotificationByTrackId(trackId)); + const duplicateResult = await axios.post(sendNotificationUrl, data, { + headers: { + authorization: getGivethIoBasicAuth(), + }, + }); + assert.equal(duplicateResult.status, 200); + assert.equal( + duplicateResult.data.message, + errorMessages.DUPLICATE_TRACK_ID, + ); + }); } diff --git a/src/types/requestResponses.ts b/src/types/requestResponses.ts index 2e7cf73..5ad4189 100644 --- a/src/types/requestResponses.ts +++ b/src/types/requestResponses.ts @@ -9,6 +9,10 @@ export type SendNotificationTypeRequest = { * @example "Made donation" */ eventName: string; + /** + * @example "Should be unique" + */ + trackId?: string; /** * @example false */ @@ -47,17 +51,7 @@ export type SendNotificationTypeRequest = { */ anonymousId?: string; }; - metadata?: { - /** - * @example "title" - */ - projectTitle?: string; - - /** - * @example "project link" - */ - projectLink?: string; - }; + metadata?: NotificationMetadata; }; export type SendNotificationRequest = { diff --git a/src/utils/errorMessages.ts b/src/utils/errorMessages.ts index 01dc1c1..2a54fae 100644 --- a/src/utils/errorMessages.ts +++ b/src/utils/errorMessages.ts @@ -93,6 +93,7 @@ export const errorMessages = { CATEGORIES_LENGTH_SHOULD_NOT_BE_MORE_THAN_FIVE: 'Please select no more than 5 categories', INVALID_NOTIFICATION_TYPE: 'Notification type invalid', + DUPLICATE_TRACK_ID: 'Duplicate trackId', CATEGORIES_MUST_BE_FROM_THE_FRONTEND_SUBSELECTION: 'This category is not valid', INVALID_TX_HASH: 'Invalid txHash', diff --git a/src/validators/schemaValidators.ts b/src/validators/schemaValidators.ts index 6384a8f..45f5fbc 100644 --- a/src/validators/schemaValidators.ts +++ b/src/validators/schemaValidators.ts @@ -31,6 +31,7 @@ export const sendNotificationValidator = Joi.object({ sendEmail: Joi.boolean(), sendSegment: Joi.boolean(), email: Joi.string(), + trackId: Joi.string(), userWalletAddress: Joi.string() .pattern(ethereumWalletAddressRegex) .required(), From 4db0a58e4047c908636e10701c2021903cf26198 Mon Sep 17 00:00:00 2001 From: Mohammad Ranjbar Z Date: Tue, 22 Nov 2022 14:40:36 +0300 Subject: [PATCH 2/4] Add creation time to notification entity related to #14 --- src/controllers/v1/notificationsController.ts | 12 ++++++--- src/routes/v1/notificationRouter.test.ts | 27 +++++++++++++++++++ src/types/requestResponses.ts | 5 ++++ src/validators/schemaValidators.ts | 1 + 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/controllers/v1/notificationsController.ts b/src/controllers/v1/notificationsController.ts index 9bc6f8f..1fd2a8a 100644 --- a/src/controllers/v1/notificationsController.ts +++ b/src/controllers/v1/notificationsController.ts @@ -43,7 +43,7 @@ import { getNotificationTypeByEventName, getNotificationTypeByEventNameAndMicroservice, } from '../../repositories/notificationTypeRepository'; -import { EMAIL_STATUSES } from '../../entities/notification'; +import { EMAIL_STATUSES, Notification } from '../../entities/notification'; import { createNewUserAddressIfNotExists } from '../../repositories/userAddressRepository'; import { SEGMENT_METADATA_SCHEMA_VALIDATOR } from '../../utils/validators/segmentAndMetadataValidators'; import { findNotificationSettingByNotificationTypeAndUserAddress } from '../../repositories/notificationSettingRepository'; @@ -67,6 +67,7 @@ export class NotificationsController { try { validateWithJoiSchema(body, sendNotificationValidator); if (body.trackId && (await findNotificationByTrackId(body.trackId))) { + // We dont throw error in this case but dont create new notification neither return { success: true, message: errorMessages.DUPLICATE_TRACK_ID, @@ -137,7 +138,7 @@ export class NotificationsController { message: errorMessages.USER_TURNED_OF_THIS_NOTIFICATION_TYPE, }; } - await createNotification({ + const notificationData: Partial = { notificationType, userAddress, email: body.email, @@ -146,7 +147,12 @@ export class NotificationsController { metadata: body?.metadata, segmentData: body.segment, projectId, - }); + }; + if (body.creationTime) { + // creationTime is optional and it's timestamp in milliseconds format + notificationData.createdAt = new Date(body.creationTime); + } + await createNotification(notificationData); return { success: true }; // add if and logic for push notification (not in mvp) diff --git a/src/routes/v1/notificationRouter.test.ts b/src/routes/v1/notificationRouter.test.ts index 2c62a80..07bf1f5 100644 --- a/src/routes/v1/notificationRouter.test.ts +++ b/src/routes/v1/notificationRouter.test.ts @@ -2203,4 +2203,31 @@ function sendNotificationTestCases() { errorMessages.DUPLICATE_TRACK_ID, ); }); + + it('should create notification successfully with passing creationTime', async () => { + const creationTime = new Date().getTime(); + const trackId = generateRandomTxHash(); + const data = { + eventName: 'Draft published', + sendEmail: false, + sendSegment: false, + userWalletAddress: generateRandomEthereumAddress(), + trackId, + creationTime, + metadata: { + projectTitle, + projectLink, + }, + }; + const result = await axios.post(sendNotificationUrl, data, { + headers: { + authorization: getGivethIoBasicAuth(), + }, + }); + assert.equal(result.status, 200); + assert.isOk(result.data); + assert.isTrue(result.data.success); + const createdNotification = await findNotificationByTrackId(trackId); + assert.equal(createdNotification?.createdAt.getTime(), creationTime); + }); } diff --git a/src/types/requestResponses.ts b/src/types/requestResponses.ts index 5ad4189..bd9a879 100644 --- a/src/types/requestResponses.ts +++ b/src/types/requestResponses.ts @@ -21,6 +21,11 @@ export type SendNotificationTypeRequest = { * @example false */ sendSegment: boolean; + /** + * @example 1667992708000 + * @description milliseconds + */ + creationTime: number; /** * @example false */ diff --git a/src/validators/schemaValidators.ts b/src/validators/schemaValidators.ts index 45f5fbc..c8ef761 100644 --- a/src/validators/schemaValidators.ts +++ b/src/validators/schemaValidators.ts @@ -32,6 +32,7 @@ export const sendNotificationValidator = Joi.object({ sendSegment: Joi.boolean(), email: Joi.string(), trackId: Joi.string(), + creationTime: Joi.number(), userWalletAddress: Joi.string() .pattern(ethereumWalletAddressRegex) .required(), From 674a1321f9ee6464a32ce39044d32cdf8813d4a4 Mon Sep 17 00:00:00 2001 From: Carlos Quintero Date: Tue, 22 Nov 2022 22:37:23 -0500 Subject: [PATCH 3/4] fix past tense typo --- src/routes/v1/notificationRouter.test.ts | 2 +- src/utils/errorMessages.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/v1/notificationRouter.test.ts b/src/routes/v1/notificationRouter.test.ts index 07bf1f5..073dee9 100644 --- a/src/routes/v1/notificationRouter.test.ts +++ b/src/routes/v1/notificationRouter.test.ts @@ -2200,7 +2200,7 @@ function sendNotificationTestCases() { assert.equal(duplicateResult.status, 200); assert.equal( duplicateResult.data.message, - errorMessages.DUPLICATE_TRACK_ID, + errorMessages.DUPLICATED_TRACK_ID, ); }); diff --git a/src/utils/errorMessages.ts b/src/utils/errorMessages.ts index 2a54fae..fa35f07 100644 --- a/src/utils/errorMessages.ts +++ b/src/utils/errorMessages.ts @@ -93,7 +93,7 @@ export const errorMessages = { CATEGORIES_LENGTH_SHOULD_NOT_BE_MORE_THAN_FIVE: 'Please select no more than 5 categories', INVALID_NOTIFICATION_TYPE: 'Notification type invalid', - DUPLICATE_TRACK_ID: 'Duplicate trackId', + DUPLICATED_TRACK_ID: 'Duplicated trackId', CATEGORIES_MUST_BE_FROM_THE_FRONTEND_SUBSELECTION: 'This category is not valid', INVALID_TX_HASH: 'Invalid txHash', From fb9b636c115afddea5e973b5487e3f002abf40f8 Mon Sep 17 00:00:00 2001 From: Carlos Quintero Date: Tue, 22 Nov 2022 22:40:58 -0500 Subject: [PATCH 4/4] fix typo --- src/controllers/v1/notificationsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/v1/notificationsController.ts b/src/controllers/v1/notificationsController.ts index 1fd2a8a..da413c0 100644 --- a/src/controllers/v1/notificationsController.ts +++ b/src/controllers/v1/notificationsController.ts @@ -70,7 +70,7 @@ export class NotificationsController { // We dont throw error in this case but dont create new notification neither return { success: true, - message: errorMessages.DUPLICATE_TRACK_ID, + message: errorMessages.DUPLICATED_TRACK_ID, }; } const userAddress = await createNewUserAddressIfNotExists(