Merge pull request #2607 from alphagov/refactor-forms-controller

stephencdaly · web-flow · commit d3441b58c485 · 2026-03-10T13:42:05.000Z
Add forms specific base controller
diff --git a/app/controllers/forms/archive_form_controller.rb b/app/controllers/forms/archive_form_controller.rb
@@ -1,7 +1,6 @@
 module Forms
-  class ArchiveFormController < WebController
+  class ArchiveFormController < FormsController
     before_action :check_user_has_permission
-    after_action :verify_authorized
 
     def archive
       return redirect_to path_to_form unless current_form.is_live?
diff --git a/app/controllers/forms/archive_welsh_controller.rb b/app/controllers/forms/archive_welsh_controller.rb
@@ -1,7 +1,6 @@
 module Forms
-  class ArchiveWelshController < WebController
+  class ArchiveWelshController < FormsController
     before_action :check_user_has_permission
-    after_action :verify_authorized
 
     def show
       return redirect_to path_to_form unless has_live_welsh_translation?
diff --git a/app/controllers/forms/archived_controller.rb b/app/controllers/forms/archived_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class ArchivedController < WebController
-    after_action :verify_authorized
-
+  class ArchivedController < FormsController
     def show_form
       authorize current_form, :can_view_form?
 
diff --git a/app/controllers/forms/change_name_controller.rb b/app/controllers/forms/change_name_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class ChangeNameController < WebController
-    after_action :verify_authorized
-
+  class ChangeNameController < FormsController
     def edit
       authorize current_form, :can_view_form?
       @name_input = NameInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms/contact_details_controller.rb b/app/controllers/forms/contact_details_controller.rb
@@ -1,6 +1,5 @@
 module Forms
-  class ContactDetailsController < WebController
-    after_action :verify_authorized
+  class ContactDetailsController < FormsController
     def new
       authorize current_form, :can_view_form?
       @contact_details_input = ContactDetailsInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms/copy_controller.rb b/app/controllers/forms/copy_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class CopyController < WebController
-    after_action :verify_authorized
-
+  class CopyController < FormsController
     def copy
       authorize current_form, :copy?
 
diff --git a/app/controllers/forms/daily_submission_batch_controller.rb b/app/controllers/forms/daily_submission_batch_controller.rb
@@ -1,7 +1,6 @@
 module Forms
-  class DailySubmissionBatchController < WebController
+  class DailySubmissionBatchController < FormsController
     before_action :check_feature_flag
-    after_action :verify_authorized
 
     def new
       authorize current_form, :can_edit_form?
diff --git a/app/controllers/forms/declaration_controller.rb b/app/controllers/forms/declaration_controller.rb
@@ -1,6 +1,5 @@
 module Forms
-  class DeclarationController < WebController
-    after_action :verify_authorized
+  class DeclarationController < FormsController
     def new
       authorize current_form, :can_view_form?
       @declaration_input = DeclarationInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms/delete_confirmation_controller.rb b/app/controllers/forms/delete_confirmation_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class DeleteConfirmationController < WebController
-    after_action :verify_authorized
-
+  class DeleteConfirmationController < FormsController
     def delete
       authorize current_form
 
diff --git a/app/controllers/forms/draft_controller.rb b/app/controllers/forms/draft_controller.rb
@@ -0,0 +1,9 @@
+class Forms::DraftController < FormsController
+  def show
+    authorize current_form, :can_view_form?
+    task_service = FormTaskListService.call(form: current_form, current_user:)
+    @task_list = task_service.all_sections
+    @task_status_counts = task_service.task_counts
+    render :show, locals: { current_form: }
+  end
+end
diff --git a/app/controllers/forms/live_controller.rb b/app/controllers/forms/live_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class LiveController < WebController
-    after_action :verify_authorized
-
+  class LiveController < FormsController
     def show_form
       authorize current_form, :can_view_form?
 
diff --git a/app/controllers/forms/make_live_controller.rb b/app/controllers/forms/make_live_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class MakeLiveController < WebController
-    after_action :verify_authorized
-
+  class MakeLiveController < FormsController
     def new
       authorize current_form, :can_make_form_live?
       @make_live_input = MakeLiveInput.new(form: current_form)
diff --git a/app/controllers/forms/metrics_controller.rb b/app/controllers/forms/metrics_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class MetricsController < WebController
-    after_action :verify_authorized
-
+  class MetricsController < FormsController
     FORM_NAME_IN_FILENAME_MAX_LENGTH = 85 # limit the filename length to 100 characters
 
     def metrics_csv
diff --git a/app/controllers/forms/payment_link_controller.rb b/app/controllers/forms/payment_link_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class PaymentLinkController < WebController
-    after_action :verify_authorized
-
+  class PaymentLinkController < FormsController
     def new
       authorize current_form, :can_view_form?
       @payment_link_input = PaymentLinkInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms/privacy_policy_controller.rb b/app/controllers/forms/privacy_policy_controller.rb
@@ -1,6 +1,5 @@
 module Forms
-  class PrivacyPolicyController < WebController
-    after_action :verify_authorized
+  class PrivacyPolicyController < FormsController
     def new
       authorize current_form, :can_view_form?
       @privacy_policy_input = PrivacyPolicyInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms/redirect_from_forms_runner_controller.rb b/app/controllers/forms/redirect_from_forms_runner_controller.rb
@@ -1,6 +1,5 @@
-class Forms::RedirectFromFormsRunnerController < WebController
+class Forms::RedirectFromFormsRunnerController < FormsController
   before_action :check_user_has_permission
-  after_action :verify_authorized
 
   def edit_question
     page_external_id = params.require(:page_external_id)
diff --git a/app/controllers/forms/share_preview_controller.rb b/app/controllers/forms/share_preview_controller.rb
@@ -1,7 +1,6 @@
 module Forms
-  class SharePreviewController < WebController
+  class SharePreviewController < FormsController
     before_action :check_user_has_permission
-    after_action :verify_authorized
 
     def new
       @share_preview_input = Forms::SharePreviewInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms/submission_attachments_controller.rb b/app/controllers/forms/submission_attachments_controller.rb
@@ -1,7 +1,6 @@
 module Forms
-  class SubmissionAttachmentsController < WebController
+  class SubmissionAttachmentsController < FormsController
     before_action :submission_type_email?
-    after_action :verify_authorized
 
     def new
       authorize current_form, :can_view_form?
diff --git a/app/controllers/forms/submission_email_controller.rb b/app/controllers/forms/submission_email_controller.rb
@@ -1,7 +1,6 @@
 module Forms
-  class SubmissionEmailController < WebController
+  class SubmissionEmailController < FormsController
     before_action :submission_email_input, except: %i[create confirm_submission_email_code]
-    after_action :verify_authorized
 
     def new
       authorize current_form, :can_view_form?
diff --git a/app/controllers/forms/unarchive_controller.rb b/app/controllers/forms/unarchive_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class UnarchiveController < WebController
-    after_action :verify_authorized
-
+  class UnarchiveController < FormsController
     def new
       authorize current_form, :can_make_form_live?
       @make_live_input = MakeLiveInput.new(form: current_form)
diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class WelshTranslationController < WebController
-    after_action :verify_authorized
-
+  class WelshTranslationController < FormsController
     def new
       authorize current_form, :can_edit_form?
       return redirect_to form_path(current_form) unless welsh_enabled?
diff --git a/app/controllers/forms/what_happens_next_controller.rb b/app/controllers/forms/what_happens_next_controller.rb
@@ -1,7 +1,5 @@
 module Forms
-  class WhatHappensNextController < WebController
-    after_action :verify_authorized
-
+  class WhatHappensNextController < FormsController
     def new
       authorize current_form, :can_view_form?
       @what_happens_next_input = WhatHappensNextInput.new(form: current_form).assign_form_values
diff --git a/app/controllers/forms_controller.rb b/app/controllers/forms_controller.rb
@@ -1,35 +1,23 @@
 class FormsController < WebController
   after_action :verify_authorized
 
-  def show
-    authorize current_form, :can_view_form?
-    task_service = FormTaskListService.call(form: current_form, current_user:)
-    @task_list = task_service.all_sections
-    @task_status_counts = task_service.task_counts
-    render :show, locals: { current_form: }
+  def current_form
+    @current_form ||= Form.find(params[:form_id])
   end
 
-  def mark_pages_section_completed
-    authorize current_form, :can_view_form?
-    @pages = current_form.pages
-    @mark_complete_input = Forms::MarkPagesSectionCompleteInput.new(mark_complete_input_params)
+  def current_live_form
+    @current_live_form ||= FormDocument::Content.from_form_document(current_form.live_form_document)
+  end
 
-    if @mark_complete_input.submit
-      success_message = if @mark_complete_input.mark_complete == "true"
-                          t("banner.success.form.pages_saved_and_section_completed")
-                        else
-                          t("banner.success.form.pages_saved")
-                        end
-      redirect_to form_path(current_form.id), success: success_message
-    else
-      @mark_complete_input.mark_complete = "false"
-      render "pages/index", locals: { current_form: }, status: :unprocessable_content
-    end
+  def current_live_welsh_form
+    @current_live_welsh_form ||= FormDocument::Content.from_form_document(current_form.live_welsh_form_document)
   end
 
-private
+  def current_archived_form
+    @current_archived_form ||= FormDocument::Content.from_form_document(current_form.archived_form_document)
+  end
 
-  def mark_complete_input_params
-    params.require(:forms_mark_pages_section_complete_input).permit(:mark_complete).merge(form: current_form)
+  def current_archived_welsh_form
+    @current_archived_welsh_form ||= FormDocument::Content.from_form_document(current_form.archived_welsh_form_document)
   end
 end
diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb
@@ -1,4 +1,4 @@
-class PagesController < WebController
+class PagesController < FormsController
   before_action :check_user_has_permission
   before_action :clear_draft_questions_data, only: %i[index move_page]
   after_action :verify_authorized
@@ -83,6 +83,23 @@ def move_page
     redirect_to form_pages_path, success: success_message
   end
 
+  def mark_pages_section_completed
+    @pages = current_form.pages
+    @mark_complete_input = Forms::MarkPagesSectionCompleteInput.new(mark_complete_input_params)
+
+    if @mark_complete_input.submit
+      success_message = if @mark_complete_input.mark_complete == "true"
+                          t("banner.success.form.pages_saved_and_section_completed")
+                        else
+                          t("banner.success.form.pages_saved")
+                        end
+      redirect_to form_path(current_form.id), success: success_message
+    else
+      @mark_complete_input.mark_complete = "false"
+      render "pages/index", locals: { current_form: }, status: :unprocessable_content
+    end
+  end
+
 private
 
   def clear_draft_questions_data
@@ -126,6 +143,10 @@ def setup_draft_question_for_existing_page
     edit_draft_question
   end
 
+  def mark_complete_input_params
+    params.require(:forms_mark_pages_section_complete_input).permit(:mark_complete).merge(form: current_form)
+  end
+
   def set_answer_type_logging_attribute
     if @draft_question.present?
       CurrentLoggingAttributes.answer_type = @draft_question.answer_type
diff --git a/app/controllers/web_controller.rb b/app/controllers/web_controller.rb
@@ -72,26 +72,6 @@ def authenticate_user!
     @current_user = current_user
   end
 
-  def current_form
-    @current_form ||= Form.find(params[:form_id])
-  end
-
-  def current_live_form
-    @current_live_form ||= FormDocument::Content.from_form_document(current_form.live_form_document)
-  end
-
-  def current_live_welsh_form
-    @current_live_welsh_form ||= FormDocument::Content.from_form_document(current_form.live_welsh_form_document)
-  end
-
-  def current_archived_form
-    @current_archived_form ||= FormDocument::Content.from_form_document(current_form.archived_form_document)
-  end
-
-  def current_archived_welsh_form
-    @current_archived_welsh_form ||= FormDocument::Content.from_form_document(current_form.archived_welsh_form_document)
-  end
-
 private
 
   def authenticate_and_check_access
diff --git a/app/views/forms/draft/show.html.erb b/app/views/forms/draft/show.html.erb
diff --git a/config/routes.rb b/config/routes.rb
@@ -19,7 +19,7 @@
   end
 
   scope "forms/:form_id" do
-    get "/" => "forms#show", as: :form
+    get "/" => "forms/draft#show", as: :form
     get "/change-name" => "forms/change_name#edit", as: :change_form_name
     post "/change-name" => "forms/change_name#update"
 
@@ -85,7 +85,7 @@
 
     scope "/pages" do
       get "/" => "pages#index", as: :form_pages
-      post "/" => "forms#mark_pages_section_completed"
+      post "/" => "pages#mark_pages_section_completed"
       post "/move-page" => "pages#move_page", as: :move_page
       get "/change-order" => "pages/change_order#new", as: :change_order_new
       post "/change-order" => "pages/change_order#create", as: :change_order_create
diff --git a/spec/controller/web_controller_spec.rb b/spec/controller/web_controller_spec.rb
@@ -3,8 +3,6 @@
 describe WebController, type: :controller do
   subject(:web_controller) { described_class.new }
 
-  let(:form) { create :form }
-
   describe "#user_ip" do
     [
       ["", nil],
@@ -80,26 +78,6 @@ def index
     end
   end
 
-  describe "#current_form" do
-    before do
-      params = ActionController::Parameters.new(form_id: form.id)
-      allow(controller).to receive(:params).and_return(params)
-    end
-
-    it "returns the current form" do
-      expect(controller.current_form).to eq form
-    end
-
-    it "memorizes the find form request so it doesn't have to repeat the calls" do
-      allow(Form).to receive(:find).with(form.id).and_return(form)
-
-      controller.current_form
-      controller.current_form
-
-      expect(Form).to have_received(:find).exactly(1).times
-    end
-  end
-
   describe "analytics" do
     let(:user) { create :user }
 
diff --git a/spec/controllers/forms_controller_spec.rb b/spec/controllers/forms_controller_spec.rb
@@ -0,0 +1,26 @@
+require "rails_helper"
+
+RSpec.describe FormsController, type: :controller do
+  subject(:controller) { described_class.new }
+
+  let(:form) { create :form }
+
+  describe "#current_form" do
+    before do
+      controller.params = ActionController::Parameters.new(form_id: form.id)
+    end
+
+    it "returns the current form" do
+      expect(controller.current_form).to eq form
+    end
+
+    it "memorizes the find form request so it doesn't have to repeat the calls" do
+      allow(Form).to receive(:find).with(form.id).and_return(form)
+
+      controller.current_form
+      controller.current_form
+
+      expect(Form).to have_received(:find).exactly(1).times
+    end
+  end
+end
diff --git a/spec/requests/forms/draft_controller_spec.rb b/spec/requests/forms/draft_controller_spec.rb
diff --git a/spec/requests/pages_controller_spec.rb b/spec/requests/pages_controller_spec.rb
diff --git a/spec/views/forms/draft/show.html.erb_spec.rb b/spec/views/forms/draft/show.html.erb_spec.rb
