TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-28 14:03:48 +03:00
Code Issues Projects Releases Wiki Activity

🐛 Fixed 'url' attribute miscalculation when when requested as the only part of fields filter (#9969)

Browse Source 
closes #9962

- Fixed the bug with url being set to /404 when id was not present on the model
- Added a functional test to cover this bug
- Refactored url decorating methods to be more clear about the nature of passed parameters
This commit is contained in:
Nazar Gargol 2018-10-15 14:47:56 +02:00 committed by GitHub
parent 561c4b208d
commit 9fd9186557
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 97 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
core/server/api/v0.1/decorators/urls.js
View File

@ -1,32 +1,32 @@
const urlService = require('../../../services/url');
const {urlFor, makeAbsoluteUrls} = require('../../../services/url/utils');
const urlsForPost = (post, options) => {
post.url = urlService.getUrlByResourceId(post.id);
const urlsForPost = (id, attrs, options) => {
attrs.url = urlService.getUrlByResourceId(id);
if (options.columns && !options.columns.includes('url')) {
delete post.url;
delete attrs.url;
}
if (options && options.context && options.context.public && options.absolute_urls) {
if (post.feature_image) {
post.feature_image = urlFor('image', {image: post.feature_image}, true);
if (attrs.feature_image) {
attrs.feature_image = urlFor('image', {image: attrs.feature_image}, true);
}
if (post.og_image) {
post.og_image = urlFor('image', {image: post.og_image}, true);
if (attrs.og_image) {
attrs.og_image = urlFor('image', {image: attrs.og_image}, true);
}
if (post.twitter_image) {
post.twitter_image = urlFor('image', {image: post.twitter_image}, true);
if (attrs.twitter_image) {
attrs.twitter_image = urlFor('image', {image: attrs.twitter_image}, true);
}
if (post.html) {
post.html = makeAbsoluteUrls(post.html, urlFor('home', true), post.url).html();
if (attrs.html) {
attrs.html = makeAbsoluteUrls(attrs.html, urlFor('home', true), attrs.url).html();
}
if (post.url) {
post.url = urlFor({relativeUrl: post.url}, true);
if (attrs.url) {
attrs.url = urlFor({relativeUrl: attrs.url}, true);
}
}
@ -35,53 +35,53 @@ const urlsForPost = (post, options) => {
// @NOTE: this block also decorates primary_tag/primary_author objects as they
// are being passed by reference in tags/authors. Might be refactored into more explicit call
// in the future, but is good enough for current use-case
if (relation === 'tags' && post.tags) {
post.tags = post.tags.map(tag => urlsForTag(tag, options));
if (relation === 'tags' && attrs.tags) {
attrs.tags = attrs.tags.map(tag => urlsForTag(tag.id, tag, options));
}
if (relation === 'author' && post.author) {
post.author = urlsForUser(post.author, options);
if (relation === 'author' && attrs.author) {
attrs.author = urlsForUser(attrs.author.id, attrs.author, options);
}
if (relation === 'authors' && post.authors) {
post.authors = post.authors.map(author => urlsForUser(author, options));
if (relation === 'authors' && attrs.authors) {
attrs.authors = attrs.authors.map(author => urlsForUser(author.id, author, options));
}
});
}
return post;
return attrs;
};
const urlsForUser = (user, options) => {
const urlsForUser = (id, attrs, options) => {
if (options && options.context && options.context.public && options.absolute_urls) {
user.url = urlFor({
relativeUrl: urlService.getUrlByResourceId(user.id)
attrs.url = urlFor({
relativeUrl: urlService.getUrlByResourceId(id)
}, true);
if (user.profile_image) {
user.profile_image = urlFor('image', {image: user.profile_image}, true);
if (attrs.profile_image) {
attrs.profile_image = urlFor('image', {image: attrs.profile_image}, true);
}
if (user.cover_image) {
user.cover_image = urlFor('image', {image: user.cover_image}, true);
if (attrs.cover_image) {
attrs.cover_image = urlFor('image', {image: attrs.cover_image}, true);
}
}
return user;
return attrs;
};
const urlsForTag = (tag, options) => {
const urlsForTag = (id, attrs, options) => {
if (options && options.context && options.context.public && options.absolute_urls) {
tag.url = urlFor({
relativeUrl: urlService.getUrlByResourceId(tag.id)
attrs.url = urlFor({
relativeUrl: urlService.getUrlByResourceId(attrs.id)
}, true);
if (tag.feature_image) {
tag.feature_image = urlFor('image', {image: tag.feature_image}, true);
if (attrs.feature_image) {
attrs.feature_image = urlFor('image', {image: attrs.feature_image}, true);
}
}
return tag;
return attrs;
};
module.exports.urlsForPost = urlsForPost;

8
core/server/api/v0.1/posts.js
View File

@ -61,7 +61,7 @@ posts = {
return models.Post.findPage(options)
.then(({data, meta}) => {
return {
posts: data.map(model => urlsForPost(model.toJSON(options), options)),
posts: data.map(model => urlsForPost(model.id, model.toJSON(options), options)),
meta: meta
};
});
@ -110,7 +110,7 @@ posts = {
}
return {
posts: [urlsForPost(model.toJSON(options), options)]
posts: [urlsForPost(model.id, model.toJSON(options), options)]
};
});
}
@ -156,7 +156,7 @@ posts = {
}));
}
const post = urlsForPost(model.toJSON(options), options);
const post = urlsForPost(model.id, model.toJSON(options), options);
// If previously was not published and now is (or vice versa), signal the change
// @TODO: `statusChanged` get's added for the API headers only. Reconsider this.
@ -204,7 +204,7 @@ posts = {
function modelQuery(options) {
return models.Post.add(options.data.posts[0], omit(options, ['data']))
.then((model) => {
const post = urlsForPost(model.toJSON(options), options);
const post = urlsForPost(model.id, model.toJSON(options), options);
if (post.status === 'published') {
// When creating a new post that is published right now, signal the change

8
core/server/api/v0.1/tags.js
View File

@ -37,7 +37,7 @@ tags = {
return models.Tag.findPage(options)
.then(({data, meta}) => {
return {
tags: data.map(model => urlsForTag(model.toJSON(options), options)),
tags: data.map(model => urlsForTag(model.id, model.toJSON(options), options)),
meta: meta
};
});
@ -81,7 +81,7 @@ tags = {
}
return {
tags: [urlsForTag(model.toJSON(options), options)]
tags: [urlsForTag(model.id, model.toJSON(options), options)]
};
});
}
@ -116,7 +116,7 @@ tags = {
return models.Tag.add(options.data.tags[0], _.omit(options, ['data']))
.then((model) => {
return {
tags: [urlsForTag(model.toJSON(options), options)]
tags: [urlsForTag(model.id, model.toJSON(options), options)]
};
});
}
@ -159,7 +159,7 @@ tags = {
}
return {
tags: [urlsForTag(model.toJSON(options), options)]
tags: [urlsForTag(model.id, model.toJSON(options), options)]
};
});
}

6
core/server/api/v0.1/users.js
View File

@ -41,7 +41,7 @@ users = {
return models.User.findPage(options)
.then(({data, meta}) => {
return {
users: data.map(model => urlsForUser(model.toJSON(options), options)),
users: data.map(model => urlsForUser(model.id, model.toJSON(options), options)),
meta: meta
};
});
@ -90,7 +90,7 @@ users = {
}
return {
users: [urlsForUser(model.toJSON(options), options)]
users: [urlsForUser(model.id, model.toJSON(options), options)]
};
});
}
@ -151,7 +151,7 @@ users = {
}
return {
users: [urlsForUser(model.toJSON(options), options)]
users: [urlsForUser(model.id, model.toJSON(options), options)]
};
});
}

41
core/test/functional/api/v0.1/public_api_spec.js
View File

@ -270,6 +270,47 @@ describe('Public API', function () {
});
});
it('browse posts: request only url fields', function (done) {
request.get(localUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available&fields=url'))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
testUtils.API.checkResponse(jsonResponse.posts[0], 'post', false, false, ['url']);
res.body.posts[0].url.should.eql('/welcome/');
done();
});
});
it('browse posts: request only url fields with include and absolute_urls', function (done) {
request.get(localUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available&fields=url&include=tags&absolute_urls=true'))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
const jsonResponse = res.body;
should.exist(jsonResponse.posts);
testUtils.API.checkResponse(jsonResponse.posts[0], 'post', false, false, ['url','tags']);
jsonResponse.posts[0].url.should.eql('http://127.0.0.1:2369/welcome/');
jsonResponse.posts[0].tags[0].url.should.eql('http://127.0.0.1:2369/tag/getting-started/');
done();
});
});
it('browse posts: request to include tags and authors with absolute_urls', function (done) {
request.get(localUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available&absolute_urls=true&include=tags,authors'))
.set('Origin', testUtils.API.getURL())

22
core/test/unit/api/v0.1/decorators/urls_spec.js
View File

@ -23,7 +23,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
columns: [],
};
urls.urlsForPost(object, options);
urls.urlsForPost(1, object, options);
should.equal(Object.keys(object).length, 0);
});
@ -35,7 +35,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
};
urlService.getUrlByResourceId.withArgs(object.id).returns('url');
urls.urlsForPost(object, options);
urls.urlsForPost(object.id, object, options);
object.url.should.equal('url');
});
@ -51,7 +51,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForPost(object, options);
urls.urlsForPost(object.id, object, options);
const urlObject = url.parse(object.feature_image);
should.exist(urlObject.protocol);
@ -69,7 +69,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForPost(object, options);
urls.urlsForPost(object.id, object, options);
const urlObject = url.parse(object.twitter_image);
@ -88,7 +88,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForPost(object, options);
urls.urlsForPost(object.id, object, options);
const urlObject = url.parse(object.og_image);
@ -107,7 +107,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForPost(object, options);
urls.urlsForPost(object.id, object, options);
const imgSrc = object.html.match(/src="([^"]+)"/)[1];
const imgSrcUrlObject = url.parse(imgSrc);
@ -128,7 +128,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
};
urlService.getUrlByResourceId.withArgs(object.id).returns('url');
urls.urlsForUser(object, options);
urls.urlsForUser(object.id, object, options);
const urlObject = url.parse(object.url);
should.exist(urlObject.protocol);
@ -146,7 +146,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForUser(object, options);
urls.urlsForUser(object.id, object, options);
const urlObject = url.parse(object.profile_image);
should.exist(urlObject.protocol);
@ -164,7 +164,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForUser(object, options);
urls.urlsForUser(object.id, object, options);
const urlObject = url.parse(object.cover_image);
should.exist(urlObject.protocol);
@ -183,7 +183,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
};
urlService.getUrlByResourceId.withArgs(object.id).returns('url');
urls.urlsForTag(object, options);
urls.urlsForTag(object.id, object, options);
const urlObject = url.parse(object.url);
should.exist(urlObject.protocol);
@ -201,7 +201,7 @@ describe('Unit: api:v0.1:decorators:urls', function () {
}
};
urls.urlsForTag(object, options);
urls.urlsForTag(object.id, object, options);
const urlObject = url.parse(object.feature_image);
should.exist(urlObject.protocol);