Merge pull request #201 from topcoder-platform/dev

Avoid duplicate broadcast notifications

Author: sachin-maheshwari

migrations/v2.0.4.sql

@@ -0,0 +1,37 @@
+
+-- query to identify the duplicate rows in bulk_message_user_refs table
+SELECT count(*), bulk_message_id, user_id  FROM "bulk_message_user_refs" 
+   GROUP BY bulk_message_id, user_id HAVING count(*) > 1;
+
+-- create temp table and store duplicate broadcast notification rows
+SELECT * INTO temptable FROM "Notifications" WHERE id IN  
+   (
+       SELECT a.notification_id FROM "bulk_message_user_refs" AS "a", 
+       "bulk_message_user_refs" AS "b" 
+       WHERE a.id < b.id 
+       AND a.bulk_message_id = b.bulk_message_id 
+       AND a.user_id = b.user_id
+    );
+
+-- DELETE duplicate rows from bulk_message_user_refs table
+DELETE FROM "bulk_message_user_refs" AS "a" 
+  USING "bulk_message_user_refs" AS "b" 
+  WHERE a.id < b.id 
+  AND a.bulk_message_id = b.bulk_message_id 
+  AND a.user_id = b.user_id;
+
+-- DELETE duplicate rows from Notifications
+DELETE FROM "Notifications" 
+WHERE id IN (SELECT id FROM temptable) 
+AND type = 'admin.notification.broadcast';
+
+-- make unique column to avoid duplicate insert 
+ALTER TABLE bulk_message_user_refs ADD UNIQUE (bulk_message_id, user_id);
+
+-- get duplicate broadcast rows
+SELECT * FROM "Notifications" AS a
+   LEFT JOIN "bulk_message_user_refs" AS b
+   ON a.id=b.notification_id
+   WHERE a.type='admin.notification.broadcast'
+   AND b.id IS NULL;
+

src/hooks/hookBulkMessage.js

@@ -116,9 +116,13 @@ async function isBroadCastMessageForUser(userId, bulkMessage, memberInfo, userGr
  * 
  * @param {Integer} userId 
  * @param {Integer} bulkMessageId 
- * @param {Integer} notificationId 
+ * @param {Object} notificationObj
  */
-async function insertUserRefs(userId, bulkMessageId, notificationId) {
+async function insertUserRefs(userId, bulkMessageId, notificationObj) {
+    let notificationId = null
+    if (notificationObj) {
+        notificationId = notificationObj.id
+    }
     try {
         const r = await models.BulkMessageUserRefs.create({
             bulk_message_id: bulkMessageId,
@@ -129,6 +133,15 @@ async function insertUserRefs(userId, bulkMessageId, notificationId) {
         return r
     } catch (e) {
         logger.error(`${logPrefix} Failed to insert userRef record for user: ${userId}, error: ${e}`)
+        if (notificationId && notificationObj) {
+             try {
+                 await notificationObj.destroy()
+                 logger.info(`Deleted/reverted duplicate/ref-transaction failed, broadcast notification ${notificationId} for user: ${userId}`)
+             } catch (error) {
+                 logger.error(`Error in deleting duplicate notification record, ${error}`)
+             }
+
+        }
         throw new Error(`insertUserRefs() : ${e}`)
     }
 }
@@ -155,7 +168,7 @@ async function createNotificationForUser(userId, bulkMessage) {
         })
         logger.info(`${logPrefix} Inserted notification record ${n.id} for current user ${userId}`)
         // TODO need to be in transaction so that rollback will be possible
-        const result = await insertUserRefs(userId, bulkMessage.id, n.id)
+        const result = await insertUserRefs(userId, bulkMessage.id, n)
         return result
     } catch (e) {
         logger.error(`${logPrefix} insert broadcast notification error: ${e} `)