From 2180cb3d98ce08d93d9151957717302ef3cb41c3 Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Fri, 26 Aug 2016 22:12:23 -0500 Subject: [PATCH 1/5] feat(api): Send flag notifications to slack --- config.json.example | 4 + package.json | 5 +- test/api/v3/unit/libs/slack.js | 97 +++++++++++++++++++++++ website/server/controllers/api-v3/chat.js | 7 ++ website/server/libs/slack.js | 48 +++++++++++ 5 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 test/api/v3/unit/libs/slack.js create mode 100644 website/server/libs/slack.js diff --git a/config.json.example b/config.json.example index 2fd677c7d4..5b03d26791 100644 --- a/config.json.example +++ b/config.json.example @@ -74,5 +74,9 @@ "APP_ID": "appId", "KEY": "key", "SECRET": "secret" + }, + "SLACK": { + "FLAGGING_URL": "https://hooks.slack.com/services/id/id/id", + "FLAGGING_FOOTER_LINK": "https://crookedneighbor.github.io/flag-o-rama/" } } diff --git a/package.json b/package.json index 71da316005..046b6c81b4 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,7 @@ "version": "3.37.0", "main": "./website/server/index.js", "dependencies": { + "@slack/client": "slackhq/node-slack-sdk#2ee794cd31326c54f38c518eef2b9d223327d939", "accepts": "^1.3.2", "amazon-payments": "0.0.4", "amplitude": "^2.0.3", @@ -89,12 +90,12 @@ "superagent": "^1.8.3", "swagger-node-express": "lefnire/swagger-node-express#habitrpg", "universal-analytics": "~0.3.2", + "useragent": "2.1.9", "uuid": "^2.0.1", "validator": "^4.9.0", "vinyl-buffer": "^1.0.0", "vinyl-source-stream": "^1.1.0", - "winston": "^2.1.0", - "useragent": "2.1.9" + "winston": "^2.1.0" }, "private": true, "engines": { diff --git a/test/api/v3/unit/libs/slack.js b/test/api/v3/unit/libs/slack.js new file mode 100644 index 0000000000..b3d579349a --- /dev/null +++ b/test/api/v3/unit/libs/slack.js @@ -0,0 +1,97 @@ +/* eslint-disable camelcase */ +import { IncomingWebhook } from '@slack/client'; +import slack from '../../../../../website/server/libs/slack'; +import { TAVERN_ID } from '../../../../../website/server/models/group'; + +describe('slack', () => { + describe('sendFlagNotification', () => { + let flagger, group, message; + + beforeEach(() => { + sandbox.stub(IncomingWebhook.prototype, 'send'); + flagger = { + id: 'flagger-id', + profile: { + name: 'flagger', + }, + }; + group = { + id: 'group-id', + privacy: 'private', + name: 'Some group', + type: 'guild', + }; + message = { + id: 'chat-id', + user: 'Author', + uuid: 'author-id', + text: 'some text', + }; + }); + + afterEach(() => { + IncomingWebhook.prototype.send.restore(); + }); + + it('sends a slack webhook', () => { + slack.sendFlagNotification({ + flagger, + group, + message, + }); + + expect(IncomingWebhook.prototype.send).to.be.calledOnce; + expect(IncomingWebhook.prototype.send).to.be.calledWith({ + text: 'flagger (flagger-id) flagged a message', + attachments: [{ + fallback: 'Flag Message', + color: 'danger', + author_name: 'Author - author-id', + title: 'Flag in Some group - (private guild)', + title_link: undefined, + text: 'some text', + footer: sandbox.match(/<.*?groupId=group-id&chatId=chat-id\|Flag this message>/), + mrkdwn_in: [ + 'text', + ], + }], + }); + }); + + it('includes a title link if guild is public', () => { + group.privacy = 'public'; + + slack.sendFlagNotification({ + flagger, + group, + message, + }); + + expect(IncomingWebhook.prototype.send).to.be.calledWithMatch({ + attachments: [sandbox.match({ + title: 'Flag in Some group', + title_link: sandbox.match(/.*\/#\/options\/groups\/guilds\/group-id/), + })], + }); + }); + + it('links to tavern', () => { + group.privacy = 'public'; + group.name = 'Tavern'; + group.id = TAVERN_ID; + + slack.sendFlagNotification({ + flagger, + group, + message, + }); + + expect(IncomingWebhook.prototype.send).to.be.calledWithMatch({ + attachments: [sandbox.match({ + title: 'Flag in Tavern', + title_link: sandbox.match(/.*\/#\/options\/groups\/tavern/), + })], + }); + }); + }); +}); diff --git a/website/server/controllers/api-v3/chat.js b/website/server/controllers/api-v3/chat.js index ea74d4238e..4dded03e9b 100644 --- a/website/server/controllers/api-v3/chat.js +++ b/website/server/controllers/api-v3/chat.js @@ -8,6 +8,7 @@ import { import _ from 'lodash'; import { removeFromArray } from '../../libs/collectionManipulators'; import { getUserInfo, getGroupUrl, sendTxn } from '../../libs/email'; +import slack from '../../libs/slack'; import pusher from '../../libs/pusher'; import nconf from 'nconf'; import Bluebird from 'bluebird'; @@ -265,6 +266,12 @@ api.flagChat = { {name: 'GROUP_URL', content: groupUrl}, ]); + slack.sendFlagNotification({ + flagger: user, + group, + message, + }); + res.respond(200, message); }, }; diff --git a/website/server/libs/slack.js b/website/server/libs/slack.js new file mode 100644 index 0000000000..e67ab26030 --- /dev/null +++ b/website/server/libs/slack.js @@ -0,0 +1,48 @@ +/* eslint-disable camelcase */ +import { IncomingWebhook } from '@slack/client'; +import { TAVERN_ID } from '../models/group'; +import nconf from 'nconf'; + +const SLACK_FLAGGING_URL = nconf.get('SLACK:FLAGGING_URL'); +const SLACK_FLAGGING_FOOTER_LINK = nconf.get('SLACK:FLAGGING_FOOTER_LINK'); +const BASE_URL = nconf.get('BASE_URL'); + +let flagSlack = new IncomingWebhook(SLACK_FLAGGING_URL); + +function sendFlagNotification ({ + flagger, + group, + message, +}) { + let titleLink; + let title = `Flag in ${group.name}`; + let text = `${flagger.profile.name} (${flagger.id}) flagged a message`; + + if (group.id === TAVERN_ID) { + titleLink = `${BASE_URL}/#/options/groups/tavern`; + } else if (group.privacy === 'public') { + titleLink = `${BASE_URL}/#/options/groups/guilds/${group.id}`; + } else { + title += ` - (${group.privacy} ${group.type})`; + } + + flagSlack.send({ + text, + attachments: [{ + fallback: 'Flag Message', + color: 'danger', + author_name: `${message.user} - ${message.uuid}`, + title, + title_link: titleLink, + text: message.text, + footer: `<${SLACK_FLAGGING_FOOTER_LINK}?groupId=${group.id}&chatId=${message.id}|Flag this message>`, + mrkdwn_in: [ + 'text', + ], + }], + }); +} + +module.exports = { + sendFlagNotification, +}; From 41851afe5fc3344328453d472bca6573b6bf71b2 Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Fri, 26 Aug 2016 23:09:54 -0500 Subject: [PATCH 2/5] fix: noop if no slack flagging url is passed in --- test/api/v3/unit/libs/slack.js | 19 +++++++++++++++++++ website/server/libs/slack.js | 12 +++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/test/api/v3/unit/libs/slack.js b/test/api/v3/unit/libs/slack.js index b3d579349a..b133380384 100644 --- a/test/api/v3/unit/libs/slack.js +++ b/test/api/v3/unit/libs/slack.js @@ -1,7 +1,10 @@ /* eslint-disable camelcase */ import { IncomingWebhook } from '@slack/client'; +import requireAgain from 'require-again'; import slack from '../../../../../website/server/libs/slack'; +import logger from '../../../../../website/server/libs/logger'; import { TAVERN_ID } from '../../../../../website/server/models/group'; +import nconf from 'nconf'; describe('slack', () => { describe('sendFlagNotification', () => { @@ -93,5 +96,21 @@ describe('slack', () => { })], }); }); + + it('noops if no flagging url is provided', () => { + sandbox.stub(nconf, 'get').withArgs('SLACK:FLAGGING_URL').returns(''); + sandbox.stub(logger, 'error'); + let reRequiredSlack = requireAgain('../../../../../website/server/libs/slack'); + + expect(logger.error).to.be.calledOnce; + + reRequiredSlack.sendFlagNotification({ + flagger, + group, + message, + }); + + expect(IncomingWebhook.prototype.send).to.not.be.called; + }); }); }); diff --git a/website/server/libs/slack.js b/website/server/libs/slack.js index e67ab26030..2c309ae430 100644 --- a/website/server/libs/slack.js +++ b/website/server/libs/slack.js @@ -1,5 +1,6 @@ /* eslint-disable camelcase */ import { IncomingWebhook } from '@slack/client'; +import logger from './logger'; import { TAVERN_ID } from '../models/group'; import nconf from 'nconf'; @@ -7,13 +8,22 @@ const SLACK_FLAGGING_URL = nconf.get('SLACK:FLAGGING_URL'); const SLACK_FLAGGING_FOOTER_LINK = nconf.get('SLACK:FLAGGING_FOOTER_LINK'); const BASE_URL = nconf.get('BASE_URL'); -let flagSlack = new IncomingWebhook(SLACK_FLAGGING_URL); +let flagSlack; + +try { + flagSlack = new IncomingWebhook(SLACK_FLAGGING_URL); +} catch (err) { + logger.error(err); +} function sendFlagNotification ({ flagger, group, message, }) { + if (!SLACK_FLAGGING_URL) { + return; + } let titleLink; let title = `Flag in ${group.name}`; let text = `${flagger.profile.name} (${flagger.id}) flagged a message`; From 6480602ee6f3fcf599dd4cde5f08f7157266b7c2 Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Sat, 27 Aug 2016 07:20:36 -0500 Subject: [PATCH 3/5] fix(test): Correct logger test --- test/api/v3/unit/libs/logger.js | 57 +++++++++++++-------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/test/api/v3/unit/libs/logger.js b/test/api/v3/unit/libs/logger.js index 20189ccbe1..3464a304e2 100644 --- a/test/api/v3/unit/libs/logger.js +++ b/test/api/v3/unit/libs/logger.js @@ -1,57 +1,46 @@ import winston from 'winston'; -import requireAgain from 'require-again'; +import logger from '../../../../../website/server/libs/logger'; -/* eslint-disable global-require */ describe('logger', () => { - let pathToLoggerLib = '../../../../../website/server/libs/logger'; - let infoSpy; - let errorSpy; + let logSpy; beforeEach(() => { - infoSpy = sandbox.stub(); - errorSpy = sandbox.stub(); - sandbox.stub(winston, 'Logger').returns({ - info: infoSpy, - error: errorSpy, - }); + logSpy = sandbox.stub(winston.Logger.prototype, 'log'); }); afterEach(() => { sandbox.restore(); }); - it('info', () => { - let attachLogger = requireAgain(pathToLoggerLib); - attachLogger.info(1, 2, 3); - expect(infoSpy).to.be.calledOnce; - expect(infoSpy).to.be.calledWith(1, 2, 3); + describe('info', () => { + it('calls winston\'s info log', () => { + logger.info(1, 2, 3); + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith('info', 1, 2, 3); + }); }); describe('error', () => { - it('with custom arguments', () => { - let attachLogger = requireAgain(pathToLoggerLib); - attachLogger.error(1, 2, 3, 4); - expect(errorSpy).to.be.calledOnce; - expect(errorSpy).to.be.calledWith(1, 2, 3, 4); + it('passes through arguments if the first arg is not an error object', () => { + logger.error(1, 2, 3, 4); + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith('error', 1, 2, 3, 4); }); - it('with error', () => { - let attachLogger = requireAgain(pathToLoggerLib); + it('parses the error and passes it to the logger when the first arg is an error object', () => { let errInstance = new Error('An error.'); - attachLogger.error(errInstance, { + logger.error(errInstance, { data: 1, }, 2, 3); - expect(errorSpy).to.be.calledOnce; - // using calledWith doesn't work - let lastCallArgs = errorSpy.lastCall.args; - expect(lastCallArgs[3]).to.equal(3); - expect(lastCallArgs[2]).to.equal(2); - expect(lastCallArgs[1]).to.eql({ - data: 1, - fullError: errInstance, - }); - expect(lastCallArgs[0]).to.eql(errInstance.stack); + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + { data: 1, fullError: errInstance }, + 2, + 3 + ); }); }); }); From 45e4c6867e7a834c74129d9111ca0b350c69002a Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Sat, 27 Aug 2016 07:49:26 -0500 Subject: [PATCH 4/5] chore(test): increase test coverage around logger lib --- test/api/v3/unit/libs/logger.js | 162 ++++++++++++++++++++++++++++---- 1 file changed, 145 insertions(+), 17 deletions(-) diff --git a/test/api/v3/unit/libs/logger.js b/test/api/v3/unit/libs/logger.js index 3464a304e2..a02e58509f 100644 --- a/test/api/v3/unit/libs/logger.js +++ b/test/api/v3/unit/libs/logger.js @@ -1,5 +1,8 @@ import winston from 'winston'; import logger from '../../../../../website/server/libs/logger'; +import { + NotFound, +} from '../../../../../website/server/libs//errors'; describe('logger', () => { let logSpy; @@ -21,26 +24,151 @@ describe('logger', () => { }); describe('error', () => { - it('passes through arguments if the first arg is not an error object', () => { - logger.error(1, 2, 3, 4); - expect(logSpy).to.be.calledOnce; - expect(logSpy).to.be.calledWith('error', 1, 2, 3, 4); + context('non-error object', () => { + it('passes through arguments if the first arg is not an error object', () => { + logger.error(1, 2, 3, 4); + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith('error', 1, 2, 3, 4); + }); }); - it('parses the error and passes it to the logger when the first arg is an error object', () => { - let errInstance = new Error('An error.'); - logger.error(errInstance, { - data: 1, - }, 2, 3); + context('error object', () => { + it('logs the stack and the err data', () => { + let errInstance = new Error('An error.'); + logger.error(errInstance, { + data: 1, + }, 2, 3); - expect(logSpy).to.be.calledOnce; - expect(logSpy).to.be.calledWith( - 'error', - errInstance.stack, - { data: 1, fullError: errInstance }, - 2, - 3 - ); + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + { data: 1, fullError: errInstance }, + 2, + 3 + ); + }); + + it('logs the stack and the err data with it\'s own fullError property', () => { + let errInstance = new Error('An error.'); + let anotherError = new Error('another error'); + + logger.error(errInstance, { + data: 1, + fullError: anotherError, + }, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + { data: 1, fullError: anotherError }, + 2, + 3 + ); + }); + + it('logs the error when errorData is null', () => { + let errInstance = new Error('An error.'); + + logger.error(errInstance, null, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + null, + 2, + 3 + ); + }); + + it('logs the error when errorData is not an object', () => { + let errInstance = new Error('An error.'); + + logger.error(errInstance, true, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + true, + 2, + 3 + ); + }); + + it('logs the error when errorData does not include isHandledError property', () => { + let errInstance = new Error('An error.'); + + logger.error(errInstance, { httpCode: 400 }, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + { httpCode: 400, fullError: errInstance }, + 2, + 3 + ); + }); + + it('logs the error when errorData includes isHandledError property but is a 500 error', () => { + let errInstance = new Error('An error.'); + + logger.error(errInstance, { + isHandledError: true, + httpCode: 502, + }, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + { httpCode: 502, isHandledError: true, fullError: errInstance }, + 2, + 3 + ); + }); + + it('logs a warning when errorData includes isHandledError property and is not a 500 error', () => { + let errInstance = new Error('An error.'); + + logger.error(errInstance, { + isHandledError: true, + httpCode: 403, + }, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'warn', + errInstance.stack, + { httpCode: 403, isHandledError: true, fullError: errInstance }, + 2, + 3 + ); + }); + + it('logs additional data from a CustomError', () => { + let errInstance = new NotFound('An error.'); + + errInstance.customField = 'Some interesting data'; + + logger.error(errInstance, {}, 2, 3); + + expect(logSpy).to.be.calledOnce; + expect(logSpy).to.be.calledWith( + 'error', + errInstance.stack, + { + fullError: { + customField: 'Some interesting data', + }, + }, + 2, + 3 + ); + }); }); }); }); From 87973d7b668fb57ac3708f364321a13c72f25fad Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Mon, 29 Aug 2016 17:01:36 -0500 Subject: [PATCH 5/5] chore: Switch out footer link for HabitRPG version --- config.json.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.json.example b/config.json.example index 5b03d26791..c742eeef5f 100644 --- a/config.json.example +++ b/config.json.example @@ -77,6 +77,6 @@ }, "SLACK": { "FLAGGING_URL": "https://hooks.slack.com/services/id/id/id", - "FLAGGING_FOOTER_LINK": "https://crookedneighbor.github.io/flag-o-rama/" + "FLAGGING_FOOTER_LINK": "https://habitrpg.github.io/flag-o-rama/" } }