Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak key people validation handling and heading #95

Merged
merged 3 commits into from
Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions app/forms/key_people_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ 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]
self.dob_day = params[:dob_day].to_i
self.dob_month = params[:dob_month].to_i
self.dob_year = params[:dob_year].to_i
process_date_fields(params)

self.key_person = add_key_person
self.date_of_birth = key_person.date_of_birth
Expand All @@ -24,13 +22,23 @@ def submit(params)
super(attributes, params[:reg_identifier])
end

validates :first_name, presence: true, length: { maximum: 35 }
validates :last_name, presence: true, length: { maximum: 35 }
validates_with KeyPeopleValidator
validate :old_enough?
validates_with DateOfBirthValidator

private

# If we can make the date fields positive integers, save those integers
# Otherwise, save as nil
def process_date_fields(params)
day = params[:dob_day].to_i
month = params[:dob_month].to_i
year = params[:dob_year].to_i

self.dob_day = day if day.positive?
self.dob_month = month if month.positive?
self.dob_year = year if year.positive?
end

def add_key_person
KeyPerson.new(first_name: first_name,
last_name: last_name,
Expand Down
2 changes: 2 additions & 0 deletions app/models/key_person.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ def set_date_of_birth
errors.add(:date_of_birth, :invalid_date)
rescue ArgumentError
errors.add(:date_of_birth, :invalid_date)
rescue TypeError
errors.add(:date_of_birth, :invalid_date)
end
end
end
34 changes: 27 additions & 7 deletions app/validators/date_of_birth_validator.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,39 @@
class DateOfBirthValidator < ActiveModel::Validator
def validate(record)
fields = { day: record.dob_day, month: record.dob_month, year: record.dob_year }
# Make sure we have any date fields to validate before proceeding
return false if all_fields_empty?(record)

fields.each do |type, field|
validate_field(record, type, field)
end
# Next, check the date fields one at a time
return false unless individual_fields_valid?(record)

# If individual fields are OK, check the validity of the date as a whole
dob_is_a_date?(record)
end

private

def validate_field(record, type, field)
return unless field_present?(record, type, field)
return unless field_is_integer?(record, type, field)
def all_fields_empty?(record)
fields = [record.dob_day, record.dob_month, record.dob_year].compact
return false if fields.any?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍬 👍

record.errors.add(:date_of_birth, :not_a_date)
true
end

def individual_fields_valid?(record)
all_fields_valid = true

fields = { day: record.dob_day, month: record.dob_month, year: record.dob_year }
fields.each do |type, field|
next if field_is_valid?(record, type, field)
all_fields_valid = false
end

all_fields_valid
end

def field_is_valid?(record, type, field)
return false unless field_present?(record, type, field)
return false unless field_is_integer?(record, type, field)
field_is_in_correct_range?(record, type, field)
end

Expand Down
41 changes: 41 additions & 0 deletions app/validators/key_people_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
class KeyPeopleValidator < ActiveModel::Validator
def validate(record)
return false unless fields_have_content?(record)
validate_first_name(record)
validate_last_name(record)
DateOfBirthValidator.new.validate(record)
end

private

def fields_have_content?(record)
fields = [record.first_name, record.last_name, record.dob_day, record.dob_month, record.dob_year]
fields.each do |field|
return true if field.present? && field.to_s.length.positive?
end
record.errors.add(:base, :not_enough_key_people)
false
end

def validate_first_name(record)
return unless field_is_present?(record, :first_name)
field_is_not_too_long?(record, :first_name, 35)
end

def validate_last_name(record)
return unless field_is_present?(record, :last_name)
field_is_not_too_long?(record, :last_name, 35)
end

def field_is_present?(record, field)
return true if record.send(field).present?
record.errors.add(field, :blank)
false
end

def field_is_not_too_long?(record, field, length)
return true if record.send(field).length < length
record.errors.add(field, :too_long)
false
end
end
3 changes: 2 additions & 1 deletion config/locales/forms/key_people_forms/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ en:
heading_limitedLiabilityPartnership: Partner details
heading_overseas: Business owner details
heading_partnership: Partner details
heading_soleTrader: Business owner details
heading_soleTrader: Business or organisation owner details
description_1_localAuthority: Provide the name and date of birth for each chief executive of this local authority or public body.
description_1_limitedCompany: Provide the name and date of birth for each director of this business.
description_1_limitedLiabilityPartnership: Provide the name and date of birth for each partner of this business.
Expand All @@ -27,6 +27,7 @@ en:
errors:
models:
key_people_form:
not_enough_key_people: "You must add the details of at least one person"
attributes:
first_name:
blank: "You must enter a first name"
Expand Down
12 changes: 12 additions & 0 deletions spec/forms/key_people_forms_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@
end
end

context "when all the date of birth fields are empty" do
before(:each) do
key_people_form.dob_day = ""
key_people_form.dob_month = ""
key_people_form.dob_year = ""
end

it "is not valid" do
expect(key_people_form).to_not be_valid
end
end

context "when a date of birth is not a valid date" do
before(:each) do
key_people_form.date_of_birth = nil
Expand Down