From 11abac9c58a792d31726807c28f9b7b5746b816a Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Mon, 27 Mar 2023 10:17:03 +0200 Subject: [PATCH] Added 100% unit test coverage for PostsExporter fixes https://github.com/TryGhost/Team/issues/2796 --- ghost/email-service/test/utils/index.js | 13 +- ghost/posts-service/lib/PostsExporter.js | 16 +- .../posts-service/test/PostsExporter.test.js | 492 ++++++++++++++++++ ghost/posts-service/test/utils/index.js | 37 +- 4 files changed, 540 insertions(+), 18 deletions(-) create mode 100644 ghost/posts-service/test/PostsExporter.test.js diff --git a/ghost/email-service/test/utils/index.js b/ghost/email-service/test/utils/index.js index 3c2178cd8f..8d8500c724 100644 --- a/ghost/email-service/test/utils/index.js +++ b/ghost/email-service/test/utils/index.js @@ -70,17 +70,20 @@ const createModelClass = (options = {}) => { ); }, findAll: async (data) => { - return Promise.resolve( - (options.findAll ?? []).map(f => createModel({...f, ...data})) - ); + const models = (options.findAll ?? []).map(f => createModel({...f, ...data})); + return Promise.resolve({ + models, + map: models.map.bind(models), + length: models.length + }); }, findPage: async (data) => { const all = options.findAll ?? []; const limit = data.limit ?? 15; const page = data.page ?? 1; - const start = (page - 1) * limit; - const end = start + limit; + const start = (page - 1) * (limit === 'all' ? all.length : limit); + const end = limit === 'all' ? all.length : (start + limit); const pageData = all.slice(start, end); return Promise.resolve( diff --git a/ghost/posts-service/lib/PostsExporter.js b/ghost/posts-service/lib/PostsExporter.js index b592d72062..a0123bbdf8 100644 --- a/ghost/posts-service/lib/PostsExporter.js +++ b/ghost/posts-service/lib/PostsExporter.js @@ -83,7 +83,7 @@ class PostsExporter { tags: post.related('tags').map(tag => tag.get('name')).join(', '), post_access: this.postAccessToString(post), email_recipients: email ? this.humanReadableEmailRecipientFilter(email?.get('recipient_filter'), labels) : null, - newsletter: newsletters.length > 1 && post.get('newsletter_id') && email ? newsletters.find(newsletter => newsletter.get('id') === post.get('newsletter_id')).get('name') : null, + newsletter: newsletters.length > 1 && post.get('newsletter_id') && email ? newsletters.find(newsletter => newsletter.get('id') === post.get('newsletter_id'))?.get('name') : null, sends: email?.get('email_count') ?? null, opens: trackOpens ? (email?.get('opened_count') ?? null) : null, clicks: showEmailClickAnalytics ? (post.get('count__clicks') ?? 0) : null, @@ -108,11 +108,11 @@ class PostsExporter { removeableColumns.push('reacted_with_more_like_this', 'reacted_with_less_like_this'); } - if (!membersEnabled && !trackClicks) { + if (membersEnabled && !trackClicks) { removeableColumns.push('clicks'); } - if (!membersEnabled && !trackOpens) { + if (membersEnabled && !trackOpens) { removeableColumns.push('opens'); } @@ -122,10 +122,7 @@ class PostsExporter { removeableColumns.push('paid_signups'); } - // Note the strict null check: we allow columns that are all zero - const columnsToRemove = removeableColumns.filter(key => mapped.every(row => !row[key] && row[key] !== 0)); - - for (const columnToRemove of columnsToRemove) { + for (const columnToRemove of removeableColumns) { for (const row of mapped) { delete row[columnToRemove]; } @@ -212,9 +209,6 @@ class PostsExporter { * @returns */ filterToString(filter, allLabels) { - if (!filter) { - return []; - } const strings = []; if (filter.$and) { // Not supported @@ -245,7 +239,7 @@ class PostsExporter { } else if (filter.status === 'paid') { strings.push('paid members'); } else if (filter.status === 'comped') { - strings.push('comped members'); + strings.push('complimentary members'); } } else { if (filter.status.$ne === 'free') { diff --git a/ghost/posts-service/test/PostsExporter.test.js b/ghost/posts-service/test/PostsExporter.test.js new file mode 100644 index 0000000000..16be9e3f50 --- /dev/null +++ b/ghost/posts-service/test/PostsExporter.test.js @@ -0,0 +1,492 @@ +const {PostsExporter} = require('../index'); +const assert = require('assert'); +const {createModelClass, createModel} = require('./utils'); + +class SettingsCache { + constructor(settings) { + this.settings = settings; + } + + get(key) { + return this.settings[key]; + } + + set(key, value) { + this.settings[key] = value; + } +} + +describe('PostsExporter', function () { + it('Can construct class', function () { + new PostsExporter({}); + }); + + describe('export', function () { + let exporter; + let models; + let post; + let settingsCache; + let settingsHelpers; + let defaultNewsletter; + + beforeEach(function () { + defaultNewsletter = { + id: createModel({}).id, + name: 'Daily Newsletter', + feedback_enabled: true + }; + + post = { + title: 'Test Post', + status: 'published', + created_at: new Date(), + published_at: new Date(), + updated_at: new Date(), + featured: false, + loaded: ['tiers','tags','authors','email'], + email: createModel({ + feedback_enabled: true, + track_clicks: true, + email_count: 256, + opened_count: 128 + }), + count__clicks: 64, + count__signups: 32, + count__paid_conversions: 16, + count__positive_feedback: 8, + count__negative_feedback: 4, + newsletter_id: defaultNewsletter.id, + authors: [ + createModel({ + name: 'Test Author' + }) + ], + tags: [ + createModel({ + name: 'Test Tag' + }) + ], + visibility: 'tiers', + tiers: [ + createModel({ + name: 'Silver' + }), + createModel({ + name: 'Gold' + }) + ] + }; + models = { + Post: createModelClass({ + findAll: [ + post + ] + }), + Newsletter: createModelClass({ + findAll: [ + defaultNewsletter + ] + }), + Label: createModelClass({ + findAll: [] + }) + }; + + settingsCache = new SettingsCache({ + members_track_sources: true, + email_track_opens: true, + email_track_clicks: true + }); + + settingsHelpers = { + isMembersEnabled: () => true, + arePaidMembersEnabled: () => true + }; + + exporter = new PostsExporter({ + models, + settingsCache, + settingsHelpers, + getPostUrl: () => 'https://example.com/post' + }); + }); + + it('Can export posts', async function () { + const posts = await exporter.export({}); + assert.equal(posts.length, 1); + + // Hides newsletter column + assert.equal(posts[0].newsletter, undefined); + }); + + it('Adds newsletter columns if multiple newsletters', async function () { + const secondNewsletter = { + id: createModel({}).id, + name: 'Weekly Newsletter', + feedback_enabled: true + }; + models.Newsletter.options.findAll.push(secondNewsletter); + models.Post.options.findAll.push({ + ...post, + newsletter_id: models.Newsletter.options.findAll[1].id + }); + const posts = await exporter.export({}); + assert.equal(posts.length, 2); + + // Shows newsletter column + assert.equal(posts[0].newsletter, 'Daily Newsletter'); + assert.equal(posts[1].newsletter, 'Weekly Newsletter'); + + // Shows feedback columns + assert.equal(posts[0].reacted_with_more_like_this, post.count__positive_feedback); + assert.equal(posts[0].reacted_with_less_like_this, post.count__negative_feedback); + }); + + it('Hides feedback columns if feedback disabled for all newsletters', async function () { + defaultNewsletter.feedback_enabled = false; + const posts = await exporter.export({}); + + // Hides feedback columns + assert.equal(posts[0].reacted_with_more_like_this, undefined); + assert.equal(posts[0].reacted_with_less_like_this, undefined); + }); + + it('Hides email related analytics when post status is draft', async function () { + const secondNewsletter = { + id: createModel({}).id, + name: 'Weekly Newsletter', + feedback_enabled: true + }; + models.Newsletter.options.findAll.push(secondNewsletter); + post.status = 'draft'; + const posts = await exporter.export({}); + + // No feedback columns + assert.equal(posts[0].reacted_with_more_like_this, undefined); + assert.equal(posts[0].reacted_with_less_like_this, undefined); + + // Sends etc + assert.equal(posts[0].sends, undefined); + assert.equal(posts[0].opens, undefined); + assert.equal(posts[0].clicks, undefined); + assert.equal(posts[0].newsletter, undefined); + + // Signups + assert.equal(posts[0].free_signups, undefined); + assert.equal(posts[0].paid_signups, undefined); + }); + + it('Hides member related columns if members disabled', async function () { + settingsHelpers.isMembersEnabled = () => false; + const posts = await exporter.export({}); + assert.equal(posts[0].email_recipients, undefined); + + // No feedback columns + assert.equal(posts[0].reacted_with_more_like_this, undefined); + assert.equal(posts[0].reacted_with_less_like_this, undefined); + + // Sends etc + assert.equal(posts[0].sends, undefined); + assert.equal(posts[0].opens, undefined); + assert.equal(posts[0].clicks, undefined); + + // Signups + assert.equal(posts[0].free_signups, undefined); + assert.equal(posts[0].paid_signups, undefined); + }); + + it('Hides clicks if disabled', async function () { + settingsCache.set('email_track_clicks', false); + const posts = await exporter.export({}); + + assert.notEqual(posts[0].email_recipients, undefined); + assert.notEqual(posts[0].reacted_with_more_like_this, undefined); + assert.notEqual(posts[0].reacted_with_less_like_this, undefined); + assert.notEqual(posts[0].sends, undefined); + assert.notEqual(posts[0].opens, undefined); + assert.notEqual(posts[0].free_signups, undefined); + assert.notEqual(posts[0].paid_signups, undefined); + + assert.equal(posts[0].clicks, undefined); + }); + + it('Hides opens if disabled', async function () { + settingsCache.set('email_track_opens', false); + const posts = await exporter.export({}); + + assert.notEqual(posts[0].email_recipients, undefined); + assert.notEqual(posts[0].reacted_with_more_like_this, undefined); + assert.notEqual(posts[0].reacted_with_less_like_this, undefined); + assert.notEqual(posts[0].sends, undefined); + assert.notEqual(posts[0].clicks, undefined); + assert.notEqual(posts[0].free_signups, undefined); + assert.notEqual(posts[0].paid_signups, undefined); + + assert.equal(posts[0].opens, undefined); + }); + + it('Hides paid member related columns if paid members disabled', async function () { + settingsHelpers.arePaidMembersEnabled = () => false; + const posts = await exporter.export({}); + + assert.notEqual(posts[0].email_recipients, undefined); + assert.notEqual(posts[0].reacted_with_more_like_this, undefined); + assert.notEqual(posts[0].reacted_with_less_like_this, undefined); + assert.notEqual(posts[0].sends, undefined); + assert.notEqual(posts[0].clicks, undefined); + assert.notEqual(posts[0].free_signups, undefined); + assert.notEqual(posts[0].opens, undefined); + + assert.equal(posts[0].paid_signups, undefined); + }); + + it('Works if relations not loaded correctly', async function () { + delete post.count__clicks; + delete post.count__signups; + delete post.count__paid_conversions; + delete post.count__positive_feedback; + delete post.count__negative_feedback; + + const posts = await exporter.export({}); + assert.equal(posts.length, 1); + + assert.equal(posts[0].clicks, 0); + assert.equal(posts[0].free_signups, 0); + assert.equal(posts[0].paid_signups, 0); + assert.equal(posts[0].reacted_with_more_like_this, 0); + assert.equal(posts[0].reacted_with_less_like_this, 0); + }); + }); + + describe('mapPostStatus', function () { + const exporter = new PostsExporter({}); + + it('Returns draft', function () { + assert.equal( + exporter.mapPostStatus('draft', false), + 'draft' + ); + }); + + it('Returns scheduled', function () { + assert.equal( + exporter.mapPostStatus('scheduled', false), + 'scheduled' + ); + }); + + it('Returns emailed only', function () { + assert.equal( + exporter.mapPostStatus('sent', false), + 'emailed only' + ); + }); + + it('Returns published and emailed', function () { + assert.equal( + exporter.mapPostStatus('published', true), + 'published and emailed' + ); + }); + + it('Returns published only', function () { + assert.equal( + exporter.mapPostStatus('published', false), + 'published only' + ); + }); + + it('Returns unsupported', function () { + assert.equal( + exporter.mapPostStatus('unsupported', false), + 'unsupported' + ); + }); + }); + + describe('postAccessToString', function () { + const exporter = new PostsExporter({}); + + it('Returns public', function () { + const access = exporter.postAccessToString( + createModel({ + visibility: 'public' + }) + ); + assert.equal( + access, + 'Public' + ); + }); + + it('Returns members', function () { + const access = exporter.postAccessToString( + createModel({ + visibility: 'members' + }) + ); + assert.equal( + access, + 'Free members' + ); + }); + + it('Returns paid', function () { + const access = exporter.postAccessToString( + createModel({ + visibility: 'paid' + }) + ); + assert.equal( + access, + 'Paid members' + ); + }); + + it('Returns empty tiers', function () { + const access = exporter.postAccessToString( + createModel({ + visibility: 'tiers', + loaded: ['tiers'], + tiers: [] + }) + ); + assert.equal( + access, + 'Nobody' + ); + }); + + it('Returns multiple tiers', function () { + const access = exporter.postAccessToString( + createModel({ + visibility: 'tiers', + loaded: ['tiers'], + tiers: [ + createModel({ + name: 'Silver' + }), + createModel({ + name: 'Gold' + }) + ] + }) + ); + assert.equal( + access, + 'Silver, Gold' + ); + }); + + it('Returns unsupported', function () { + const access = exporter.postAccessToString( + createModel({ + visibility: 'unsupported' + }) + ); + assert.equal( + access, + 'unsupported' + ); + }); + }); + + describe('humanReadableEmailRecipientFilter', function () { + const exporter = new PostsExporter({}); + let labels; + + beforeEach(function () { + labels = [ + createModel({ + slug: 'imported', + name: 'Imported' + }), + createModel({ + slug: 'vip', + name: 'VIP' + }) + ]; + }); + + it('Returns all', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('all'), + 'all' + ); + }); + + it('Returns empty', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter(''), + '' + ); + }); + + it('Returns labels', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('label:imported', labels), + 'Imported' + ); + + assert.equal( + exporter.humanReadableEmailRecipientFilter('label:imported,label:vip', labels), + 'Imported, VIP' + ); + }); + + it('Returns invalid labels', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('label:invalidone', labels), + 'invalidone' + ); + }); + + it('Returns status', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('status:free'), + 'free members' + ); + + assert.equal( + exporter.humanReadableEmailRecipientFilter('status:-free'), + 'paid members' + ); + + assert.equal( + exporter.humanReadableEmailRecipientFilter('status:paid'), + 'paid members' + ); + + assert.equal( + exporter.humanReadableEmailRecipientFilter('status:comped'), + 'complimentary members' + ); + + assert.equal( + exporter.humanReadableEmailRecipientFilter('status:-paid'), + 'free members' + ); + }); + + it('Ignores AND', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('status:free+status:paid', labels), + '' + ); + }); + + it('Single brackets filter', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('(status:free)', labels), + 'free members' + ); + }); + + it('Ignores invalid filters', function () { + assert.equal( + exporter.humanReadableEmailRecipientFilter('sdgsdgsdg sdg sdg sdgs dgs', labels), + 'sdgsdgsdg sdg sdg sdgs dgs' + ); + }); + }); +}); diff --git a/ghost/posts-service/test/utils/index.js b/ghost/posts-service/test/utils/index.js index f3d689a041..1de8cc7814 100644 --- a/ghost/posts-service/test/utils/index.js +++ b/ghost/posts-service/test/utils/index.js @@ -12,7 +12,10 @@ const createModel = (propertiesAndRelations) => { } if (Array.isArray(propertiesAndRelations[relation])) { return Promise.resolve({ - models: propertiesAndRelations[relation] + models: propertiesAndRelations[relation], + toJSON: () => { + return propertiesAndRelations[relation].map(m => m.toJSON()); + } }); } return Promise.resolve(propertiesAndRelations[relation]); @@ -24,6 +27,13 @@ const createModel = (propertiesAndRelations) => { if (!propertiesAndRelations.loaded.includes(relation)) { throw new Error(`Model.related('${relation}') was used on a test model that didn't explicitly loaded that relation.`); } + if (Array.isArray(propertiesAndRelations[relation])) { + const arr = [...propertiesAndRelations[relation]]; + arr.toJSON = () => { + return arr.map(m => m.toJSON()); + }; + return arr; + } return propertiesAndRelations[relation]; }, get: (property) => { @@ -45,6 +55,7 @@ const createModel = (propertiesAndRelations) => { const createModelClass = (options = {}) => { return { ...options, + options, add: async (properties) => { return Promise.resolve(createModel(properties)); }, @@ -60,8 +71,30 @@ const createModelClass = (options = {}) => { ); }, findAll: async (data) => { + const models = (options.findAll ?? []).map(f => createModel({...f, ...data})); + return Promise.resolve({ + models, + map: models.map.bind(models), + length: models.length + }); + }, + findPage: async (data) => { + const all = options.findAll ?? []; + const limit = data.limit ?? 15; + const page = data.page ?? 1; + + const start = (page - 1) * (limit === 'all' ? all.length : limit); + const end = limit === 'all' ? all.length : (start + limit); + + const pageData = all.slice(start, end); return Promise.resolve( - (options.findAll ?? []).map(f => createModel({...f, ...data})) + { + data: pageData.map(f => createModel({...f, ...data})), + meta: { + page, + limit + } + } ); }, transaction: async (callback) => {