Moved usage of preview options from globalTemplateOptions to localTemplateOptions

refs https://github.com/TryGhost/Team/issues/1097

globalTemplateOptions are supposed to be static with localTemplateOptions being merged in per-request, however the per-request preview data was being extracted and set in the global options. Comments suggest that the global data should be static and eventually updated via other means, the usage of the request object to get per-request preview data is working against that.

- adjusted the preview handler to return an object rather than changing properties by reference on a passed in object
- moved preview data fetching out of `getSiteData()` used in `updateGlobalTemplateOptions()` and into `updateLocalTemplateOptions()` so that we're not relying on the request object in `updateGlobalTemplateOptions()`
This commit is contained in:
Kevin Ansfield 2021-09-30 10:36:05 +01:00
parent 4a6bedce7b
commit 8a17e723a1
3 changed files with 30 additions and 24 deletions

View File

@ -87,12 +87,9 @@ async function getProductAndPricesData() {
}
}
function getSiteData(req) {
function getSiteData() {
let siteData = settingsCache.getPublic();
// @TODO: it would be nicer if this was proper middleware somehow...
siteData = preview.handle(req, siteData);
// theme-only computed property added to @site
if (settingsCache.get('members_signup_access') === 'none') {
const escapedUrl = encodeURIComponent(urlUtils.urlFor({relativeUrl: '/rss/'}, true));
@ -108,7 +105,7 @@ async function updateGlobalTemplateOptions(req, res, next) {
// Static information, same for every request unless the settings change
// @TODO: bind this once and then update based on events?
// @TODO: decouple theme layer from settings cache using the Content API
const siteData = getSiteData(req);
const siteData = getSiteData();
const labsData = labs.getAll();
const themeData = {
@ -157,10 +154,18 @@ function updateLocalTemplateData(req, res, next) {
function updateLocalTemplateOptions(req, res, next) {
const localTemplateOptions = hbs.getLocalTemplateOptions(res.locals);
// adjust @site.url for http/https based on the incoming request
const siteData = {
url: urlUtils.urlFor('home', {secure: req.secure, trailingSlash: false}, true)
};
// @TODO: it would be nicer if this was proper middleware somehow...
const previewData = preview.handle(req);
// update site data with any preview values from the request
Object.assign(siteData, previewData);
const member = req.member ? {
uuid: req.member.uuid,
email: req.member.email,

View File

@ -14,7 +14,7 @@ function decodeValue(value) {
return value;
}
function getPreviewData(previewHeader, siteData) {
function getPreviewData(previewHeader) {
// Keep the string shorter with short codes for certain parameters
const supportedSettings = {
c: 'accent_color',
@ -25,23 +25,27 @@ function getPreviewData(previewHeader, siteData) {
let opts = new URLSearchParams(previewHeader);
const previewData = {};
opts.forEach((value, key) => {
value = decodeValue(value);
if (supportedSettings[key]) {
_.set(siteData, supportedSettings[key], value);
_.set(previewData, supportedSettings[key], value);
}
});
siteData._preview = previewHeader;
previewData._preview = previewHeader;
return siteData;
return previewData;
}
module.exports._PREVIEW_HEADER_NAME = PREVIEW_HEADER_NAME;
module.exports.handle = (req, siteData) => {
module.exports.handle = (req) => {
let previewData = {};
if (req && req.header(PREVIEW_HEADER_NAME)) {
siteData = getPreviewData(req.header(PREVIEW_HEADER_NAME), siteData);
previewData = getPreviewData(req.header(PREVIEW_HEADER_NAME));
}
return siteData;
return previewData;
};

View File

@ -66,6 +66,7 @@ describe('Themes middleware', function () {
.returns(fakeSiteData);
sandbox.stub(hbs, 'updateTemplateOptions');
sandbox.stub(hbs, 'updateLocalTemplateOptions');
});
it('mounts active theme if not yet mounted', function (done) {
@ -194,7 +195,7 @@ describe('Themes middleware', function () {
});
describe('Preview Mode', function () {
it('calls updateTemplateOptions with correct data when one parameter is set', function (done) {
it('calls updateLocalTemplateOptions with correct data when one parameter is set', function (done) {
const previewString = 'c=%23000fff';
req.header = () => {
return previewString;
@ -204,13 +205,11 @@ describe('Themes middleware', function () {
try {
should.not.exist(err);
hbs.updateTemplateOptions.calledOnce.should.be.true();
const templateOptions = hbs.updateTemplateOptions.firstCall.args[0];
hbs.updateLocalTemplateOptions.calledOnce.should.be.true();
const templateOptions = hbs.updateLocalTemplateOptions.firstCall.args[1];
const data = templateOptions.data;
data.should.be.an.Object().with.properties('site', 'labs', 'config');
should.equal(data.site, fakeSiteData);
data.should.be.an.Object().with.properties('site');
data.site.should.be.an.Object().with.properties('accent_color', '_preview');
data.site._preview.should.eql(previewString);
@ -223,7 +222,7 @@ describe('Themes middleware', function () {
});
});
it('calls updateTemplateOptions with correct data when two parameters are set', function (done) {
it('calls updateLocalTemplateOptions with correct data when two parameters are set', function (done) {
const previewString = 'c=%23000fff&icon=%2Fcontent%2Fimages%2Fmyimg.png';
req.header = () => {
return previewString;
@ -233,13 +232,11 @@ describe('Themes middleware', function () {
try {
should.not.exist(err);
hbs.updateTemplateOptions.calledOnce.should.be.true();
const templateOptions = hbs.updateTemplateOptions.firstCall.args[0];
hbs.updateLocalTemplateOptions.calledOnce.should.be.true();
const templateOptions = hbs.updateLocalTemplateOptions.firstCall.args[1];
const data = templateOptions.data;
data.should.be.an.Object().with.properties('site', 'labs', 'config');
should.equal(data.site, fakeSiteData);
data.should.be.an.Object().with.properties('site');
data.site.should.be.an.Object().with.properties('accent_color', 'icon', '_preview');
data.site._preview.should.eql(previewString);