Skip to content

Commit

Permalink
🐛 Fixed meta twitter_image output when using data: post.{slug} for …
Browse files Browse the repository at this point in the history
…routes

refs #10082

```
routes:
  /news/:
    data: post.news
```

The twitter_image was not available, because the context is [news, post] and the data is in `data.post`.
The context helper was incorrect. I think it is still not fully correct, but only focused on this use case.
The meta layer needs a full refactoring.
  • Loading branch information
kirrg001 committed Mar 12, 2019
1 parent 34fad7e commit fef0aa4
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 9 deletions.
31 changes: 22 additions & 9 deletions core/server/data/meta/context_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,29 @@ function getContextObject(data, context) {
/**
* If the data object does not contain the requested context, we return the fallback object.
*/
var blog = {
cover_image: settingsCache.get('cover_image'),
twitter: settingsCache.get('twitter'),
facebook: settingsCache.get('facebook')
},
contextObject;
const blog = {
cover_image: settingsCache.get('cover_image'),
twitter: settingsCache.get('twitter'),
facebook: settingsCache.get('facebook')
};

context = _.includes(context, 'page') || _.includes(context, 'amp') ? 'post' : context;
contextObject = data[context] || blog;
return contextObject;
let chosenContext;

// @TODO: meta layer is very broken, it's really hard to understand what it's doing
// The problem is that handlebars root object is structured differently. Sometimes the object is flat on data
// and sometimes the object is part of a key e.g. data.post. This needs to be prepared at the very first stage and not in each helper.
if (_.includes(context, 'page') || _.includes(context, 'amp') && data.post) {
chosenContext = data.post;
} else if (_.includes(context, 'post') && data.post) {
chosenContext = data.post;
} else if (data[context]) {
// @NOTE: This is confusing as hell. It tries to get data[['author']], which works, but coincidence?
chosenContext = data[context];
} else {
chosenContext = blog;
}

return chosenContext;
}

module.exports = getContextObject;
12 changes: 12 additions & 0 deletions core/test/unit/data/meta/twitter_image_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ describe('getTwitterImage', function () {
twitterImageUrl.should.match(/\/content\/images\/my-special-twitter-image\.jpg$/);
});

it('should return absolute url for Twitter image in post context', function () {
var twitterImageUrl = getTwitterImage({
context: ['news', 'post'],
post: {
feature_image: '/content/images/my-test-image.jpg',
twitter_image: '/content/images/my-special-twitter-image.jpg'
}
});
twitterImageUrl.should.not.equal('/content/images/my-special-twitter-image.jpg');
twitterImageUrl.should.match(/\/content\/images\/my-special-twitter-image\.jpg$/);
});

it('should return absolute url for feature image in post context', function () {
var twitterImageUrl = getTwitterImage({
context: ['post'],
Expand Down

2 comments on commit fef0aa4

@kirrg001
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gargol Urgh sorry this commit message needs to be unified with the other meta fixes. Let me know if you need help

@naz
Copy link
Member

@naz naz commented on fef0aa4 Mar 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grouped them up a little in the release notes, let me know if it's understandable enough :)

Please sign in to comment.