From 66af0a46c27669b40af0faafe2b8b4f7fa834494 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Fri, 13 Apr 2018 15:39:17 -0300 Subject: [PATCH 1/3] Add parameter to REST chat.react endpoint, to make it work like a toggle --- packages/rocketchat-api/server/v1/chat.js | 2 +- packages/rocketchat-reactions/setReaction.js | 18 +++++- tests/end-to-end/api/05-chat.js | 64 +++++++++++++++++++- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/packages/rocketchat-api/server/v1/chat.js b/packages/rocketchat-api/server/v1/chat.js index aa8404b75f251..08f86842fd019 100644 --- a/packages/rocketchat-api/server/v1/chat.js +++ b/packages/rocketchat-api/server/v1/chat.js @@ -273,7 +273,7 @@ RocketChat.API.v1.addRoute('chat.react', { authRequired: true }, { throw new Meteor.Error('error-emoji-param-not-provided', 'The required "emoji" param is missing.'); } - Meteor.runAsUser(this.userId, () => Meteor.call('setReaction', emoji, msg._id)); + Meteor.runAsUser(this.userId, () => Meteor.call('setReaction', emoji, msg._id, this.bodyParams.shouldReact)); return RocketChat.API.v1.success(); } diff --git a/packages/rocketchat-reactions/setReaction.js b/packages/rocketchat-reactions/setReaction.js index 89df6b8139f6e..222808a36c9ef 100644 --- a/packages/rocketchat-reactions/setReaction.js +++ b/packages/rocketchat-reactions/setReaction.js @@ -2,7 +2,7 @@ import _ from 'underscore'; Meteor.methods({ - setReaction(reaction, messageId) { + setReaction(reaction, messageId, shouldReact) { if (!Meteor.userId()) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setReaction' }); } @@ -39,12 +39,21 @@ Meteor.methods({ return false; } - if (message.reactions && message.reactions[reaction] && message.reactions[reaction].usernames.indexOf(user.username) !== -1) { + const userAlreadyReacted = message.reactions && message.reactions[reaction] && message.reactions[reaction].usernames.indexOf(user.username) !== -1; + const removeUserReaction = () => { message.reactions[reaction].usernames.splice(message.reactions[reaction].usernames.indexOf(user.username), 1); - + }; + const removeMessageReactionIfNeed = () => { if (message.reactions[reaction].usernames.length === 0) { delete message.reactions[reaction]; } + }; + if (userAlreadyReacted) { + if (shouldReact) { + throw new Meteor.Error('error-not-allowed', 'You already reacted with this message.', { method: 'setReaction' }); + } + removeUserReaction(); + removeMessageReactionIfNeed(); if (_.isEmpty(message.reactions)) { delete message.reactions; @@ -55,6 +64,9 @@ Meteor.methods({ RocketChat.callbacks.run('setReaction', messageId, reaction); } } else { + if (shouldReact === false) { + throw new Meteor.Error('error-not-allowed', 'You still haven\'t reacted with this message.', { method: 'setReaction' }); + } if (!message.reactions) { message.reactions = {}; } diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index 9d5b3f21455fd..b2c4d372ffbfb 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -178,7 +178,22 @@ describe('[Chat]', function() { .end(done); }); - describe('/chat.react', () => { + describe('[/chat.react]', () => { + it('should return statusCode: 400 and error when try unreact a message that\'s no reacted yet', (done) => { + request.post(api('chat.react')) + .set(credentials) + .send({ + emoji: ':squid:', + messageId: message._id, + shouldReact: false + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); it('should return statusCode: 200 when the emoji is valid', (done) => { request.post(api('chat.react')) .set(credentials) @@ -193,7 +208,51 @@ describe('[Chat]', function() { }) .end(done); }); - + it('should return statusCode: 400 and error when try react a message that\'s already reacted', (done) => { + request.post(api('chat.react')) + .set(credentials) + .send({ + emoji: ':squid:', + messageId: message._id, + shouldReact: true + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); + it('should return statusCode: 200 when unreact a message with flag, shouldReact: false', (done) => { + request.post(api('chat.react')) + .set(credentials) + .send({ + emoji: ':squid:', + messageId: message._id, + shouldReact: false + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + }) + .end(done); + }); + it('should return statusCode: 200 when react a message with flag, shouldReact: true', (done) => { + request.post(api('chat.react')) + .set(credentials) + .send({ + emoji: ':squid:', + messageId: message._id, + shouldReact: true + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + }) + .end(done); + }); it('should return statusCode: 200 when the emoji is valid and has no colons', (done) => { request.post(api('chat.react')) .set(credentials) @@ -208,7 +267,6 @@ describe('[Chat]', function() { }) .end(done); }); - it('should return statusCode: 200 for reaction property when the emoji is valid', (done) => { request.post(api('chat.react')) .set(credentials) From 8beaa597acf652f103d431503f0d93f1ad5513ba Mon Sep 17 00:00:00 2001 From: Unknown Date: Tue, 19 Jun 2018 17:26:12 -0300 Subject: [PATCH 2/3] Removing the exception throw if the /chat.react behavior was the same --- packages/rocketchat-reactions/setReaction.js | 11 ++++------- tests/end-to-end/api/05-chat.js | 12 ++++++------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/rocketchat-reactions/setReaction.js b/packages/rocketchat-reactions/setReaction.js index 222808a36c9ef..6eaf8c51eec21 100644 --- a/packages/rocketchat-reactions/setReaction.js +++ b/packages/rocketchat-reactions/setReaction.js @@ -39,7 +39,7 @@ Meteor.methods({ return false; } - const userAlreadyReacted = message.reactions && message.reactions[reaction] && message.reactions[reaction].usernames.indexOf(user.username) !== -1; + const userAlreadyReacted = Boolean(message.reactions) && message.reactions[reaction] && message.reactions[reaction].usernames.indexOf(user.username) !== -1; const removeUserReaction = () => { message.reactions[reaction].usernames.splice(message.reactions[reaction].usernames.indexOf(user.username), 1); }; @@ -48,10 +48,10 @@ Meteor.methods({ delete message.reactions[reaction]; } }; + if (userAlreadyReacted === shouldReact) { + return; + } if (userAlreadyReacted) { - if (shouldReact) { - throw new Meteor.Error('error-not-allowed', 'You already reacted with this message.', { method: 'setReaction' }); - } removeUserReaction(); removeMessageReactionIfNeed(); @@ -64,9 +64,6 @@ Meteor.methods({ RocketChat.callbacks.run('setReaction', messageId, reaction); } } else { - if (shouldReact === false) { - throw new Meteor.Error('error-not-allowed', 'You still haven\'t reacted with this message.', { method: 'setReaction' }); - } if (!message.reactions) { message.reactions = {}; } diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index f049d65ff024b..07736412471b0 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -364,7 +364,7 @@ describe('[Chat]', function() { }); }); describe('[/chat.react]', () => { - it('should return statusCode: 400 and error when try unreact a message that\'s no reacted yet', (done) => { + it('should return statusCode: 200 and success when try unreact a message that\'s no reacted yet', (done) => { request.post(api('chat.react')) .set(credentials) .send({ @@ -373,9 +373,9 @@ describe('[Chat]', function() { shouldReact: false }) .expect('Content-Type', 'application/json') - .expect(400) + .expect(200) .expect((res) => { - expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('success', true); }) .end(done); }); @@ -408,7 +408,7 @@ describe('[Chat]', function() { }) .end(done); }); - it('should return statusCode: 400 and error when try react a message that\'s already reacted', (done) => { + it('should return statusCode: 200 and success when try react a message that\'s already reacted', (done) => { request.post(api('chat.react')) .set(credentials) .send({ @@ -417,9 +417,9 @@ describe('[Chat]', function() { shouldReact: true }) .expect('Content-Type', 'application/json') - .expect(400) + .expect(200) .expect((res) => { - expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('success', true); }) .end(done); }); From fc8b4c7306042fa636193eb9fc7cd5d0b242df7c Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 19 Jun 2018 18:35:53 -0300 Subject: [PATCH 3/3] fix review --- package-lock.json | 7 ++----- packages/rocketchat-reactions/setReaction.js | 19 +++++++++---------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index cdef413604d4b..4bccc97faab10 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4473,7 +4473,6 @@ "version": "0.1.7", "resolved": "https://registry.npmjs.org/errno/-/errno-0.1.7.tgz", "integrity": "sha512-MfrRBDWzIWifgq6tJj60gkAwtLNb6sQPlcFrSOflcP1aFmmruKQ2wRnze/8V6kgyz7H3FF8Npzv78mZ7XLLflg==", - "optional": true, "requires": { "prr": "1.0.1" } @@ -9478,8 +9477,7 @@ "natives": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/natives/-/natives-1.1.4.tgz", - "integrity": "sha512-Q29yeg9aFKwhLVdkTAejM/HvYG0Y1Am1+HUkFQGn5k2j8GS+v60TVmZh6nujpEAj/qql+wGUrlryO8bF+b1jEg==", - "optional": true + "integrity": "sha512-Q29yeg9aFKwhLVdkTAejM/HvYG0Y1Am1+HUkFQGn5k2j8GS+v60TVmZh6nujpEAj/qql+wGUrlryO8bF+b1jEg==" }, "natural-compare": { "version": "1.4.0", @@ -11211,8 +11209,7 @@ "prr": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/prr/-/prr-1.0.1.tgz", - "integrity": "sha1-0/wRS6BplaRexok/SEzrHXj19HY=", - "optional": true + "integrity": "sha1-0/wRS6BplaRexok/SEzrHXj19HY=" }, "pseudomap": { "version": "1.0.2", diff --git a/packages/rocketchat-reactions/setReaction.js b/packages/rocketchat-reactions/setReaction.js index 6eaf8c51eec21..2cf5a8c7bdc79 100644 --- a/packages/rocketchat-reactions/setReaction.js +++ b/packages/rocketchat-reactions/setReaction.js @@ -1,6 +1,14 @@ /* globals msgStream */ import _ from 'underscore'; +const removeUserReaction = (message, reaction, username) => { + message.reactions[reaction].usernames.splice(message.reactions[reaction].usernames.indexOf(username), 1); + if (message.reactions[reaction].usernames.length === 0) { + delete message.reactions[reaction]; + } + return message; +}; + Meteor.methods({ setReaction(reaction, messageId, shouldReact) { if (!Meteor.userId()) { @@ -40,20 +48,11 @@ Meteor.methods({ } const userAlreadyReacted = Boolean(message.reactions) && message.reactions[reaction] && message.reactions[reaction].usernames.indexOf(user.username) !== -1; - const removeUserReaction = () => { - message.reactions[reaction].usernames.splice(message.reactions[reaction].usernames.indexOf(user.username), 1); - }; - const removeMessageReactionIfNeed = () => { - if (message.reactions[reaction].usernames.length === 0) { - delete message.reactions[reaction]; - } - }; if (userAlreadyReacted === shouldReact) { return; } if (userAlreadyReacted) { - removeUserReaction(); - removeMessageReactionIfNeed(); + removeUserReaction(message, reaction, user.username); if (_.isEmpty(message.reactions)) { delete message.reactions;