Fixed error in newsletter editing limit checks (#14817)

refs https://github.com/TryGhost/Team/issues/1583
refs https://ghost.slack.com/archives/C02G9E68C/p1652397268702749?thread_ts=1652397192.822389&cid=C02G9E68C

Used `model.status` instead of `model.get('status')`, resulting in undefined, resulting in `!== 'active'` to return true. Also added a test case for editing active newsletters.
This commit is contained in:
Simon Backx 2022-05-13 10:15:35 +02:00 committed by GitHub
parent d3f29e53de
commit a95e9d0b7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 71 additions and 1 deletions

View File

@ -191,7 +191,7 @@ class NewslettersService {
const {cleanedAttrs, emailsToVerify} = await this.prepAttrsForEmailVerification(attrs, originalNewsletter);
if (originalNewsletter.status !== 'active' && cleanedAttrs.status === 'active') {
if (originalNewsletter.get('status') !== 'active' && cleanedAttrs.status === 'active') {
await this.limitService.errorIfWouldGoOverLimit('newsletters', sharedOptions);
}

View File

@ -1302,6 +1302,52 @@ Object {
}
`;
exports[`Newsletters API Host Settings: newsletter limits Max limit Editing an active newsletter doesn't fail 1: [body] 1`] = `
Object {
"newsletters": Array [
Object {
"body_font_category": "serif",
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"description": null,
"footer_content": null,
"header_image": null,
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"name": "Updated active newsletter name",
"sender_email": "jamie@example.com",
"sender_name": "Jamie",
"sender_reply_to": "newsletter",
"show_badge": true,
"show_feature_image": true,
"show_header_icon": true,
"show_header_name": true,
"show_header_title": true,
"slug": "old-newsletter",
"sort_order": 2,
"status": "archived",
"subscribe_on_signup": true,
"title_alignment": "center",
"title_font_category": "serif",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"visibility": "members",
},
],
}
`;
exports[`Newsletters API Host Settings: newsletter limits Max limit Editing an active newsletter doesn't fail 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "678",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-cache-invalidate": "/*",
"x-powered-by": "Express",
}
`;
exports[`Newsletters API Host Settings: newsletter limits Max limit Editing an archived newsletter doesn't fail 1: [body] 1`] = `
Object {
"newsletters": Array [

View File

@ -474,6 +474,30 @@ describe('Newsletters API', function () {
location: anyLocationFor('newsletters')
});
});
it('Editing an active newsletter doesn\'t fail', async function () {
const allNewsletters = await models.Newsletter.findAll();
const newsletterCount = allNewsletters.filter(n => n.get('status') === 'active').length;
assert.equal(newsletterCount, 3, 'This test expects to have 3 current active newsletters');
const activeNewsletter = allNewsletters.find(n => n.get('status') !== 'active');
assert.ok(activeNewsletter, 'This test expects to have an active newsletter in the test fixtures');
const id = activeNewsletter.id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
name: 'Updated active newsletter name'
}]
})
.expectStatus(200)
.matchBodySnapshot({
newsletters: [newsletterSnapshot]
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
it('Editing an archived newsletter doesn\'t fail', async function () {
const allNewsletters = await models.Newsletter.findAll();