diff --git a/app/assets/stylesheets/downtimes.scss b/app/assets/stylesheets/downtimes.scss index a58374f6f..b98b9f22b 100644 --- a/app/assets/stylesheets/downtimes.scss +++ b/app/assets/stylesheets/downtimes.scss @@ -10,6 +10,10 @@ .downtimes__table-link { @include govuk-media-query($from: "tablet") { - white-space: nowrap; + white-space: nowrap; } } + +.remove-link, .remove-link:visited{ + color: govuk-colour("red"); +} diff --git a/app/controllers/downtimes_controller.rb b/app/controllers/downtimes_controller.rb index 3f2b1768e..4f4806ae0 100644 --- a/app/controllers/downtimes_controller.rb +++ b/app/controllers/downtimes_controller.rb @@ -1,7 +1,6 @@ class DowntimesController < ApplicationController before_action :require_govuk_editor before_action :load_edition, except: [:index] - before_action :process_params, only: %i[update] layout "design_system" @@ -39,19 +38,33 @@ def edit def update @downtime = Downtime.for(@edition.artefact) - if params["commit"] == "Cancel downtime" + if params[:downtime] + datetime_validation_errors = datetime_validation_errors(downtime_params, %w[start_time end_time]) + if datetime_validation_errors.empty? && @downtime.update(downtime_params) + DowntimeScheduler.schedule_publish_and_expiry(@downtime) + flash[:success] = "#{edition_link} downtime message re-scheduled (from #{view_context.downtime_datetime(@downtime)})".html_safe + redirect_to downtimes_path + else + @downtime.valid? # Make sure the model validations have run + datetime_validation_errors.each do |name, message| + # Remove any default messages for this field added by the model validation + @downtime.errors.delete(name) + @downtime.errors.add(name, message) + end + render :edit + end + else DowntimeRemover.destroy_immediately(@downtime) flash[:success] = "#{edition_link} downtime message cancelled".html_safe redirect_to downtimes_path - elsif @downtime.update(downtime_params) - DowntimeScheduler.schedule_publish_and_expiry(@downtime) - flash[:success] = "#{edition_link} downtime message re-scheduled (from #{view_context.downtime_datetime(@downtime)})".html_safe - redirect_to downtimes_path - else - render :edit end end + def destroy + @downtime = Downtime.for(@edition.artefact) + render :delete + end + private def downtime_params @@ -67,10 +80,6 @@ def load_edition @edition = Edition.find(params[:edition_id]) end - def process_params - squash_multiparameter_datetime_attributes(downtime_params, %w[start_time end_time]) - end - def edition_link view_context.link_to(@edition.title, edit_edition_downtime_path(@edition), class: "link-inherit bold") end diff --git a/app/models/downtime.rb b/app/models/downtime.rb index 40357ec82..54ab6c736 100644 --- a/app/models/downtime.rb +++ b/app/models/downtime.rb @@ -9,7 +9,7 @@ class Downtime belongs_to :artefact, optional: true validate :start_time_precedes_end_time - validate :end_time_is_in_future, on: :create + validate :end_time_is_in_future validates :message, :start_time, :end_time, :artefact, presence: true def self.for(artefact) diff --git a/app/views/downtimes/_form.html.erb b/app/views/downtimes/_form.html.erb index c6e9a7da1..f3649fda2 100644 --- a/app/views/downtimes/_form.html.erb +++ b/app/views/downtimes/_form.html.erb @@ -7,32 +7,32 @@ year: { name: "downtime[start_time(1i)]", label: "Year", - value: params.dig("downtime", "start_time(1i)"), + value: params.dig("downtime", "start_time(1i)") || @downtime.start_time&.year, width: 4, }, month: { name: "downtime[start_time(2i)]", label: "Month", - value: params.dig("downtime", "start_time(2i)"), + value: params.dig("downtime", "start_time(2i)") || @downtime.start_time&.month, width: 2, }, day: { id: "downtime_start_time", name: "downtime[start_time(3i)]", label: "Day", - value: params.dig("downtime", "start_time(3i)"), + value: params.dig("downtime", "start_time(3i)") || @downtime.start_time&.day, width: 2, }, hour: { name: "downtime[start_time(4i)]", label: "Hour", - value: params.dig("downtime", "start_time(4i)"), + value: params.dig("downtime", "start_time(4i)") || @downtime.start_time&.hour, width: 2, }, minute: { name: "downtime[start_time(5i)]", label: "Minute", - value: params.dig("downtime", "start_time(5i)"), + value: params.dig("downtime", "start_time(5i)") || @downtime.start_time&.minute, width: 2, }, } %> @@ -44,32 +44,32 @@ year: { name: "downtime[end_time(1i)]", label: "Year", - value: params.dig("downtime", "end_time(1i)"), + value: params.dig("downtime", "end_time(1i)") || @downtime.end_time&.year, width: 4, }, month: { name: "downtime[end_time(2i)]", label: "Month", - value: params.dig("downtime", "end_time(2i)"), + value: params.dig("downtime", "end_time(2i)") || @downtime.end_time&.month, width: 2, }, day: { id: "downtime_end_time", name: "downtime[end_time(3i)]", label: "Day", - value: params.dig("downtime", "end_time(3i)"), + value: params.dig("downtime", "end_time(3i)") || @downtime.end_time&.day, width: 2, }, hour: { name: "downtime[end_time(4i)]", label: "Hour", - value: params.dig("downtime", "end_time(4i)"), + value: params.dig("downtime", "end_time(4i)") || @downtime.end_time&.hour, width: 2, }, minute: { name: "downtime[end_time(5i)]", label: "Minute", - value: params.dig("downtime", "end_time(5i)"), + value: params.dig("downtime", "end_time(5i)") || @downtime.end_time&.minute, width: 2, }, } %> @@ -85,7 +85,7 @@ hint: "Message is auto-generated once a schedule has been made.", id: "downtime_message", name: "downtime[message]", - value: params.dig("downtime", "message"), + value: params.dig("downtime", "message") || @downtime.message, error_items: errors_for(f.object.errors, :message), } %> diff --git a/app/views/downtimes/delete.html.erb b/app/views/downtimes/delete.html.erb new file mode 100644 index 000000000..d6e332a98 --- /dev/null +++ b/app/views/downtimes/delete.html.erb @@ -0,0 +1,19 @@ +<% content_for :page_title, 'Remove downtime message' %> +<% content_for :title, 'Remove downtime message' %> + +
+ <%= render "govuk_publishing_components/components/lead_paragraph", { + text: "Are you sure you want to remove the scheduled downtime message for #{@downtime.artefact.name}?", + margin_bottom: 8 + } %> + + <%= form_for @downtime, url: edition_downtime_path(@edition), html: { class: 'form well remove-top-margin', 'data-module': 'downtime-message' } do |f| %> +
+ <%= render "govuk_publishing_components/components/button", { + text: "Remove", + destructive: true + } %> + <%= link_to "Cancel", downtimes_path, class: "govuk-link govuk-link--no-visited-state" %> +
+ <% end %> +
diff --git a/app/views/downtimes/edit.html.erb b/app/views/downtimes/edit.html.erb index 9d263c669..9318d0b24 100644 --- a/app/views/downtimes/edit.html.erb +++ b/app/views/downtimes/edit.html.erb @@ -1,19 +1,27 @@ -<% content_for :page_title, 'Re-schedule downtime message' %> - - +<% content_for :page_title, 'Edit downtime message' %> +<% content_for :title, 'Edit downtime message' %> +<% content_for :title_context, @downtime.artefact.name %> +<% unless @downtime.errors.empty? %> + <% content_for :error_summary, render("shared/error_summary", { object: @downtime }) %> +<% end %> -
-

- <%= @downtime.artefact.name %> - Re-schedule downtime message -

-
+
+ <%= render "govuk_publishing_components/components/lead_paragraph", { + text: "Downtime message appear on the service's start page one day before the downtime is due to occur.", + margin_bottom: 6 + } %> <%= form_for @downtime, url: edition_downtime_path(@edition), html: { class: 'form well remove-top-margin', 'data-module': 'downtime-message' } do |f| %> <%= render 'form', f: f %> - <%= f.submit 'Re-schedule downtime message', class: 'js-submit btn btn-success' %> - <%= f.submit 'Cancel downtime', class: 'add-left-margin btn btn-danger' %> +
+ <%= render "govuk_publishing_components/components/button", { + text: "Save", + value: "save", + name: "save" + } %> + + <%= link_to "Cancel", downtimes_path, class: "govuk-link govuk-link--no-visited-state" %> + <%= link_to "Remove", edition_destroy_downtime_path, class: "govuk-link remove-link" %> +
<% end %> +
diff --git a/config/routes.rb b/config/routes.rb index b3bf7baf2..cbc156764 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,7 +49,8 @@ resource :downtime, only: %i[new create] end constraints FeatureConstraint.new("design_system_downtime_edit") do - resource :downtime, only: %i[edit update] + resource :downtime, only: %i[edit update destroy] + get "downtime" => "downtimes#destroy", as: :destroy_downtime end resource :downtime, only: %i[new create edit update destroy], controller: "legacy_downtimes" end diff --git a/test/functional/downtimes_controller_test.rb b/test/functional/downtimes_controller_test.rb index de68c5db8..11b330757 100644 --- a/test/functional/downtimes_controller_test.rb +++ b/test/functional/downtimes_controller_test.rb @@ -31,7 +31,7 @@ class DowntimesControllerTest < ActionController::TestCase should "redirect to the downtime index page" do DowntimeScheduler.stubs(:schedule_publish_and_expiry) post :create, params: { edition_id: edition.id, downtime: downtime_params } - assert_redirected_to controller: "legacy_downtimes", action: "index" + assert_redirected_to controller: "downtimes", action: "index" end end @@ -107,13 +107,13 @@ class DowntimesControllerTest < ActionController::TestCase context "cancelling scheduled downtime" do should "invoke the DowntimeRemover" do DowntimeRemover.expects(:destroy_immediately).with(downtime) - put :update, params: { edition_id: edition.id, downtime: downtime_params, commit: "Cancel downtime" } + put :update, params: { edition_id: edition.id, commit: "Cancel downtime" } end should "redirect to the downtime index" do DowntimeRemover.stubs(:destroy_immediately) - put :update, params: { edition_id: edition.id, downtime: downtime_params, commit: "Cancel downtime" } - assert_redirected_to controller: "legacy_downtimes", action: "index" + put :update, params: { edition_id: edition.id, commit: "Cancel downtime" } + assert_redirected_to controller: "downtimes", action: "index" end end @@ -127,7 +127,7 @@ class DowntimesControllerTest < ActionController::TestCase create_downtime DowntimeScheduler.stubs(:schedule_publish_and_expiry) put :update, params: { edition_id: edition.id, downtime: downtime_params, commit: "Re-schedule downtime message" } - assert_redirected_to controller: "legacy_downtimes", action: "index" + assert_redirected_to controller: "downtimes", action: "index" end end @@ -165,6 +165,14 @@ class DowntimesControllerTest < ActionController::TestCase end end + context "#destroy" do + should "render the page ok" do + create_downtime + get :destroy, params: { edition_id: edition.id } + assert_response :ok + end + end + def edition @edition ||= FactoryBot.create(:transaction_edition) end diff --git a/test/integration/downtime_integration_test.rb b/test/integration/downtime_integration_test.rb index 9084a338d..b4c68ea36 100644 --- a/test/integration/downtime_integration_test.rb +++ b/test/integration/downtime_integration_test.rb @@ -18,6 +18,7 @@ class DowntimeIntegrationTest < JavascriptIntegrationTest test_strategy = Flipflop::FeatureSet.current.test! test_strategy.switch!(:design_system_downtime_index_page, true) test_strategy.switch!(:design_system_downtime_new, true) + test_strategy.switch!(:design_system_downtime_edit, true) end test "Scheduling new downtime" do @@ -44,10 +45,12 @@ class DowntimeIntegrationTest < JavascriptIntegrationTest visit root_path click_link "Downtime" click_link "Edit downtime" - legacy_enter_end_time first_of_july_next_year_at_nine_thirty_pm_bst + enter_to_date_and_time first_of_july_next_year_at_nine_thirty_pm_bst + + find("textarea#downtime_message").click assert_match("This service will be unavailable from midday to 9:30pm on #{day} 1 July.", page.find_field("Message").value) - click_on "Re-schedule downtime message" + click_on "Save" assert page.has_content?("downtime message re-scheduled") assert page.has_content?("midday to 9:30pm on 1 July") @@ -60,7 +63,8 @@ class DowntimeIntegrationTest < JavascriptIntegrationTest visit root_path click_link "Downtime" click_link "Edit downtime" - click_on "Cancel downtime" + click_link "Remove" + click_on "Remove" assert page.has_content?("downtime message cancelled") assert_no_downtime_scheduled diff --git a/test/integration/downtime_with_invalid_dates_test.rb b/test/integration/downtime_with_invalid_dates_test.rb index 05fd3a518..8830d56cf 100644 --- a/test/integration/downtime_with_invalid_dates_test.rb +++ b/test/integration/downtime_with_invalid_dates_test.rb @@ -18,6 +18,7 @@ class DowntimeWithInvalidDates < ActionDispatch::IntegrationTest test_strategy = Flipflop::FeatureSet.current.test! test_strategy.switch!(:design_system_downtime_index_page, true) test_strategy.switch!(:design_system_downtime_new, true) + test_strategy.switch!(:design_system_downtime_edit, true) end test "Scheduling new downtime with invalid dates" do @@ -48,6 +49,54 @@ class DowntimeWithInvalidDates < ActionDispatch::IntegrationTest assert page.has_field?("downtime[end_time(5i)]", with: end_time.min.to_s) end + test "Rescheduling new downtime with invalid dates" do + DowntimeScheduler.stubs(:schedule_publish_and_expiry) + create_downtime + + visit root_path + click_link "Downtime" + click_link "Edit downtime" + + start_time = 1.day.ago + end_time = start_time - 1.day + enter_from_date_and_time start_time + enter_to_date_and_time end_time + + find("textarea#downtime_message").click + + click_button "Save" + + assert page.has_link?("End time must be in the future", href: "#downtime_end_time") + assert page.has_link?("Start time must be earlier than end time", href: "#downtime_start_time") + assert page.has_field?("downtime[start_time(1i)]", with: start_time.year.to_s) + assert page.has_field?("downtime[start_time(2i)]", with: start_time.month.to_s) + assert page.has_field?("downtime[start_time(3i)]", with: start_time.day.to_s) + assert page.has_field?("downtime[start_time(4i)]", with: start_time.hour.to_s) + assert page.has_field?("downtime[start_time(5i)]", with: start_time.min.to_s) + assert page.has_field?("downtime[end_time(1i)]", with: end_time.year.to_s) + assert page.has_field?("downtime[end_time(2i)]", with: end_time.month.to_s) + assert page.has_field?("downtime[end_time(3i)]", with: end_time.day.to_s) + assert page.has_field?("downtime[end_time(4i)]", with: end_time.hour.to_s) + assert page.has_field?("downtime[end_time(5i)]", with: end_time.min.to_s) + end + + test "Rescheduling downtime with missing day field" do + DowntimeScheduler.stubs(:schedule_publish_and_expiry) + create_downtime + + visit root_path + click_link "Downtime" + click_link "Edit downtime" + + fill_in "downtime[end_time(3i)]", with: "" + + find("textarea#downtime_message").click + + click_on "Save" + + assert page.has_content?("End time format is invalid") + end + def enter_from_date_and_time(start_time) enter_date_and_time("start", start_time) end @@ -63,4 +112,25 @@ def enter_date_and_time(prefix, time) fill_in "downtime[#{prefix}_time(4i)]", with: time.hour.to_s fill_in "downtime[#{prefix}_time(5i)]", with: time.min.to_s end + + def create_downtime + Downtime.create!( + artefact: @edition.artefact, + start_time: first_of_july_next_year_at_midday_bst, + end_time: first_of_july_next_year_at_six_pm_bst, + message: "foo", + ) + end + + def first_of_july_next_year_at_midday_bst + Time.zone.local(next_year, 7, 1, 12, 0) + end + + def first_of_july_next_year_at_six_pm_bst + Time.zone.local(next_year, 7, 1, 18, 0) + end + + def next_year + Time.zone.now.next_year.year + end end diff --git a/test/models/downtime_test.rb b/test/models/downtime_test.rb index e8cc7305d..dea694fdc 100644 --- a/test/models/downtime_test.rb +++ b/test/models/downtime_test.rb @@ -37,13 +37,6 @@ class DowntimeTest < ActiveSupport::TestCase assert_includes downtime.errors[:end_time], "must be in the future" end - should "validate end time is in future only on create" do - downtime = FactoryBot.create(:downtime) - downtime.assign_attributes(start_time: Time.zone.today - 3, end_time: Time.zone.yesterday) - - assert downtime.valid? - end - should "validate start time is earlier than end time" do downtime = FactoryBot.build(:downtime, start_time: Time.zone.today + 2, end_time: Time.zone.today + 1)