Merge pull request #1659 from alphagov/ldeb-refactor-page-delete

Refactor controller actions and views for page delete confirmation

app/controllers/forms/delete_confirmation_controller.rb

@@ -16,11 +16,7 @@ def destroy
 
       if @delete_confirmation_input.valid?
         if @delete_confirmation_input.confirmed?
-          if params[:page_id].present?
-            delete_page(current_form, @page)
-          else
-            delete_form(current_form)
-          end
+          delete_form(current_form)
         else
           redirect_to @back_url
         end
@@ -52,19 +48,10 @@ def previous_page(id)
     def load_page_variables
       @delete_confirmation_input = DeleteConfirmationInput.new
 
-      if params[:page_id].present?
-        @page = PageRepository.find(page_id: params[:page_id], form_id: current_form.id)
-        @url = destroy_page_path(current_form, @page.id)
-        @confirm_deletion_legend = t("forms_delete_confirmation_input.confirm_deletion_page")
-        @item_name = @page.question_text
-        @back_url = edit_question_path(current_form, @page.id)
-      else
-        @url = destroy_form_path(current_form)
-        @confirm_deletion_legend = t("forms_delete_confirmation_input.confirm_deletion")
-        @confirm_deletion_live_form = t("forms_delete_confirmation_input.confirm_deletion_live_form") if current_form.live?
-        @item_name = current_form.name
-        @back_url = form_path(current_form)
-      end
+      @url = destroy_form_path(current_form)
+      @confirm_deletion_legend = t("forms_delete_confirmation_input.confirm_deletion")
+      @item_name = current_form.name
+      @back_url = form_path(current_form)
     end
 
     def delete_form(form)
@@ -76,15 +63,5 @@ def delete_form(form)
         raise StandardError, "Deletion unsuccessful"
       end
     end
-
-    def delete_page(form, page)
-      success_url = form_pages_path(form)
-
-      if PageRepository.destroy(page)
-        redirect_to success_url, status: :see_other, success: "Successfully deleted ‘#{page.question_text}’"
-      else
-        raise StandardError, "Deletion unsuccessful"
-      end
-    end
   end
 end

app/controllers/pages_controller.rb

@@ -9,6 +9,40 @@ def index
     render :index, locals: { current_form: }
   end
 
+  def delete
+    @page = PageRepository.find(page_id: params[:page_id], form_id: current_form.id)
+    @url = destroy_page_path(current_form, @page.id)
+    @confirm_deletion_legend = t("forms_delete_confirmation_input.confirm_deletion_page")
+    @item_name = @page.question_text
+    @back_url = edit_question_path(current_form, @page.id)
+
+    @delete_confirmation_input = Forms::DeleteConfirmationInput.new
+  end
+
+  def destroy
+    @page = PageRepository.find(page_id: params[:page_id], form_id: current_form.id)
+    @url = destroy_page_path(current_form, @page.id)
+    @confirm_deletion_legend = t("forms_delete_confirmation_input.confirm_deletion_page")
+    @item_name = @page.question_text
+    @back_url = edit_question_path(current_form, @page.id)
+
+    @delete_confirmation_input = Forms::DeleteConfirmationInput.new(
+      params.require(:forms_delete_confirmation_input).permit(:confirm),
+    )
+    if @delete_confirmation_input.valid?
+      if @delete_confirmation_input.confirmed?
+        delete_page(current_form, @page)
+      else
+        redirect_to @back_url
+      end
+    else
+      render :delete
+    end
+  rescue StandardError
+    flash[:message] = "Deletion unsuccessful"
+    redirect_to @back_url
+  end
+
   def start_new_question
     clear_draft_questions_data
     redirect_to type_of_answer_create_path(form_id: current_form.id)
@@ -35,7 +69,7 @@ def clear_draft_questions_data
   end
 
   def check_user_has_permission
-    authorize current_form, :can_view_form?
+    authorize current_form, :can_edit_form?
   end
 
   def page
@@ -70,4 +104,14 @@ def setup_draft_question_for_existing_page
     end
     edit_draft_question
   end
+
+  def delete_page(form, page)
+    success_url = form_pages_path(form)
+
+    if PageRepository.destroy(page)
+      redirect_to success_url, status: :see_other, success: "Successfully deleted ‘#{page.question_text}’"
+    else
+      raise StandardError, "Deletion unsuccessful"
+    end
+  end
 end

app/input_objects/forms/delete_confirmation_input.rb

@@ -1,2 +1,5 @@
 class Forms::DeleteConfirmationInput < ConfirmActionInput
