Improve the configuration model

app/helpers/tooltips_helper.rb

@@ -31,7 +31,7 @@ def completed_calls_help_text
   end
 
   def shared_cases_help_text
-    t('tooltips.shared_cases', shared_reset: Config.shared_reset).strip
+    t('tooltips.shared_cases', shared_reset: Config.shared_reset_days).strip
   end
 
   def unconfirmed_support_help_text

app/models/concerns/shareable.rb

@@ -5,7 +5,7 @@ module Shareable
   def still_shared?
     # Verify that a pregnancy has not been shared in the past six days
     return false if resolved_without_fund
-    recent = versions.where('created_at > ?', Config.shared_reset.days.ago)
+    recent = versions.where('created_at > ?', Config.shared_reset_days.days.ago)
     return true if recent.any?(&:marked_shared?)
     false
   end

app/models/config.rb

@@ -25,6 +25,30 @@ class Config < ApplicationRecord
     time_zone: "Time zone to use for displaying dates. Default is Eastern. Valid options are Eastern, Central, Mountain, Pacific, Alaska, Hawaii, Arizona, Indiana (East), or Puerto Rico."
   }.freeze
 
+  DEFAULTS = {
+    insurance: nil,
+    external_pledge_source: nil,
+    pledge_limit_help_text: nil,
+    language: nil,
+    resources_url: nil,
+    practical_support_guidance_url: nil,
+    fax_service: nil,
+    referred_by: nil,
+    practical_support: nil,
+    hide_practical_support: false,
+    start_of_week: 'monday',
+    budget_bar_max: 1_000,
+    voicemail: 12,
+    days_to_keep_fulfilled_patients: 90,
+    days_to_keep_all_patients: 365,
+    shared_reset_days: 6,
+    hide_budget_bar: false,
+    aggregate_statistics: false,
+    hide_standard_dropdown_values: false,
+    county: nil,
+    time_zone: "Eastern"
+  }.freeze
+
   enum config_key: {
     insurance: 0,
     external_pledge_source: 1,
@@ -141,7 +165,7 @@ def self.config_to_bool(key)
 
   def self.budget_bar_max
     budget_max = Config.find_or_create_by(config_key: 'budget_bar_max').options.try :last
-    budget_max ||= 1_000
+    budget_max ||= DEFAULTS['budget_bar_max']
     budget_max.to_i
   end
 
@@ -151,34 +175,34 @@ def self.hide_practical_support?
 
   def self.start_day
     start = Config.find_or_create_by(config_key: 'start_of_week').options.try :last
-    start ||= "monday"
+    start ||= DEFAULTS['start_of_week']
     start.downcase.to_sym
   end
 
   def self.time_zone
     tz = Config.find_or_create_by(config_key: 'time_zone').options.try :last
-    tz ||= "Eastern"
+    tz ||= DEFAULTS['time_zone']
     ActiveSupport::TimeZone.new(TIME_ZONE[tz])
   end
 
   def self.archive_fulfilled_patients
     archive_days = Config.find_or_create_by(config_key: 'days_to_keep_fulfilled_patients').options.try :last
     # default 3 months
-    archive_days ||= 90
+    archive_days ||= DEFAULTS['days_to_keep_fulfilled_patients']
     archive_days.to_i
   end
 
   def self.archive_all_patients
     archive_days = Config.find_or_create_by(config_key: 'days_to_keep_all_patients').options.try :last
     # default 1 year
-    archive_days ||= 365
+    archive_days ||= DEFAULTS['days_to_keep_call_patients']
     archive_days.to_i
   end
 
-  def self.shared_reset
+  def self.shared_reset_days
     shared_reset_days = Config.find_or_create_by(config_key: 'shared_reset').options.try :last
     # default 6 days
-    shared_reset_days ||= 6
+    shared_reset_days ||= DEFAULTS['shared_reset']
     shared_reset_days.to_i
   end
 
@@ -225,10 +249,11 @@ def validate_config
 
       # run the validators and get a boolean, exit if all are true
       # (see comment above in `clean_config_value` for an explainer)
-      return if validators.all? { |validator| method(validator).call }
+      validation_errors = validators.map { |validator| method(validator).call }
+      return if validation_errors.all? { |validation_error| validation_error.nil? }
 
       errors.add(:invalid_value_for,
-        "#{config_key.humanize(capitalize: false)}: #{options.join(', ')}")
+        "#{config_key.humanize(capitalize: false)}: #{validation_errors.compact.join(', ')}")
     end
 
     # generic cleaner for words (so we have standardized capitalization)
