Merge pull request #1857 from alphagov/refactor-scheduling-datetime

Some refactorings in the datetime validation for scheduling

app/controllers/admin/schedulings_controller.rb

@@ -1,18 +1,15 @@
 class Admin::SchedulingsController < ApplicationController
   before_action :skip_slimmer
-  before_action :load_country_and_edition, only: %i[new create destroy]
+  before_action :load_country_and_edition
   before_action :redirect_unless_can_schedule
 
   def new; end
 
   def create
     begin
-      if scheduled_publication_time.present?
-        @edition.scheduled_publication_time = scheduled_publication_time
-
-        if @edition.save && @edition.schedule_for_publication(current_user)
-          redirect_to admin_country_path(@country.slug), notice: "#{@country.name} travel advice is scheduled to publish on #{@edition.scheduled_publication_time.strftime('%B %d, %Y %H:%M %Z')}." and return
-        end
+      @edition.scheduled_publication_time = scheduled_publication_time
+      if @edition.save && @edition.schedule_for_publication(current_user)
+        redirect_to admin_country_path(@country.slug), notice: "#{@country.name} travel advice is scheduled to publish on #{@edition.scheduled_publication_time.strftime('%B %d, %Y %H:%M %Z')}." and return
       end
     rescue StandardError => e
       @edition.errors.add(:scheduled_publication_time, e)
@@ -33,18 +30,17 @@ def destroy
 private
 
   def redirect_unless_can_schedule
-    redirect_to admin_country_path(@country.slug) and return unless can_schedule_edition?
+    redirect_to admin_country_path(@country.slug) unless can_schedule_edition?
   end
 
   def scheduled_publication_time
-    year, month, day, hour, minute = scheduling_params.to_h.sort.map { |_, v| v }
-
-    raise StandardError, "cannot be blank" if [year, month, day, hour, minute].any?(&:blank?)
-    raise StandardError, "is not in the correct format" unless year.to_s.match?(/^\d{4}$/) && month.match?(/^\d{1,2}$/) \
-      && day.match?(/^\d{1,2}$/) && hour.match?(/^\d{1,2}$/) && minute.match?(/^\d{1,2}$/) && Date.valid_date?(year.to_i, month.to_i, day.to_i)
+    raise StandardError, "cannot be blank" if scheduling_params.values.any?(&:blank?)
 
     begin
-      Time.zone.local(year.to_i, month.to_i, day.to_i, hour.to_i, minute.to_i)
+      year, month, day, hour, minute = scheduling_params.to_h.sort.map { |_, v| Integer(v) }
+      raise ArgumentError unless year.to_s.match?(/^\d{4}$/) && Date.valid_date?(year, month, day)
+
+      Time.zone.local(year, month, day, hour, minute)
     rescue ArgumentError
       raise StandardError, "is not in the correct format"
     end