Ability to delete multiple attachments while updating an issue (#13072).

git-svn-id: http://svn.redmine.org/redmine/trunk@15650 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/issue.rb

@@ -59,6 +59,7 @@ class Issue < ActiveRecord::Base
 
   DONE_RATIO_OPTIONS = %w(issue_field issue_status)
 
+  attr_accessor :deleted_attachment_ids
   attr_reader :current_journal
   delegate :notes, :notes=, :private_notes, :private_notes=, :to => :current_journal, :allow_nil => true
 
@@ -109,7 +110,7 @@ class Issue < ActiveRecord::Base
               :force_updated_on_change, :update_closed_on, :set_assigned_to_was
   after_save {|issue| issue.send :after_project_change if !issue.id_changed? && issue.project_id_changed?}
   after_save :reschedule_following_issues, :update_nested_set_attributes,
-             :update_parent_attributes, :create_journal
+             :update_parent_attributes, :delete_selected_attachments, :create_journal
   # Should be after_create but would be called before previous after_save callbacks
   after_save :after_create_from_copy
   after_destroy :update_parent_attributes
@@ -403,6 +404,10 @@ class Issue < ActiveRecord::Base
     write_attribute(:description, arg)
   end
 
+  def deleted_attachment_ids
+    Array(@deleted_attachment_ids).map(&:to_i)
+  end
+
   # Overrides assign_attributes so that project and tracker get assigned first
   def assign_attributes_with_project_and_tracker_first(new_attributes, *args)
     return if new_attributes.nil?
@@ -465,6 +470,9 @@ class Issue < ActiveRecord::Base
     :if => lambda {|issue, user| (issue.new_record? || issue.attributes_editable?(user)) &&
       user.allowed_to?(:manage_subtasks, issue.project)}
 
+  safe_attributes 'deleted_attachment_ids',
+    :if => lambda {|issue, user| issue.attachments_deletable?(user)}
+
   def safe_attribute_names(user=nil)
     names = super
     names -= disabled_core_fields
@@ -1586,6 +1594,13 @@ class Issue < ActiveRecord::Base
     end
   end
 
+  def delete_selected_attachments
+    if deleted_attachment_ids.present?
+      objects = attachments.where(:id => deleted_attachment_ids.map(&:to_i))
+      attachments.delete(objects)
+    end
+  end
+
   # Callback on file attachment
   def attachment_added(attachment)
     if current_journal && !attachment.new_record?

app/views/issues/_edit.html.erb

@@ -38,9 +38,29 @@
   
       <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %>
       </fieldset>
-  
+
       <fieldset><legend><%= l(:label_attachment_plural) %></legend>
-      <p><%= render :partial => 'attachments/form', :locals => {:container => @issue} %></p>
+        <% if @issue.attachments.any? && @issue.safe_attribute?('deleted_attachment_ids') %>
+        <div class="contextual"><%= link_to l(:label_edit_attachments), '#', :onclick => "$('#existing-attachments').toggle(); return false;" %></div>
+        <div id="existing-attachments" style="<%= @issue.deleted_attachment_ids.blank? ? 'display:none;' : '' %>">
+          <% @issue.attachments.each do |attachment| %>
+          <span class="existing-attachment">
+            <%= text_field_tag '', attachment.filename, :class => "filename", :disabled => true %>
+            <label>
+              <%= check_box_tag 'issue[deleted_attachment_ids][]',
+                                attachment.id,
+                                @issue.deleted_attachment_ids.include?(attachment.id),
+                                :id => nil, :class => "deleted_attachment" %> <%= l(:button_delete) %>
+            </label>
+          </span>
+          <% end %>
+          <hr />
+        </div>
+        <% end %>
+
+        <div id="new-attachments" style="display:inline-block;">
+          <%= render :partial => 'attachments/form', :locals => {:container => @issue} %>
+        </div>
       </fieldset>
     <% end %>
     </div>

public/javascripts/attachments.js

@@ -189,3 +189,8 @@ function setupFileDrop() {
 }
 
 $(document).ready(setupFileDrop);
+$(document).ready(function(){
+  $("input.deleted_attachment").change(function(){
+    $(this).parents('.existing-attachment').toggleClass('deleted', $(this).is(":checked"));
+  }).change();
+});

public/stylesheets/application.css

@@ -363,7 +363,7 @@ div.square {
   overflow: hidden;
   width: .6em; height: .6em;
 }
-.contextual {float:right; white-space: nowrap; line-height:1.4em;margin-top:5px; padding-left: 10px; font-size:0.9em;}
+.contextual {float:right; white-space: nowrap; line-height:1.4em;margin:5px 0px; padding-left: 10px; font-size:0.9em;}
 .contextual input, .contextual select {font-size:0.9em;}
 .message .contextual { margin-top: 0; }
 
@@ -673,14 +673,16 @@ span.required {color: #bb0000;}
 .check_box_group.bool_cf {border:0; background:inherit;}
 .check_box_group.bool_cf label {display: inline;}
 
-#attachments_fields input.description {margin-left:4px; width:340px;}
-#attachments_fields span {display:block; white-space:nowrap;}
-#attachments_fields input.filename {border:0; height:1.8em; width:250px; color:#555; background-color:inherit; background:url(../images/attachment.png) no-repeat 1px 50%; padding-left:18px;}
+#attachments_fields input.description, #existing-attachments input.description {margin-left:4px; width:340px;}
+#attachments_fields>span, #existing-attachments>span {display:block; white-space:nowrap;}
+#attachments_fields input.filename, #existing-attachments .filename {border:0; width:250px; color:#555; background-color:inherit; background:url(../images/attachment.png) no-repeat 1px 50%; padding-left:18px;}
+#attachments_fields input.filename {height:1.8em;}
 #attachments_fields .ajax-waiting input.filename {background:url(../images/hourglass.png) no-repeat 0px 50%;}
 #attachments_fields .ajax-loading input.filename {background:url(../images/loading.gif) no-repeat 0px 50%;}
 #attachments_fields div.ui-progressbar { width: 100px; height:14px; margin: 2px 0 -5px 8px; display: inline-block; }
 a.remove-upload {background: url(../images/delete.png) no-repeat 1px 50%; width:1px; display:inline-block; padding-left:16px;}
 a.remove-upload:hover {text-decoration:none !important;}
+.existing-attachment.deleted .filename {text-decoration:line-through; color:#999 !important;}
 
 div.fileover { background-color: lavender; }
 
@@ -1137,8 +1139,6 @@ a.close-icon:hover {background-image:url('../images/close_hl.png');}
   background-position: 0% 50%;
   background-repeat: no-repeat;
   padding-left: 16px;
-}
-a.icon-only {
   display: inline-block;
   width: 0;
   height: 16px;
@@ -1148,7 +1148,7 @@ a.icon-only {
   font-size: 8px;
   vertical-align: text-bottom;
 }
-a.icon-only::after {
+.icon-only::after {
   content: " ";
 }
 

test/functional/issues_controller_test.rb

@@ -3761,6 +3761,44 @@ class IssuesControllerTest < ActionController::TestCase
     end
   end
 
+  def test_put_update_with_attachment_deletion_should_create_a_single_journal
+    set_tmp_attachments_directory
+    @request.session[:user_id] = 2
+
+    journal = new_record(Journal) do
+      assert_difference 'Attachment.count', -2 do
+        put :update,
+            :id => 3,
+            :issue => {
+              :notes => 'Removing attachments',
+              :deleted_attachment_ids => ['1', '5']
+            }
+      end
+    end
+    assert_equal 'Removing attachments', journal.notes
+    assert_equal 2, journal.details.count
+  end
+
+  def test_put_update_with_attachment_deletion_and_failure_should_preserve_selected_attachments
+    set_tmp_attachments_directory
+    @request.session[:user_id] = 2
+
+    assert_no_difference 'Journal.count' do
+      assert_no_difference 'Attachment.count' do
+        put :update,
+            :id => 3,
+            :issue => {
+              :subject => '',
+              :notes => 'Removing attachments',
+              :deleted_attachment_ids => ['1', '5']
+            }
+      end
+    end
+    assert_select 'input[name=?][value="1"][checked=checked]', 'issue[deleted_attachment_ids][]'
+    assert_select 'input[name=?][value="5"][checked=checked]', 'issue[deleted_attachment_ids][]'
+    assert_select 'input[name=?][value="6"]:not([checked])', 'issue[deleted_attachment_ids][]'
+  end
+
   def test_put_update_with_no_change
     issue = Issue.find(1)
     issue.journals.clear