TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-12-29 13:52:10 +03:00
Code Issues Projects Releases Wiki Activity

Replaced schema.isPost in slack service /w custom fn

Browse Source 
- This is the only piece of server code that relies on the schema.checks, the rest are all frontend
- IMO this code should actually check for the post properties that the slack message needs
- OR it should switch based on the event type
- either way there's no need to have a shared util for this simple use case
- especially becaue it's confusing the use case for it and creating cross-coupling between server and frontend
This commit is contained in:
Hannah Wolfe 2022-04-05 14:14:39 +01:00
parent 4ee2fcd869
commit 11867ab43a
No known key found for this signature in database
GPG Key ID: AB586C3B5AE5C037
2 changed files with 12 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
core/server/services/slack.js
View File

@ -6,7 +6,6 @@ const {blogIcon} = require('../lib/image');
const urlUtils = require('../../shared/url-utils');
const urlService = require('./url');
const settingsCache = require('../../shared/settings-cache');
const schema = require('../data/schema').checks;
const moment = require('moment');
// Used to receive post.published model event, but also the slack.test event from the API which iirc this was done to avoid circular deps a long time ago
@ -37,6 +36,15 @@ function getSlackSettings() {
};
}
/**
* @TODO: change this function to check for the properties we depend on
* @param {Object} data
* @returns {boolean}
*/
function hasPostProperties(data) {
return Object.prototype.hasOwnProperty.call(data, 'html') && Object.prototype.hasOwnProperty.call(data, 'title') && Object.prototype.hasOwnProperty.call(data, 'slug');
}
function ping(post) {
let message;
let title;
@ -47,7 +55,7 @@ function ping(post) {
let blogTitle = settingsCache.get('title');
// If this is a post, we want to send the link of the post
if (schema.isPost(post)) {
if (hasPostProperties(post)) {
message = urlService.getUrlByResourceId(post.id, {absolute: true});
title = post.title ? post.title : null;
author = post.authors ? post.authors[0] : null;
@ -79,7 +87,7 @@ function ping(post) {
return;
}
if (schema.isPost(post)) {
if (hasPostProperties(post)) {
slackData = {
// We are handling the case of test notification here by checking
// if it is a post or a test message to check webhook working.

14
test/unit/server/services/slack.test.js
View File

@ -11,7 +11,6 @@ const events = require('../../../../core/server/lib/common/events');
const logging = require('@tryghost/logging');
const imageLib = require('../../../../core/server/lib/image');
const urlService = require('../../../../core/server/services/url');
const schema = require('../../../../core/server/data/schema').checks;
const settingsCache = require('../../../../core/shared/settings-cache');
// Test data
@ -94,14 +93,12 @@ describe('Slack', function () {
});
describe('ping()', function () {
let isPostStub;
let settingsCacheStub;
let slackReset;
let makeRequestStub;
const ping = slack.__get__('ping');
beforeEach(function () {
isPostStub = sinon.stub(schema, 'isPost');
sinon.stub(urlService, 'getUrlByResourceId');
settingsCacheStub = sinon.stub(settingsCache, 'get');
@ -127,7 +124,6 @@ describe('Slack', function () {
const post = testUtils.DataGenerator.forKnex.createPost({slug: 'webhook-test'});
urlService.getUrlByResourceId.withArgs(post.id, {absolute: true}).returns('http://myblog.com/' + post.slug + '/');
isPostStub.returns(true);
settingsCacheStub.withArgs('slack_url').returns(slackURL);
// execute code
@ -135,7 +131,6 @@ describe('Slack', function () {
// assertions
makeRequestStub.calledOnce.should.be.true();
isPostStub.calledTwice.should.be.true();
urlService.getUrlByResourceId.calledOnce.should.be.true();
settingsCacheStub.calledWith('slack_url').should.be.true();
@ -157,7 +152,6 @@ describe('Slack', function () {
let requestUrl;
let requestData;
isPostStub.returns(false);
settingsCacheStub.withArgs('slack_url').returns(slackURL);
// execute code
@ -165,7 +159,6 @@ describe('Slack', function () {
// assertions
makeRequestStub.calledOnce.should.be.true();
isPostStub.calledTwice.should.be.true();
urlService.getUrlByResourceId.called.should.be.false();
settingsCacheStub.calledWith('slack_url').should.be.true();
@ -198,7 +191,6 @@ describe('Slack', function () {
it('does not make a request if post is a page', function () {
const post = testUtils.DataGenerator.forKnex.createPost({type: 'page'});
isPostStub.returns(true);
settingsCacheStub.withArgs('slack_url').returns(slackURL);
// execute code
@ -206,14 +198,12 @@ describe('Slack', function () {
// assertions
makeRequestStub.calledOnce.should.be.false();
isPostStub.calledOnce.should.be.true();
urlService.getUrlByResourceId.calledOnce.should.be.true();
settingsCacheStub.calledWith('slack_url').should.be.true();
});
it('does not send webhook for \'welcome\' post', function () {
const post = testUtils.DataGenerator.forKnex.createPost({slug: 'welcome'});
isPostStub.returns(true);
settingsCacheStub.withArgs('slack_url').returns(slackURL);
// execute code
@ -221,7 +211,6 @@ describe('Slack', function () {
// assertions
makeRequestStub.calledOnce.should.be.false();
isPostStub.calledOnce.should.be.true();
urlService.getUrlByResourceId.calledOnce.should.be.true();
settingsCacheStub.calledWith('slack_url').should.be.true();
});
@ -235,8 +224,7 @@ describe('Slack', function () {
// assertions
makeRequestStub.calledOnce.should.be.false();
isPostStub.calledOnce.should.be.true();
urlService.getUrlByResourceId.called.should.be.false();
urlService.getUrlByResourceId.called.should.be.true();
settingsCacheStub.calledWith('slack_url').should.be.true();
});
});