diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb
index 101e5443a..aacbc4f99 100644
--- a/app/controllers/documents_controller.rb
+++ b/app/controllers/documents_controller.rb
@@ -75,7 +75,7 @@ class DocumentsController < ApplicationController
@document.safe_attributes = params[:document]
if @document.save
flash[:notice] = l(:notice_successful_update)
- redirect_to document_path(@document)
+ add_attachment
else
render :action => 'edit'
end
diff --git a/app/models/document.rb b/app/models/document.rb
index a1b561b3c..b5a9f2958 100644
--- a/app/models/document.rb
+++ b/app/models/document.rb
@@ -40,9 +40,12 @@ class Document < ActiveRecord::Base
)
acts_as_activity_provider :scope => proc {preload(:project)}
+ attr_writer :deleted_attachment_ids
+
validates_presence_of :project, :title, :category
validates_length_of :title, :maximum => 255
+ after_save :delete_selected_attachments
after_create_commit :send_notification
scope :visible, (lambda do |*args|
@@ -51,6 +54,10 @@ class Document < ActiveRecord::Base
end)
safe_attributes 'category_id', 'title', 'description', 'custom_fields', 'custom_field_values'
+ safe_attributes(
+ 'deleted_attachment_ids',
+ :if => lambda {|document, user| document.attachments_deletable?(user)}
+ )
def visible?(user=User.current)
!user.nil? && user.allowed_to?(:view_documents, project)
@@ -75,8 +82,18 @@ class Document < ActiveRecord::Base
project.notified_users.reject {|user| !visible?(user)}
end
+ def deleted_attachment_ids
+ Array(@deleted_attachment_ids).map(&:to_i)
+ end
+
private
+ def delete_selected_attachments
+ deleted_ids = deleted_attachment_ids
+ return if deleted_ids.blank?
+ attachments.where(:id => deleted_ids).destroy_all
+ end
+
def send_notification
if Setting.notified_events.include?('document_added')
Mailer.deliver_document_added(self, User.current)
diff --git a/app/views/documents/_form.html.erb b/app/views/documents/_form.html.erb
index 45b63611f..64c55ff17 100644
--- a/app/views/documents/_form.html.erb
+++ b/app/views/documents/_form.html.erb
@@ -12,12 +12,30 @@
<% @document.custom_field_values.each do |value| %>
<%= custom_field_tag_with_label :document, value %>
<% end %>
-
<%= wikitoolbar_for 'document_description' %>
-<% if @document.new_record? %>
-
-
<%= render :partial => 'attachments/form', :locals => {:container => @document} %>
+
+
diff --git a/test/functional/documents_controller_test.rb b/test/functional/documents_controller_test.rb
index 3426963a8..1aec958e3 100644
--- a/test/functional/documents_controller_test.rb
+++ b/test/functional/documents_controller_test.rb
@@ -224,36 +224,90 @@ class DocumentsControllerTest < Redmine::ControllerTest
end
def test_update
+ set_tmp_attachments_directory
@request.session[:user_id] = 2
- put(
- :update,
- :params => {
- :id => 1,
- :document => {
- :title => 'test_update'
+ assert_difference 'Attachment.count' do
+ put(
+ :update,
+ :params => {
+ :id => 1,
+ :document => {
+ :title => 'test_update'
+ },
+ :attachments => {
+ '1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}
+ }
}
- }
- )
+ )
+ end
assert_redirected_to '/documents/1'
document = Document.find(1)
assert_equal 'test_update', document.title
end
def test_update_with_failure
+ set_tmp_attachments_directory
@request.session[:user_id] = 2
- put(
- :update,
- :params => {
- :id => 1,
- :document => {
- :title => ''
+ assert_no_difference 'Attachment.count' do
+ put(
+ :update,
+ :params => {
+ :id => 1,
+ :document => {
+ :title => ''
+ },
+ :attachments => {
+ '1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}
+ }
}
- }
- )
+ )
+ end
assert_response :success
assert_select_error /title cannot be blank/i
end
+ def test_update_with_deleted_attachment_ids
+ set_tmp_attachments_directory
+ @request.session[:user_id] = 2
+ document = Document.find(1)
+ attachment = document.attachments.first
+ assert_difference 'Attachment.count', -1 do
+ put(
+ :update,
+ :params => {
+ :id => 1,
+ :document => {
+ :title => 'test_update',
+ :deleted_attachment_ids => [attachment.id]
+ }
+ }
+ )
+ end
+ document.reload
+ refute_includes document.attachments, attachment
+ end
+
+ def test_update_with_deleted_attachment_ids_and_failure_should_preserve_selected_attachments
+ set_tmp_attachments_directory
+ @request.session[:user_id] = 2
+ document = Document.find(1)
+ attachment = document.attachments.first
+ assert_no_difference 'Attachment.count' do
+ put(
+ :update,
+ :params => {
+ :id => 1,
+ :document => {
+ :title => '',
+ :deleted_attachment_ids => [attachment.id]
+ }
+ }
+ )
+ end
+ document.reload
+ assert_includes document.attachments, attachment
+ end
+
def test_destroy
set_tmp_attachments_directory
@request.session[:user_id] = 2