+  def to_partial_path
+    "input_objects/forms/delete_confirmation_input"
+  end
 end

app/policies/form_policy.rb

@@ -12,7 +12,8 @@ def can_view_form?
     user.groups.include?(form.group) || user.is_organisations_admin?(form.group&.organisation)
   end
 
-  alias_method :delete?, :can_view_form?
+  alias_method :can_edit_form?, :can_view_form?
+  alias_method :delete?, :can_edit_form?
   alias_method :destroy?, :delete?
 
   def can_add_page_routing_conditions?

app/views/forms/delete_confirmation/delete.html.erb

@@ -1,14 +1,9 @@
 <% set_page_title(title_with_error_prefix(@confirm_deletion_legend, @delete_confirmation_input.errors.any?)) %>
 <% content_for :back_link, govuk_back_link_to(@back_url) %>
-<%= form_with(model: @delete_confirmation_input, url: @url, method: 'delete') do |f| %>
-  <% if @delete_confirmation_input&.errors.any? %>
-    <%= f.govuk_error_summary %>
-  <% end %>
-  <span class="govuk-caption-l"><%= @item_name %></span>
-  <%= f.govuk_collection_radio_buttons :confirm,
-                                       @delete_confirmation_input.values, ->(option) { option }, ->(option) { t('helpers.label.confirm_action_input.options.' + "#{option}") },
-                                       legend: { text: @confirm_deletion_legend, size: 'l', tag: 'h1' },
-                                       hint: {text: @confirm_deletion_live_form} %>
 
-  <%= f.govuk_submit %>
-<% end %>
+<%= render(
+  @delete_confirmation_input,
+  url: @url,
+  caption_text: @item_name,
+  legend_text: @confirm_deletion_legend,
+) %>

app/views/input_objects/forms/_delete_confirmation_input.html.erb

@@ -0,0 +1,12 @@
+<%= form_with(model: delete_confirmation_input, url:, method: 'delete') do |f| %>
+  <% if delete_confirmation_input&.errors.any? %>
+    <%= f.govuk_error_summary %>
+  <% end %>
+  <span class="govuk-caption-l"><%= caption_text %></span>
+  <%= f.govuk_collection_radio_buttons :confirm,
+                                       delete_confirmation_input.values, ->(option) { option }, ->(option) { t('helpers.label.confirm_action_input.options.' + "#{option}") },
+                                       legend: { text: legend_text, size: 'l', tag: 'h1' }
+                                       %>
+
+  <%= f.govuk_submit %>
+<% end %>

app/views/pages/delete.html.erb

@@ -0,0 +1,10 @@
+<% set_page_title(title_with_error_prefix(@confirm_deletion_legend, @delete_confirmation_input.errors.any?)) %>
+<% content_for :back_link, govuk_back_link_to(@back_url) %>
+
+<%= render(
+  @delete_confirmation_input,
+  url: @url,
+  caption_text: @item_name,
+  legend_text: @confirm_deletion_legend,
+  hint_text: nil,
+) %>

config/locales/en.yml

@@ -398,7 +398,6 @@ en:
         title: Optional task
   forms_delete_confirmation_input:
     confirm_deletion: Are you sure you want to delete this draft?
-    confirm_deletion_live_form: Deleting this draft will not remove the live form.
     confirm_deletion_page: Are you sure you want to delete this page?
   group_members:
     create:

config/routes.rb

@@ -144,8 +144,8 @@
         end
 
         scope "/delete" do
-          get "/" => "forms/delete_confirmation#delete", as: :delete_page
-          delete "/" => "forms/delete_confirmation#destroy", as: :destroy_page
+          get "/" => "pages#delete", as: :delete_page
+          delete "/" => "pages#destroy", as: :destroy_page
         end
       end
     end

spec/requests/forms/delete_confirmation_controller_spec.rb

@@ -35,30 +35,6 @@
         end
       end
     end
-
-    describe "Given a valid page" do
-      before do
-        ActiveResource::HttpMock.respond_to do |mock|
-          mock.get "/api/v1/forms/2", headers, form.to_json, 200
-        end
-
-        allow(PageRepository).to receive(:find).and_return(page)
-
-        get delete_page_path(form_id: form.id, page_id: page.id)
-      end
-
-      it "Reads the form through the page repository" do
-        expect(PageRepository).to have_received(:find)
-      end
-
-      context "when current user is not in group for form the page is in" do
-        let(:membership) { nil }
-
-        it "returns an error" do
-          expect(response).to have_http_status :forbidden
-        end
-      end
-    end
   end
 
   describe "#destroy" do
