Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improvement: set activeTimezone when transforming dates #7142

Merged
merged 1 commit into from Jul 26, 2016
Merged

improvement: set activeTimezone when transforming dates #7142

merged 1 commit into from Jul 26, 2016

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Jul 25, 2016 •
edited

no issue

Set activeTimezone settings property, when running 006 migration.
Right now: we only have a default active timezone (for the blog) configured - which is UTC timezone. But when your ghost server runs in a non UTC timezone, then your blog would be in UTC and all your published_at posts dates get shifted to UTC timezone. This can be very confusing.

So we are setting the activeTimezone settings property to the server timezone (or to the one which is configured in config.js), when running the migration.

Only tested manual.

@kirrg001 kirrg001 changed the title [WIP] improvement: set activeTimezone when transforming dates improvement: set activeTimezone when transforming dates Jul 25, 2016
@kevinansfield
Copy link
Contributor

If I'm reading this correctly the timezone can be set to something that is not in our condensed list of timezones even if the timezone falls into one of our timezone groups.

Eg. for me moment.tz.guess() returns Europe/London which is correct but in our timezones.json that would equate to Europe/Dublin.

If we're OK with that then there will need to be some changes to the client to deal with settings.activeTimezone being set to something that's not in the selectable list - although that's probably a good thing as it will also allow us to later use moment.tz.guess() during the setup process to match the blog timezone to the blog owner's timezone.

@kirrg001
Copy link
Contributor Author

kirrg001 commented Jul 26, 2016 •
edited

Interesting.
#7143 will not allow setting any timezone string anymore, because of moment. I think we have to adapt our timezones.json.

@kevinansfield
Copy link
Contributor

kevinansfield commented Jul 26, 2016 •
edited

I think #7143 needs to be changed to use moment.tz.zone(givenTimezone) to check whether a zone is valid or not? I'm working on a small client PR so that the client copes with a manual timezone override.

timezones.json is a client-focused utility for condensing the list of timezones to select from as far as I'm aware, I don't think we should be using that to validate timezones?

@kirrg001
Copy link
Contributor Author

changed #7143 to allow all valid moment timezones
thanks @kevinansfield

@kirrg001 kirrg001 force-pushed the improvement/care-about-active-timezone branch from 05e4b26 to dde8231 Compare July 26, 2016 09:35
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Jul 26, 2016
@kevinansfield
Handle manual timezone override
ad7fb3f 
refs TryGhost/Ghost#7142, TryGhost/Ghost#7143
- moves timezone selection template and logic into a component
- detect if the current `activeTimezone` is not in our pre-defined list of timezones, if it isn't:
  - add a line indicating that there has been a manual override with the current `activeTimezone` value
  - add a blank option to the timezone select list
@kevinansfield kevinansfield mentioned this pull request Jul 26, 2016
Merged
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Jul 26, 2016
@kevinansfield
Handle automatic timezone override
96f2d40 
refs TryGhost/Ghost#7142, TryGhost/Ghost#7143
- moves timezone selection template and logic into a component
- detect if the current `activeTimezone` is not in our pre-defined list of timezones, if it isn't:
  - add a line indicating that there has been an override with the current `activeTimezone` value
  - add a blank option to the timezone select list
@kevinansfield kevinansfield merged commit e5a0471 into TryGhost:master Jul 26, 2016
chris-brown pushed a commit to chris-brown/Ghost that referenced this pull request Aug 14, 2016
@kirrg001 @chris-brown
improvement: set activeTimezone when transforming dates (TryGhost#7142)
1374539 
no issue
- sets `settings.activeTimezone` to best-guess based on current server time when performing the timezones migration in order to prevent unexpected changes in timezone when upgrading
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
@kirrg001 @geekhuyang
improvement: set activeTimezone when transforming dates (TryGhost#7142)
faa0c51 
no issue
- sets `settings.activeTimezone` to best-guess based on current server time when performing the timezones migration in order to prevent unexpected changes in timezone when upgrading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kirrg001 @kevinansfield