diff --git a/ghost/member-attribution/lib/history.js b/ghost/member-attribution/lib/history.js index 7dcad7fc77..ed644042af 100644 --- a/ghost/member-attribution/lib/history.js +++ b/ghost/member-attribution/lib/history.js @@ -1,19 +1,24 @@ -/** - * @typedef {UrlHistoryItem[]} UrlHistoryArray - */ - /** * @typedef {Object} UrlHistoryItem * @prop {string} path * @prop {number} time */ +/** + * @typedef {UrlHistoryItem[]} UrlHistoryArray + */ + /** * Represents a validated history */ class UrlHistory { + /** + * @private + * @param {UrlHistoryArray} urlHistory + */ constructor(urlHistory) { - this.history = urlHistory && UrlHistory.isValidHistory(urlHistory) ? urlHistory : []; + /** @private */ + this.history = urlHistory; } get length() { @@ -27,12 +32,34 @@ class UrlHistory { yield* this.history.slice().reverse(); } + /** + * @private + * @param {any[]} history + * @returns {boolean} + */ static isValidHistory(history) { - return Array.isArray(history) && !history.find(item => !this.isValidHistoryItem(item)); + for (const item of history) { + if (typeof item?.path !== 'string' || !Number.isSafeInteger(item?.time)) { + return false; + } + } + return true; } - static isValidHistoryItem(item) { - return !!item && !!item.path && !!item.time && typeof item.path === 'string' && typeof item.time === 'number' && Number.isSafeInteger(item.time); + /** + * @param {unknown} urlHistory + * @returns {UrlHistory} + */ + static create(urlHistory) { + if (!Array.isArray(urlHistory)) { + return new UrlHistory([]); + } + + if (!this.isValidHistory(urlHistory)) { + return new UrlHistory([]); + } + + return new UrlHistory(urlHistory); } } diff --git a/ghost/member-attribution/lib/service.js b/ghost/member-attribution/lib/service.js index 2001ab32bd..2b5e3f5318 100644 --- a/ghost/member-attribution/lib/service.js +++ b/ghost/member-attribution/lib/service.js @@ -2,8 +2,8 @@ const UrlHistory = require('./history'); class MemberAttributionService { /** - * - * @param {Object} deps + * + * @param {Object} deps * @param {Object} deps.attributionBuilder * @param {Object} deps.models * @param {Object} deps.models.MemberCreatedEvent @@ -15,12 +15,12 @@ class MemberAttributionService { } /** - * - * @param {import('./history').UrlHistoryArray} historyArray + * + * @param {import('./history').UrlHistoryArray} historyArray * @returns {import('./attribution').Attribution} */ getAttribution(historyArray) { - const history = new UrlHistory(historyArray); + const history = UrlHistory.create(historyArray); return this.attributionBuilder.getAttribution(history); } @@ -60,7 +60,7 @@ class MemberAttributionService { /** * Returns the parsed attribution for a member creation event - * @param {string} memberId + * @param {string} memberId * @returns {Promise} */ async getMemberCreatedAttribution(memberId) { @@ -78,7 +78,7 @@ class MemberAttributionService { /** * Returns the last attribution for a given subscription ID - * @param {string} subscriptionId + * @param {string} subscriptionId * @returns {Promise} */ async getSubscriptionCreatedAttribution(subscriptionId) { diff --git a/ghost/member-attribution/test/attribution.test.js b/ghost/member-attribution/test/attribution.test.js index ba3970cd3f..3b1738583f 100644 --- a/ghost/member-attribution/test/attribution.test.js +++ b/ghost/member-attribution/test/attribution.test.js @@ -56,18 +56,18 @@ describe('AttributionBuilder', function () { }); it('Returns empty if empty history', function () { - const history = new UrlHistory([]); + const history = UrlHistory.create([]); should(attributionBuilder.getAttribution(history)).match({id: null, type: null, url: null}); }); it('Returns last url', function () { - const history = new UrlHistory([{path: '/dir/not-last', time: 123}, {path: '/dir/test/', time: 123}]); + const history = UrlHistory.create([{path: '/dir/not-last', time: 123}, {path: '/dir/test/', time: 123}]); should(attributionBuilder.getAttribution(history)).match({type: 'url', id: null, url: '/test/'}); }); it('Returns last post', function () { - const history = new UrlHistory([ - {path: '/dir/my-post', time: 123}, + const history = UrlHistory.create([ + {path: '/dir/my-post', time: 123}, {path: '/dir/test', time: 124}, {path: '/dir/unknown-page', time: 125} ]); @@ -75,25 +75,25 @@ describe('AttributionBuilder', function () { }); it('Returns last post even when it found pages', function () { - const history = new UrlHistory([ - {path: '/dir/my-post', time: 123}, - {path: '/dir/my-page', time: 124}, + const history = UrlHistory.create([ + {path: '/dir/my-post', time: 123}, + {path: '/dir/my-page', time: 124}, {path: '/dir/unknown-page', time: 125} ]); should(attributionBuilder.getAttribution(history)).match({type: 'post', id: 123, url: '/my-post'}); }); it('Returns last page if no posts', function () { - const history = new UrlHistory([ - {path: '/dir/other', time: 123}, - {path: '/dir/my-page', time: 124}, + const history = UrlHistory.create([ + {path: '/dir/other', time: 123}, + {path: '/dir/my-page', time: 124}, {path: '/dir/unknown-page', time: 125} ]); should(attributionBuilder.getAttribution(history)).match({type: 'page', id: 845, url: '/my-page'}); }); it('Returns all null for invalid histories', function () { - const history = new UrlHistory('invalid'); + const history = UrlHistory.create('invalid'); should(attributionBuilder.getAttribution(history)).match({ type: null, id: null, @@ -102,7 +102,7 @@ describe('AttributionBuilder', function () { }); it('Returns all null for empty histories', function () { - const history = new UrlHistory([]); + const history = UrlHistory.create([]); should(attributionBuilder.getAttribution(history)).match({ type: null, id: null, diff --git a/ghost/member-attribution/test/history.test.js b/ghost/member-attribution/test/history.test.js index 383fa0a648..91316633fa 100644 --- a/ghost/member-attribution/test/history.test.js +++ b/ghost/member-attribution/test/history.test.js @@ -4,72 +4,56 @@ require('./utils'); const UrlHistory = require('../lib/history'); describe('UrlHistory', function () { - describe('Constructor', function () { - it('sets history to empty array if invalid', function () { - const history = new UrlHistory('invalid'); - should(history.history).eql([]); - }); - it('sets history to empty array if missing', function () { - const history = new UrlHistory(); - should(history.history).eql([]); - }); - }); - - describe('Validation', function () { - it('isValidHistory returns false for non arrays', function () { - should(UrlHistory.isValidHistory('string')).eql(false); - should(UrlHistory.isValidHistory()).eql(false); - should(UrlHistory.isValidHistory(12)).eql(false); - should(UrlHistory.isValidHistory(null)).eql(false); - should(UrlHistory.isValidHistory({})).eql(false); - should(UrlHistory.isValidHistory(NaN)).eql(false); - - should(UrlHistory.isValidHistory([ + it('sets history to empty array if invalid', function () { + const inputs = [ + 'string', + undefined, + 12, + null, + {}, + NaN, + [ { time: 1, path: '/test' }, 't' - ])).eql(false); - }); - - it('isValidHistory returns true for valid arrays', function () { - should(UrlHistory.isValidHistory([])).eql(true); - should(UrlHistory.isValidHistory([ - { - time: 1, - path: '/test' - } - ])).eql(true); - }); - - it('isValidHistoryItem returns false for invalid objects', function () { - should(UrlHistory.isValidHistoryItem({})).eql(false); - should(UrlHistory.isValidHistoryItem('test')).eql(false); - should(UrlHistory.isValidHistoryItem(0)).eql(false); - should(UrlHistory.isValidHistoryItem()).eql(false); - should(UrlHistory.isValidHistoryItem(NaN)).eql(false); - should(UrlHistory.isValidHistoryItem([])).eql(false); - - should(UrlHistory.isValidHistoryItem({ + ], + [{}], + ['test'], + [0], + [undefined], + [NaN], + [[]], + [{ time: 'test', path: 'test' - })).eql(false); - - should(UrlHistory.isValidHistoryItem({ + }], + [{ path: 'test' - })).eql(false); - - should(UrlHistory.isValidHistoryItem({ + }], + [{ time: 123 - })).eql(false); - }); + }] + ]; - it('isValidHistoryItem returns true for valid objects', function () { - should(UrlHistory.isValidHistoryItem({ - time: 123, - path: '/time' - })).eql(true); - }); + for (const input of inputs) { + const history = UrlHistory.create(input); + should(history.history).eql([]); + } + }); + + it('sets history for valid arrays', function () { + const inputs = [ + [], + [{ + time: Date.now(), + path: '/test' + }] + ]; + for (const input of inputs) { + const history = UrlHistory.create(input); + should(history.history).eql(input); + } }); });