Refactor public controllers to use shared submission processing concern

Author: stefan-burke

app/controllers/concerns/submission_processing.rb

@@ -0,0 +1,78 @@
+module SubmissionProcessing
+  extend ActiveSupport::Concern
+
+  # Common method to process a submission for either form or code
+  def process_submission
+    # Initialize a new submission, handling the case where no fields are enabled
+    @submission = @form.submissions.new
+    @submission.device = @device
+
+    # Only attempt to update with submission params if they exist in the request
+    # This handles forms that have no enabled fields
+    @submission.assign_attributes(submission_params) if params[:submission].present?
+
+    # Associate the submission with the current user if logged in
+    @submission.user = current_user if current_user
+
+    # Check if this is a code submission and if the code has already been claimed
+    if defined?(@code) && @code && @code.claimed?
+      @submission.errors.add(:base, "This code has already been used")
+      return render "public_forms/show", status: :unprocessable_entity
+    end
+
+    if @submission.save
+      # Mark the code as claimed if this is a code submission
+      @code.claim! if defined?(@code) && @code
+
+      # Send the submission email only if target_email_address is present
+      send_submission_email
+
+      # Redirect to thanks page instead of rendering
+      # This prevents form resubmission on refresh
+      if defined?(@code) && @code
+        redirect_to code_thanks_path(@code)
+      else
+        redirect_to form_thanks_path(@form.code, @device)
+      end
+    else
+      render "public_forms/show", status: :unprocessable_entity
+    end
+  end
+
+  def send_submission_email
+    if @form.target_email_address.present?
+      begin
+        SubmissionMailer.new_submission(@submission).deliver_later
+        # Email will be sent asynchronously, so we mark it as pending
+        # It will be updated when the job completes
+      rescue => e
+        # Log the error but continue with the form submission
+        Rails.logger.error "Failed to queue submission email: #{e.message}"
+        @submission.mark_as_failed! if @submission.respond_to?(:mark_as_failed!)
+      end
+    end
+  end
+
+  def check_form_access
+    # Redirect to login if the form requires login and user is not logged in
+    if @form.require_login && !logged_in?
+      store_location # Store the current URL to redirect back after login
+      redirect_to login_path, alert: "You need to log in to access this form"
+    end
+  end
+
+  def submission_params
+    if params[:submission].present?
+      params.require(:submission).permit(
+        :name,
+        :email_address,
+        :phone,
+        :address,
+        :postcode
+      )
+    else
+      # Return an empty hash if there are no submission parameters
+      {}
+    end
+  end
+end

app/controllers/public_codes_controller.rb

@@ -1,73 +1,29 @@
 class PublicCodesController < ApplicationController
+  include SubmissionProcessing
+
   skip_before_action :require_login
   before_action :set_code
+  before_action :set_form_and_device
+  before_action :check_form_access, only: [:show, :create]
   layout "public_forms"
 
   def show
-    if @code.form.require_login? && !logged_in?
-      store_location
-      redirect_to login_path
-      return
-    end
-
     # If code has already been claimed, show the thank you page
     if @code.claimed?
-      # Still set the form and device for the thank you page
-      @form = @code.form
-      @device = @code.device
       render "public_forms/thanks"
       return
     end
 
-    @form = @code.form
-    @device = @code.device
     @submission = Submission.new
     render "public_forms/show"
   end
 
   def thanks
-    @form = @code.form
-    @device = @code.device
     render "public_forms/thanks"
   end
 
   def create
-    @form = @code.form
-    @device = @code.device
-    @submission = @form.submissions.new(submission_params)
-    @submission.device = @device
-
-    # Check if the code has already been claimed
-    if @code.claimed?
-      @submission.errors.add(:base, "This code has already been used")
-      return render "public_forms/show", status: :unprocessable_entity
-    end
-
-    # Associate the submission with the current user if logged in
-    @submission.user = current_user if current_user
-
-    if @submission.save
-      # Mark the code as claimed
-      @code.claim!
-
-      # Send the submission email only if target_email_address is present
-      if @form.target_email_address.present?
-        begin
-          SubmissionMailer.new_submission(@submission).deliver_later
-          # Email will be sent asynchronously, so we mark it as pending
-          # It will be updated when the job completes
-        rescue => e
-          # Log the error but continue with the form submission
-          Rails.logger.error "Failed to queue submission email: #{e.message}"
-          @submission.mark_as_failed! if @submission.respond_to?(:mark_as_failed!)
-        end
-      end
-
-      # Redirect to thanks page instead of rendering
-      redirect_to code_thanks_path(@code)
-    else
-      render "public_forms/show", status: :unprocessable_entity
-    end
+    process_submission
   end
 
   private
