replaces reform & custom error handling

Replaces reform by a simple layer of Disposable +
ActiveModel::Validations for contracts.

Additionally removes the custom error handling where OpenProject added
symbols to keep the ability to identify faulty attributes while having
non standard format (deviates from `%{attribute} %{message}`).

Changes to active record now allow us to define the format of a message
on i18n level, e.g. `%{message}`. Therefore the patching can be removed.

Reform plans to remove support for ActiveModel::Validations in version
4.0 at the latest but even today, support for it is hapazard. As we do
not need the full stack of Reform anyway, we can solely rely on
Disposable.
pull/8590/head
ulferts 4 years ago committed by Oliver Günther
parent 5dcf92f61e
commit e2961fba14
  1. 8
      Gemfile
  2. 9
      Gemfile.lock
  3. 6
      app/contracts/attribute_help_texts/base_contract.rb
  4. 6
      app/contracts/custom_actions/execute_contract.rb
  5. 6
      app/contracts/delete_contract.rb
  6. 7
      app/contracts/groups/base_contract.rb
  7. 8
      app/contracts/members/base_contract.rb
  8. 62
      app/contracts/model_contract.rb
  9. 6
      app/contracts/oauth/application_contract.rb
  10. 8
      app/contracts/projects/archive_contract.rb
  11. 6
      app/contracts/projects/base_contract.rb
  12. 6
      app/contracts/projects/instantiate_template_contract.rb
  13. 8
      app/contracts/projects/unarchive_contract.rb
  14. 7
      app/contracts/queries/base_contract.rb
  15. 5
      app/contracts/queries/update_contract.rb
  16. 1
      app/contracts/relations/base_contract.rb
  17. 1
      app/contracts/relations/create_contract.rb
  18. 13
      app/contracts/roles/base_contract.rb
  19. 6
      app/contracts/roles/create_contract.rb
  20. 6
      app/contracts/users/base_contract.rb
  21. 6
      app/contracts/users/update_contract.rb
  22. 10
      app/contracts/versions/base_contract.rb
  23. 6
      app/contracts/versions/delete_contract.rb
  24. 6
      app/contracts/work_packages/create_contract.rb
  25. 6
      app/helpers/error_message_helper.rb
  26. 4
      app/models/custom_actions/actions/assigned_to.rb
  27. 8
      app/models/custom_actions/actions/base.rb
  28. 14
      app/models/custom_actions/actions/strategies/validate_in_range.rb
  29. 4
      app/models/custom_actions/validate_allowed_value.rb
  30. 2
      app/models/member.rb
  31. 7
      app/models/query.rb
  32. 1
      app/models/version.rb
  33. 8
      config/application.rb
  34. 18
      config/locales/en.yml
  35. 7
      lib/api/errors/error_base.rb
  36. 6
      lib/api/utilities/endpoints/modify.rb
  37. 2
      lib/api/v3/custom_actions/custom_actions_api.rb
  38. 78
      lib/open_project/patches/active_model_errors.rb
  39. 79
      lib/open_project/patches/reform.rb
  40. 1
      lib/plugins/acts_as_customizable/init.rb
  41. 7
      lib/plugins/acts_as_customizable/lib/acts_as_customizable.rb
  42. 49
      lib/plugins/acts_as_customizable/lib/human_attribute_name.rb
  43. 30
      modules/backlogs/spec/contracts/work_packages/update_contract_spec.rb
  44. 6
      modules/bim/app/contracts/bim/bcf/concerns/manage_bcf_guarded.rb
  45. 14
      modules/bim/app/contracts/bim/ifc_models/base_contract.rb
  46. 6
      modules/bim/app/representers/bim/bcf/api/v2_1/errors/error_mapper.rb
  47. 4
      modules/bim/spec/contracts/bcf/issues/create_contract_spec.rb
  48. 29
      modules/boards/spec/requests/api/v3/grids/grids_resource_spec.rb
  49. 10
      modules/costs/app/contracts/time_entries/base_contract.rb
  50. 8
      modules/costs/app/contracts/time_entries/create_contract.rb
  51. 6
      modules/costs/app/contracts/time_entries/update_contract.rb
  52. 19
      modules/grids/app/contracts/grids/base_contract.rb
  53. 6
      modules/grids/app/contracts/grids/delete_contract.rb
  54. 29
      modules/my_page/spec/requests/api/v3/grids/grids_resource_spec.rb
  55. 8
      spec/models/custom_actions/actions/assigned_to_spec.rb

