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

🐛 Fixed missing "value" property for settings Admin API v2

Browse source 
closes #10518

- we had a very generic logic to remove "unwanted" null values
  - copied from v0.1
  - originally added in 7d4107fec4
- this logic transformed: settings = [{key: 'key', value: null}] to [{key: 'key'}], which is wrong
- i've removed this generic logic completely, because i don't know which purpose it serves
- if there a specific case where we want to remove null values, we should either use the JSON schema or use a specific serializer for the target resource
- added tests to proof that settings API behaves as it should
- one test failed because we removed the isNull logic -> if you send published_at = null on a published post
  - the model layer has a piece of logic to force a date if you set published_at to null if the status is published
  - protected
This commit is contained in:
kirrg001 2019-03-04 17:00:07 +01:00
parent 98b4940f70
commit 449bae9a48
6 changed files with 28 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
core/server/api/shared/serializers/input/all.js
View file

@ -35,20 +35,5 @@ module.exports = {
}
debug(frame.options);
},
add(apiConfig, frame) {
// CASE: will remove unwanted null values
_.each(frame.data[apiConfig.docName], (value, index) => {
if (!_.isObject(frame.data[apiConfig.docName][index])) {
return;
}
frame.data[apiConfig.docName][index] = _.omitBy(frame.data[apiConfig.docName][index], _.isNull);
});
},
edit() {
return this.add(...arguments);
}
};

5
core/server/api/v2/utils/serializers/input/settings.js
View file

@ -7,9 +7,10 @@ module.exports = {
frame.data = {settings: [{key: frame.data, value: frame.options}]};
}
// prepare data
// CASE: transform objects/arrays into string (we store stringified objects in the db)
// @TODO: This belongs into the model layer?
frame.data.settings.forEach((setting) => {
if (!_.isString(setting.value)) {
if (_.isObject(setting.value)) {
setting.value = JSON.stringify(setting.value);
}
});

22
core/test/acceptance/old/admin/settings_spec.js
View file

@ -97,7 +97,22 @@ describe('Settings API', function () {
changedValue = [],
settingToChange = {
settings: [
{key: 'title', value: changedValue}
{
key: 'title',
value: changedValue
},
{
key: 'ghost_head',
value: null
},
{
key: 'navigation',
value: {label: 'label1'}
},
{
key: 'slack',
value: JSON.stringify({username: 'username'})
}
]
};
@ -115,10 +130,13 @@ describe('Settings API', function () {
return done(err);
}
var putBody = res.body;
const putBody = res.body;
res.headers['x-cache-invalidate'].should.eql('/*');
should.exist(putBody);
putBody.settings[0].value.should.eql(JSON.stringify(changedValue));
should.equal(putBody.settings[1].value, null);
should.equal(putBody.settings[2].value, JSON.stringify({label: 'label1'}));
should.equal(putBody.settings[3].value, JSON.stringify({username: 'username'}));
localUtils.API.checkResponse(putBody, 'settings');
done();
});

4
core/test/regression/api/v2/admin/posts_spec.js
View file

@ -144,8 +144,8 @@ describe('Posts API', function () {
.expect(200);
})
.then((res) => {
// @NOTE: you cannot modify published_at manually, that's why the resource won't change.
should.not.exist(res.headers['x-cache-invalidate']);
// @NOTE: if you set published_at to null and the post is published, we set it to NOW in model layer
should.exist(res.headers['x-cache-invalidate']);
should.exist(res.body.posts);
should.exist(res.body.posts[0].published_at);
});

9
core/test/unit/api/shared/serializers/handle_spec.js
View file

@ -28,27 +28,24 @@ describe('Unit: api/shared/serializers/handle', function () {
it('ensure serializers are called with apiConfig and frame', function () {
const allStub = sinon.stub();
const addStub = sinon.stub();
sinon.stub(shared.serializers.input.all, 'all').get(() => allStub);
sinon.stub(shared.serializers.input.all, 'add').get(() => addStub);
const apiSerializers = {
all: sinon.stub().resolves(),
posts: {
all: sinon.stub().resolves(),
add: sinon.stub().resolves()
browse: sinon.stub().resolves()
}
};
const apiConfig = {docName: 'posts', method: 'add'};
const apiConfig = {docName: 'posts', method: 'browse'};
const frame = {};
const stubsToCheck = [
allStub,
addStub,
apiSerializers.all,
apiSerializers.posts.all,
apiSerializers.posts.add
apiSerializers.posts.browse
];
return shared.serializers.handle.input(apiConfig, apiSerializers, frame)

24
core/test/unit/api/shared/serializers/input/all_spec.js
View file

@ -78,28 +78,4 @@ describe('Unit: v2/utils/serializers/input/all', function () {
});
});
});
describe('add', function () {
it('removes null values', function () {
const frame = {
data: {
posts: [{
a: null,
b: true,
c: null
}]
}
};
const apiConfig = {
docName: 'posts'
};
shared.serializers.input.all.add(apiConfig, frame);
should.not.exist(frame.data.posts[0].a);
should.exist(frame.data.posts[0].b);
should.not.exist(frame.data.posts[0].c);
});
});
});