@@ -113,33 +89,5 @@
         expect(form).not_to have_been_deleted
       end
     end
-
-    describe "Given a valid page" do
-      before do
-        allow(PageRepository).to receive_messages(find: page, destroy: true)
-
-        delete destroy_page_path(form_id: form.id, page_id: page.id, forms_delete_confirmation_input: { confirm: "yes" })
-      end
-
-      it "destroys the page through the page repository" do
-        expect(PageRepository).to have_received(:destroy)
-      end
-
-      it "redirects you to the page" do
-        expect(response).to redirect_to(form_pages_path(form.id))
-      end
-
-      context "when current user is not in group for form" do
-        let(:membership) { nil }
-
-        it "returns an error" do
-          expect(response).to have_http_status :forbidden
-        end
-
-        it "does not call destroy through the page repository" do
-          expect(PageRepository).not_to have_received(:destroy)
-        end
-      end
-    end
   end
 end

spec/requests/pages_controller_spec.rb

@@ -4,9 +4,10 @@
   let(:form_response) { build :form, id: 2 }
 
   let(:group) { create(:group, organisation: standard_user.organisation) }
+  let(:membership) { create :membership, group:, user: standard_user }
 
   before do
-    Membership.create!(group_id: group.id, user: standard_user, added_by: standard_user)
+    membership
     login_as_standard_user
   end
 
@@ -86,8 +87,8 @@
     end
   end
 
-  describe "Deleting an existing page" do
-    describe "Given a valid page" do
+  describe "#delete" do
+    describe "given a valid page" do
       let(:page) do
         Page.new({
           id: 1,
@@ -113,13 +114,25 @@
       end
 
       it "renders the delete page template" do
-        expect(response).to render_template("forms/delete_confirmation/delete")
+        expect(response).to render_template("pages/delete")
+      end
+
+      it "reads the form through the page repository" do
+        expect(PageRepository).to have_received(:find)
+      end
+
+      context "when current user is not in group for form the page is in" do
+        let(:membership) { nil }
+
+        it "returns an error" do
+          expect(response).to have_http_status :forbidden
+        end
       end
     end
   end
 
-  describe "Destroying an existing page" do
-    describe "Given a valid page" do
+  describe "#destroy" do
+    describe "given a valid page" do
       let(:page) do
         Page.new({
           id: 1,
@@ -149,9 +162,25 @@
         delete destroy_page_path(form_id: 2, page_id: 1, forms_delete_confirmation_input: { confirm: "yes" })
       end
 
-      it "Redirects you to the page index screen" do
+      it "redirects you to the page index screen" do
         expect(response).to redirect_to(form_pages_path)
       end
+
+      it "destroys the page through the page repository" do
+        expect(PageRepository).to have_received(:destroy)
+      end
+
+      context "when current user is not in group for form" do
+        let(:membership) { nil }
+
+        it "returns an error" do
+          expect(response).to have_http_status :forbidden
+        end
+
+        it "does not call destroy through the page repository" do
+          expect(PageRepository).not_to have_received(:destroy)
+        end
+      end
     end
   end
 

spec/views/forms/delete_confirmation/delete.html.erb_spec.rb

@@ -0,0 +1,41 @@
+require "rails_helper"
+
+RSpec.describe "forms/delete_confirmation/delete" do
+  let(:form) { build :form, id: 1, name: "Test form" }
+
+  before do
+    assign(:back_url, form_path(form_id: 1))
+    assign(:confirm_deletion_legend, "Are you sure you want to delete this draft?")
+    assign(:delete_confirmation_input, Forms::DeleteConfirmationInput.new)
+    assign(:item_name, form.name)
+    assign(:url, destroy_form_path(form_id: 1))
+
+    render
+  end
+
+  it "has a page title" do
+    expect(view.content_for(:title)).to include "Are you sure you want to delete this draft?"
+  end
+
+  it "has a heading" do
+    expect(rendered).to have_css "h1", text: "Are you sure you want to delete this draft?"
+  end
+
+  it "has a heading caption with the question text" do
+    expect(rendered).to have_css ".govuk-caption-l", text: "Test form"
+  end
+
+  it "has a back link to the form page" do
+    expect(view.content_for(:back_link)).to have_link "Back", href: "/forms/1"
+  end
+
+  it "has a delete confirmation input to confirm deletion of the form" do
+    expect(rendered).to render_template "input_objects/forms/_delete_confirmation_input"
+  end
+
+  describe "delete confirmation input" do
+    it "posts the confirm value to the destroy action" do
+      expect(rendered).to have_element "form", action: "/forms/1/delete", method: "post"
+    end
+  end
+end