From f32bfc7e36a0c0eeba4e46b264a1c614564e3e55 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 18 Feb 2020 15:20:48 +0100 Subject: [PATCH] fix(subscriptions): Don't publish undefined Fix #3088 --- .../notifications/notificationsMiddleware.js | 34 ++++++---- .../notificationsMiddleware.spec.js | 66 +++++++++++++++---- 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/backend/src/middleware/notifications/notificationsMiddleware.js b/backend/src/middleware/notifications/notificationsMiddleware.js index 4636b8e9f..26c27bb10 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.js +++ b/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) - await notifyUsersOfMention('Post', post.id, idsOfUsers, 'mentioned_in_post', context) + if (post) { + 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 notifyUsersOfMention('Comment', comment.id, idsOfUsers, 'mentioned_in_comment', context) - if (context.user.id !== postAuthor.id) - await notifyUsersOfComment('Comment', comment.id, postAuthor.id, 'commented_on_post', context) + await publishNotifications( + notifyUsersOfMention('Comment', comment.id, idsOfUsers, 'mentioned_in_comment', context), + notifyUsersOfComment('Comment', comment.id, postAuthor.id, 'commented_on_post', context), + ) return comment } @@ -29,7 +39,7 @@ const postAuthorOfComment = async (commentId, { context }) => { try { postAuthorId = await session.readTransaction(transaction => { return transaction.run( - ` + ` MATCH (author:User)-[:WROTE]->(:Post)<-[:COMMENTS]-(:Comment { id: $commentId }) RETURN author { .id } as authorId `, @@ -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 - return pubsub.publish(NOTIFICATION_ADDED, { notificationAdded: notification }) + const notifications = await writeTxResultPromise + 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 - return pubsub.publish(NOTIFICATION_ADDED, { notificationAdded: notification }) + const notifications = await writeTxResultPromise + return notifications } finally { session.close() } diff --git a/backend/src/middleware/notifications/notificationsMiddleware.spec.js b/backend/src/middleware/notifications/notificationsMiddleware.spec.js index 95c0037b8..16e33bdfb 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.spec.js +++ b/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 @al-capone 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( - query({ - query: notificationQuery, - variables: { - read: false, - }, + it('publishes `NOTIFICATION_ADDED` to me', async () => { + await createPostAction() + expect(publishSpy).toHaveBeenCalledWith( + 'NOTIFICATION_ADDED', + 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) }) }) })