adapt the json_validator from activerecord_json_validator

The error messages need to be more fine grained than what can be achieved with the original gem

Gemfile

@@ -97,8 +97,8 @@ gem 'svg-graph', '~> 2.2.0'
 
 gem 'date_validator', '~> 0.12.0'
 gem 'email_validator', '~> 2.2.3'
+gem 'json_schemer', '~> 0.2.18'
 gem 'ruby-duration', '~> 3.2.0'
-gem 'activerecord_json_validator', '~> 2.0.0'
 
 # provide compatible filesystem information for available storage
 gem 'sys-filesystem', '~> 1.4.0', require: false

Gemfile.lock

@@ -246,9 +246,6 @@ GEM
       multi_json (~> 1.11, >= 1.11.2)
       rack (>= 2.0.8, < 3)
       railties (>= 5.2.4.1)
-    activerecord_json_validator (2.0.1)
-      activerecord (>= 4.2.0, < 7)
-      json_schemer (~> 0.2.18)
     activestorage (6.1.4.1)
       actionpack (= 6.1.4.1)
       activejob (= 6.1.4.1)
@@ -973,7 +970,6 @@ DEPENDENCIES
   activerecord-import (~> 1.2.0)
   activerecord-nulldb-adapter (~> 0.7.0)
   activerecord-session_store (~> 2.0.0)
-  activerecord_json_validator (~> 2.0.0)
   acts_as_list (~> 1.0.1)
   acts_as_tree (~> 2.9.0)
   addressable (~> 2.8.0)
@@ -1023,6 +1019,7 @@ DEPENDENCIES
   html-pipeline (~> 2.14.0)
   htmldiff
   i18n-js (~> 3.9.0)
+  json_schemer (~> 0.2.18)
   json_spec (~> 1.1.4)
   ladle
   launchy (~> 2.5.0)

app/contracts/user_preferences/update_contract.rb

@@ -32,15 +32,16 @@ module UserPreferences
   class UpdateContract < BaseContract
     property :settings
 
-    USER_PREFERENCE_SCHEMA = Rails.root.join('config/schemas/api/user_preferences.schema.json')
-
     validates :settings,
-              presence: true,
+              exclusion: { in: [{}] },
               json: {
                 message: ->(errors) do
                   errors.map { |error| JSONSchemer::Errors.pretty(error) }
                 end,
-                schema: USER_PREFERENCE_SCHEMA
+                schema: ->(*) {
+                  UserPreferences::Schema.schema
+                },
+                if: -> { model.settings.present? }
               }
   end
 end

app/models/user_preference.rb

@@ -41,6 +41,8 @@ class UserPreference < ApplicationRecord
   # as boolean with ? suffix
   def method_missing(method_name, *args)
     key = method_name.to_s
+    return super unless supported_settings_method?(key)
+
     action = key[-1]
 
     case action
@@ -57,7 +59,7 @@ class UserPreference < ApplicationRecord
   # We respond to all methods as we retrieve
   # the key from settings
   def respond_to_missing?(method_name, include_private = false)
-    UserPreferences::Schema.properties.include?(method_name.to_s.gsub(/\?|=\z/, '')) || super
+    supported_settings_method?(method_name) || super
   end
 
   def [](attr_name)
@@ -138,4 +140,8 @@ class UserPreference < ApplicationRecord
   def time_zone_correctness
     errors.add(:time_zone, :inclusion) if time_zone.present? && canonical_time_zone.nil?
   end
+
+  def supported_settings_method?(method_name)
+    UserPreferences::Schema.properties.include?(method_name.to_s.gsub(/\?|=\z/, ''))
+  end
 end

app/models/user_preferences/schema.rb

@@ -32,8 +32,10 @@ class UserPreferences::Schema
   class << self
     PATH = Rails.root.join('config/schemas/api/user_preferences.schema.json')
 
+    # TODO: check if Time zones can be verified within the schema
+
     def schema
-      @schema ||= JSON::load(PATH)
+      @schema ||= JSON::parse(File.read(PATH))
     end
 
     def properties

app/validators/json_validator.rb