@ -285,13 +285,13 @@ gem 'bootsnap', '~> 1.4.5', require: false
# API gems
gem 'grape', '~> 1.3.0'
gem 'roar', '~> 1.1.0'
# CORS for API
gem 'rack-cors', '~> 1.1.1'
gem 'reform', '~> 2.2.0'
gem 'reform-rails', '~> 0.1.7'
gem 'roar', '~> 1.1.0'
# Required for contracts
gem 'disposable', '~> 0.4.7'
platforms :mri, :mingw, :x64_mingw do
group :postgres do
@ -302,7 +302,7 @@ platforms :mri, :mingw, :x64_mingw do
gem 'activerecord-nulldb-adapter', '~> 0.4.0'
# Have application level locks on the database to have a mutex shared between workers/hosts.
# We e.g. emply this to safeguard the creation of journals.
# We e.g. employ this to safeguard the creation of journals.
gem 'with_advisory_lock', '~> 4.6.0'
end

@ -792,12 +792,6 @@ GEM
recaptcha (5.5.0)
json
redcarpet (3.5.0)
reform (2.2.4)
disposable (>= 0.4.1)
representable (>= 2.4.0, < 3.1.0)
reform-rails (0.1.7)
activemodel (>= 3.2)
reform (>= 2.2.0)
regexp_parser (1.7.0)
representable (3.0.4)
declarative (< 0.1.0)
@ -1023,6 +1017,7 @@ DEPENDENCIES
deckar01-task_list (~> 2.3.1)
delayed_cron_job (~> 0.7.2)
delayed_job_active_record (~> 4.1.4)
disposable (~> 0.4.7)
doorkeeper (~> 5.3.1)
equivalent-xml (~> 0.6)
escape_utils (~> 1.0)
@ -1104,8 +1099,6 @@ DEPENDENCIES
rails-i18n (~> 6.0.0)
rails_12factor
rdoc (>= 2.4.2)
reform (~> 2.2.0)
reform-rails (~> 0.1.7)
reporting_engine!
request_store (~> 1.5.0)
responders (~> 3.0)

@ -32,11 +32,7 @@ module AttributeHelpTexts
class BaseContract < ::ModelContract
include Attachments::ValidateReplacements
def validate
validate_user_allowed_to_manage
super
end
validate :validate_user_allowed_to_manage
def self.model
AttributeHelpText

@ -28,10 +28,8 @@
# See docs/COPYRIGHT.rdoc for more details.
#++
require 'model_contract'
module CustomActions
class ExecuteContract < Reform::Contract
class ExecuteContract < ModelContract
property :lock_version
property :work_package_id
@ -43,7 +41,7 @@ module CustomActions
def work_package_visible
return unless model.work_package_id
unless WorkPackage.visible.where(id: model.work_package_id).exists?
unless WorkPackage.visible(user).where(id: model.work_package_id).exists?
errors.add(:work_package_id, :does_not_exist)
end
end

@ -37,11 +37,7 @@ class DeleteContract < ModelContract
end
end
def validate
user_allowed
super
end
validate :user_allowed
def user_allowed
unless authorized?

@ -28,12 +28,7 @@
module Groups
class BaseContract < ::ModelContract
def validate
user_allowed_to_manage
super
end
validate :user_allowed_to_manage
def user_allowed_to_manage
unless user.admin?

@ -35,12 +35,8 @@ module Members
attribute :roles
def validate
user_allowed_to_manage
roles_grantable
super
end
validate :user_allowed_to_manage
validate :roles_grantable
def user_allowed_to_manage
if model.project && !user.allowed_to?(:manage_members, model.project)

