Highlight fields with errors & update validations.

app/assets/stylesheets/application.css.scss

@@ -92,6 +92,16 @@
   margin: 10px 0px 15px 10px;
 }
 
+// Adds a red border around invalid input fields after form submission.
+.field_with_errors
+{
+  input, textarea
+  {
+    border-color: red;
+    border-width: 2px;
+  }
+}
+
 .accessibility
 {
   input { margin-top: 4px; }

app/helpers/admin/form_helper.rb

@@ -33,5 +33,14 @@ def nested_categories(categories)
         end
       end.join.html_safe
     end
+
+    def error_class_for(model, attribute, field)
+      return if model.errors[attribute].blank?
+      'field_with_errors' if field_contains_errors?(model, attribute, field)
+    end
+
+    def field_contains_errors?(model, attribute, field)
+      model.errors[attribute].select { |error| error.include?(field) }.present?
+    end
   end
 end

app/models/location.rb

@@ -67,15 +67,9 @@ class Location < ActiveRecord::Base
   # For example, the urls field is an array that can contain multiple URLs.
   # To be able to validate each URL in the array, we have to use a
   # custom array validator. See app/validators/array_validator.rb
-  validates :urls, array: {
-    format: { with: %r{\Ahttps?://([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z}i,
-              message: "%{value} #{I18n.t('errors.messages.invalid_url')}",
-              allow_blank: true } }
-
-  validates :emails, :admin_emails, array: {
-    format: { with: /\A([^@\s]+)@((?:(?!-)[-a-z0-9]+(?<!-)\.)+[a-z]{2,})\z/i,
-              message: "%{value} #{I18n.t('errors.messages.invalid_email')}",
-              allow_blank: true } }
+  validates :urls, array: { url: true }
+
+  validates :emails, :admin_emails, array: { email: true }
 
   # Only call Google's geocoding service if the address has changed
   # to avoid unnecessary requests that affect our rate limit.

app/models/organization.rb

@@ -12,10 +12,7 @@ class Organization < ActiveRecord::Base
   # For example, the urls field is an array that can contain multiple URLs.
   # To be able to validate each URL in the array, we have to use a
   # custom array validator. See app/validators/array_validator.rb
-  validates :urls, array: {
-    format: { with: %r{\Ahttps?://([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z}i,
-              message: "%{value} #{I18n.t('errors.messages.invalid_url')}",
-              allow_blank: true } }
+  validates :urls, array: { url: true }
 
   serialize :urls, Array
 

app/models/service.rb

@@ -14,14 +14,9 @@ class Service < ActiveRecord::Base
   validates :name, :description, :location,
             presence: { message: I18n.t('errors.messages.blank_for_service') }
 
-  validates :urls, array: {
-    format: { with: %r{\Ahttps?://([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z}i,
-              message: "%{value} #{I18n.t('errors.messages.invalid_url')}",
-              allow_blank: true } }
+  validates :urls, array: { url: true }
 
-  validate :service_area_format, if: (proc do |s|
-    s.service_areas.is_a?(Array) && SETTINGS[:valid_service_areas].present?
-  end)
+  validates :service_areas, array: { service_area: true }
 
   auto_strip_attributes :audience, :description, :eligibility, :fees,
                         :how_to_apply, :name, :short_desc, :wait
@@ -33,14 +28,4 @@ class Service < ActiveRecord::Base
   serialize :keywords, Array
   serialize :service_areas, Array
   serialize :urls, Array
-
-  def service_area_format
-    return unless service_areas.present?
-    valid_service_areas = SETTINGS[:valid_service_areas]
-    return unless (service_areas - valid_service_areas).size != 0
-    error_message = 'At least one service area is improperly formatted, ' \
-      'or is not an accepted city or county name. Please make sure all ' \
-      'words are capitalized.'
-    errors.add(:service_areas, error_message)
-  end
 end

app/validators/email_validator.rb

@@ -1,8 +1,9 @@
 class EmailValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
+    return if value.blank?
     default_message = "#{value} #{I18n.t('errors.messages.invalid_email')}"
 
-    unless value =~ /.+@.+\..+/i
+    unless value =~ /\A([^@\s]+)@((?:(?!-)[-a-z0-9]+(?<!-)\.)+[a-z]{2,})\z/i
       record.errors[attribute] << (options[:message] || default_message)
     end
   end

app/validators/service_area_validator.rb

@@ -0,0 +1,10 @@
+class ServiceAreaValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    return if value.blank?
+    default_message = "#{value} #{I18n.t('errors.messages.invalid_service_area')}"
+
+    unless SETTINGS[:valid_service_areas].include?(value)
+      record.errors[attribute] << (options[:message] || default_message)
+    end
+  end
+end

app/validators/url_validator.rb

@@ -1,5 +1,6 @@
 class UrlValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
+    return if value.blank?
     default_message = "#{value} #{I18n.t('errors.messages.invalid_url')}"
 
     unless value =~ %r{\Ahttps?:\/\/([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z}i

app/views/admin/locations/forms/_admin_email_fields.html.haml

@@ -1,6 +1,6 @@
 = field_set_tag do
   .row
-    .col-xs-6
+    .col-sm-6
       = email_field_tag 'location[admin_emails][]', '', class: 'form-control'
   = link_to 'Delete this admin permanently', '#', class: 'btn btn-danger delete_attribute'
   %hr

app/views/admin/locations/forms/_admin_emails.html.haml

@@ -11,8 +11,8 @@
     - @location.admin_emails.each_with_index do |admin, i|
       = field_set_tag do
         .row
-          .col-xs-6
-            = text_field_tag 'location[admin_emails][]', admin, class: 'form-control', id: "location_admin_emails_#{i}"
+          %div{ class: "col-sm-6 #{error_class_for(@location, :admin_emails, admin)}" }
+            = email_field_tag 'location[admin_emails][]', admin, class: 'form-control', id: "location_admin_emails_#{i}"
         = link_to 'Delete this admin permanently', '#', class: 'btn btn-danger delete_attribute'
         %hr
   = link_to_add_array_fields 'Add a new admin email', :locations, :admin_email

app/views/admin/locations/forms/_description.html.haml

@@ -3,5 +3,5 @@
     = f.label :description, 'Description'
     %span.desc
       A description of the location's services.
-  %p
+  = field_set_tag do
     = f.text_area :description, required: false, class: 'form-control', rows: 10

app/views/admin/locations/forms/_emails.html.haml

@@ -9,7 +9,7 @@
     - @location.emails.each_with_index do |email, i|
       = field_set_tag do
         .row
-          .col-md-6
+          %div{ class: "col-sm-6 #{error_class_for(@location, :emails, email)}" }
             = email_field_tag 'location[emails][]', email, class: 'form-control', id: "location_emails_#{i}"
         = link_to 'Delete this email permanently', '#', class: 'btn btn-danger delete_attribute'
         %hr

app/views/admin/locations/forms/_location_name.html.haml

@@ -3,5 +3,7 @@
     = f.label :name, 'Location Name'
     %span.desc
       The name of the organization at a location, such as a branch, department, or similar.
-  %p
-    = f.text_field :name, required: false, maxlength: 255, class: 'form-control'
+  = field_set_tag do
+    .row
+      .col-sm-6
+        = f.text_field :name, required: false, maxlength: 255, class: 'form-control'

app/views/admin/locations/forms/_short_desc.html.haml

@@ -3,5 +3,5 @@
     = f.label :short_desc, 'Short Description'
     %span.desc
       A short summary of the description of services.
-  %p
+  = field_set_tag do
     = f.text_area :short_desc, class: 'form-control'

app/views/admin/locations/forms/_urls.html.haml

@@ -9,7 +9,9 @@
   - if @location.urls.present?
     - @location.urls.each_with_index do |url, i|
       = field_set_tag do
-        = url_field_tag 'location[urls][]', url, class: 'form-control', id: "location_urls_#{i}"
+        .row
+          %div{ class: "col-sm-6 #{error_class_for(@location, :urls, url)}" }
+            = url_field_tag 'location[urls][]', url, class: 'form-control', id: "location_urls_#{i}"
         = link_to 'Delete this website permanently', '#', class: 'btn btn-danger delete_attribute'
         %hr
   = link_to_add_array_fields 'Add a new website', :locations, :url

app/views/admin/organizations/forms/_name.html.haml

@@ -1,5 +1,7 @@
 .inst-box
   %header
     = f.label :name, 'Organization Name'
-  %p
-    = f.text_field :name, required: false, maxlength: 255, class: 'form-control'
+  = field_set_tag do
+    .row
+      .col-sm-6
+        = f.text_field :name, required: false, maxlength: 255, class: 'form-control'

app/views/admin/organizations/forms/_urls.html.haml

@@ -9,7 +9,9 @@
   - if @organization.urls.present?
     - @organization.urls.each_with_index do |url, i|
       = field_set_tag do
-        = url_field_tag 'organization[urls][]', url, class: 'form-control', id: "organization_urls_#{i}"
+        .row
+          %div{ class: "col-sm-6 #{error_class_for(@organization, :urls, url)}" }
+            = url_field_tag 'organization[urls][]', url, class: 'form-control', id: "organization_urls_#{i}"
         = link_to 'Delete this website permanently', '#', class: 'btn btn-danger delete_attribute'
         %hr
   = link_to_add_array_fields 'Add a new website', :organizations, :url

app/views/admin/services/forms/_description.html.haml

@@ -3,5 +3,5 @@
     = f.label :description, 'Description'
     %span.desc
       A description of the service.
-  %p
+  = field_set_tag do
     = f.text_area :description, required: false, class: 'form-control', rows: 5

app/views/admin/services/forms/_name.html.haml

@@ -3,5 +3,7 @@
     = f.label :name, 'Service Name'
     %span.desc
       The name of the service.
-  %p
-    = f.text_field :name, required: false, maxlength: 255, class: 'form-control'
+  = field_set_tag do
+    .row
+      .col-sm-6
+        = f.text_field :name, required: false, maxlength: 255, class: 'form-control'

app/views/admin/services/forms/_service_areas.html.haml

@@ -10,7 +10,9 @@
   - if @service.service_areas.present?
     - @service.service_areas.each_with_index do |service_area, i|
       = field_set_tag do
-        = text_field_tag 'service[service_areas][]', service_area, class: 'form-control', id: "service_service_areas_#{i}"
+        .row
+          %div{ class: "col-sm-6 #{error_class_for(@service, :service_areas, service_area)}" }
+            = text_field_tag 'service[service_areas][]', service_area, class: 'form-control', id: "service_service_areas_#{i}"
         = link_to 'Delete this service area permanently', '#', class: 'btn btn-danger delete_attribute'
         %hr
   = link_to_add_array_fields 'Add a new service area', :services, :service_area

app/views/admin/services/forms/_urls.html.haml

@@ -9,7 +9,9 @@
   - if @service.urls.present?
     - @service.urls.each_with_index do |url, i|
       = field_set_tag do
-        = url_field_tag 'service[urls][]', url, class: 'form-control', id: "service_urls_#{i}"
+        .row
+          %div{ class: "col-sm-6 #{error_class_for(@service, :urls, url)}" }
+            = url_field_tag 'service[urls][]', url, class: 'form-control', id: "service_urls_#{i}"
         = link_to 'Delete this website permanently', '#', class: 'btn btn-danger delete_attribute'
         %hr
   = link_to_add_array_fields 'Add a new website', :services, :url

config/locales/en.yml

@@ -48,6 +48,7 @@ en:
       invalid_email: 'is not a valid email'
       invalid_fax: 'is not a valid US fax number'
       invalid_phone: 'is not a valid US phone number'
+      invalid_service_area: 'is not a valid service area'
       invalid_state: 'Please enter a valid 2-letter state abbreviation'
       invalid_url: 'is not a valid URL'
       invalid_zip: 'is not a valid ZIP code'

spec/api/patch_service_spec.rb

@@ -64,7 +64,7 @@
     expect(response.status).to eq(422)
     expect(json['message']).to eq('Validation failed for resource.')
     expect(json['errors'].first['service_areas'].first).
-      to include('At least one service area is improperly formatted')
+      to eq('Belmont, CA is not a valid service area')
   end
 
   it 'returns 422 when service_areas is empty String' do

spec/features/admin/locations/update_admin_emails_spec.rb

@@ -2,7 +2,7 @@
 
 feature 'Update admin_emails' do
   background do
-    create(:location)
+    @location = create(:location)
     login_super_admin
     visit '/admin/locations/vrs-services'
   end
@@ -36,11 +36,24 @@
     expect(total_admins.length).to eq 1
   end
 
+  scenario 'with 2 admin_emails but only one is invalid', :js do
+    @location.update!(admin_emails: ['[email protected]'])
+    visit '/admin/locations/vrs-services'
+    click_link 'Add a new admin'
+    admin_emails = page.
+        all(:xpath, "//input[@name='location[admin_emails][]']")
+    fill_in admin_emails[-1][:id], with: 'Alexandria'
+    click_button 'Save changes'
+    total_fields_with_errors = page.all(:css, '.field_with_errors')
+    expect(total_fields_with_errors.length).to eq 1
+  end
+
   scenario 'with invalid admin', :js do
     click_link 'Add a new admin'
     fill_in 'location[admin_emails][]', with: 'moncefsamaritanhouse.com'
     click_button 'Save changes'
     expect(page).
       to have_content 'moncefsamaritanhouse.com is not a valid email'
+    expect(page).to have_css('.field_with_errors')
   end
 end

spec/features/admin/locations/update_emails_spec.rb

@@ -35,6 +35,7 @@
     fill_in 'location_emails_0', with: 'example.org'
     click_button 'Save changes'
     expect(page).to have_content 'example.org is not a valid email'
+    expect(page).to have_css('.field_with_errors')
   end
 
   scenario 'by adding 2 new emails', :js do
@@ -58,4 +59,16 @@
     total_emails = all(:xpath, "//input[@name='location[emails][]']")
     expect(total_emails.length).to eq 1
   end
+
+  scenario 'with 2 emails but only one is invalid', :js do
+    @location.update!(emails: ['[email protected]'])
+    visit '/admin/locations/vrs-services'
+    click_link 'Add a new general email'
+    emails = page.
+        all(:xpath, "//input[@name='location[emails][]']")
+    fill_in emails[-1][:id], with: 'Alexandria'
+    click_button 'Save changes'
+    total_fields_with_errors = page.all(:css, '.field_with_errors')
+    expect(total_fields_with_errors.length).to eq 1
+  end
 end

spec/features/admin/locations/update_name_spec.rb

@@ -11,6 +11,7 @@
     fill_in 'location_name', with: ''
     click_button 'Save changes'
     expect(page).to have_content "Name can't be blank for Location"
+    expect(page).to have_css('.field_with_errors')
   end
 
   scenario 'with valid location name' do

spec/features/admin/locations/update_urls_spec.rb

@@ -27,12 +27,25 @@
     expect(total_urls.length).to eq 1
   end
 
+  scenario 'with 2 urls but only one is invalid', :js do
+    @location.update!(urls: ['http://ruby.org'])
+    visit '/admin/locations/vrs-services'
+    click_link 'Add a new website'
+    urls = page.
+        all(:xpath, "//input[@name='location[urls][]']")
+    fill_in urls[-1][:id], with: 'Alexandria'
+    click_button 'Save changes'
+    total_fields_with_errors = page.all(:css, '.field_with_errors')
+    expect(total_fields_with_errors.length).to eq 1
+  end
+
   scenario 'with invalid website' do
     @location.update!(urls: ['http://ruby.org'])
     visit '/admin/locations/vrs-services'
     fill_in 'location_urls_0', with: 'www.monfresh.com'
     click_button 'Save changes'
     expect(page).to have_content 'www.monfresh.com is not a valid URL'
+    expect(page).to have_css('.field_with_errors')
   end
 
   scenario 'with valid website' do

spec/features/admin/organizations/update_urls_spec.rb

@@ -27,12 +27,25 @@
     expect(total_urls.length).to eq 1
   end
 
+  scenario 'with 2 urls but only one is invalid', :js do
+    @org.update!(urls: ['http://ruby.org'])
+    visit '/admin/organizations/parent-agency'
+    click_link 'Add a new website'
+    urls = page.
+        all(:xpath, "//input[@name='organization[urls][]']")
+    fill_in urls[-1][:id], with: 'Alexandria'
+    click_button 'Save changes'
+    total_fields_with_errors = page.all(:css, '.field_with_errors')
+    expect(total_fields_with_errors.length).to eq 1
+  end
+
   scenario 'with invalid website' do
     @org.update!(urls: ['http://ruby.org'])
     visit '/admin/organizations/parent-agency'
     fill_in 'organization_urls_0', with: 'www.monfresh.com'
     click_button 'Save changes'
     expect(page).to have_content 'www.monfresh.com is not a valid URL'
+    expect(page).to have_css('.field_with_errors')
   end
 
   scenario 'with valid website' do