@@ -0,0 +1,134 @@
+# The code in here, with the exception of the error handling has been copied from the
+# activerecord_json_validator gem.
+
+# frozen_string_literal: true
+
+class JsonValidator < ActiveModel::EachValidator
+  def initialize(options)
+    options.reverse_merge!(schema: nil)
+    options.reverse_merge!(options: {})
+    @attributes = options[:attributes]
+
+    super
+
+    inject_setter_method(options[:class], @attributes)
+  end
+
+  # Validate the JSON value with a JSON schema path or String
+  def validate_each(record, attribute, value)
+    # Validate value with JSON Schemer
+    errors = JSONSchemer.schema(schema(record), **options.fetch(:options)).validate(value).to_a
+
+    # Everything is good if we don’t have any errors and we got valid JSON value
+    return if errors.empty? && record.send(:"#{attribute}_invalid_json").blank?
+
+    # Add error message to the attribute
+    errors.each do |error|
+      add_error(record, error)
+    end
+  end
+
+  protected
+
+  # Redefine the setter method for the attributes, since we want to
+  # catch JSON parsing errors.
+  def inject_setter_method(klass, attributes)
+    attributes.each do |attribute|
+      # rubocop:disable Style/DocumentDynamicEvalDefinition
+      klass.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+        attr_reader :"#{attribute}_invalid_json"
+        define_method "#{attribute}=" do |args|
+          begin
+            @#{attribute}_invalid_json = nil
+            args = ::ActiveSupport::JSON.decode(args) if args.is_a?(::String)
+            super(args)
+          rescue ActiveSupport::JSON.parse_error
+            @#{attribute}_invalid_json = args
+            super({})
+          end
+        end
+      RUBY
+      # rubocop:enable Style/DocumentDynamicEvalDefinition
+    end
+  end
+
+  # Return a valid schema, recursively calling
+  # itself until it gets a non-Proc/non-Symbol value.
+  def schema(record, schema = nil)
+    schema ||= options.fetch(:schema)
+
+    case schema
+    when Proc then schema(record, record.instance_exec(&schema))
+    when Symbol then schema(record, record.send(schema))
+    else schema
+    end
+  end
+
+  def add_error(record, error)
+    data_pointer, type, schema = error.values_at('data_pointer', 'type', 'schema')
+    path = data_pointer.split('/', 3)[1..]
+
+    case type
+    when 'required'
+      add_blank_error(record, error, path)
+    when 'null', 'string', 'boolean', 'integer', 'number', 'array', 'object'
+      add_type_mismatch_error(record, path, type)
+    when 'schema'
+      add_schema_violated_error(record, path)
+    when 'format'
+      add_format_error(record, path, schema.fetch('format'))
+    when 'enum'
+      add_enum_error(record, path)
+    else
+      add_invalid_error(record, path)
+    end
+  end
+
+  def add_blank_error(record, error, path)
+    keys = error.dig('details', 'missing_keys')
+
+    keys.each do |key|
+      if path.nil?
+        record.errors.add(key, :blank)
+      else
+        record.errors.add(path[0], :blank_nested, property: (path[1..] + [key]).join('/'))
+      end
+    end
+  end
+
+  def add_type_mismatch_error(record, path, type)
+    if path.length == 1
+      record.errors.add(path[0], :type_mismatch, type: type)
+    else
+      record.errors.add(path[0], :type_mismatch_nested, type: type, path: path[1])
+    end
+  end
+
+  def add_schema_violated_error(record, path)
+    if path.length == 1
+      record.errors.add(path[0], :unknown_property)
+    else
+      record.errors.add(path[0], :unknown_property_nested, path: path[1])
+    end
+  end
+
+  def add_format_error(record, path, expected)
+    if path.length == 1
+      record.errors.add(path[0], :format, expected: expected)
+    else
+      record.errors.add(path[0], :format_nested, expected: expected, path: path[1])
+    end
+  end
+
+  def add_enum_error(record, path)
+    if path.length == 1
+      record.errors.add(path[0], :inclusion)
+    else
+      record.errors.add(path[0], :inclusion_nested, path: path[1])
+    end
+  end
+
+  def add_invalid_error(record, path)
+    record.errors.add(path[0], :invalid)
+  end
+end

config/locales/en.yml

@@ -594,6 +594,7 @@ en:
         before: "must be before %{date}."
         before_or_equal_to: "must be before or equal to %{date}."
         blank: "can't be blank."
+        blank_nested: "needs to have the property '%{property}' set."
         cant_link_a_work_package_with_a_descendant: "A work package cannot be linked to one of its subtasks."
         circular_dependency: "This relation would create a circular dependency."
         confirmation: "doesn't match %{attribute}."
@@ -607,11 +608,14 @@ en:
         even: "must be even."
         exclusion: "is reserved."
         file_too_large: "is too large (maximum size is %{count} Bytes)."
+        format: "does not match the expected format '%{expected}'."
+        format_nested: "does not match the expected format '%{expected}' at path '%{path}'."
         greater_than: "must be greater than %{count}."
         greater_than_or_equal_to: "must be greater than or equal to %{count}."
         greater_than_or_equal_to_start_date: "must be greater than or equal to the start date."
         greater_than_start_date: "must be greater than the start date."
         inclusion: "is not set to one of the allowed values."
+        inclusion_nested: "is not set to one of the allowed values at path '%{path}'."
         invalid: "is invalid."
         invalid_url: 'is not a valid URL.'
         invalid_url_scheme: 'is not a supported protocol (allowed: %{allowed_schemes}).'
@@ -632,7 +636,11 @@ en:
         taken: "has already been taken."
         too_long: "is too long (maximum is %{count} characters)."
         too_short: "is too short (minimum is %{count} characters)."
+        type_mismatch: "is not of type '%{type}'"
+        type_mismatch_nested: "is not of type '%{type}' at path '%{path}'"
         unchangeable: "cannot be changed."
+        unknown_property: "is not a known property."
+        unknown_property_nested: "has the unknown path '%{path}'."
         unremovable: "cannot be removed."
         wrong_length: "is the wrong length (should be %{count} characters)."
       models:

config/schemas/api/user_preferences.schema.json

@@ -16,14 +16,12 @@
                     "type": "boolean"
                 },
                 "comments_sorting": {
-                    "type": "string"
+                    "type": "string",
+                    "enum": ["asc", "desc"]
                 },
                 "auto_hide_popups": {
                     "type": "boolean"
                 },
-                "self_notified": {
-                    "type": "boolean"
-                },
                 "daily_reminders": {
                     "$ref": "#/definitions/DailyReminders"
                 }
@@ -33,11 +31,10 @@
                 "comments_sorting",
                 "daily_reminders",
                 "hide_mail",
-                "self_notified",
                 "time_zone",
                 "warn_on_leaving_unsaved"
             ],
-            "title": "Welcome"
+            "title": "UserPreferences"
         },
         "DailyReminders": {
             "type": "object",

db/migrate/20210825183540_make_user_preferences_json.rb

@@ -3,6 +3,8 @@ require_relative './migration_utils/utils'
 class MakeUserPreferencesJson < ActiveRecord::Migration[6.1]
   include ::Migration::Utils
 
+  # TODO: remove self_notified and switch type of warn_on_leaving_unsaved in DB to boolean (currently is 1/0)
+
   class UserPreferenceWithOthers < ::UserPreference
     self.table_name = 'user_preferences'
     serialize :others, Hash

spec/contracts/user_preferences/update_contract_spec.rb

@@ -35,7 +35,24 @@ describe UserPreferences::UpdateContract do
 
   let(:current_user) { FactoryBot.build_stubbed(:user) }
   let(:user) { FactoryBot.build_stubbed :user }
-  let(:user_preference) { FactoryBot.build_stubbed(:user_preference, user: user) }
+  let(:user_preference) do
+    FactoryBot.build_stubbed(:user_preference,
+                             user: user,
+                             settings: settings&.with_indifferent_access)
+  end
+  let(:settings) do
+    {
+      hide_mail: true,
+      auto_hide_popups: true,
+      comments_sorting: 'desc',
+      daily_reminders: {
+        enabled: true,
+        times: %w[08:00:00+00:00 12:00:00+00:00]
+      },
+      time_zone: 'Brasilia',
+      warn_on_leaving_unsaved: true
+    }
+  end
   let(:contract) { described_class.new(user_preference, current_user) }
 
   context 'when current_user is admin' do
@@ -78,4 +95,154 @@ describe UserPreferences::UpdateContract do
   context 'when current_user is a regular user' do
     it_behaves_like 'contract user is unauthorized'
   end
+
+  context 'when the settings are nil' do
+    let(:settings) { nil }
+
+    it_behaves_like 'contract is invalid', settings: :exclusion
+  end
+
+  context 'without a hide_mail value' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          enabled: true,
+          times: %w[08:00:00+00:00 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', hide_mail: :blank
+  end
+
+  context 'with a string for hide_mail' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          enabled: true,
+          times: %w[08:00:00+00:00 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: 'yes please'
+      }
+    end
+
+    it_behaves_like 'contract is invalid', hide_mail: :type_mismatch
+  end
+
+  context 'with a field within the daily_reminders having the wrong type' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          enabled: 'sure',
+          times: %w[08:00:00+00:00 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', daily_reminders: :type_mismatch_nested
+  end
+
+  context 'with a field within the daily_reminders missing' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          times: %w[08:00:00+00:00 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', daily_reminders: :blank_nested
+  end
+
+  context 'with an extra property' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          enabled: true,
+          times: %w[08:00:00+00:00 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: true,
+        foo: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', foo: :unknown_property
+  end
+
+  context 'with an extra property within the daily_reminders' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          enabled: true,
+          times: %w[08:00:00+00:00 12:00:00+00:00],
+          foo: true
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', daily_reminders: :unknown_property_nested
+  end
+
+  context 'with an invalid time for the daily_reminders' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'desc',
+        daily_reminders: {
+          enabled: true,
+          times: %w[abc 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', daily_reminders: :format_nested
+  end
+
+  context 'with an invalid order for comments_sorting' do
+    let(:settings) do
+      {
+        auto_hide_popups: true,
+        comments_sorting: 'up',
+        daily_reminders: {
+          enabled: true,
+          times: %w[08:00:00+00:00 12:00:00+00:00]
+        },
+        time_zone: 'Brasilia',
+        warn_on_leaving_unsaved: true,
+        hide_mail: true
+      }
+    end
+
+    it_behaves_like 'contract is invalid', comments_sorting: :inclusion
+  end
 end

spec/models/user_preference_spec.rb

@@ -82,6 +82,15 @@ describe UserPreference do
     end
   end
 
+  describe 'an unsupported method' do
+    context 'for created_at (key not in the schema)' do
+      it 'raises an error' do
+        expect { preference.created_at }
+          .to raise_error NoMethodError
+      end
+    end
+  end
+
   describe 'sort order' do
     it_behaves_like 'accepts real and false booleans',
                     :comments_in_reverse_order=,