From 3e60941054246221f25125750462138e059f5f1a Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 30 May 2017 11:40:39 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20=20Add=20=3Fformats=20param=20to=20?= =?UTF-8?q?Posts=20API=20(#8305)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #8275 - Adds support for `formats` param - Returns `html` by default - Can optionally return other formats by providing a comma-separated list --- core/server/api/posts.js | 8 +- core/server/api/utils.js | 23 ++- core/server/data/schema/checks.js | 2 +- core/server/models/post.js | 24 ++++ core/test/functional/routes/api/posts_spec.js | 132 ++++++++++++++++++ .../integration/model/model_posts_spec.js | 50 ++++++- core/test/utils/api.js | 13 +- 7 files changed, 235 insertions(+), 17 deletions(-) diff --git a/core/server/api/posts.js b/core/server/api/posts.js index 671ccada12..0ddbee82cf 100644 --- a/core/server/api/posts.js +++ b/core/server/api/posts.js @@ -37,7 +37,7 @@ posts = { * @returns {Promise} Posts Collection with Meta */ browse: function browse(options) { - var extraOptions = ['status'], + var extraOptions = ['status', 'formats'], permittedOptions, tasks; @@ -62,7 +62,7 @@ posts = { tasks = [ utils.validate(docName, {opts: permittedOptions}), utils.handlePublicPermissions(docName, 'browse'), - utils.convertOptions(allowedIncludes), + utils.convertOptions(allowedIncludes, dataProvider.Post.allowedFormats), modelQuery ]; @@ -79,7 +79,7 @@ posts = { * @return {Promise} Post */ read: function read(options) { - var attrs = ['id', 'slug', 'status', 'uuid'], + var attrs = ['id', 'slug', 'status', 'uuid', 'formats'], tasks; /** @@ -96,7 +96,7 @@ posts = { tasks = [ utils.validate(docName, {attrs: attrs, opts: options.opts || []}), utils.handlePublicPermissions(docName, 'read'), - utils.convertOptions(allowedIncludes), + utils.convertOptions(allowedIncludes, dataProvider.Post.allowedFormats), modelQuery ]; diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 521626f178..b3a33674e3 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -122,7 +122,7 @@ utils = { name: {} }, // these values are sanitised/validated separately - noValidation = ['data', 'context', 'include', 'filter', 'forUpdate', 'transacting'], + noValidation = ['data', 'context', 'include', 'filter', 'forUpdate', 'transacting', 'formats'], errors = []; _.each(options, function (value, key) { @@ -243,12 +243,16 @@ utils = { return this.trimAndLowerCase(fields); }, + prepareFormats: function prepareFormats(formats, allowedFormats) { + return _.intersection(this.trimAndLowerCase(formats), allowedFormats); + }, + /** * ## Convert Options * @param {Array} allowedIncludes * @returns {Function} doConversion */ - convertOptions: function convertOptions(allowedIncludes) { + convertOptions: function convertOptions(allowedIncludes, allowedFormats) { /** * Convert our options from API-style to Model-style * @param {Object} options @@ -258,11 +262,20 @@ utils = { if (options.include) { options.include = utils.prepareInclude(options.include, allowedIncludes); } + if (options.fields) { options.columns = utils.prepareFields(options.fields); delete options.fields; } + if (options.formats) { + options.formats = utils.prepareFormats(options.formats, allowedFormats); + } + + if (options.formats && options.columns) { + options.columns = options.columns.concat(options.formats); + } + return options; }; }, @@ -274,7 +287,7 @@ utils = { * @param {String} docName * @returns {Promise(Object)} resolves to the original object if it checks out */ - checkObject: function (object, docName, editId) { + checkObject: function checkObject(object, docName, editId) { if (_.isEmpty(object) || _.isEmpty(object[docName]) || _.isEmpty(object[docName][0])) { return Promise.reject(new errors.BadRequestError({ message: i18n.t('errors.api.utils.noRootKeyProvided', {docName: docName}) @@ -306,10 +319,10 @@ utils = { return Promise.resolve(object); }, - checkFileExists: function (fileData) { + checkFileExists: function checkFileExists(fileData) { return !!(fileData.mimetype && fileData.path); }, - checkFileIsValid: function (fileData, types, extensions) { + checkFileIsValid: function checkFileIsValid(fileData, types, extensions) { var type = fileData.mimetype, ext = path.extname(fileData.name).toLowerCase(); diff --git a/core/server/data/schema/checks.js b/core/server/data/schema/checks.js index 59eaba8b6a..06cf5bdc3a 100644 --- a/core/server/data/schema/checks.js +++ b/core/server/data/schema/checks.js @@ -1,5 +1,5 @@ function isPost(jsonData) { - return jsonData.hasOwnProperty('html') && jsonData.hasOwnProperty('markdown') && + return jsonData.hasOwnProperty('html') && jsonData.hasOwnProperty('title') && jsonData.hasOwnProperty('slug'); } diff --git a/core/server/models/post.js b/core/server/models/post.js index 0ac9207f5c..29c00ae893 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -477,12 +477,31 @@ Post = ghostBookshelf.Model.extend({ defaultColumnsToFetch: function defaultColumnsToFetch() { return ['id', 'published_at', 'slug', 'author_id']; }, + /** + * If the `formats` option is not used, we return `html` be default. + * Otherwise we return what is requested e.g. `?formats=mobiledoc,plaintext` + */ + formatsToJSON: function formatsToJSON(attrs, options) { + var defaultFormats = ['html'], + formatsToKeep = options.formats || defaultFormats; + + // Iterate over all known formats, and if they are not in the keep list, remove them + _.each(Post.allowedFormats, function (format) { + if (formatsToKeep.indexOf(format) === -1) { + delete attrs[format]; + } + }); + + return attrs; + }, toJSON: function toJSON(options) { options = options || {}; var attrs = ghostBookshelf.Model.prototype.toJSON.call(this, options); + attrs = this.formatsToJSON(attrs, options); + if (!options.columns || (options.columns && options.columns.indexOf('author') > -1)) { attrs.author = attrs.author || attrs.author_id; delete attrs.author_id; @@ -505,6 +524,8 @@ Post = ghostBookshelf.Model.extend({ return this.isPublicContext() ? 'page:false' : 'page:false+status:published'; } }, { + allowedFormats: ['markdown', 'mobiledoc', 'html', 'plaintext', 'amp'], + orderDefaultOptions: function orderDefaultOptions() { return { status: 'ASC', @@ -580,6 +601,9 @@ Post = ghostBookshelf.Model.extend({ edit: ['forUpdate'] }; + // The post model additionally supports having a formats option + options.push('formats'); + if (validOptions[methodName]) { options = options.concat(validOptions[methodName]); } diff --git a/core/test/functional/routes/api/posts_spec.js b/core/test/functional/routes/api/posts_spec.js index ccafe205ee..4a480e7218 100644 --- a/core/test/functional/routes/api/posts_spec.js +++ b/core/test/functional/routes/api/posts_spec.js @@ -54,6 +54,138 @@ describe('Post API', function () { testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); _.isBoolean(jsonResponse.posts[0].page).should.eql(true); + + done(); + }); + }); + + it('can retrieve a single post format', function (done) { + request.get(testUtils.API.getApiQuery('posts/?formats=mobiledoc')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + should.exist(jsonResponse.posts); + testUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(5); + testUtils.API.checkResponse(jsonResponse.posts[0], 'post', ['mobiledoc'], ['html']); + testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); + _.isBoolean(jsonResponse.posts[0].page).should.eql(true); + + done(); + }); + }); + + it('can retrieve multiple post formats', function (done) { + request.get(testUtils.API.getApiQuery('posts/?formats=plaintext,mobiledoc,amp')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + should.exist(jsonResponse.posts); + testUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(5); + testUtils.API.checkResponse(jsonResponse.posts[0], 'post', ['mobiledoc', 'plaintext', 'amp'], ['html']); + testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); + _.isBoolean(jsonResponse.posts[0].page).should.eql(true); + + done(); + }); + }); + + it('can handle unknown post formats', function (done) { + request.get(testUtils.API.getApiQuery('posts/?formats=plaintext,mobiledo')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + should.exist(jsonResponse.posts); + testUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(5); + testUtils.API.checkResponse(jsonResponse.posts[0], 'post', ['plaintext'], ['html']); + testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); + _.isBoolean(jsonResponse.posts[0].page).should.eql(true); + + done(); + }); + }); + + it('can handle empty formats (default html is expected)', function (done) { + request.get(testUtils.API.getApiQuery('posts/?formats=')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + should.exist(jsonResponse.posts); + testUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(5); + testUtils.API.checkResponse(jsonResponse.posts[0], 'post'); + testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); + _.isBoolean(jsonResponse.posts[0].page).should.eql(true); + + done(); + }); + }); + + it('fields and formats', function (done) { + request.get(testUtils.API.getApiQuery('posts/?formats=mobiledoc,html&fields=id,title')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + should.exist(jsonResponse.posts); + testUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(5); + + testUtils.API.checkResponse( + jsonResponse.posts[0], + 'post', + null, + null, + ['mobiledoc', 'id', 'title', 'html'] + ); + + testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + done(); }); }); diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index d7e63338fc..8612f544c0 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -43,7 +43,9 @@ describe('Post Model', function () { beforeEach(testUtils.setup('owner', 'posts', 'apps')); - function checkFirstPostData(firstPost) { + function checkFirstPostData(firstPost, options) { + options = options || {}; + should.not.exist(firstPost.author_id); firstPost.author.should.be.an.Object(); firstPost.url.should.equal('/html-ipsum/'); @@ -60,10 +62,29 @@ describe('Post Model', function () { firstPost.published_by.name.should.equal(DataGenerator.Content.users[0].name); firstPost.tags[0].name.should.equal(DataGenerator.Content.tags[0].name); - // Formats // @TODO change / update this for mobiledoc in - firstPost.markdown.should.match(/HTML Ipsum Presents/); - firstPost.html.should.match(/HTML Ipsum Presents/); + if (options.formats) { + if (options.formats.indexOf('markdown') !== -1) { + firstPost.markdown.should.match(/HTML Ipsum Presents/); + } + + if (options.formats.indexOf('html') !== -1) { + firstPost.html.should.match(/HTML Ipsum Presents/); + } + + if (options.formats.indexOf('plaintext') !== -1) { + /** + * NOTE: this is null, not undefined, so it was returned + * The plaintext value is generated. + */ + should.equal(firstPost.plaintext, null); + } + } else { + firstPost.html.should.match(/HTML Ipsum Presents/); + should.equal(firstPost.plaintext, undefined); + should.equal(firstPost.markdown, undefined); + should.equal(firstPost.amp, undefined); + } } describe('findAll', function () { @@ -98,6 +119,27 @@ describe('Post Model', function () { done(); }).catch(done); }); + + it('can findAll, use formats option', function (done) { + var options = { + formats: ['markdown', 'plaintext'], + include: ['author', 'fields', 'tags', 'created_by', 'updated_by', 'published_by'] + }; + + PostModel.findAll(options) + .then(function (results) { + should.exist(results); + results.length.should.be.above(0); + + var posts = results.models.map(function (model) { + return model.toJSON(options); + }), firstPost = _.find(posts, {title: testUtils.DataGenerator.Content.posts[0].title}); + + checkFirstPostData(firstPost, options); + + done(); + }).catch(done); + }); }); describe('findPage', function () { diff --git a/core/test/utils/api.js b/core/test/utils/api.js index fbd333244c..fad8ebd34c 100644 --- a/core/test/utils/api.js +++ b/core/test/utils/api.js @@ -19,8 +19,13 @@ var _ = require('lodash'), slugs: ['slugs'], slug: ['slug'], // object / model level - // Post API swaps author_id to author, and always returns a computed 'url' property - post: _(schema.posts).keys().without('author_id').concat('author', 'url').value(), + // Post API + post: _(schema.posts).keys() + // does not return all formats by default + .without('markdown', 'mobiledoc', 'amp', 'plaintext') + // swaps author_id to author, and always returns a computed 'url' property + .without('author_id').concat('author', 'url') + .value(), // User API always removes the password field user: _(schema.users).keys().without('password').without('ghost_auth_access_token').value(), // Tag API swaps parent_id to parent @@ -78,8 +83,10 @@ function checkResponseValue(jsonResponse, expectedProperties) { providedProperties.length.should.eql(expectedProperties.length); } -function checkResponse(jsonResponse, objectType, additionalProperties, missingProperties) { +function checkResponse(jsonResponse, objectType, additionalProperties, missingProperties, onlyProperties) { var checkProperties = expectedProperties[objectType]; + + checkProperties = onlyProperties ? onlyProperties : checkProperties; checkProperties = additionalProperties ? checkProperties.concat(additionalProperties) : checkProperties; checkProperties = missingProperties ? _.xor(checkProperties, missingProperties) : checkProperties;