Skip to content

Commit

Permalink
Add logging / error handling to GovukTimezone
Browse files Browse the repository at this point in the history
The more I think about it, the more I worry that having the gem forcibly
set time_zone to London without any warnings could lead to a confusing /
hard to debug problem down the line.

For example, someone trying to set config.time_zone explicitly to UTC,
or trying to set it to something else entirely, and then being confused
when it's still London.

There's no way I can think of to distinguish the "time_zone is UTC
because that's the default" situation from the "time_zone is UTC because
it's been set explicitly" situation. Logging an info message at least
increases the chance that someone will notice what's going on if they
are trying to set it to UTC.

We're also logging the situation where time_zone is set to London - this
config is fairly widespread, and it will become redundant following this
change to the gem. However it's harmless, so I don't think it's worth
chasing up all the other edge cases.

All other values of time_zone should be considered errors, as it would
be very confusing to have them silently overridden.
  • Loading branch information
richardTowers committed Jun 21, 2024
1 parent 83e58ac commit e804b35
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
9 changes: 9 additions & 0 deletions lib/govuk_app_config/govuk_timezone.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
module GovukTimezone
def self.configure(config)
case config.time_zone
when "UTC"
Rails.logger.info "govuk_app_config changing time_zone from UTC (the default) to London"
when "London"
Rails.logger.info "govuk_app_config always sets time_zone to London - there is no need to set config.time_zone in your app"
else
raise "govuk_app_config prevents configuring time_zones other than London - config.time_zone was set to #{config.time_zone}"
end

config.time_zone = "London"
end
end
26 changes: 23 additions & 3 deletions spec/lib/govuk_timezone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,30 @@

RSpec.describe GovukError do
describe ".configure" do
it "should set time_zone to London" do
config = double(:config)
expect(config).to receive(:time_zone=).with("London")
let(:config) { Rails::Railtie::Configuration.new }
let(:logger) { instance_double("ActiveSupport::Logger") }

before do
allow(Rails).to receive(:logger).and_return(logger)
end

it "should override the default UTC time_zone to London" do
config.time_zone = "UTC"
expect(logger).to receive(:info)
GovukTimezone.configure(config)
expect(config.time_zone).to eq("London")
end

it "should leave time_zones set to London as London" do
config.time_zone = "London"
expect(logger).to receive(:info)
GovukTimezone.configure(config)
expect(config.time_zone).to eq("London")
end

it "should raise an error if configured with any other time zone" do
config.time_zone = "Shanghai"
expect { GovukTimezone.configure(config) }.to raise_error(/govuk_app_config prevents configuring time_zones/)
end
end
end

0 comments on commit e804b35

Please sign in to comment.