@@ -79,13 +35,8 @@ def set_code
     redirect_to root_path
   end
 
-  def submission_params
-    params.require(:submission).permit(
-      :name,
-      :email_address,
-      :phone,
-      :address,
-      :postcode
-    )
+  def set_form_and_device
+    @form = @code.form
+    @device = @code.device
   end
 end

app/controllers/public_forms_controller.rb

@@ -1,5 +1,6 @@
 class PublicFormsController < ApplicationController
   require "rqrcode"
+  include SubmissionProcessing
 
   # Skip login for public form views, but check if the form requires login
   skip_before_action :require_login
@@ -13,32 +14,7 @@ def show
   end
 
   def create
-    @submission = @form.submissions.new(submission_params)
-    @submission.device = @device
-
-    # Associate the submission with the current user if logged in
-    @submission.user = current_user if current_user
-
-    if @submission.save
-      # Send the submission email only if target_email_address is present
-      if @form.target_email_address.present?
-        begin
-          SubmissionMailer.new_submission(@submission).deliver_later
-          # Email will be sent asynchronously, so we mark it as pending
-          # It will be updated when the job completes
-        rescue => e
-          # Log the error but continue with the form submission
-          Rails.logger.error "Failed to queue submission email: #{e.message}"
-          @submission.mark_as_failed!
-        end
-      end
-
-      # Redirect to thanks page instead of rendering
-      # This prevents form resubmission on refresh
-      redirect_to form_thanks_path(@form.code, @device)
-    else
-      render :show, status: :unprocessable_entity
-    end
+    process_submission
   end
 
   def thanks
@@ -91,22 +67,4 @@ def set_form_and_device_for_qr
     blank_qr = ChunkyPNG::Image.new(200, 200, ChunkyPNG::Color::WHITE)
     send_data blank_qr.to_s, type: "image/png", disposition: "inline"
   end
-
-  def check_form_access
-    # Redirect to login if the form requires login and user is not logged in
-    if @form.require_login && !logged_in?
-      store_location # Store the current URL to redirect back after login
-      redirect_to login_path, alert: "You need to log in to access this form"
-    end
-  end
-
-  def submission_params
-    params.require(:submission).permit(
-      :name,
-      :email_address,
-      :phone,
-      :address,
-      :postcode
-    )
-  end
 end

spec/migrations/ensure_form_codes_spec.rb

@@ -0,0 +1,58 @@
+require "rails_helper"
+
+RSpec.describe "EnsureFormCodes migration", type: :migration do
+  let(:migration_path) { File.join(Rails.root, "db/migrate/20250523081500_ensure_form_codes.rb") }
+  let(:migration_class) { "EnsureFormCodes" }
+
+  # Helper to create a form directly bypassing validations
+  def create_form_without_code(attrs = {})
+    attrs = {name: "Form without code", button_text: "Submit"}.merge(attrs)
+
+    # Override the generate_code callback temporarily
+    Form.skip_callback(:validation, :before, :generate_code, raise: false)
+
+    form = Form.create!(attrs)
+
+    # Reset the code to nil
+    form.update_column(:code, nil)
+
+    Form.set_callback(:validation, :before, :generate_code)
+
+    form.reload
+  end
+
+  it "adds codes to all forms that don't have one" do
+    # Create a form with a nil code
+    form1 = create_form_without_code(name: "Form with nil code")
+    expect(form1.code).to be_nil
+
+    # Create a form with an empty code
+    form2 = create_form_without_code(name: "Form with empty code")
+    form2.update_column(:code, "")
+    form2.reload
+    expect(form2.code).to eq("")
+
+    # Create a form with a valid code
+    form3 = Form.create!(name: "Form with code", button_text: "Submit")
+    original_code = form3.code
+    expect(original_code).not_to be_nil
+
+    # Load and run the migration
+    require migration_path
+    migration = migration_class.constantize.new
+    migration.up
+
+    # Check that forms have codes now
+    form1.reload
+    expect(form1.code).not_to be_nil
+    expect(form1.code.length).to eq(12)
+
+    form2.reload
+    expect(form2.code).not_to be_nil
+    expect(form2.code.length).to eq(12)
+
+    # Check that existing codes are preserved
+    form3.reload
+    expect(form3.code).to eq(original_code)
+  end
+end