From d4e6355eb3f220787e9374eeef6fb496f213f0d1 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Thu, 16 Feb 2012 21:00:11 +0000 Subject: Better handling of attachments when issue validation fails (#10253). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8891 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- test/functional/attachments_controller_test.rb | 8 ++ test/functional/issues_controller_test.rb | 129 +++++++++++++++++++++ .../issues_controller_transaction_test.rb | 35 +++++- 3 files changed, 167 insertions(+), 5 deletions(-) (limited to 'test/functional') diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb index 2cf79672d..311c72588 100644 --- a/test/functional/attachments_controller_test.rb +++ b/test/functional/attachments_controller_test.rb @@ -210,6 +210,14 @@ class AttachmentsControllerTest < ActionController::TestCase set_tmp_attachments_directory end + def test_show_file_without_container_should_be_denied + attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2) + + @request.session[:user_id] = 2 + get :show, :id => attachment.id + assert_response 403 + end + def test_download_text_file get :download, :id => 4 assert_response :success diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 6f1bea304..371de69a6 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1663,6 +1663,69 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 59, File.size(attachment.diskfile) end + def test_post_create_with_failure_should_save_attachments + set_tmp_attachments_directory + @request.session[:user_id] = 2 + + assert_no_difference 'Issue.count' do + assert_difference 'Attachment.count' do + post :create, :project_id => 1, + :issue => { :tracker_id => '1', :subject => '' }, + :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'}} + assert_response :success + assert_template 'new' + end + end + + attachment = Attachment.first(:order => 'id DESC') + assert_equal 'testfile.txt', attachment.filename + assert File.exists?(attachment.diskfile) + assert_nil attachment.container + + assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token} + assert_tag 'span', :content => /testfile.txt/ + end + + def test_post_create_with_failure_should_keep_saved_attachments + set_tmp_attachments_directory + attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2) + @request.session[:user_id] = 2 + + assert_no_difference 'Issue.count' do + assert_no_difference 'Attachment.count' do + post :create, :project_id => 1, + :issue => { :tracker_id => '1', :subject => '' }, + :attachments => {'p0' => {'token' => attachment.token}} + assert_response :success + assert_template 'new' + end + end + + assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token} + assert_tag 'span', :content => /testfile.txt/ + end + + def test_post_create_should_attach_saved_attachments + set_tmp_attachments_directory + attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2) + @request.session[:user_id] = 2 + + assert_difference 'Issue.count' do + assert_no_difference 'Attachment.count' do + post :create, :project_id => 1, + :issue => { :tracker_id => '1', :subject => 'Saved attachments' }, + :attachments => {'p0' => {'token' => attachment.token}} + assert_response 302 + end + end + + issue = Issue.first(:order => 'id DESC') + assert_equal 1, issue.attachments.count + + attachment.reload + assert_equal issue, attachment.container + end + context "without workflow privilege" do setup do Workflow.delete_all(["role_id = ?", Role.anonymous.id]) @@ -2301,6 +2364,72 @@ class IssuesControllerTest < ActionController::TestCase assert mail.body.include?('testfile.txt') end + def test_put_update_with_failure_should_save_attachments + set_tmp_attachments_directory + @request.session[:user_id] = 2 + + assert_no_difference 'Journal.count' do + assert_difference 'Attachment.count' do + put :update, :id => 1, + :issue => { :subject => '' }, + :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'}} + assert_response :success + assert_template 'edit' + end + end + + attachment = Attachment.first(:order => 'id DESC') + assert_equal 'testfile.txt', attachment.filename + assert File.exists?(attachment.diskfile) + assert_nil attachment.container + + assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token} + assert_tag 'span', :content => /testfile.txt/ + end + + def test_put_update_with_failure_should_keep_saved_attachments + set_tmp_attachments_directory + attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2) + @request.session[:user_id] = 2 + + assert_no_difference 'Journal.count' do + assert_no_difference 'Attachment.count' do + put :update, :id => 1, + :issue => { :subject => '' }, + :attachments => {'p0' => {'token' => attachment.token}} + assert_response :success + assert_template 'edit' + end + end + + assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token} + assert_tag 'span', :content => /testfile.txt/ + end + + def test_put_update_should_attach_saved_attachments + set_tmp_attachments_directory + attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2) + @request.session[:user_id] = 2 + + assert_difference 'Journal.count' do + assert_difference 'JournalDetail.count' do + assert_no_difference 'Attachment.count' do + put :update, :id => 1, + :notes => 'Attachment added', + :attachments => {'p0' => {'token' => attachment.token}} + assert_redirected_to '/issues/1' + end + end + end + + attachment.reload + assert_equal Issue.find(1), attachment.container + + journal = Journal.first(:order => 'id DESC') + assert_equal 1, journal.details.size + assert_equal 'testfile.txt', journal.details.first.value + end + def test_put_update_with_attachment_that_fails_to_save set_tmp_attachments_directory diff --git a/test/functional/issues_controller_transaction_test.rb b/test/functional/issues_controller_transaction_test.rb index 75c0b6a83..58af193ac 100644 --- a/test/functional/issues_controller_transaction_test.rb +++ b/test/functional/issues_controller_transaction_test.rb @@ -58,7 +58,33 @@ class IssuesControllerTransactionTest < ActionController::TestCase assert_no_difference 'Journal.count' do assert_no_difference 'TimeEntry.count' do - assert_no_difference 'Attachment.count' do + put :update, + :id => issue.id, + :issue => { + :fixed_version_id => 4, + :lock_version => (issue.lock_version - 1) + }, + :notes => 'My notes', + :time_entry => { :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first.id } + end + end + + assert_response :success + assert_template 'edit' + assert_tag 'div', :attributes => {:class => 'conflict'} + assert_tag 'input', :attributes => {:name => 'conflict_resolution', :value => 'overwrite'} + assert_tag 'input', :attributes => {:name => 'conflict_resolution', :value => 'add_notes'} + assert_tag 'input', :attributes => {:name => 'conflict_resolution', :value => 'cancel'} + end + + def test_update_stale_issue_should_save_attachments + set_tmp_attachments_directory + issue = Issue.find(2) + @request.session[:user_id] = 2 + + assert_no_difference 'Journal.count' do + assert_no_difference 'TimeEntry.count' do + assert_difference 'Attachment.count' do put :update, :id => issue.id, :issue => { @@ -74,10 +100,9 @@ class IssuesControllerTransactionTest < ActionController::TestCase assert_response :success assert_template 'edit' - assert_tag 'div', :attributes => {:class => 'conflict'} - assert_tag 'input', :attributes => {:name => 'conflict_resolution', :value => 'overwrite'} - assert_tag 'input', :attributes => {:name => 'conflict_resolution', :value => 'add_notes'} - assert_tag 'input', :attributes => {:name => 'conflict_resolution', :value => 'cancel'} + attachment = Attachment.first(:order => 'id DESC') + assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token} + assert_tag 'span', :content => /testfile.txt/ end def test_update_stale_issue_without_notes_should_not_show_add_notes_option -- cgit v1.2.3