Users can add people with relevant convictions

app/controllers/conviction_details_forms_controller.rb

@@ -1,9 +1,13 @@
-class ConvictionDetailsFormsController < FormsController
+class ConvictionDetailsFormsController < PersonFormsController
   def new
     super(ConvictionDetailsForm, "conviction_details_form")
   end
 
   def create
     super(ConvictionDetailsForm, "conviction_details_form")
   end
+
+  def delete_person
+    super(ConvictionDetailsForm, "conviction_details_form")
+  end
 end

app/controllers/key_people_forms_controller.rb

@@ -1,37 +1,13 @@
-class KeyPeopleFormsController < FormsController
+class KeyPeopleFormsController < PersonFormsController
   def new
     super(KeyPeopleForm, "key_people_form")
   end
 
   def create
-    if params[:commit] == I18n.t("key_people_forms.new.add_person_link")
-      submit_and_add_another
-    else
-      super(KeyPeopleForm, "key_people_form")
-    end
-  end
-
-  def submit_and_add_another
-    return unless set_up_form(KeyPeopleForm, "key_people_form", params["key_people_form"][:reg_identifier])
-
-    respond_to do |format|
-      if @key_people_form.submit(params["key_people_form"])
-        format.html { redirect_to_correct_form }
-      else
-        format.html { render :new }
-      end
-    end
+    super(KeyPeopleForm, "key_people_form")
   end
 
   def delete_person
-    return unless set_up_form(KeyPeopleForm, "key_people_form", params[:reg_identifier])
-
-    respond_to do |format|
-      # Check if there are any matches first, to avoid a Mongoid error
-      people_with_id = @transient_registration.keyPeople.where(id: params[:id])
-      people_with_id.first.delete if people_with_id.any?
-
-      format.html { redirect_to_correct_form }
-    end
+    super(KeyPeopleForm, "key_people_form")
   end
 end

app/controllers/person_forms_controller.rb

@@ -0,0 +1,35 @@
+class PersonFormsController < FormsController
+  def create(form_class, form)
+    if params[:commit] == I18n.t("#{form}s.new.add_person_link")
+      submit_and_add_another(form_class, form)
+    else
+      super(form_class, form)
+    end
+  end
+
+  def submit_and_add_another(form_class, form)
+    return unless set_up_form(form_class, form, params[form][:reg_identifier])
+
+    form_instance_variable = instance_variable_get("@#{form}")
+
+    respond_to do |format|
+      if form_instance_variable.submit(params[form])
+        format.html { redirect_to_correct_form }
+      else
+        format.html { render :new }
+      end
+    end
+  end
+
+  def delete_person(form_class, form)
+    return unless set_up_form(form_class, form, params[:reg_identifier])
+
+    respond_to do |format|
+      # Check if there are any matches first, to avoid a Mongoid error
+      people_with_id = @transient_registration.keyPeople.where(id: params[:id])
+      people_with_id.first.delete if people_with_id.any?
+
+      format.html { redirect_to_correct_form }
+    end
+  end
+end

app/forms/conviction_details_form.rb

@@ -1,17 +1,38 @@
-class ConvictionDetailsForm < BaseForm
-  # TODO: Define accessible attributes, eg attr_accessor :field
+class ConvictionDetailsForm < PersonForm
+  attr_accessor :position
 
-  def initialize(transient_registration)
-    super
-    # TODO: Define params to get from transient_registration, eg self.field = @transient_registration.field
+  def position?
+    true
   end
 
-  def submit(params)
-    # Assign the params for validation and pass them to the BaseForm method for updating
-    # TODO: Define allowed params, eg self.field = params[:field]
-    # TODO: Include attributes to update in the attributes hash, eg { field: field }
-    attributes = {}
+  def maximum_people_in_type
+    nil
+  end
+
+  def minimum_people_in_type
+    1
+  end
+
+  private
+
+  def person_type
+    "relevant"
+  end
+
+  # Adding the new person directly to @transient_registration.keyPeople immediately updates the object,
+  # regardless of validation. So instead we copy all existing people into a new array and modify that.
+  def list_of_people_to_keep
+    people = []
+
+    @transient_registration.keyPeople.each do |person|
+      # We need to copy the person before adding to the array to avoid a 'conflicting modifications' Mongo error (10151)
+      people << person.clone
+    end
+
+    people
+  end
 
-    super(attributes, params[:reg_identifier])
+  def age_cutoff_date
+    (Date.today - 16.years) + 1.day
   end
 end

app/forms/key_people_form.rb

@@ -1,114 +1,59 @@
-class KeyPeopleForm < BaseForm
+class KeyPeopleForm < PersonForm
   attr_accessor :business_type
-  attr_accessor :first_name, :last_name, :dob_day, :dob_month, :dob_year, :key_person, :date_of_birth
 
   def initialize(transient_registration)
     super
     # We only use this for the correct microcopy
     self.business_type = @transient_registration.business_type
 
     # If there's only one key person, we can pre-fill the fields so users can easily edit them
-    prefill_form if can_only_have_one_key_person? && @transient_registration.keyPeople.present?
+    prefill_form if can_only_have_one_person_in_type? && @transient_registration.key_people.present?
   end
 
