fix(subscriptions): Don't publish undefined

Fix #3088

backend/src/middleware/notifications/notificationsMiddleware.js

@@ -2,11 +2,21 @@ import extractMentionedUsers from './mentions/extractMentionedUsers'
 import { validateNotifyUsers } from '../validation/validationMiddleware'
 import { pubsub, NOTIFICATION_ADDED } from '../../server'
 
+const publishNotifications = async (...promises) => {
+  const notifications = await Promise.all(promises)
+  notifications
+    .flat()
+    .forEach(notificationAdded => pubsub.publish(NOTIFICATION_ADDED, { notificationAdded }))
+}
+
 const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo) => {
   const idsOfUsers = extractMentionedUsers(args.content)
   const post = await resolve(root, args, context, resolveInfo)
-  if (post && idsOfUsers && idsOfUsers.length)
+  if (post) {
-    await notifyUsersOfMention('Post', post.id, idsOfUsers, 'mentioned_in_post', context)
+    await publishNotifications(
+      notifyUsersOfMention('Post', post.id, idsOfUsers, 'mentioned_in_post', context),
+    )
+  }
   return post
 }
 
@@ -16,10 +26,10 @@ const handleContentDataOfComment = async (resolve, root, args, context, resolveI
   const comment = await resolve(root, args, context, resolveInfo)
   const [postAuthor] = await postAuthorOfComment(comment.id, { context })
   idsOfUsers = idsOfUsers.filter(id => id !== postAuthor.id)
-  if (idsOfUsers && idsOfUsers.length)
+  await publishNotifications(
-    await notifyUsersOfMention('Comment', comment.id, idsOfUsers, 'mentioned_in_comment', context)
+    notifyUsersOfMention('Comment', comment.id, idsOfUsers, 'mentioned_in_comment', context),
-  if (context.user.id !== postAuthor.id)
+    notifyUsersOfComment('Comment', comment.id, postAuthor.id, 'commented_on_post', context),
-    await notifyUsersOfComment('Comment', comment.id, postAuthor.id, 'commented_on_post', context)
+  )
   return comment
 }
 
@@ -43,6 +53,7 @@ const postAuthorOfComment = async (commentId, { context }) => {
 }
 
 const notifyUsersOfMention = async (label, id, idsOfUsers, reason, context) => {
+  if (!(idsOfUsers && idsOfUsers.length)) return []
   await validateNotifyUsers(label, reason)
   let mentionedCypher
   switch (reason) {
@@ -91,8 +102,8 @@ const notifyUsersOfMention = async (label, id, idsOfUsers, reason, context) => {
     return notificationTransactionResponse.records.map(record => record.get('notification'))
   })
   try {
-    const [notification] = await writeTxResultPromise
+    const notifications = await writeTxResultPromise
-    return pubsub.publish(NOTIFICATION_ADDED, { notificationAdded: notification })
+    return notifications
   } catch (error) {
     throw new Error(error)
   } finally {
@@ -101,6 +112,7 @@ const notifyUsersOfMention = async (label, id, idsOfUsers, reason, context) => {
 }
 
 const notifyUsersOfComment = async (label, commentId, postAuthorId, reason, context) => {
+  if (!(context.user.id !== postAuthorId)) return []
   await validateNotifyUsers(label, reason)
   const session = context.driver.session()
   const writeTxResultPromise = await session.writeTransaction(async transaction => {
@@ -121,8 +133,8 @@ const notifyUsersOfComment = async (label, commentId, postAuthorId, reason, cont
     return notificationTransactionResponse.records.map(record => record.get('notification'))
   })
   try {
-    const [notification] = await writeTxResultPromise
+    const notifications = await writeTxResultPromise
-    return pubsub.publish(NOTIFICATION_ADDED, { notificationAdded: notification })
+    return notifications
   } finally {
     session.close()
   }

backend/src/middleware/notifications/notificationsMiddleware.spec.js

@@ -2,9 +2,10 @@ import { gql } from '../../helpers/jest'
 import { cleanDatabase } from '../../db/factories'
 import { createTestClient } from 'apollo-server-testing'
 import { getNeode, getDriver } from '../../db/neo4j'
-import createServer from '../../server'
+import createServer, { pubsub } from '../../server'
 
 let server, query, mutate, notifiedUser, authenticatedUser
+let publishSpy
 const driver = getDriver()
 const neode = getNeode()
 const categoryIds = ['cat9']
@@ -36,6 +37,7 @@ const createCommentMutation = gql`
 
 beforeAll(async () => {
   await cleanDatabase()
+  publishSpy = jest.spyOn(pubsub, 'publish')
   const createServerResult = createServer({
     context: () => {
       return {
@@ -52,6 +54,7 @@ beforeAll(async () => {
 })
 
 beforeEach(async () => {
+  publishSpy.mockClear()
   notifiedUser = await neode.create(
     'User',
     {
@@ -259,7 +262,15 @@ describe('notifications', () => {
           await createPostAction()
           const expectedContent =
             'Hey <a class="mention" data-mention-id="you" href="/profile/you/al-capone" target="_blank">@al-capone</a> how do you do?'
-          const expected = expect.objectContaining({
+          await expect(
+            query({
+              query: notificationQuery,
+              variables: {
+                read: false,
+              },
+            }),
+          ).resolves.toMatchObject({
+            errors: undefined,
             data: {
               notifications: [
                 {
@@ -275,15 +286,22 @@ describe('notifications', () => {
               ],
             },
           })
+        })
 
-          await expect(
+        it('publishes `NOTIFICATION_ADDED` to me', async () => {
-            query({
+          await createPostAction()
-              query: notificationQuery,
+          expect(publishSpy).toHaveBeenCalledWith(
-              variables: {
+            'NOTIFICATION_ADDED',
-                read: false,
+            expect.objectContaining({
-              },
+              notificationAdded: expect.objectContaining({
+                reason: 'mentioned_in_post',
+                to: expect.objectContaining({
+                  id: 'you',
+                }),
+              }),
             }),
-          ).resolves.toEqual(expected)
+          )
+          expect(publishSpy).toHaveBeenCalledTimes(1)
         })
 
         describe('updates the post and mentions me again', () => {
@@ -429,6 +447,11 @@ describe('notifications', () => {
               }),
             ).resolves.toEqual(expected)
           })
+
+          it('does not publish `NOTIFICATION_ADDED`', async () => {
+            await createPostAction()
+            expect(publishSpy).not.toHaveBeenCalled()
+          })
         })
       })
 
@@ -554,10 +577,6 @@ describe('notifications', () => {
 
           it('sends no notification', async () => {
             await createCommentOnPostAction()
-            const expected = expect.objectContaining({
-              data: { notifications: [] },
-            })
-
             await expect(
               query({
                 query: notificationQuery,
@@ -565,7 +584,26 @@ describe('notifications', () => {
                   read: false,
                 },
               }),
-            ).resolves.toEqual(expected)
+            ).resolves.toMatchObject({
+              data: { notifications: [] },
+              errors: undefined,
+            })
+          })
+
+          it('does not publish `NOTIFICATION_ADDED` to authenticated user', async () => {
+            await createCommentOnPostAction()
+            expect(publishSpy).toHaveBeenCalledWith(
+              'NOTIFICATION_ADDED',
+              expect.objectContaining({
+                notificationAdded: expect.objectContaining({
+                  reason: 'commented_on_post',
+                  to: expect.objectContaining({
+                    id: 'postAuthor', // that's expected, it's not me but the post author
+                  }),
+                }),
+              }),
+            )
+            expect(publishSpy).toHaveBeenCalledTimes(1)
           })
         })
       })