From fb3505af99a312fe9435403c36f3255de4eb2b09 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Wed, 27 Oct 2021 11:23:36 -0300 Subject: [PATCH 01/11] fixed users not receiving emails --- app/lib/server/functions/createDirectRoom.js | 4 +++- app/utils/lib/getDefaultSubscriptionPref.js | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/lib/server/functions/createDirectRoom.js b/app/lib/server/functions/createDirectRoom.js index bdf87b460642..f98596d1d281 100644 --- a/app/lib/server/functions/createDirectRoom.js +++ b/app/lib/server/functions/createDirectRoom.js @@ -6,6 +6,7 @@ import { callbacks } from '../../../callbacks/server'; import { Rooms, Subscriptions } from '../../../models/server'; import { settings } from '../../../settings/server'; import { getDefaultSubscriptionPref } from '../../../utils/server'; +import { Users } from '../../../models'; const generateSubscription = (fname, name, user, extra) => ({ alert: false, @@ -90,13 +91,14 @@ export const createDirectRoom = function(members, roomExtraData = {}, options = } else { members.forEach((member) => { const otherMembers = sortedMembers.filter(({ _id }) => _id !== member._id); + const memberWithPreferences = Users.findOneByUsername(member.username, { fields: { username: 1, 'settings.preferences': 1 } }); Subscriptions.upsert({ rid, 'u._id': member._id }, { ...options.creator === member._id && { $set: { open: true } }, $setOnInsert: generateSubscription( getFname(otherMembers), getName(otherMembers), - member, + memberWithPreferences, { ...options.subscriptionExtra, ...options.creator !== member._id && { open: members.length > 2 }, diff --git a/app/utils/lib/getDefaultSubscriptionPref.js b/app/utils/lib/getDefaultSubscriptionPref.js index 294a7d50a734..e8f70d0fd5e3 100644 --- a/app/utils/lib/getDefaultSubscriptionPref.js +++ b/app/utils/lib/getDefaultSubscriptionPref.js @@ -1,6 +1,5 @@ export const getDefaultSubscriptionPref = (userPref) => { const subscription = {}; - const { desktopNotifications, mobileNotifications, @@ -8,6 +7,7 @@ export const getDefaultSubscriptionPref = (userPref) => { highlights, } = (userPref.settings && userPref.settings.preferences) || {}; + if (Array.isArray(highlights) && highlights.length) { subscription.userHighlights = highlights; } @@ -26,6 +26,5 @@ export const getDefaultSubscriptionPref = (userPref) => { subscription.emailNotifications = emailNotificationMode; subscription.emailPrefOrigin = 'user'; } - return subscription; }; From 246ef842589ecc0804cff16002472e9c03909981 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Wed, 27 Oct 2021 11:25:40 -0300 Subject: [PATCH 02/11] lint --- app/utils/lib/getDefaultSubscriptionPref.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/utils/lib/getDefaultSubscriptionPref.js b/app/utils/lib/getDefaultSubscriptionPref.js index e8f70d0fd5e3..b18239de048e 100644 --- a/app/utils/lib/getDefaultSubscriptionPref.js +++ b/app/utils/lib/getDefaultSubscriptionPref.js @@ -1,5 +1,6 @@ export const getDefaultSubscriptionPref = (userPref) => { const subscription = {}; + const { desktopNotifications, mobileNotifications, @@ -7,7 +8,6 @@ export const getDefaultSubscriptionPref = (userPref) => { highlights, } = (userPref.settings && userPref.settings.preferences) || {}; - if (Array.isArray(highlights) && highlights.length) { subscription.userHighlights = highlights; } From fe8db6cf2d1980aa3a09809df4da82e21b6161a6 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Wed, 27 Oct 2021 11:26:48 -0300 Subject: [PATCH 03/11] small change --- app/utils/lib/getDefaultSubscriptionPref.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/utils/lib/getDefaultSubscriptionPref.js b/app/utils/lib/getDefaultSubscriptionPref.js index b18239de048e..5c086f4adacd 100644 --- a/app/utils/lib/getDefaultSubscriptionPref.js +++ b/app/utils/lib/getDefaultSubscriptionPref.js @@ -1,4 +1,4 @@ -export const getDefaultSubscriptionPref = (userPref) => { +export const agetDefaultSubscriptionPref = (userPref) => { const subscription = {}; const { @@ -26,5 +26,6 @@ export const getDefaultSubscriptionPref = (userPref) => { subscription.emailNotifications = emailNotificationMode; subscription.emailPrefOrigin = 'user'; } + return subscription; }; From 383c078160aa87bae5e0dc5c90696c989856ec5d Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Wed, 27 Oct 2021 11:27:23 -0300 Subject: [PATCH 04/11] typo --- app/utils/lib/getDefaultSubscriptionPref.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/utils/lib/getDefaultSubscriptionPref.js b/app/utils/lib/getDefaultSubscriptionPref.js index 5c086f4adacd..294a7d50a734 100644 --- a/app/utils/lib/getDefaultSubscriptionPref.js +++ b/app/utils/lib/getDefaultSubscriptionPref.js @@ -1,4 +1,4 @@ -export const agetDefaultSubscriptionPref = (userPref) => { +export const getDefaultSubscriptionPref = (userPref) => { const subscription = {}; const { From bd7fdb9924d441c459e84a7e853d1de5fbc57c28 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Wed, 24 Nov 2021 11:17:25 -0300 Subject: [PATCH 05/11] stopped searching for members with preferences at each iteration --- app/lib/server/functions/createDirectRoom.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/lib/server/functions/createDirectRoom.js b/app/lib/server/functions/createDirectRoom.js index f98596d1d281..5147785dccba 100644 --- a/app/lib/server/functions/createDirectRoom.js +++ b/app/lib/server/functions/createDirectRoom.js @@ -89,16 +89,15 @@ export const createDirectRoom = function(members, roomExtraData = {}, options = $setOnInsert: generateSubscription(members[0].name || members[0].username, members[0].username, members[0], { ...options.subscriptionExtra }), }); } else { - members.forEach((member) => { + const membersWithPreferences = Users.find({}, { username: 1, 'settings.preferences': 1 }); + membersWithPreferences.forEach((member) => { const otherMembers = sortedMembers.filter(({ _id }) => _id !== member._id); - const memberWithPreferences = Users.findOneByUsername(member.username, { fields: { username: 1, 'settings.preferences': 1 } }); - Subscriptions.upsert({ rid, 'u._id': member._id }, { ...options.creator === member._id && { $set: { open: true } }, $setOnInsert: generateSubscription( getFname(otherMembers), getName(otherMembers), - memberWithPreferences, + member, { ...options.subscriptionExtra, ...options.creator !== member._id && { open: members.length > 2 }, From aee238efb55ebf0dc51df767784ba00a64a4d7ba Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Thu, 25 Nov 2021 13:47:58 -0300 Subject: [PATCH 06/11] fixes + tests --- app/lib/server/functions/createDirectRoom.js | 5 +- tests/end-to-end/api/04-direct-message.js | 56 ++++++++++++++++++++ tests/end-to-end/api/userPreferences.ts | 23 ++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 tests/end-to-end/api/userPreferences.ts diff --git a/app/lib/server/functions/createDirectRoom.js b/app/lib/server/functions/createDirectRoom.js index 5147785dccba..d7c313bb75e6 100644 --- a/app/lib/server/functions/createDirectRoom.js +++ b/app/lib/server/functions/createDirectRoom.js @@ -6,7 +6,7 @@ import { callbacks } from '../../../callbacks/server'; import { Rooms, Subscriptions } from '../../../models/server'; import { settings } from '../../../settings/server'; import { getDefaultSubscriptionPref } from '../../../utils/server'; -import { Users } from '../../../models'; +import { Users } from '../../../models/server/raw'; const generateSubscription = (fname, name, user, extra) => ({ alert: false, @@ -89,7 +89,8 @@ export const createDirectRoom = function(members, roomExtraData = {}, options = $setOnInsert: generateSubscription(members[0].name || members[0].username, members[0].username, members[0], { ...options.subscriptionExtra }), }); } else { - const membersWithPreferences = Users.find({}, { username: 1, 'settings.preferences': 1 }); + const memberIds = members.map((member) => member._id); + const membersWithPreferences = Users.find({ _id: { $in: memberIds } }, { username: 1, 'settings.preferences': 1 }); membersWithPreferences.forEach((member) => { const otherMembers = sortedMembers.filter(({ _id }) => _id !== member._id); Subscriptions.upsert({ rid, 'u._id': member._id }, { diff --git a/tests/end-to-end/api/04-direct-message.js b/tests/end-to-end/api/04-direct-message.js index 33dce8b18c5a..117519d2a02c 100644 --- a/tests/end-to-end/api/04-direct-message.js +++ b/tests/end-to-end/api/04-direct-message.js @@ -8,6 +8,7 @@ import { directMessage, apiUsername, apiEmail, + methodCall, } from '../../data/api-data.js'; import { password, adminUsername } from '../../data/user.js'; import { deleteRoom } from '../../data/rooms.helper'; @@ -481,19 +482,27 @@ describe('[Direct Messages]', function() { }); describe('/im.create', () => { + let user; + let userCredentials; let otherUser; let roomId; + let userPrefRoomId; before(async () => { + user = await createUser(); otherUser = await createUser(); + userCredentials = await login(user.username, password); }); after(async () => { if (roomId) { await deleteRoom({ type: 'd', roomId }); + await deleteRoom({ type: 'd', userPrefRoomId }); } await deleteUser(otherUser); + await deleteUser(user); otherUser = undefined; + user = undefined; }); it('creates a DM between two other parties (including self)', (done) => { @@ -546,6 +555,53 @@ describe('[Direct Messages]', function() { }) .end(done); }); + + it('should create dm with correct notification preferences', () => { + it('should save user preferences', (done) => { + request.post(methodCall('saveUserPreferences')) + .set(userCredentials) + .send({ + message: JSON.stringify({ + method: 'saveUserPreferences', + params: [{ emailNotificationMode: 'nothing' }], + }), + }) + .end(done); + }); + + it('should create a DM', (done) => { + request.post(api('im.create')) + .set(userCredentials) + .send({ + usernames: [user.username, otherUser.username].join(','), + }) + .expect(200) + .expect('Content-Type', 'application/json') + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('room').and.to.be.an('object'); + expect(res.body.room).to.have.property('usernames').and.to.have.members([user.username, otherUser.username]); + roomId = res.body.room._id; + }) + .end(done); + }); + + it('should return the right user notification preferences in the dm', (done) => { + request.get(api('subscriptions.getOne')) + .set(userCredentials) + .query({ + roomId: userPrefRoomId._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('subscription').and.to.be.an('object'); + expect(res.body).to.have.nested.property('subscription.emailNotifications').and.to.be.equal('nothing'); + }) + .end(done); + }); + }); }); describe('/im.delete', () => { diff --git a/tests/end-to-end/api/userPreferences.ts b/tests/end-to-end/api/userPreferences.ts new file mode 100644 index 000000000000..ab0d210dde20 --- /dev/null +++ b/tests/end-to-end/api/userPreferences.ts @@ -0,0 +1,23 @@ +import { getCredentials, request, credentials, methodCall } from '../../data/api-data.js'; + +describe('User Preferences', function() { + this.retries(0); + + before((done) => getCredentials(done)); + + describe('[/saveUserPreferences]', () => { + it('should save correctly user preferences', (done) => { + request.post(methodCall('saveUserPreferences')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'saveUserPreferences', + params: [{ emailNotificationMode: 'nothing' }], + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .end(done); + }); + }); +}); From e487b4c45e5fcad935e9666d29990d8825aab1a8 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Thu, 25 Nov 2021 14:03:43 -0300 Subject: [PATCH 07/11] removed file --- tests/end-to-end/api/userPreferences.ts | 23 ----------------------- 1 file changed, 23 deletions(-) delete mode 100644 tests/end-to-end/api/userPreferences.ts diff --git a/tests/end-to-end/api/userPreferences.ts b/tests/end-to-end/api/userPreferences.ts deleted file mode 100644 index ab0d210dde20..000000000000 --- a/tests/end-to-end/api/userPreferences.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { getCredentials, request, credentials, methodCall } from '../../data/api-data.js'; - -describe('User Preferences', function() { - this.retries(0); - - before((done) => getCredentials(done)); - - describe('[/saveUserPreferences]', () => { - it('should save correctly user preferences', (done) => { - request.post(methodCall('saveUserPreferences')) - .set(credentials) - .send({ - message: JSON.stringify({ - method: 'saveUserPreferences', - params: [{ emailNotificationMode: 'nothing' }], - }), - }) - .expect('Content-Type', 'application/json') - .expect(200) - .end(done); - }); - }); -}); From 5050ff38b20645015f73304edbdfbf00f6d2b962 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Mon, 29 Nov 2021 09:01:59 -0300 Subject: [PATCH 08/11] switched from upsert to update --- app/lib/server/functions/createDirectRoom.js | 39 ++++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/app/lib/server/functions/createDirectRoom.js b/app/lib/server/functions/createDirectRoom.js index d7c313bb75e6..dddf16486f0c 100644 --- a/app/lib/server/functions/createDirectRoom.js +++ b/app/lib/server/functions/createDirectRoom.js @@ -1,14 +1,16 @@ import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions'; import { Meteor } from 'meteor/meteor'; +import { Random } from 'meteor/random'; import { Apps } from '../../../apps/server'; import { callbacks } from '../../../callbacks/server'; -import { Rooms, Subscriptions } from '../../../models/server'; +import { Rooms } from '../../../models/server'; import { settings } from '../../../settings/server'; import { getDefaultSubscriptionPref } from '../../../utils/server'; -import { Users } from '../../../models/server/raw'; +import { Users, Subscriptions } from '../../../models/server/raw'; const generateSubscription = (fname, name, user, extra) => ({ + _id: Random.id(), alert: false, unread: 0, userMentions: 0, @@ -84,27 +86,32 @@ export const createDirectRoom = function(members, roomExtraData = {}, options = const rid = room?._id || Rooms.insert(roomInfo); if (members.length === 1) { // dm to yourself - Subscriptions.upsert({ rid, 'u._id': members[0]._id }, { + Subscriptions.update({ rid, 'u._id': members[0]._id }, { $set: { open: true }, $setOnInsert: generateSubscription(members[0].name || members[0].username, members[0].username, members[0], { ...options.subscriptionExtra }), - }); + }, { upsert: true }); } else { const memberIds = members.map((member) => member._id); const membersWithPreferences = Users.find({ _id: { $in: memberIds } }, { username: 1, 'settings.preferences': 1 }); + membersWithPreferences.forEach((member) => { const otherMembers = sortedMembers.filter(({ _id }) => _id !== member._id); - Subscriptions.upsert({ rid, 'u._id': member._id }, { - ...options.creator === member._id && { $set: { open: true } }, - $setOnInsert: generateSubscription( - getFname(otherMembers), - getName(otherMembers), - member, - { - ...options.subscriptionExtra, - ...options.creator !== member._id && { open: members.length > 2 }, - }, - ), - }); + Subscriptions.update( + { rid, 'u._id': member._id }, + { + ...options.creator === member._id && { $set: { open: true } }, + $setOnInsert: generateSubscription( + getFname(otherMembers), + getName(otherMembers), + member, + { + ...options.subscriptionExtra, + ...options.creator !== member._id && { open: members.length > 2 }, + }, + ), + }, + { upsert: true }, + ); }); } From b8c066ae25821202c881241141a0a29f0b273442 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Mon, 29 Nov 2021 10:22:24 -0300 Subject: [PATCH 09/11] fixed user pref tests --- tests/end-to-end/api/04-direct-message.js | 37 ++++++++++++++--------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/tests/end-to-end/api/04-direct-message.js b/tests/end-to-end/api/04-direct-message.js index 117519d2a02c..a89afa538b3d 100644 --- a/tests/end-to-end/api/04-direct-message.js +++ b/tests/end-to-end/api/04-direct-message.js @@ -482,27 +482,19 @@ describe('[Direct Messages]', function() { }); describe('/im.create', () => { - let user; - let userCredentials; let otherUser; let roomId; - let userPrefRoomId; before(async () => { - user = await createUser(); otherUser = await createUser(); - userCredentials = await login(user.username, password); }); after(async () => { if (roomId) { await deleteRoom({ type: 'd', roomId }); - await deleteRoom({ type: 'd', userPrefRoomId }); } await deleteUser(otherUser); - await deleteUser(user); otherUser = undefined; - user = undefined; }); it('creates a DM between two other parties (including self)', (done) => { @@ -556,9 +548,26 @@ describe('[Direct Messages]', function() { .end(done); }); - it('should create dm with correct notification preferences', () => { - it('should save user preferences', (done) => { - request.post(methodCall('saveUserPreferences')) + describe('should create dm with correct notification preferences', () => { + let user; + let userCredentials; + let userPrefRoomId; + + before(async () => { + user = await createUser(); + userCredentials = await login(user.username, password); + }); + + after(async () => { + if (userPrefRoomId) { + await deleteRoom({ type: 'd', roomId: userPrefRoomId }); + } + await deleteUser(user); + user = undefined; + }); + + it('should save user preferences', async () => { + await request.post(methodCall('saveUserPreferences')) .set(userCredentials) .send({ message: JSON.stringify({ @@ -566,7 +575,7 @@ describe('[Direct Messages]', function() { params: [{ emailNotificationMode: 'nothing' }], }), }) - .end(done); + .expect(200); }); it('should create a DM', (done) => { @@ -581,7 +590,7 @@ describe('[Direct Messages]', function() { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('room').and.to.be.an('object'); expect(res.body.room).to.have.property('usernames').and.to.have.members([user.username, otherUser.username]); - roomId = res.body.room._id; + userPrefRoomId = res.body.room._id; }) .end(done); }); @@ -590,7 +599,7 @@ describe('[Direct Messages]', function() { request.get(api('subscriptions.getOne')) .set(userCredentials) .query({ - roomId: userPrefRoomId._id, + roomId: userPrefRoomId, }) .expect('Content-Type', 'application/json') .expect(200) From 84d22db33e265aec34e5acc4b3f09409646933a9 Mon Sep 17 00:00:00 2001 From: Leonardo Ostjen Couto Date: Thu, 9 Dec 2021 12:56:40 -0300 Subject: [PATCH 10/11] fixed merge conflict --- tests/end-to-end/api/04-direct-message.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/end-to-end/api/04-direct-message.js b/tests/end-to-end/api/04-direct-message.js index e3d65e6395d9..6fc85534cb9c 100644 --- a/tests/end-to-end/api/04-direct-message.js +++ b/tests/end-to-end/api/04-direct-message.js @@ -635,7 +635,9 @@ describe('[Direct Messages]', function() { expect(res.body).to.have.nested.property('subscription.emailNotifications').and.to.be.equal('nothing'); }) .end(done); - + }); + }); + async function testRoomFNameForUser(testCredentials, roomId, fullName) { return request.get(api('subscriptions.getOne')) .set(testCredentials) From 322a03cfdab58c7f8382a42083c4f2ef6ec59485 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Fri, 17 Dec 2021 18:47:00 -0300 Subject: [PATCH 11/11] Apply suggestions from code review --- app/lib/server/functions/createDirectRoom.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/lib/server/functions/createDirectRoom.js b/app/lib/server/functions/createDirectRoom.js index dddf16486f0c..74d2e3418218 100644 --- a/app/lib/server/functions/createDirectRoom.js +++ b/app/lib/server/functions/createDirectRoom.js @@ -86,7 +86,7 @@ export const createDirectRoom = function(members, roomExtraData = {}, options = const rid = room?._id || Rooms.insert(roomInfo); if (members.length === 1) { // dm to yourself - Subscriptions.update({ rid, 'u._id': members[0]._id }, { + Subscriptions.updateOne({ rid, 'u._id': members[0]._id }, { $set: { open: true }, $setOnInsert: generateSubscription(members[0].name || members[0].username, members[0].username, members[0], { ...options.subscriptionExtra }), }, { upsert: true }); @@ -96,7 +96,7 @@ export const createDirectRoom = function(members, roomExtraData = {}, options = membersWithPreferences.forEach((member) => { const otherMembers = sortedMembers.filter(({ _id }) => _id !== member._id); - Subscriptions.update( + Subscriptions.updateOne( { rid, 'u._id': member._id }, { ...options.creator === member._id && { $set: { open: true } },