From 17940bf02cd135fefd7614a2e1db582b632aa9af Mon Sep 17 00:00:00 2001
From: Jan Sandbrink <j.sandbrink@finn.de>
Date: Fri, 3 Jul 2015 14:25:54 +0200
Subject: [PATCH] change validation of WP children

- do not add error on a symbol called :#123 (123 is the childrens ID)
    - this is against the semantics of ActiveRecord::Errors
    - creating symbols in a loop used to be a bad idea (might be better today)
    - it breaks error messages in the APIv3 (which is due to APIv3 working beyond the borders of AR errors, not the best of ideas...)
- use :base instead, the place for unlocatable errors
- include the fact that #123 refers to a child element in the error message

Basically I feel that this makes the children validation cleaner and better. But somehow I fear, that there is some code out there that depends on the fact that error keys like :#123 existed...
---
 app/models/work_package/validations.rb             | 5 +++--
 config/locales/en.yml                              | 1 +
 spec/requests/api/v3/work_package_resource_spec.rb | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/app/models/work_package/validations.rb b/app/models/work_package/validations.rb
index 8e48373a4d..1bb2ae5ad3 100644
--- a/app/models/work_package/validations.rb
+++ b/app/models/work_package/validations.rb
@@ -125,8 +125,9 @@ module WorkPackage::Validations
 
   def validate_children
     children.select { |c| !c.valid? }.each do |child|
-      child.errors.each do |_, value|
-        errors.add(:"##{child.id}", value)
+      child.errors.each do |attribute, message|
+        full_message = child.errors.full_message(attribute, message)
+        errors.add(:base, "#{I18n.t('label_child_element')} ##{child.id}: #{full_message}")
       end
     end
   end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 94212f1148..5b4be0f001 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -674,6 +674,7 @@ en:
   label_changes_details: "Details of all changes"
   label_changeset_plural: "Changesets"
   label_checked: "checked"
+  label_child_element: "Child element"
   label_chronological_order: "In chronological order"
   label_close_versions: "Close completed versions"
   label_closed_work_packages: "closed"
diff --git a/spec/requests/api/v3/work_package_resource_spec.rb b/spec/requests/api/v3/work_package_resource_spec.rb
index ee0b1e4cb8..bf17f05f6d 100644
--- a/spec/requests/api/v3/work_package_resource_spec.rb
+++ b/spec/requests/api/v3/work_package_resource_spec.rb
@@ -897,7 +897,9 @@ h4. things we like
 
           it_behaves_like 'multiple errors of the same type with messages' do
             let(:message) {
-              [child_1.id, child_2.id].map { |id| "##{id} cannot be in another project." }
+              [child_1.id, child_2.id].map { |id|
+                "Child element ##{id}: Parent cannot be in another project."
+              }
             }
           end
         end