Skip to content

Commit

Permalink
🐛 config.theme.timezone must not be overwritten (TryGhost#7232)
Browse files Browse the repository at this point in the history
closes TryGhost#7182

When calling `config.set()` in the settings api, we want to set the active timezone of the blog to make it available in our `settingsCache`. But because the `theme` object in the `set` prototype was already set to `Etc/UTC` as default, the `_.merge` function would always overwrite our `activeTimezone` with the default value.

This PR changes the code in the way, that we always set 'Etc/UTC' for the timezone as default, _until_ we fetched our settings and therefore the `activeTimezone` setting, so we can overwrite it.

This issue had not only influence on the date helper, but everywhere in our codebase, where we rely on reading the `timezone` from our config, instead of our settings. The `{{@blog.timezone}}` helper reflected that quiet well, as it would always show `Etc/UTC`
  • Loading branch information
aileen authored and mixonic committed Oct 28, 2016
1 parent e5a51b7 commit 9f1c1c8
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
14 changes: 10 additions & 4 deletions core/server/config/index.js
Expand Up @@ -118,6 +118,15 @@ ConfigManager.prototype.set = function (config) {
this._config.theme.navigation = config.theme.navigation;
}

// Special case for theme.timezone, which should be overridden not merged
if (config && config.theme && config.theme.timezone) {
this._config.theme.timezone = config.theme.timezone;
} else {
// until we have set the timezone from settings, we use the default
this._config.theme = this._config.theme ? this._config.theme : {};
this._config.theme.timezone = 'Etc/UTC';
}

// Protect against accessing a non-existant object.
// This ensures there's always at least a paths object
// because it's referenced in multiple places.
Expand Down Expand Up @@ -201,10 +210,7 @@ ConfigManager.prototype.set = function (config) {
},
theme: {
// normalise the URL by removing any trailing slash
url: this._config.url ? this._config.url.replace(/\/$/, '') : '',

// default timezone
timezone: 'Etc/UTC'
url: this._config.url ? this._config.url.replace(/\/$/, '') : ''
},
routeKeywords: {
tag: 'tag',
Expand Down
32 changes: 31 additions & 1 deletion core/test/unit/config_spec.js
Expand Up @@ -36,7 +36,8 @@ describe('Config', function () {
title: 'casper',
description: 'casper',
logo: 'casper',
cover: 'casper'
cover: 'casper',
timezone: 'Etc/UTC'
}
});
});
Expand All @@ -57,6 +58,35 @@ describe('Config', function () {
themeConfig.should.have.property('description', 'casper');
themeConfig.should.have.property('logo', 'casper');
themeConfig.should.have.property('cover', 'casper');
themeConfig.should.have.property('timezone', 'Etc/UTC');
});
});

describe('Timezone default', function () {
it('should use timezone from settings when set', function () {
var themeConfig = config.theme;

// Check values are as we expect
themeConfig.should.have.property('timezone', 'Etc/UTC');
themeConfig.should.have.property('url');

configUtils.set({
theme: {
timezone: 'Africa/Cairo'
}
});

config.theme.should.have.property('timezone', 'Africa/Cairo');
config.theme.should.have.property('url');
});

it('should set theme object with timezone by default', function () {
var themeConfig = configUtils.defaultConfig;

// Check values are as we expect
themeConfig.should.have.property('theme');
themeConfig.theme.should.have.property('timezone', 'Etc/UTC');
themeConfig.theme.should.have.property('url');
});
});

Expand Down

0 comments on commit 9f1c1c8

Please sign in to comment.