-  def submit(params)
-    # Assign the params for validation and pass them to the BaseForm method for updating
-    self.first_name = params[:first_name]
-    self.last_name = params[:last_name]
-    process_date_fields(params)
-
-    self.key_person = add_key_person
-    self.date_of_birth = key_person.date_of_birth
-
-    attributes = if fields_have_content?
-                   { keyPeople: all_key_people }
-                 else
-                   {}
-                 end
-
-    super(attributes, params[:reg_identifier])
-  end
-
-  validates_with KeyPeopleValidator
-  validate :old_enough?
-
-  def maximum_key_people
+  def maximum_people_in_type
     return unless business_type.present?
     key_people_limits[business_type.to_sym][:maximum]
   end
 
-  def minimum_key_people
+  def minimum_people_in_type
     # Business type should always be set, but use 1 as the default, just in case
     return 1 unless business_type.present?
     key_people_limits[business_type.to_sym][:minimum]
   end
 
-  def number_of_existing_key_people
-    @transient_registration.keyPeople.count
-  end
-
-  def can_only_have_one_key_person?
-    return false unless maximum_key_people.present?
-    maximum_key_people == 1
-  end
-
-  def enough_key_people?
-    return false if number_of_existing_key_people < minimum_key_people
-    true
-  end
-
-  def fields_have_content?
-    fields = [first_name, last_name, dob_day, dob_month, dob_year]
-    fields.each do |field|
-      return true if field.present? && field.to_s.length.positive?
-    end
-    false
-  end
-
   private
 
-  def prefill_form
-    self.first_name = @transient_registration.keyPeople.first.first_name
-    self.last_name = @transient_registration.keyPeople.first.last_name
-    self.dob_day = @transient_registration.keyPeople.first.dob_day
-    self.dob_month = @transient_registration.keyPeople.first.dob_month
-    self.dob_year = @transient_registration.keyPeople.first.dob_year
-  end
-
-  def process_date_fields(params)
-    self.dob_day = format_date_field_value(params[:dob_day])
-    self.dob_month = format_date_field_value(params[:dob_month])
-    self.dob_year = format_date_field_value(params[:dob_year])
-  end
-
-  # If we can make the date fields positive integers, use those integers
-  # Otherwise, return nil
-  def format_date_field_value(value)
-    # If this isn't a valid integer, .to_i returns 0
-    integer_value = value.to_i
-    return integer_value if integer_value.positive?
+  def person_type
+    "key"
   end
 
-  def add_key_person
-    KeyPerson.new(first_name: first_name,
-                  last_name: last_name,
-                  dob_day: dob_day,
-                  dob_month: dob_month,
-                  dob_year: dob_year,
-                  person_type: "key")
+  def prefill_form
+    self.first_name = @transient_registration.key_people.first.first_name
+    self.last_name = @transient_registration.key_people.first.last_name
+    self.dob_day = @transient_registration.key_people.first.dob_day
+    self.dob_month = @transient_registration.key_people.first.dob_month
+    self.dob_year = @transient_registration.key_people.first.dob_year
   end
 
-  def all_key_people
-    # If there's only one key person allowed, just replace existing data
-    return [key_person] if can_only_have_one_key_person?
-
-    existing_key_people = []
-    # Adding the new key person directly to @transient_registration.keyPeople immediately updates the object,
-    # regardless of validation. So instead we copy the existing key people into a new array and modify that.
-    @transient_registration.keyPeople.each do |person|
-      existing_key_people << person
+  # Adding the new key person directly to @transient_registration.keyPeople immediately updates the object,
+  # regardless of validation. So instead we copy all existing people into a new array and modify that.
+  def list_of_people_to_keep
+    people = []
+
+    # If there's only one key person allowed, we want to discard any existing key people, but keep people with
+    # relevant convictions. Otherwise, we copy all the keyPeople, regardless of type.
+    existing_people = if can_only_have_one_person_in_type?
+                        @transient_registration.relevant_people
+                      else
+                        @transient_registration.keyPeople
+                      end
+
+    existing_people.each do |person|
+      # We need to copy the person before adding to the array to avoid a 'conflicting modifications' Mongo error (10151)
+      people << person.clone
     end
-    existing_key_people << key_person
+
+    people
   end
 
   def key_people_limits
@@ -122,9 +67,7 @@ def key_people_limits
     }
   end
 
-  def old_enough?
-    return false unless date_of_birth.present?
-
+  def age_cutoff_date
     age_limits = {
       limitedCompany: 16.years,
       limitedLiabilityPartnership: 17.years,
@@ -133,12 +76,11 @@ def old_enough?
       partnership: 17.years,
       soleTrader: 17.years
     }
-    age_cutoff_date = (Date.today - age_limits[business_type.to_sym]) + 1.day
 
-    return true if date_of_birth < age_cutoff_date
+    (Date.today - age_limits[business_type.to_sym]) + 1.day
+  end
 
-    error_message = "age_limit_#{business_type}".to_sym
-    errors.add(:date_of_birth, error_message)
-    false
+  def age_limit_error_message
+    "age_limit_#{business_type}".to_sym
   end
 end