Fixed: No validation errors when entering an invalid "Parent task" (#11979).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10615 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 520d2c3dcebabe629146cfe2899e0f9706262d57..13a21e07a2371fe1da3e931cae56300285599c77 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -416,7 +416,9 @@ class Issue < ActiveRecord::Base
     end
 
     if attrs['parent_issue_id'].present?
-      attrs.delete('parent_issue_id') unless Issue.visible(user).exists?(attrs['parent_issue_id'].to_i)
+      unless Issue.visible(user).exists?(attrs['parent_issue_id'].to_i)
+        @invalid_parent_issue_id = attrs.delete('parent_issue_id')
+      end
     end
 
     if attrs['custom_field_values'].present?
@@ -550,7 +552,9 @@ class Issue < ActiveRecord::Base
     end
 
     # Checks parent issue assignment
-    if @parent_issue
+    if @invalid_parent_issue_id.present?
+      errors.add :parent_issue_id, :invalid
+    elsif @parent_issue
       if !valid_parent_project?(@parent_issue)
         errors.add :parent_issue_id, :invalid
       elsif !new_record?
@@ -947,17 +951,19 @@ class Issue < ActiveRecord::Base
   end
 
   def parent_issue_id=(arg)
-    parent_issue_id = arg.blank? ? nil : arg.to_i
-    if parent_issue_id && @parent_issue = Issue.find_by_id(parent_issue_id)
+    s = arg.to_s.strip.presence
+    if s && (m = s.match(%r{\A#?(\d+)\z})) && (@parent_issue = Issue.find_by_id(m[1]))
       @parent_issue.id
     else
       @parent_issue = nil
-      nil
+      @invalid_parent_issue_id = arg
     end
   end
 
   def parent_issue_id
-    if instance_variable_defined? :@parent_issue
+    if @invalid_parent_issue_id
+      @invalid_parent_issue_id
+    elsif instance_variable_defined? :@parent_issue
       @parent_issue.nil? ? nil : @parent_issue.id
     else
       parent_id

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 01eb26c9631018b6264f69f453f6975a9c531625..b897c472e0dd993767727a6c3064b78c81c46e99 100644 (file)
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -1945,24 +1945,42 @@ class IssuesControllerTest < ActionController::TestCase
                  :issue => {:tracker_id => 1,
                             :subject => 'This is a child issue',
                             :parent_issue_id => 2}
+
+      assert_response 302
     end
     issue = Issue.find_by_subject('This is a child issue')
     assert_not_nil issue
     assert_equal Issue.find(2), issue.parent
   end
 
-  def test_post_create_subissue_with_non_numeric_parent_id
+  def test_post_create_subissue_with_non_visible_parent_id_should_not_validate
     @request.session[:user_id] = 2
 
-    assert_difference 'Issue.count' do
+    assert_no_difference 'Issue.count' do
       post :create, :project_id => 1,
                  :issue => {:tracker_id => 1,
                             :subject => 'This is a child issue',
-                            :parent_issue_id => 'ABC'}
+                            :parent_issue_id => '4'}
+
+      assert_response :success
+      assert_select 'input[name=?][value=?]', 'issue[parent_issue_id]', '4'
+      assert_error_tag :content => /Parent task is invalid/i
+    end
+  end
+
+  def test_post_create_subissue_with_non_numeric_parent_id_should_not_validate
+    @request.session[:user_id] = 2
+
+    assert_no_difference 'Issue.count' do
+      post :create, :project_id => 1,
+                 :issue => {:tracker_id => 1,
+                            :subject => 'This is a child issue',
+                            :parent_issue_id => '01ABC'}
+
+      assert_response :success
+      assert_select 'input[name=?][value=?]', 'issue[parent_issue_id]', '01ABC'
+      assert_error_tag :content => /Parent task is invalid/i
     end
-    issue = Issue.find_by_subject('This is a child issue')
-    assert_not_nil issue
-    assert_nil issue.parent
   end
 
   def test_post_create_private

diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 0747f5c8421313e1b4b89268a5bad8767e0a9a3a..365ecad683ca4279ab4d953ac8ce4a2fb0eacbe1 100644 (file)
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -92,6 +92,20 @@ class IssueTest < ActiveSupport::TestCase
     end
   end
 
+  def test_create_with_parent_issue_id
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :subject => 'Group assignment', :parent_issue_id => 1)
+    assert_save issue
+    assert_equal 1, issue.parent_issue_id
+    assert_equal Issue.find(1), issue.parent
+  end
+
+  def test_create_with_invalid_parent_issue_id
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :subject => 'Group assignment', :parent_issue_id => '01ABC')
+    assert !issue.save
+    assert_equal '01ABC', issue.parent_issue_id
+    assert_include 'Parent task is invalid', issue.errors.full_messages
+  end
+
   def assert_visibility_match(user, issues)
     assert_equal issues.collect(&:id).sort, Issue.all.select {|issue| issue.visible?(user)}.collect(&:id).sort
   end