@@ -243,12 +268,16 @@ def titleize_capitalization
 
     # generic validator for numerics
     def validate_number
-      options.last =~ /\A\d+\z/
+      if options.last !=~ /\A\d+\z/
+        'Must be number'
+      end
     end
 
     # validator for singletons (no lists allowed)
     def validate_singleton
-      options.length == 1
+      if options.length != 1
+        'No lists allowed'
+      end
     end
 
     ### URL fields
@@ -262,7 +291,7 @@ def validate_url
       url = UriService.new(maybe_url).uri
 
       # uriservice returns nil if there's a problem.
-      return false if !url
+      return false unless url
 
 
       config_value['options'] = [url]
@@ -283,7 +312,9 @@ def validate_url
     ].freeze
 
     def validate_start_of_week
-      START_OF_WEEK.include?(options.last.capitalize)
+      unless START_OF_WEEK.include?(options.last.capitalize)
+        "Must be a day of the week (or the word monthly)"
+      end
     end
 
     ### Time zone
@@ -301,38 +332,45 @@ def validate_start_of_week
     }.stringify_keys!
 
     def validate_time_zone
-      TIME_ZONE.keys.include?(options.last.titleize)
+      unless TIME_ZONE.keys.include?(options.last.titleize)
+        "Timezone provided is not supported"
+      end
     end
 
     ### Practical support
 
     def validate_yes_or_no
       # allow yes or no, to be nice (technically only yes is considered)
-      options.last =~ /\A(yes|no)\z/i
+      if !options.last =~ /\A(yes|no)\z/i
+        "Field must be either 'yes' or 'no'"
+      end
     end
 
     ### Patient archive
     ARCHIVE_MIN_DAYS = 60   # 2 months
     ARCHIVE_MAX_DAYS = 550  # 1.5 years
 
     def validate_patient_archive
-      validate_number && options.last.to_i.between?(ARCHIVE_MIN_DAYS, ARCHIVE_MAX_DAYS)
+      if !validate_number || !options.last.to_i.between?(ARCHIVE_MIN_DAYS, ARCHIVE_MAX_DAYS)
+        "Must be between #{ARCHIVE_MIN_DAYS} and #{ARCHIVE_MAX_DAYS} days."
+      end
     end
 
     ### shared reset
     SHARED_MIN_DAYS = 2      # 2 days
     SHARED_MAX_DAYS = 7 * 6  # 6 weeks
 
     def validate_shared_reset
-      validate_number && options.last.to_i.between?(SHARED_MIN_DAYS, SHARED_MAX_DAYS)
+      if !validate_number || !options.last.to_i.between?(SHARED_MIN_DAYS, SHARED_MAX_DAYS)
+        "Must be between #{SHARED_MIN_DAYS} and #{SHARED_MAX_DAYS} days."
+      end
     end
 
     def validate_length
       total_length = 0
       options.each do |option|
         total_length += option.length
-        return false if total_length > 4000
       end
-      true
+      "Length of provided values is too long (over 4000 characters)" if total_length > 4000
     end
 end

test/models/config_test.rb

@@ -199,8 +199,8 @@ class ConfigTest < ActiveSupport::TestCase
 
     describe 'archive_patients' do
       it 'should return proper defaults' do
-        assert_equal 90, Config.archive_fulfilled_patients
-        assert_equal 365, Config.archive_all_patients
+        assert_equal Config::DEFAULTS['days_to_keep_fulfilled_patients'], Config.archive_fulfilled_patients
+        assert_equal Config::DEFAULTS['days_to_keep_all_patients'], Config.archive_all_patients
       end
 
       it 'should validate bounds' do
@@ -293,9 +293,9 @@ class ConfigTest < ActiveSupport::TestCase
       end
     end
 
-    describe 'shared_reset' do
+    describe 'shared_reset_days' do
       it 'should return proper default' do
-        assert_equal 6, Config.shared_reset
+        assert_equal Config::DEFAULTS['shared_reset_days'], Config.shared_reset_days
       end
 
       it 'should validate bounds' do