@ -28,10 +28,24 @@
# See docs/COPYRIGHT.rdoc for more details.
#++
require 'reform'
require 'reform/form/active_model/model_validations'
class ModelContract < Disposable::Twin
require "disposable/twin/composition" # Expose.
include Expose
feature Setup
feature Setup::SkipSetter
feature Default
include ActiveModel::Validations
extend ActiveModel::Naming
extend ActiveModel::Translation
delegate :id,
to: :model
# Allows human_attribute_name to resolve custom fields correctly
extend Redmine::Acts::Customizable::HumanAttributeName
class ModelContract < Reform::Contract
class << self
def writable_attributes
@writable_attributes ||= []
@ -41,10 +55,6 @@ class ModelContract < Reform::Contract
@writable_conditions ||= []
end
def attribute_validations
@attribute_validations ||= []
end
def attribute_permissions
@attribute_permissions ||= {}
end
@ -59,15 +69,25 @@ class ModelContract < Reform::Contract
attribute_aliases[db] = outside
end
def property(name, options = {}, &block)
if twin = options.delete(:form)
options[:twin] = twin
end
if validates_options = options[:validates]
validates name, validates_options
end
super
end
def attribute(attribute, options = {}, &block)
property attribute
add_writable(attribute, options[:writeable])
attribute_permission(attribute, options[:permission])
if block
attribute_validations << block
end
validate(attribute, &block) if block
end
def default_attribute_permission(permission)
@ -126,11 +146,9 @@ class ModelContract < Reform::Contract
writable_attributes.include?(attribute.to_s)
end
def validate
def validate(*args)
super()
readonly_attributes_unchanged
run_attribute_validations
super
# Allow subclasses to check only contract errors
return errors.empty? unless validate_model?
@ -141,7 +159,7 @@ class ModelContract < Reform::Contract
# order to have them available at one place.
# This is something we need as long as we have validations split
# among the model and its contract.
errors.merge!(model.errors, [])
errors.merge!(model.errors)
errors.empty?
end
@ -154,7 +172,11 @@ class ModelContract < Reform::Contract
end
def self.model
@model ||= name.deconstantize.singularize.constantize
@model ||= begin
name.deconstantize.singularize.constantize
rescue NameError
ActiveRecord::Base
end
end
# use activerecord as the base scope instead of 'activemodel' to be compatible
@ -198,14 +220,6 @@ class ModelContract < Reform::Contract
changed
end
def run_attribute_validations
attribute_validations.each { |validation| instance_exec(&validation) }
end
def attribute_validations
collect_ancestor_attributes(:attribute_validations)
end
def collect_ancestor_attribute_aliases
@ancestor_attribute_aliases ||= collect_ancestor_attributes(:attribute_aliases)
end

@ -34,11 +34,7 @@ module OAuth
::Doorkeeper::Application
end
def validate
validate_client_credential_user
super
end
validate :validate_client_credential_user
attribute :name
attribute :redirect_uri

@ -30,12 +30,8 @@ module Projects
class ArchiveContract < ModelContract
include Projects::Archiver
def validate
validate_admin_only
validate_no_foreign_wp_references
super
end
validate :validate_admin_only
validate :validate_no_foreign_wp_references
protected

@ -52,11 +52,7 @@ module Projects
validate_templated_set_by_admin
end
def validate
validate_user_allowed_to_manage
super
end
validate :validate_user_allowed_to_manage
def assignable_parents
Project

@ -34,11 +34,7 @@ module Projects
.where(templated: true)
end
def validate
validate_user_allowed_to_instantiate_template
super
end
validate :validate_user_allowed_to_instantiate_template
private

@ -30,12 +30,8 @@ module Projects
class UnarchiveContract < ModelContract
include Projects::Archiver
def validate
validate_admin_only
validate_all_ancestors_active
super
end
validate :validate_admin_only
validate :validate_all_ancestors_active
protected

@ -58,11 +58,8 @@ module Queries
Query
end
def validate
validate_project
user_allowed_to_make_public
super
end
validate :validate_project
validate :user_allowed_to_make_public
def validate_project
errors.add :project, :error_not_found if project_id.present? && !project_visible?

@ -31,10 +31,7 @@ require 'queries/base_contract'
module Queries
class UpdateContract < BaseContract
def validate
user_allowed_to_change
super
end
validate :user_allowed_to_change
##
# Check if the current user may save the changes

@ -51,6 +51,7 @@ module Relations
def validate!(*args)
# same as before_validation callback
model.send(:reverse_if_needed)
super
end

@ -1,4 +1,5 @@
#-- encoding: UTF-8
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2020 the OpenProject GmbH

@ -31,11 +31,7 @@ module Roles
attribute :name
attribute :assignable
def validate
check_permission_prerequisites
super
end
validate :check_permission_prerequisites
def assignable_permissions
if model.is_a?(GlobalRole)
@ -83,10 +79,9 @@ module Roles
def add_unmet_dependency_error(selected, unmet)
errors.add(:permissions,
I18n.t(:'activerecord.errors.models.role.permissions.dependency_missing',
permission: I18n.t("permission_#{selected}"),
dependency: I18n.t("permission_#{unmet}")),
error_symbol: :dependency_missing)
:dependency_missing,
permission: I18n.t("permission_#{selected}"),
dependency: I18n.t("permission_#{unmet}"))
end
end
end

@ -30,11 +30,7 @@ module Roles
class CreateContract < BaseContract
attribute :type
def validate
type_in_allowed
super
end
validate :type_in_allowed
private

@ -48,11 +48,7 @@ module Users
User
end
def validate
existing_auth_source
super
end
validate :existing_auth_source
private

@ -32,11 +32,7 @@ require 'users/base_contract'
module Users
class UpdateContract < BaseContract
def validate
user_allowed_to_update
super
end
validate :user_allowed_to_update
private

