jro_apps/redmine Commit - r10404:2b797fa82fdd

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

Jean-Philippe Lang -

r10404:2b797fa82fdd

parent child

Context file:

r10404:2b797fa82fdd

Collapse all files

app/models/issue.rb +12 -6

                  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?
                  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?
                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

test/functional/issues_controller_test.rb +24 -6

                               :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

test/unit/issue_test.rb +14 0

                  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

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages