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

Timed updates #3369

Merged
merged 2 commits into from
Jul 3, 2024
Merged

Timed updates #3369

merged 2 commits into from
Jul 3, 2024

Conversation

KludgeKML
Copy link
Contributor

@KludgeKML KludgeKML commented Jul 1, 2024

https://trello.com/c/upXxaeBR

WHAT

Add in a helper method (before_update_time?) which can be used to make banner (or other layout) changes at a fixed time. Using this method rather than rolling a new one each time we have to make timed updates will prevent problems related to time zone settings (such as comparing a Time object - which does support time zones - with a Date object - which does not - causing an update to be an hour out during BST)

WHY

Currently all deployments in banner text (non-emergency and perhaps emergency too) are manually triggered using GitHub Actions. Attempts to schedule banner updates using Ruby date functions (by merging the changes early), have caused issues with time-sensitive features due to server time differences from UK time (GMT/BST). A unique banner text deployment failed to update at the expected time because the server time did not match UK time. Automating the deployment to account for this time difference between server and UK time is essential to ensure timely updates without requiring human intervention at midnight.

TESTING

We've deployed a version of this branch to integration with a nearer time, and observed that the banner vanished at the correct time.

RELATED PRS

Update dev docs to point to new helper method

Follow these steps if you are doing a Rails upgrade.

Copy link
Contributor

@leenagupte leenagupte left a comment

Choose a reason for hiding this comment

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

Thanks for adding tests, and for testing the "switchover" time 🎉

Is this intention for this helper to be long lived or to be deleted after the global banner is taken down?

Could copy some of the detail from the Trello card into the commit so that the detail isn't lost when the card is eventually archived? Thanks.

@@ -0,0 +1,5 @@
module TimedUpdateHelper
def before_update_time?(year:, month:, day:, hour:, minute:)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: I'm not sure about this name, but I can't think of anything better!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I also couldn't think of anything better. Let's sleep on it and see if a better name turns up in the morning.

@KludgeKML
Copy link
Contributor Author

Thanks for adding tests, and for testing the "switchover" time 🎉

Is this intention for this helper to be long lived or to be deleted after the global banner is taken down?

I think I'd say longer-lived? Like, for as long as we're doing it this way rather than it being a publishable object, we should encourage people to use this method rather than roll their own and risk forgetting about timezones.

Could copy some of the detail from the Trello card into the commit so that the detail isn't lost when the card is eventually archived? Thanks.

Yeah, I'll do that in the morning.

@leenagupte
Copy link
Contributor

I think I'd say longer-lived? Like, for as long as we're doing it this way rather than it being a publishable object, we should encourage people to use this method rather than roll their own and risk forgetting about timezones.

That's great. If it's not part of the trello ticket, then can updating the docs be added to it?

@KludgeKML
Copy link
Contributor Author

That's great. If it's not part of the trello ticket, then can updating the docs be added to it?

It is in the trello card. I'll pop in an update PR for govuk-developer-docs.

Copy link
Contributor

@leenagupte leenagupte left a comment

Choose a reason for hiding this comment

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

Thanks for updating the description. ⭐

@KludgeKML KludgeKML merged commit bef6269 into main Jul 3, 2024
11 checks passed
@KludgeKML KludgeKML deleted the timed-updates branch July 3, 2024 10:24
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.

2 participants