Skip to content

Commit

Permalink
Fix invalid date behaviour
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lauraghiorghisor-tw committed Apr 20, 2024
1 parent 1a4c0e6 commit cd328f6
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
2 changes: 1 addition & 1 deletion app/controllers/admin/schedulings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 33 additions & 0 deletions spec/controllers/admin/schedulings_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down

0 comments on commit cd328f6

Please sign in to comment.