Merge pull request #750 from opf/fix/work_package_parent_validation

Fixed a bug with non-functional validation for work package parents

config/locales/de.yml

@@ -216,10 +216,11 @@ de:
           attributes:
             due_date:
               not_start_date: "ist nicht am Startdatum, obwohl die bei Meilensteinen Pflicht ist"
-            parent:
+            parent_id:
               cannot_be_milestone: "darf kein Meilenstein sein"
               cannot_be_in_another_project: "darf nicht in einem anderen Projekt sein"
               not_a_valid_parent: "ist ungültig"
+              does_not_exist: "existiert nicht"
         project_association:
           identical_projects: "kann nicht von einem Projekt auf sich selbst erstellt werden"
           project_association_not_allowed: "erlaubt keine Projekt-Abhängigkeiten"

config/locales/en.yml

@@ -217,10 +217,11 @@ en:
           attributes:
             due_date:
               not_start_date: "is not on start date, although this is required for milestones"
-            parent:
+            parent_id:
               cannot_be_milestone: "cannot be a milestone"
               cannot_be_in_another_project: "cannot be in another project"
               not_a_valid_parent: "is invaild"
+              does_not_exist: "does not exist"
         user:
           attributes:
             password:

doc/CHANGELOG.md

@@ -36,11 +36,12 @@ See doc/COPYRIGHT.rdoc for more details.
 * `#3266` Fix: [Work package tracking] % done in work package status cannot be modified
 * `#3291` Fix: Internal error when clicking on member
 * `#3303` Fix: [Work package tracking] Search results are linked to wrong location
+* `#3322` [Data migration] Journal entries display changes to custom fields
 * `#3331` use permitted_params for group_controller
 * `#3363` [Timelines] Autocompleter broken multiple times in timelines edit
 * `#3390` [Design] Implement new look for header and project navigation
 * Change global search keyboard shortcut to 's' and project menu shortcut to 'p'
-* `#3322` [Data migration] Journal entries display changes to custom fields
+* Fixed a small bug with a non-functional validation for parents when creating a work package
 
 ## 3.0.0pre37
 

lib/open_project/nested_set/root_id_handling.rb

@@ -84,10 +84,14 @@ module OpenProject::NestedSet
       end
 
       def validate_correct_parent
+        # calling parent triggers the loading, so if we could not load one, the parent does not exist
+        if parent_id && !parent
+          errors.add :parent_id, :does_not_exist
+        end
         # Checks parent issue assignment
         if parent
           if !Setting.cross_project_work_package_relations? && parent.project_id != self.project_id
-            errors.add :parent, :cannot_be_in_another_project
+            errors.add :parent_id, :cannot_be_in_another_project
           elsif !new_record?
             # moving an existing issue
             if parent.root_id != root_id
@@ -95,7 +99,7 @@ module OpenProject::NestedSet
             elsif move_possible?(parent)
               # move accepted inside tree
             else
-              errors.add :parent, :not_a_valid_parent
+              errors.add :parent_id, :not_a_valid_parent
             end
           end
         end
@@ -129,7 +133,7 @@ module OpenProject::NestedSet
       # of the parent. If no parent is provided, the new node defines it's own
       # set.
       def initial_root_id
-        if parent_id
+        if parent
           self.root_id = parent.root_id
         else
           self.root_id = id

spec/models/work_package_spec.rb

@@ -1313,6 +1313,17 @@ describe WorkPackage do
     end
   end
 
+  describe "parent work package" do
+    describe "with parent_id for a not existing work package" do
+      let(:project) { FactoryGirl.create(:project) }
+      let(:invalid_work_package) { FactoryGirl.build(:work_package, :project => project, :parent_id => 1) }
+
+      it 'should raise an error' do
+        invalid_work_package.should_not be_valid
+      end
+    end
+  end
+
   describe 'custom fields' do
     it 'should not duplicate error messages when invalid' do
       cf1 = FactoryGirl.create(:work_package_custom_field, :is_required => true)

test/unit/issue_nested_set_test.rb

@@ -51,7 +51,7 @@ class IssueNestedSetTest < ActiveSupport::TestCase
                              :parent_id => issue.id }
     end
     assert !child.save
-    refute_empty child.errors[:parent]
+    refute_empty child.errors[:parent_id]
   end
 
   def test_creating_a_child_in_different_project_should_validate_if_allowed
@@ -65,7 +65,7 @@ class IssueNestedSetTest < ActiveSupport::TestCase
                              :parent_id => issue.id }
     end
     assert child.save
-    assert_empty child.errors[:parent]
+    assert_empty child.errors[:parent_id]
   end
 
 
@@ -116,7 +116,7 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     child.reload
     child.parent_id = grandchild.id
     assert !child.save
-    refute_empty child.errors[:parent]
+    refute_empty child.errors[:parent_id]
   end
 
   def test_moving_an_issue_should_keep_valid_relations_only