@ -38,13 +38,9 @@ module Versions
:new_record?,
to: :model
def validate
user_allowed_to_manage
validate_project_is_set
validate_sharing_included
super
end
validate :user_allowed_to_manage
validate :validate_project_is_set
validate :validate_sharing_included
attribute :name
attribute :description

@ -30,11 +30,7 @@ module Versions
class DeleteContract < ::DeleteContract
delete_permission :manage_versions
def validate
validate_no_work_packages_attached
super
end
validate :validate_no_work_packages_attached
protected

@ -39,11 +39,7 @@ module WorkPackages
default_attribute_permission :add_work_packages
def validate
user_allowed_to_add
super
end
validate :user_allowed_to_add
private

@ -45,11 +45,7 @@ module ErrorMessageHelper
error_messages = errors.full_messages
errors.details.each do |attribute, details|
details.each do |error|
object.errors.add(attribute, error[:error], **error.except(:error))
end
end
object.errors.merge!(errors)
render_error_messages_partial(error_messages, object: object)
end

@ -91,8 +91,8 @@ class CustomActions::Actions::AssignedTo < CustomActions::Actions::Base
def validate_me_value(errors)
if has_me_value? && !User.current.logged?
errors.add :actions,
I18n.t(:'activerecord.errors.models.custom_actions.not_logged_in', name: human_name),
error_symbol: :not_logged_in
:not_logged_in,
name: human_name
end
end

@ -97,16 +97,16 @@ class CustomActions::Actions::Base
def validate_value_required(errors)
if required? && values.empty?
errors.add :actions,
I18n.t(:'activerecord.errors.models.custom_actions.empty', name: human_name),
error_symbol: :empty
:empty,
name: human_name
end
end
def validate_only_one_value(errors)
if !multi_value? && values.length > 1
errors.add :actions,
I18n.t(:'activerecord.errors.models.custom_actions.only_one_allowed', name: human_name),
error_symbol: :only_one_allowed
:only_one_allowed,
name: human_name
end
end
end

@ -52,20 +52,18 @@ module CustomActions::Actions::Strategies::ValidateInRange
def validate_smaller_than_maximum(errors)
if maximum && values[0] > maximum
errors.add :actions,
I18n.t(:'activerecord.errors.models.custom_actions.smaller_than_or_equal_to',
name: human_name,
count: maximum),
error_symbol: :smaller_than_or_equal_to
:smaller_than_or_equal_to,
name: human_name,
count: maximum
end
end
def validate_greater_than_minimum(errors)
if minimum && values[0] < minimum
errors.add :actions,
I18n.t(:'activerecord.errors.models.custom_actions.greater_than_or_equal_to',
name: human_name,
count: minimum),
error_symbol: :greater_than_or_equal_to
:greater_than_or_equal_to,
name: human_name,
count: minimum
end
end
end

@ -28,8 +28,8 @@ module CustomActions::ValidateAllowedValue
allowed_ids = allowed_values.map { |v| v[:value] }
if values.to_set != (allowed_ids & values).to_set
errors.add attribute,
I18n.t(:'activerecord.errors.models.custom_actions.inclusion', name: human_name),
error_symbol: :inclusion
:inclusion,
name: human_name
end
end
end

@ -30,7 +30,7 @@
class Member < ApplicationRecord
extend DeprecatedAlias
belongs_to :principal, foreign_key: 'user_id'
has_many :member_roles, dependent: :destroy, autosave: true
has_many :member_roles, dependent: :destroy, autosave: true, validate: false
has_many :roles, through: :member_roles
belongs_to :project

@ -122,7 +122,8 @@ class Query < ApplicationRecord
(column_names - available_names).each do |name|
errors.add :column_names,
I18n.t(:error_invalid_query_column, value: name)
:invalid,
value: name
end
end
@ -131,14 +132,14 @@ class Query < ApplicationRecord
sort_criteria.each do |name, _dir|
unless available_criteria.include? name.to_s
errors.add :sort_criteria, I18n.t(:error_invalid_sort_criterion, value: name)
errors.add :sort_criteria, :invalid, value: name
end
end
end
def validate_group_by
unless group_by.blank? || groupable_columns.map(&:name).map(&:to_s).include?(group_by.to_s)
errors.add :group_by, I18n.t(:error_invalid_group_by, value: group_by)
errors.add :group_by, :invalid, value: group_by
end
end

@ -46,7 +46,6 @@ class Version < ApplicationRecord
validates_format_of :effective_date, with: /\A\d{4}-\d{2}-\d{2}\z/, message: :not_a_date, allow_nil: true
validates_format_of :start_date, with: /\A\d{4}-\d{2}-\d{2}\z/, message: :not_a_date, allow_nil: true
validates_inclusion_of :status, in: VERSION_STATUSES
validates_inclusion_of :sharing, in: VERSION_SHARINGS
validate :validate_start_date_before_effective_date
scope_classes ::Versions::Scopes::OrderBySemverName

