Prevented registering multiple Slack event listeners

refs https://github.com/TryGhost/Toolbox/issues/592

- in tests, we boot Ghost over and over
- this inits all the services each time
- it turns out that the Slack event listener is registered 80+ times
- to prevent this, we can check if it has already been registered, like
  we do with webhooks: 4639396c3a/ghost/core/core/server/services/webhooks/listen.js (L67-L69)
- not sure about this pattern in general, but it's something we can
  review in coming weeks
This commit is contained in:
Daniel Lockyer 2023-06-12 14:41:41 +02:00 committed by Daniel Lockyer
parent 4639396c3a
commit af8c0dc7a5
2 changed files with 14 additions and 9 deletions

View File

@ -153,7 +153,7 @@ function ping(post) {
} }
} }
function listener(model, options) { function slackListener(model, options) {
// CASE: do not ping slack if we import a database // CASE: do not ping slack if we import a database
// TODO: refactor post.published events to never fire on importing // TODO: refactor post.published events to never fire on importing
if (options && options.importing) { if (options && options.importing) {
@ -163,15 +163,20 @@ function listener(model, options) {
ping(model.toJSON()); ping(model.toJSON());
} }
function testPing() { function slackTestPing() {
ping({ ping({
message: 'Heya! This is a test notification from your Ghost blog :smile:. Seems to work fine!' message: 'Heya! This is a test notification from your Ghost blog :smile:. Seems to work fine!'
}); });
} }
function listen() { function listen() {
events.on('post.published', listener); if (!events.hasRegisteredListener('post.published', 'slackListener')) {
events.on('slack.test', testPing); events.on('post.published', slackListener);
}
if (!events.hasRegisteredListener('slack.test', 'slackTestPing')) {
events.on('slack.test', slackTestPing);
}
} }
// Public API // Public API

View File

@ -32,8 +32,8 @@ describe('Slack', function () {
it('listen() should initialise event correctly', function () { it('listen() should initialise event correctly', function () {
slack.listen(); slack.listen();
eventStub.calledTwice.should.be.true(); eventStub.calledTwice.should.be.true();
eventStub.firstCall.calledWith('post.published', slack.__get__('listener')).should.be.true(); eventStub.firstCall.calledWith('post.published', slack.__get__('slackListener')).should.be.true();
eventStub.secondCall.calledWith('slack.test', slack.__get__('testPing')).should.be.true(); eventStub.secondCall.calledWith('slack.test', slack.__get__('slackTestPing')).should.be.true();
}); });
it('listener() calls ping() with toJSONified model', function () { it('listener() calls ping() with toJSONified model', function () {
@ -47,7 +47,7 @@ describe('Slack', function () {
const pingStub = sinon.stub(); const pingStub = sinon.stub();
const resetSlack = slack.__set__('ping', pingStub); const resetSlack = slack.__set__('ping', pingStub);
const listener = slack.__get__('listener'); const listener = slack.__get__('slackListener');
listener(testModel); listener(testModel);
@ -69,7 +69,7 @@ describe('Slack', function () {
const pingStub = sinon.stub(); const pingStub = sinon.stub();
const resetSlack = slack.__set__('ping', pingStub); const resetSlack = slack.__set__('ping', pingStub);
const listener = slack.__get__('listener'); const listener = slack.__get__('slackListener');
listener(testModel, {importing: true}); listener(testModel, {importing: true});
@ -82,7 +82,7 @@ describe('Slack', function () {
it('testPing() calls ping() with default message', function () { it('testPing() calls ping() with default message', function () {
const pingStub = sinon.stub(); const pingStub = sinon.stub();
const resetSlack = slack.__set__('ping', pingStub); const resetSlack = slack.__set__('ping', pingStub);
const testPing = slack.__get__('testPing'); const testPing = slack.__get__('slackTestPing');
testPing(); testPing();