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.

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

Use Time.zone.parse instead of Date.parse in ManualUpdatesPresenter #2959

Merged

Conversation

davidgisbey
Copy link
Contributor

@davidgisbey davidgisbey commented Oct 16, 2023

Description

At the moment, it uses Date.parse which doesn't account for the difference in timing between the timestamp (UTC) and local time (BST).

This causes issues with some updates being a day off if they were published between 00.00 & 00.59.

Time.zone.parse parses the date correctly into the "London" timezone. We still need to convert it back into a date though as it groups on year, then by identical dates, so all updates for a day are grouped.

Screenshots

Before

image

After

image

Trello card

https://trello.com/c/idVD4RQT/1794-update-history-shows-the-wrong-date

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@davidgisbey davidgisbey changed the title Use Time.zone.parse in ManualUpdatesPresenter Use Time.zone.parse instead of Date.parse in ManualUpdatesPresenter Oct 16, 2023
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2959 October 16, 2023 17:08 Inactive
@davidgisbey davidgisbey force-pushed the use-time-zone-parse-for-parsing-updated-at-time-for-manuals branch from 1ff602c to 8a48a3e Compare October 16, 2023 17:26
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2959 October 16, 2023 17:26 Inactive
@davidgisbey davidgisbey force-pushed the use-time-zone-parse-for-parsing-updated-at-time-for-manuals branch from 8a48a3e to 16f898a Compare October 16, 2023 17:33
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2959 October 16, 2023 17:34 Inactive
@davidgisbey davidgisbey force-pushed the use-time-zone-parse-for-parsing-updated-at-time-for-manuals branch from 16f898a to f30ef1c Compare October 18, 2023 07:53
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2959 October 18, 2023 07:53 Inactive
At the moment, it uses Date.parse which doesn't account for the difference
in timing between the timestamp (UTC) and local time (BST).

This causes issues with some updates being a day off if they were published
between 00.00 & 00.59.

Time.zone.parse parses the date correctly into the "London" timezone.
We still need to convert it back into a date though as it groups on
year, then by identical dates, so all updates for a day are grouped.
@davidgisbey davidgisbey force-pushed the use-time-zone-parse-for-parsing-updated-at-time-for-manuals branch from f30ef1c to ca02d69 Compare October 18, 2023 08:17
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2959 October 18, 2023 08:17 Inactive
Copy link
Contributor

@ryanb-gds ryanb-gds left a comment

Choose a reason for hiding this comment

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

I'd have probably just used Time.zone.parse in the test as well to save changing the date but this will do the job. Nice one

@davidgisbey davidgisbey merged commit 6aed5b5 into main Oct 18, 2023
6 checks passed
@davidgisbey davidgisbey deleted the use-time-zone-parse-for-parsing-updated-at-time-for-manuals branch October 18, 2023 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants