Add FormsController base controller

stephencdaly · stephencdaly · commit efd1b5644795 · 2026-03-10T11:38:56.000Z
Move form-specific methods from WebController into a new
FormsController which all controllers that handle ations related to
an individual form inherit from.

We need to add a new form-specific after_action and don't want to
further pollute WebController with form-specific code.

Move the verify_authorized after_action to the FormsController to
avoid duplication and to ensure that all form-related controllers have
authorisation checks.
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
@@ -1,6 +1,4 @@
-class Forms::DraftController < WebController
-  after_action :verify_authorized
-
+class Forms::DraftController < FormsController
   def show
     authorize current_form, :can_view_form?
     task_service = FormTaskListService.call(form: current_form, current_user:)
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
@@ -0,0 +1,23 @@
+class FormsController < WebController
+  after_action :verify_authorized
+
+  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
+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
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/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
