From cd328f694bdb2288e59e40488c536b82da4471c9 Mon Sep 17 00:00:00 2001 From: Laura Ghiorghisor Date: Sat, 20 Apr 2024 18:04:05 +0100 Subject: [PATCH] Fix invalid date behaviour During testing it was discovered that the generic date incorrect error was showing for seemingly correct dates. The current implementation makes use of an Integer check to ascertain whether the input is a correct digit. Nonetheless, Integer will interpret input such as "0_" as octal rather than decimal, so values like "08" or "09" will error. We conmfirmed with manual tests that this was the case and added some more validation test coverage. The fix is to drop any leading 0s from the string before applying Integer(), with the caveat that "00" should still be passed as 0. A regex was introduced to do this. --- .../admin/schedulings_controller.rb | 2 +- .../admin/schedulings_controller_spec.rb | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/schedulings_controller.rb b/app/controllers/admin/schedulings_controller.rb index 83e43263b..8424fb4d4 100644 --- a/app/controllers/admin/schedulings_controller.rb +++ b/app/controllers/admin/schedulings_controller.rb @@ -37,7 +37,7 @@ def scheduled_publication_time raise StandardError, "cannot be blank" if scheduling_params.values.any?(&:blank?) begin - year, month, day, hour, minute = scheduling_params.to_h.sort.map { |_, v| Integer(v) } + year, month, day, hour, minute = scheduling_params.to_h.sort.map { |_, v| Integer(v.sub!(/^0*(?=.*.)/, "")) } raise ArgumentError unless year.to_s.match?(/^\d{4}$/) && Date.valid_date?(year, month, day) Time.zone.local(year, month, day, hour, minute) diff --git a/spec/controllers/admin/schedulings_controller_spec.rb b/spec/controllers/admin/schedulings_controller_spec.rb index c9cdb0f63..049990ba0 100644 --- a/spec/controllers/admin/schedulings_controller_spec.rb +++ b/spec/controllers/admin/schedulings_controller_spec.rb @@ -65,6 +65,39 @@ end context "invalid params" do + [ + %w[scheduled_publication_time 08 07 02345], + %w[scheduled_publication_time 8 9 002345], + %w[scheduled_publication_time 1 8 0002345], + %w[scheduled_publication_time 9 06 0000002345], + ].each do |param_base_name, day, month, year| + it "is valid for day #{day}, month #{month} and year #{year}" do + params = generate_scheduling_params(Time.zone.now) + params.merge!({ "#{param_base_name}(3i)" => day, "#{param_base_name}(2i)" => month, "#{param_base_name}(1i)" => year }) + + post :create, params: { edition_id: @edition.id, scheduling: params } + + expect(response.body).not_to match(/Scheduled publication time is not in the correct format/) + end + end + + [ + %w[scheduled_publication_time 1 8], + %w[scheduled_publication_time 8 9], + %w[scheduled_publication_time 9 45], + %w[scheduled_publication_time 08 09], + %w[scheduled_publication_time 00 00], + ].each do |param_base_name, hour, minute| + it "is valid for hour #{hour} and minute #{minute} with or without leading 0s" do + params = generate_scheduling_params(Time.zone.now) + params.merge!({ "#{param_base_name}(4i)" => hour, "#{param_base_name}(5i)" => minute }) + + post :create, params: { edition_id: @edition.id, scheduling: params } + + expect(response.body).not_to match(/Scheduled publication time is not in the correct format/) + end + end + [ ["scheduled_publication_time", "1", "asdf"], ["scheduled_publication_time", "2", "a"],