Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-17 23:44:39 -05:00
Code Issues Activity

Refactored extra-attrs output serializer util

Browse source 
- after discussing on a code review, we agreed the existing code was
  very verbose and it was hard to reason about
- this is because of the incredibly long if-statements we have lingering around
- the general approach here was to extract all the includes into
  variables at the top, replace them in the code and then simplify the logic
- whilst doing this, I wrote some more tests to cover the other cases
- this commit also switches to assert, as it's neater to write the
  assertions in tests
This commit is contained in:
Daniel Lockyer 2025-02-06 11:32:22 +01:00 • committed by Daniel Lockyer
parent 2bd95b3efc
commit 31e1048db3
2 changed files with 70 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
ghost/core/core/server/api/endpoints/utils/serializers/output/utils/extra-attrs.js
View file

@ -8,31 +8,32 @@ const readingMinutes = require('@tryghost/helpers').utils.readingMinutes;
* @returns {void} - modifies attrs
*/
module.exports.forPost = (options, model, attrs) => {
// This function is split up in 3 conditions for 3 different purposes:
const columnsIncludesCustomExcerpt = options.columns?.includes('custom_excerpt');
const columnsIncludesExcerpt = options.columns?.includes('excerpt');
const columnsIncludesPlaintext = options.columns?.includes('plaintext');
const columnsIncludesReadingTime = options.columns?.includes('reading_time');
const formatsIncludesPlaintext = options.formats?.includes('plaintext');
// 1. Gets excerpt from post's plaintext. If custom_excerpt exists, it overrides the excerpt but the key remains excerpt.
if (Object.prototype.hasOwnProperty.call(options, 'columns') || options.columns?.includes('excerpt') || (options.columns?.includes('excerpt') && options.formats?.includes('plaintext'))) {
if (options.columns?.includes('excerpt')) {
if (!attrs.custom_excerpt || attrs.custom_excerpt === null) {
let plaintext = model.get('plaintext');
if (plaintext) {
attrs.excerpt = plaintext.substring(0, 500);
} else {
attrs.excerpt = null;
}
if (!options.columns.includes('custom_excerpt')) {
delete attrs.custom_excerpt;
}
if (columnsIncludesExcerpt) {
if (!attrs.custom_excerpt) {
let plaintext = model.get('plaintext');
if (plaintext) {
attrs.excerpt = plaintext.substring(0, 500);
} else {
attrs.excerpt = attrs.custom_excerpt;
if (!options.columns?.includes('custom_excerpt')) {
delete attrs.custom_excerpt;
}
attrs.excerpt = null;
}
} else {
attrs.excerpt = attrs.custom_excerpt;
}
if (!columnsIncludesCustomExcerpt) {
delete attrs.custom_excerpt;
}
}
// 2. Displays plaintext if requested by user as a field. Also works if used as format.
if (options.columns?.includes('plaintext') || options.formats?.includes('plaintext')) {
// 2. Displays plaintext if requested via `columns` or `formats`
if (columnsIncludesPlaintext || formatsIncludesPlaintext) {
let plaintext = model.get('plaintext');
if (plaintext) {
attrs.plaintext = plaintext;
@ -41,15 +42,14 @@ module.exports.forPost = (options, model, attrs) => {
}
}
// 3. Displays excerpt if no columns was requested - specifically needed for the Admin Posts API.
// 3. Displays excerpt if no columns was requested - specifically needed for the Admin Posts API
if (!Object.prototype.hasOwnProperty.call(options, 'columns')) {
let plaintext = model.get('plaintext');
let customExcerpt = model.get('custom_excerpt');
if (customExcerpt !== null) {
attrs.excerpt = customExcerpt;
} else {
const plaintext = model.get('plaintext');
if (plaintext) {
attrs.excerpt = plaintext.substring(0, 500);
} else {
@ -58,10 +58,8 @@ module.exports.forPost = (options, model, attrs) => {
}
}
// reading_time still only works when used along with formats=html.
if (!Object.prototype.hasOwnProperty.call(options, 'columns') ||
(options.columns?.includes('reading_time'))) {
// 4. Add `reading_time` if no columns were requested, or if `reading_time` was requested via `columns`
if (!Object.prototype.hasOwnProperty.call(options, 'columns') || columnsIncludesReadingTime) {
if (attrs.html) {
let additionalImages = 0;

58
ghost/core/test/unit/api/canary/utils/serializers/output/utils/extra-attrs.test.js
View file

@ -1,4 +1,4 @@
const should = require('should');
const assert = require('assert/strict');
const sinon = require('sinon');
const extraAttrsUtil = require('../../../../../../../../core/server/api/endpoints/utils/serializers/output/utils/extra-attrs');
@ -19,33 +19,67 @@ describe('Unit: endpoints/utils/serializers/output/utils/extra-attrs', function
it('respects custom excerpt', function () {
const attrs = {custom_excerpt: 'custom excerpt'};
extraAttrsUtil.forPost(options, model, attrs);
attrs.excerpt.should.eql(attrs.custom_excerpt);
assert.equal(attrs.excerpt, attrs.custom_excerpt);
});
it('no custom excerpt', function () {
const attrs = {};
extraAttrsUtil.forPost(options, model, attrs);
model.get.called.should.be.true();
attrs.excerpt.should.eql(new Array(501).join('A'));
assert.ok(model.get.called);
assert.equal(attrs.excerpt, new Array(501).join('A'));
});
it('has excerpt when plaintext is null', function () {
model.get.withArgs('plaintext').returns(null);
const attrs = {};
extraAttrsUtil.forPost(options, model, attrs);
model.get.called.should.be.true();
attrs.should.have.property('excerpt');
(attrs.excerpt === null).should.be.true();
assert.ok(model.get.called);
assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'excerpt'), true);
assert.equal(attrs.excerpt, null);
});
it('has plaintext when columns includes plaintext', function () {
const attrs = {};
extraAttrsUtil.forPost({
columns: ['plaintext']
}, model, attrs);
assert.ok(model.get.called);
assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'plaintext'), true);
});
it('has plaintext when formats includes plaintext', function () {
const attrs = {};
extraAttrsUtil.forPost({
formats: ['plaintext']
}, model, attrs);
assert.ok(model.get.called);
assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'plaintext'), true);
});
it('has excerpt when no columns are passed', function () {
delete options.columns;
const attrs = {};
extraAttrsUtil.forPost(options, model, attrs);
model.get.called.should.be.true();
attrs.should.have.property('excerpt');
extraAttrsUtil.forPost({}, model, attrs);
assert.ok(model.get.called);
assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'excerpt'), true);
});
it('has reading_time when no columns are passed', function () {
const attrs = {
html: 'html'
};
extraAttrsUtil.forPost({}, model, attrs);
assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'reading_time'), true);
});
it('has reading_time when columns includes reading_time', function () {
const attrs = {
html: 'html'
};
extraAttrsUtil.forPost({
columns: ['reading_time']
}, model, attrs);
assert.equal(Object.prototype.hasOwnProperty.call(attrs, 'reading_time'), true);
});
});
});