Refactored to use individual slack settings (#2384)

refs: TryGhost/Team#1625

- we want to remove backwards compatibility code for slack being a single setting

Co-authored-by: Rishabh <zrishabhgarg@gmail.com>
This commit is contained in:
Hannah Wolfe 2022-05-16 11:38:32 +01:00 committed by GitHub
parent de2d3977ed
commit 385240e03d
13 changed files with 70 additions and 271 deletions

View File

@ -1166,3 +1166,14 @@ add|ember-template-lint|require-input-label|48|12|48|12|16e2b28dce448cbd2dd1cdc1
remove|ember-template-lint|require-input-label|19|8|19|8|aefef8f18b7648ad42353400a95bf5d4ccc426bc|1652227200000|1662595200000|1667782800000|app/components/editor-labs/modals/preview/email.hbs
remove|ember-template-lint|no-action|2|48|2|48|74d234e67cc2c695fd41431692bb2974c2feddcf|1652054400000|1662422400000|1665014400000|app/templates/editor.hbs
remove|ember-template-lint|no-action|35|108|35|108|374dcf5faec6721e81711cb589f7966ab6063a14|1652054400000|1662422400000|1665014400000|app/templates/editor.hbs
add|ember-template-lint|no-action|52|55|52|55|2c1e9f8787439af0488bbeeed6adc276a525e585|1652659200000|1663027200000|1668211200000|app/templates/settings/integrations/slack.hbs
add|ember-template-lint|no-action|53|59|53|59|126db4f8e2a36d157b01c874381f548fd8d4a79e|1652659200000|1663027200000|1668211200000|app/templates/settings/integrations/slack.hbs
add|ember-template-lint|no-action|78|55|78|55|f790efaece9bf04396b25c2fedf4625a426f2467|1652659200000|1663027200000|1668211200000|app/templates/settings/integrations/slack.hbs
add|ember-template-lint|no-passed-in-event-handlers|52|48|52|48|138f75fcc7408e61c79185c37265529c752af8b3|1652659200000|1663027200000|1668211200000|app/templates/settings/integrations/slack.hbs
add|ember-template-lint|no-passed-in-event-handlers|78|48|78|48|8fb95409c6333d15490f0777239f6e04641215e6|1652659200000|1663027200000|1668211200000|app/templates/settings/integrations/slack.hbs
remove|ember-template-lint|no-action|52|55|52|55|6a97879be420bcaec8193dad451443ab160931a2|1652054400000|1662422400000|1665014400000|app/templates/settings/integrations/slack.hbs
remove|ember-template-lint|no-action|56|59|56|59|e8badded2afd5d0c8426b85b6d0e7ac0bef55efb|1652054400000|1662422400000|1665014400000|app/templates/settings/integrations/slack.hbs
remove|ember-template-lint|no-action|78|55|78|55|856c8ab1d5835ec424c83eb6b73888b26dccdc51|1652054400000|1662422400000|1665014400000|app/templates/settings/integrations/slack.hbs
remove|ember-template-lint|no-action|82|59|82|59|e8badded2afd5d0c8426b85b6d0e7ac0bef55efb|1652054400000|1662422400000|1665014400000|app/templates/settings/integrations/slack.hbs
remove|ember-template-lint|no-passed-in-event-handlers|52|48|52|48|a893d9d149388aa9da024f59db6522bb9335bf8a|1652054400000|1662422400000|1665014400000|app/templates/settings/integrations/slack.hbs
remove|ember-template-lint|no-passed-in-event-handlers|78|48|78|48|1ab458a0888727af2b3a7a661a1e7d5f00ac0ad5|1652054400000|1662422400000|1665014400000|app/templates/settings/integrations/slack.hbs

View File

@ -1,10 +1,8 @@
import classic from 'ember-classic-decorator';
import {action} from '@ember/object';
import {empty} from '@ember/object/computed';
import {inject as service} from '@ember/service';
/* eslint-disable ghost/ember/alias-model-in-controller */
import Controller from '@ember/controller';
import boundOneWay from 'ghost-admin/utils/bound-one-way';
import {isInvalidError} from 'ember-ajax/errors';
import {task} from 'ember-concurrency';
@ -16,51 +14,21 @@ export default class SlackController extends Controller {
@service settings;
leaveSettingsTransition = null;
slackArray = null;
init() {
super.init(...arguments);
this.slackArray = [];
}
@boundOneWay('settings.slack.firstObject')
slackSettings;
@empty('slackSettings.url')
testNotificationDisabled;
get testNotificationDisabled() {
const slackUrl = this.settings.get('slackUrl');
return !slackUrl;
}
@action
save() {
this.saveTask.perform();
}
@action
updateURL(value) {
value = typeof value === 'string' ? value.trim() : value;
this.set('slackSettings.url', value);
this.get('slackSettings.errors').clear();
}
@action
updateUsername(value) {
value = typeof value === 'string' ? value.trimLeft() : value;
this.set('slackSettings.username', value);
this.get('slackSettings.errors').clear();
}
@action
triggerDirtyState() {
let slack = this.slackSettings;
let slackArray = this.slackArray;
let settings = this.settings;
// Hack to trigger the `isDirty` state on the settings model by setting a new Array
// for slack rather that replacing the existing one which would still point to the
// same reference and therfore not setting the model into a dirty state
slackArray.clear().pushObject(slack);
settings.set('slack', slackArray);
}
@action
toggleLeaveSettingsModal(transition) {
let leaveTransition = this.leaveSettingsTransition;
@ -90,7 +58,6 @@ export default class SlackController extends Controller {
leaveSettings() {
let transition = this.leaveSettingsTransition;
let settings = this.settings;
let slackArray = this.slackArray;
if (!transition) {
this.notifications.showAlert('Sorry, there was an error in the application. Please let the Ghost team know what happened.', {type: 'error'});
@ -99,22 +66,14 @@ export default class SlackController extends Controller {
// roll back changes on model props
settings.rollbackAttributes();
slackArray.clear();
return transition.retry();
}
@(task(function* () {
let slack = this.slackSettings;
let settings = this.settings;
let slackArray = this.slackArray;
try {
yield slack.validate();
// clear existing objects in slackArray to make sure we only push the validated one
slackArray.clear().pushObject(slack);
yield settings.set('slack', slackArray);
return yield settings.save();
yield this.settings.validate();
return yield this.settings.save();
} catch (error) {
if (error) {
this.notifications.showAPIError(error);
@ -134,10 +93,12 @@ export default class SlackController extends Controller {
notifications.showNotification('Test notification sent', {type: 'info', key: 'slack-test.send.success', description: 'Check your Slack channel for the test message'});
return true;
} catch (error) {
notifications.showAPIError(error, {key: 'slack-test:send'});
if (error) {
notifications.showAPIError(error, {key: 'slack-test:send'});
if (!isInvalidError(error)) {
throw error;
if (!isInvalidError(error)) {
throw error;
}
}
}
}).drop())

View File

@ -18,7 +18,6 @@ import SettingValidator from 'ghost-admin/validators/setting';
import SetupValidator from 'ghost-admin/validators/setup';
import SigninValidator from 'ghost-admin/validators/signin';
import SignupValidator from 'ghost-admin/validators/signup';
import SlackIntegrationValidator from 'ghost-admin/validators/slack-integration';
import SnippetValidator from 'ghost-admin/validators/snippet';
import TagSettingsValidator from 'ghost-admin/validators/tag-settings';
import TierBenefitItemValidator from 'ghost-admin/validators/tier-benefit-item';
@ -67,7 +66,6 @@ export default Mixin.create({
setup: SetupValidator,
signin: SigninValidator,
signup: SignupValidator,
slackIntegration: SlackIntegrationValidator,
tag: TagSettingsValidator,
user: UserValidator,
member: MemberValidator,

View File

@ -24,7 +24,8 @@ export default Model.extend(ValidationEngine, {
isPrivate: attr('boolean'),
publicHash: attr('string'),
password: attr('string'),
slack: attr('slack-settings'),
slackUrl: attr('string'),
slackUsername: attr('string'),
amp: attr('boolean'),
ampGtagId: attr('string'),
firstpromoter: attr('boolean'),

View File

@ -1,16 +0,0 @@
import EmberObject, {computed} from '@ember/object';
import ValidationEngine from 'ghost-admin/mixins/validation-engine';
import {isBlank} from '@ember/utils';
export default EmberObject.extend(ValidationEngine, {
// values entered here will act as defaults
url: '',
username: '',
validationType: 'slackIntegration',
isActive: computed('url', function () {
let url = this.url;
return !isBlank(url);
})
});

View File

@ -44,22 +44,22 @@
<div class="gh-setting-title">Webhook URL</div>
<div class="gh-setting-desc">Automatically send newly published posts to a channel in Slack or any Slack-compatible service like Discord or Mattermost.</div>
<div class="gh-setting-content-extended">
<GhFormGroup @errors={{this.slackSettings.errors}} @hasValidated={{this.slackSettings.hasValidated}} @property="url">
<GhFormGroup @errors={{this.settings.errors}} @hasValidated={{this.settings.hasValidated}} @property="slackUrl">
<GhTextInput
@placeholder="https://hooks.slack.com/services/..."
@name="slack[url]"
@value={{readonly this.slackSettings.url}}
@input={{action "updateURL" value="target.value"}}
@name="slack_url"
@value={{readonly this.settings.slackUrl}}
@input={{action (mut this.settings.slackUrl) value="target.value"}}
@focus-out={{action "validate" "slackUrl" target=this.settings}}
@keyEvents={{hash
Enter=(action "save")
}}
@focus-out={{action "triggerDirtyState"}}
data-test-slack-url-input={{true}}
/>
{{#unless this.slackSettings.errors.url}}
{{#unless this.settings.errors}}
<p>Set up a new incoming webhook <a href="https://my.slack.com/apps/new/A0F7XDUAZ-incoming-webhooks" target="_blank" rel="noopener noreferrer">here</a>, and grab the URL.</p>
{{else}}
<GhErrorMessage @errors={{this.slackSettings.errors}} @property="url" data-test-error="slack-url" />
<GhErrorMessage @errors={{this.settings.errors}} @property="slackUrl" data-test-error="slack-url" />
{{/unless}}
</GhFormGroup>
</div>
@ -70,20 +70,19 @@
<div class="gh-setting-title">Username</div>
<div class="gh-setting-desc">The username to display messages from</div>
<div class="gh-setting-content-extended">
<GhFormGroup @errors={{this.slackSettings.errors}} @hasValidated={{this.slackSettings.hasValidated}} @property="username">
<GhFormGroup @errors={{this.settings.errors}} @hasValidated={{this.settings.hasValidated}} @property="username">
<GhTextInput
@placeholder="Ghost"
@name="slack[username]"
@value={{readonly this.slackSettings.username}}
@input={{action "updateUsername" value="target.value"}}
@name="slack_username"
@value={{readonly this.settings.slackUsername}}
@input={{action (mut this.settings.slackUsername) value="target.value"}}
@keyEvents={{hash
Enter=(action "save")
}}
@focus-out={{action "triggerDirtyState"}}
data-test-slack-username-input={{true}}
/>
{{#if this.slackSettings.errors.username}}
<GhErrorMessage @errors={{this.slackSettings.errors}} @property="username" />
{{#if this.settings.errors}}
<GhErrorMessage @errors={{this.settings.errors}} @property="slackUsername" />
{{/if}}
</GhFormGroup>
</div>

View File

@ -1,40 +0,0 @@
/* eslint-disable camelcase */
import SlackObject from 'ghost-admin/models/slack-integration';
import Transform from '@ember-data/serializer/transform';
import {isArray as isEmberArray} from '@ember/array';
import {isEmpty} from '@ember/utils';
export default class SlackSettings extends Transform {
deserialize(serialized) {
let settingsArray;
try {
settingsArray = JSON.parse(serialized) || [];
} catch (e) {
settingsArray = [];
}
if (isEmpty(settingsArray)) {
settingsArray.push({url: '', username: ''});
}
let slackObjs = settingsArray.map(itemDetails => SlackObject.create(itemDetails));
return slackObjs;
}
serialize(deserialized) {
let settingsArray;
if (isEmberArray(deserialized)) {
settingsArray = deserialized.map((item) => {
let url = (item.get('url') || '').trim();
let username = (item.get('username') || '').trim();
if (url) {
return {url, username};
}
}).compact();
} else {
settingsArray = [];
}
return JSON.stringify(settingsArray);
}
}

View File

@ -1,8 +1,9 @@
import BaseValidator from './base';
import validator from 'validator';
import {isBlank} from '@ember/utils';
export default BaseValidator.create({
properties: ['title', 'description', 'password'],
properties: ['title', 'description', 'password', 'slackUrl'],
title(model) {
let title = model.get('title');
@ -27,6 +28,19 @@ export default BaseValidator.create({
if (isPrivate && password === '') {
model.get('errors').add('password', 'Password must be supplied');
this.invalidate();
}
},
slackUrl(model) {
let slackUrl = model.get('slackUrl');
if (!isBlank(slackUrl) && !validator.isURL(slackUrl, {require_protocol: true})) {
model.get('errors').add(
'slackUrl',
'The URL must be in a format like https://hooks.slack.com/services/<your personal key>'
);
this.invalidate();
}
}

View File

@ -1,24 +0,0 @@
import BaseValidator from './base';
import validator from 'validator';
import {isBlank} from '@ember/utils';
export default BaseValidator.create({
properties: ['url'],
url(model) {
let url = model.get('url');
let hasValidated = model.get('hasValidated');
// eslint-disable-next-line camelcase
if (!isBlank(url) && !validator.isURL(url, {require_protocol: true})) {
model.get('errors').add(
'url',
'The URL must be in a format like https://hooks.slack.com/services/<your personal key>'
);
this.invalidate();
}
hasValidated.addObject('url');
}
});

View File

@ -117,11 +117,11 @@ export default [
id: 16,
created_at: '2016-05-05T15:04:03.115Z',
created_by: 1,
key: 'slack',
key: 'slack_username',
group: 'slack',
updated_at: '2016-05-05T18:33:09.168Z',
updated_at: '2022-05-05T18:33:09.168Z',
updated_by: 1,
value: '[{"url":"", "username":"Ghost"}]'
value: 'Ghost'
},
{
id: 17,
@ -262,5 +262,15 @@ export default [
updated_at: '2022-02-21T13:47:00.000Z',
updated_by: 1,
value: 'true'
},
{
id: 33,
created_at: '2016-05-05T15:04:03.115Z',
created_by: 1,
key: 'slack_url',
group: 'slack',
updated_at: '2022-05-05T18:33:09.168Z',
updated_by: 1,
value: ''
}
];

View File

@ -80,10 +80,12 @@ describe('Acceptance: Settings - Integrations - Slack', function () {
let [newRequest] = this.server.pretender.handledRequests.slice(-1);
let params = JSON.parse(newRequest.requestBody);
let [result] = JSON.parse(params.settings.findBy('key', 'slack').value);
expect(result.url).to.equal('https://hooks.slack.com/services/1275958430');
expect(result.username).to.equal('SlackBot');
let urlResult = params.settings.findBy('key', 'slack_url').value;
let usernameResult = params.settings.findBy('key', 'slack_username').value;
expect(urlResult).to.equal('https://hooks.slack.com/services/1275958430');
expect(usernameResult).to.equal('SlackBot');
expect(find('[data-test-error="slack-url"]'), 'inline validation response')
.to.not.exist;

View File

@ -1,51 +0,0 @@
import SlackIntegration from 'ghost-admin/models/slack-integration';
import {describe, it} from 'mocha';
import {A as emberA} from '@ember/array';
import {expect} from 'chai';
import {setupTest} from 'ember-mocha';
describe('Unit: Transform: slack-settings', function () {
setupTest();
it('deserializes settings json', function () {
let transform = this.owner.lookup('transform:slack-settings');
let serialized = '[{"url":"http://myblog.com/blogpost1","username":"SlackBot"}]';
let result = transform.deserialize(serialized);
expect(result.length).to.equal(1);
expect(result[0]).to.be.instanceof(SlackIntegration);
expect(result[0].get('url')).to.equal('http://myblog.com/blogpost1');
expect(result[0].get('username')).to.equal('SlackBot');
});
it('deserializes empty array', function () {
let transform = this.owner.lookup('transform:slack-settings');
let serialized = '[]';
let result = transform.deserialize(serialized);
expect(result.length).to.equal(1);
expect(result[0]).to.be.instanceof(SlackIntegration);
expect(result[0].get('url')).to.equal('');
expect(result[0].get('username')).to.equal('');
});
it('serializes array of Slack settings', function () {
let transform = this.owner.lookup('transform:slack-settings');
let deserialized = emberA([
SlackIntegration.create({url: 'http://myblog.com/blogpost1', username: 'SlackBot'})
]);
let result = transform.serialize(deserialized);
expect(result).to.equal('[{"url":"http://myblog.com/blogpost1","username":"SlackBot"}]');
});
it('serializes empty SlackIntegration objects', function () {
let transform = this.owner.lookup('transform:slack-settings');
let deserialized = emberA([
SlackIntegration.create({url: ''})
]);
let result = transform.serialize(deserialized);
expect(result).to.equal('[]');
});
});

View File

@ -1,66 +0,0 @@
import SlackObject from 'ghost-admin/models/slack-integration';
import validator from 'ghost-admin/validators/slack-integration';
import {
describe,
it
} from 'mocha';
import {expect} from 'chai';
const testInvalidUrl = function (url) {
let slackObject = SlackObject.create({url});
validator.check(slackObject, 'url');
expect(validator.get('passed'), `"${url}" passed`).to.be.false;
expect(slackObject.get('errors').errorsFor('url').toArray()).to.deep.equal([{
attribute: 'url',
message: 'The URL must be in a format like https://hooks.slack.com/services/<your personal key>'
}]);
expect(slackObject.get('hasValidated')).to.include('url');
};
const testValidUrl = function (url) {
let slackObject = SlackObject.create({url});
validator.check(slackObject, 'url');
expect(validator.get('passed'), `"${url}" failed`).to.be.true;
expect(slackObject.get('hasValidated')).to.include('url');
};
describe('Unit: Validator: slack-integration', function () {
it('fails on invalid url values', function () {
let invalidUrls = [
'test@example.com',
'/has spaces',
'no-leading-slash',
'http://example.com/with spaces'
];
invalidUrls.forEach(function (url) {
testInvalidUrl(url);
});
});
it('passes on valid url values', function () {
let validUrls = [
'https://hooks.slack.com/services/;alskdjf',
'https://hooks.slack.com/services/123445678',
'https://hooks.slack.com/services/some_webhook',
'https://discordapp.com/api/webhooks/380692408364433418/mGLHSRyEoUaTvY91Te16WOT8Obn-BrJoiTNoxeUqhb6klKERb9xaZkUBYC5AeduwYCCy/slack'
];
validUrls.forEach(function (url) {
testValidUrl(url);
});
});
it('validates url by default', function () {
let slackObject = SlackObject.create();
validator.check(slackObject);
expect(slackObject.get('errors').errorsFor('url')).to.be.empty;
expect(validator.get('passed')).to.be.true;
});
});