@ -126,6 +126,14 @@ module OpenProject
# Fall back to default locale
config.i18n.fallbacks = true
# Activate being able to specify the format in which full_message works.
# Doing this, it is e.g. possible to avoid having the format of '%{attribute} %{message}' which
# will always prepend the attribute name to the error message.
# The formats can then be specified using the `format:` key within the [local].yml file in every
# layer of activerecord.errors down to the individual leve of the message, e.g.
# activerecord.errors.models.project.attributes.types.format
config.active_model.i18n_customize_full_message = true
# Enable cascade key lookup for i18n
I18n.backend.class.send(:include, I18n::Backend::Cascade)

@ -611,6 +611,7 @@ en:
not_an_integer: "(%{name}) is not an integer."
smaller_than_or_equal_to: "(%{name}) must be smaller than or equal to %{count}."
greater_than_or_equal_to: "(%{name}) must be greater than or equal to %{count}."
format: "%{message}"
doorkeeper/application:
attributes:
redirect_uri:
@ -645,6 +646,15 @@ en:
error_not_found: "not found"
public:
error_unauthorized: "- The user has no permission to create public views."
group_by:
invalid: "Can't group by: %{value}"
format: "%{message}"
column_names:
invalid: "Invalid query column: %{value}"
format: "%{message}"
sort_criteria:
invalid: "Can't sort by column: %{value}"
format: "%{message}"
group_by_hierarchies_exclusive: "is mutually exclusive with group by '%{group_by}'. You cannot activate both."
filters:
custom_fields:
@ -667,8 +677,9 @@ en:
must_not_be_ssh: "must not be an SSH url."
no_directory: "is not a directory."
role:
permissions:
dependency_missing: "need to also include '%{dependency}' as '%{permission}' is selected."
attributes:
permissions:
dependency_missing: "need to also include '%{dependency}' as '%{permission}' is selected."
time_entry:
attributes:
hours:
@ -1113,9 +1124,6 @@ en:
error_failed_to_delete_entry: 'Failed to delete this entry.'
error_in_dependent: "Error attempting to alter dependent object: %{dependent_class} #%{related_id} - %{related_subject}: %{error}"
error_invalid_selected_value: "Invalid selected value."
error_invalid_group_by: "Can't group by: %{value}"
error_invalid_query_column: "Invalid query column: %{value}"
error_invalid_sort_criterion: "Can't sort by column: %{value}"
error_journal_attribute_not_present: "Journal does not contain attribute %{attribute}."
error_pdf_export_too_many_columns: "Too many columns selected for the PDF export. Please reduce the number of columns."
error_pdf_failed_to_export: "The PDF export could not be saved: %{error}"

@ -87,11 +87,12 @@ module API
errors.keys.each do |attribute|
api_attribute_name = ::API::Utilities::PropertyNameConverter.from_ar_name(attribute)
errors.symbols_and_messages_for(attribute).each do |symbol, full_message, _|
errors.symbols_and_messages_for(attribute).each do |symbol, message|
api_errors << if symbol == :error_readonly
::API::Errors::UnwritableProperty.new(api_attribute_name, full_message)
::API::Errors::UnwritableProperty.new(api_attribute_name, message)
else
::API::Errors::Validation.new(api_attribute_name, full_message)
::API::Errors::Validation.new(api_attribute_name, message)
end
end
end

@ -59,10 +59,8 @@ module API
errors = ActiveModel::Errors.new call.result
call.dependent_results.each do |dr|
dr.errors.keys.each do |field|
dr.errors.symbols_and_messages_for(field).each do |symbol, full_message, _|
errors.add :base, symbol, message: dependent_error_message(dr.result, full_message)
end
dr.errors.full_messages.each do |full_message|
errors.add :base, :dependent_invalid, message: dependent_error_message(dr.result, full_message)
end
end

@ -63,7 +63,7 @@ module API
end
after_validation do
contract = ::CustomActions::ExecuteContract.new(parsed_params)
contract = ::CustomActions::ExecuteContract.new(parsed_params, current_user)
unless contract.valid?
fail ::API::Errors::ErrorBase.create_and_merge_errors(contract.errors)

@ -28,81 +28,19 @@
# See docs/COPYRIGHT.rdoc for more details.
#++
# This patch should no longer be necessary.
# But we have references to symbolds_and_messages_for as well as for symbols_for all over
# the code base.
module OpenProject::ActiveModelErrorsPatch
##
# ActiveRecord errors do provide no means to access the symbols initially used to create an
# error. E.g. errors.add :foo, :bar instantly translates :bar, making it hard to write code
# dependent on specific errors (which we use in the APIv3).
# We therefore add a second information store containing pairs of [symbol, translated_message].
def add(attribute, message = :invalid, options = {})
error_symbol = options.fetch(:error_symbol) { message }
super(attribute, message, options)
if store_new_symbols?
if error_symbol.is_a?(Symbol)
symbol = error_symbol
partial_message = normalize_message(attribute, message, options)
full_message = full_message(attribute, partial_message)
else
symbol = :unknown
full_message = message
end
writable_symbols_and_messages_for(attribute) << [symbol, full_message, partial_message]
end
end
def symbols_and_messages_for(attribute)
writable_symbols_and_messages_for(attribute).dup
end
def symbols_for(attribute)
symbols_and_messages_for(attribute).map(&:first)
end
def full_message(attribute, message)
return message if attribute == :base
# if a model acts_as_customizable it will inject attributes like 'custom_field_1' into itself
# using attr_name_override we resolve names of such attributes.
# The rest of the method should reflect the original method implementation of ActiveModel
attr_name_override = nil
match = /\Acustom_field_(?<id>\d+)\z/.match(attribute)
if match
attr_name_override = CustomField.find_by(id: match[:id]).name
end
symbols = details[attribute].map { |e| e[:error] }
messages = full_messages_for(attribute)
attr_name = attribute.to_s.tr('.', '_').humanize
attr_name = @base.class.human_attribute_name(attribute, default: attr_name)
I18n.t(:"errors.format", default: '%{attribute} %{message}',
attribute: attr_name_override || attr_name,
message: message)
symbols.zip(messages)
end
# Need to do the house keeping along with AR::Errors
# so that the symbols are removed when a new validation round starts
def clear
super
@error_symbols = Hash.new
end
private
def error_symbols
@error_symbols ||= Hash.new
end
def writable_symbols_and_messages_for(attribute)
error_symbols[attribute.to_sym] ||= []
end
# Kind of a hack: We need the possibility to temporarily disable symbol storing in the subclass
# Reform::Contract::Errors, because otherwise we end up with duplicate entries
# I feel dirty for doing that, but on the other hand I see no other way out... Please, stop me!
def store_new_symbols?
@store_new_symbols = true if @store_new_symbols.nil?
@store_new_symbols
def symbols_for(attribute)
details[attribute].map { |r| r[:error] }
end
end

@ -1,79 +0,0 @@
#-- encoding: UTF-8
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2020 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++
module OpenProject
module Patches
module Reform
def merge!(errors, prefix = [])
@store_new_symbols = false
super(errors, prefix)
@store_new_symbols = true
errors.keys.each do |attribute|
errors.symbols_and_messages_for(attribute).each do |symbol, full_message, partial_message|
symbols_and_messages = writable_symbols_and_messages_for(attribute)
next if symbols_and_messages && symbols_and_messages.any? do |sam|
sam[0] === symbol &&
sam[1] === full_message &&
sam[2] === partial_message
end
symbols_and_messages << [symbol, full_message, partial_message]
end
end
end
end
end
end
require "reform/form/active_model/validations"
Reform::Form.class_eval do
include Reform::Form::ActiveModel::Validations
end
Reform::Contract.class_eval do
include Reform::Form::ActiveModel::Validations
end
Reform::Form::ActiveModel::Validations::Validator.class_eval do
##
# use activerecord as the base scope instead of 'activemodel' to be compatible
# to the messages we have already stored
def self.i18n_scope
:activerecord
end
end
require 'reform/contract'
class Reform::Form::ActiveModel::Errors
prepend OpenProject::Patches::Reform
end

@ -28,4 +28,5 @@
#-- encoding: UTF-8
require File.dirname(__FILE__) + '/lib/acts_as_customizable'
require File.dirname(__FILE__) + '/lib/human_attribute_name'
ActiveRecord::Base.send(:include, Redmine::Acts::Customizable)

@ -61,6 +61,7 @@ module Redmine
module InstanceMethods
def self.included(base)
base.extend ClassMethods
base.extend HumanAttributeName
end
def available_custom_fields
@ -238,11 +239,11 @@ module Redmine
custom_value
.errors
.symbols_and_messages_for(attribute)
.each do |symbol, _, partial_message|
.details[attribute]
.each do |hash|
# Use the generated message by the custom field
# as it may contain specific parameters (e.g., :too_long requires :count)
errors.add(name, partial_message, error_symbol: symbol)
errors.add(name, hash[:error], **hash.except(:error))
end
end
end

@ -0,0 +1,49 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2020 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++
module Redmine
module Acts
module Customizable
module HumanAttributeName
# If a model acts_as_customizable it will inject attributes like 'custom_field_1' into itself.
# Using this method, they can now be i18ned same as every other attribute. This is for example
# for error messages following the format of '%{attribute} %{message}' where `attribute` is resolved
# by calling IncludingClass.human_attribute_name
def human_attribute_name(attribute, options = {})
match = /\Acustom_field_(?<id>\d+)\z/.match(attribute)
if match
CustomField.find_by(id: match[:id]).name
else
super
end
end
end
end
end
end

@ -31,24 +31,24 @@ require 'spec_helper'
describe WorkPackages::UpdateContract do
let(:work_package) do
FactoryBot.create(:work_package,
done_ratio: 50,
estimated_hours: 6.0,
project: project)
done_ratio: 50,
estimated_hours: 6.0,
project: project)
end
let(:member) { FactoryBot.create(:user, member_in_project: project, member_through_role: role) }
let (:project) { FactoryBot.create(:project) }
let(:project) { FactoryBot.create(:project) }
let(:current_user) { member }
let(:permissions) {
[
:view_work_packages,
:view_work_package_watchers,
:edit_work_packages,
:add_work_package_watchers,
:delete_work_package_watchers,
:manage_work_package_relations,
:add_work_package_notes
let(:permissions) do
%i[
view_work_packages
view_work_package_watchers
edit_work_packages
add_work_package_watchers
delete_work_package_watchers
manage_work_package_relations
add_work_package_notes
]
}
end
let(:role) { FactoryBot.create :role, permissions: permissions }
let(:changed_values) { [] }
@ -105,7 +105,7 @@ describe WorkPackages::UpdateContract do
let(:changed_values) { ['remaining_hours'] }
it('is invalid') do
expect(contract.errors.symbols_for('remaining_hours')).to match_array([:error_readonly])
expect(contract.errors.symbols_for(:remaining_hours)).to match_array([:error_readonly])
end
end
end

@ -34,11 +34,7 @@ module Bim::Bcf
extend ActiveSupport::Concern
included do
def validate
validate_user_allowed_to_manage
super
end
validate :validate_user_allowed_to_manage
private

@ -41,15 +41,11 @@ module Bim
::Bim::IfcModels::IfcModel
end
def validate
user_allowed_to_manage
user_is_uploader
ifc_attachment_existent
ifc_attachment_is_ifc
uploader_is_ifc_attachment_author
super
end
validate :user_allowed_to_manage
validate :user_is_uploader
validate :ifc_attachment_existent
validate :ifc_attachment_is_ifc
validate :uploader_is_ifc_attachment_author
def user_allowed_to_manage
if model.project && !user.allowed_to?(:manage_ifc_models, model.project)

@ -46,9 +46,9 @@ module Bim::Bcf::API::V2_1::Errors
def self.map(original_errors)
mapped_errors = ActiveModel::Errors.new(new)
original_errors.send(:error_symbols).each do |key, errors|
errors.map(&:first).each do |error|
mapped_errors.add(error_key_mapper(key), error)
original_errors.details.each do |key, errors|
errors.each do |error|
mapped_errors.add(error_key_mapper(key), error[:error], *error.except(:error))
end
end

@ -33,8 +33,8 @@ describe Bim::Bcf::Issues::CreateContract do
it_behaves_like 'issues contract' do
let(:issue) do
Bim::Bcf::Issue.new(uuid: issue_uuid,
work_package: issue_work_package,
index: issue_index)
work_package: issue_work_package,
index: issue_index)
end
let(:permissions) { [:manage_bcf] }

@ -258,17 +258,9 @@ describe 'API v3 Grids resource for Board Grids', type: :request, content_type:
it 'responds with 422 and mentions the error' do
expect(subject.status).to eq 422
expect(subject.body)
.to be_json_eql('Error'.to_json)
.at_path('_type')
expect(subject.body)
.to be_json_eql("Widgets is outside of the grid.".to_json)
.at_path('_embedded/errors/0/message')
expect(subject.body)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.at_path('_embedded/errors/1/message')
expect(JSON.parse(subject.body)['_embedded']['errors'].map { |e| e['message'] })
.to match_array ["Widgets is outside of the grid.",
"Number of rows must be greater than 0."]
end
it 'does not persist the changes to widgets' do
@ -398,17 +390,10 @@ describe 'API v3 Grids resource for Board Grids', type: :request, content_type:
.to be_json_eql('Error'.to_json)
.at_path('_type')
expect(subject.body)
.to be_json_eql("Widgets is outside of the grid.".to_json)
.at_path('_embedded/errors/0/message')
expect(subject.body)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.at_path('_embedded/errors/1/message')
expect(subject.body)
.to be_json_eql("Number of columns must be greater than 0.".to_json)
.at_path('_embedded/errors/2/message')
expect(JSON.parse(subject.body)['_embedded']['errors'].map { |e| e['message'] })
.to match_array ["Widgets is outside of the grid.",
"Number of rows must be greater than 0.",
"Number of columns must be greater than 0."]
end
end

@ -43,13 +43,9 @@ module TimeEntries
TimeEntry
end
def validate
validate_hours_are_in_range
validate_project_is_set
validate_work_package
super
end
validate :validate_hours_are_in_range
validate :validate_project_is_set
validate :validate_work_package
attribute :project_id
attribute :work_package_id

@ -30,12 +30,8 @@
module TimeEntries
class CreateContract < BaseContract
def validate
user_allowed_to_add
validate_user_current_user
super
end
validate :user_allowed_to_add
validate :validate_user_current_user
private

@ -32,12 +32,12 @@ module TimeEntries
class UpdateContract < BaseContract
include UnchangedProject
def validate
validate :validate_user_allowed_to_update
def validate_user_allowed_to_update
unless user_allowed_to_update?
errors.add :base, :error_unauthorized
end
super
end
##

@ -44,24 +44,19 @@ module Grids
attribute_alias :type, :scope
def validate
validate_allowed
validate_registered_widgets
validate_widget_collisions
validate_widgets_within
validate_widgets_start_before_end
run_registration_validations
super
end
attribute :widgets
attribute :name
attribute :options
validate :validate_allowed
validate :validate_registered_widgets
validate :validate_widget_collisions
validate :validate_widgets_within
validate :validate_widgets_start_before_end
validate :run_registration_validations
def self.model
Grid
end

@ -32,11 +32,7 @@ require 'grids/base_contract'
module Grids
class DeleteContract < BaseContract
def validate
validate_delete_allowed
super
end
validate :validate_delete_allowed
##
# Check whether this grid can be deleted.

@ -247,17 +247,9 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do
it 'responds with 422 and mentions the error' do
expect(subject.status).to eq 422
expect(subject.body)
.to be_json_eql('Error'.to_json)
.at_path('_type')
expect(subject.body)
.to be_json_eql("Widgets is outside of the grid.".to_json)
.at_path('_embedded/errors/0/message')
expect(subject.body)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.at_path('_embedded/errors/1/message')
expect(JSON.parse(subject.body)['_embedded']['errors'].map { |e| e['message'] })
.to match_array ["Widgets is outside of the grid.",
"Number of rows must be greater than 0."]
end
it 'does not persist the changes to widgets' do
@ -383,17 +375,10 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do
.to be_json_eql('Error'.to_json)
.at_path('_type')
expect(subject.body)
.to be_json_eql("Widgets is outside of the grid.".to_json)
.at_path('_embedded/errors/0/message')
expect(subject.body)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.at_path('_embedded/errors/1/message')
expect(subject.body)
.to be_json_eql("Number of columns must be greater than 0.".to_json)
.at_path('_embedded/errors/2/message')
expect(JSON.parse(subject.body)['_embedded']['errors'].map { |e| e['message'] })
.to match_array ["Widgets is outside of the grid.",
"Number of rows must be greater than 0.",
"Number of columns must be greater than 0."]
end
end
end

@ -39,13 +39,13 @@ describe CustomActions::Actions::AssignedTo, type: :model do
FactoryBot.build_stubbed(:user)]
allow(User)
.to receive_message_chain(:active_or_registered, :select, :order_by_name)
.and_return(users)
.and_return(users)
else
users = [FactoryBot.build_stubbed(:user),
FactoryBot.build_stubbed(:group)]
allow(Principal)
.to receive_message_chain(:active_or_registered, :select, :order_by_name)
.and_return(users)
.and_return(users)
end
[{ value: nil, label: '-' },
@ -105,7 +105,7 @@ describe CustomActions::Actions::AssignedTo, type: :model do
end
it 'validates the me value when executing' do
errors = ActiveModel::Errors.new(work_package)
errors = ActiveModel::Errors.new(CustomAction.new)
subject.validate errors
expect(errors.symbols_for(:actions)).to be_empty
end
@ -118,7 +118,7 @@ describe CustomActions::Actions::AssignedTo, type: :model do
end
it 'validates the me value when executing' do
errors = ActiveModel::Errors.new(work_package)
errors = ActiveModel::Errors.new(CustomAction.new)
subject.validate errors
expect(errors.symbols_for(:actions)).to include :not_logged_in
end

Loading…
Cancel
Save