From 93ee61d1176debec5d030ce0b260632634e8c279 Mon Sep 17 00:00:00 2001 From: jplang Date: Wed, 13 Jul 2016 20:04:14 +0000 Subject: [PATCH] Ability to delete multiple attachments while updating an issue (#13072). git-svn-id: https://svn.redmine.org/redmine/trunk@15650 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue.rb | 17 +++++++++- app/views/issues/_edit.html.erb | 24 ++++++++++++-- public/javascripts/attachments.js | 5 +++ public/stylesheets/application.css | 14 ++++----- test/functional/issues_controller_test.rb | 38 +++++++++++++++++++++++ 5 files changed, 88 insertions(+), 10 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 5b6ae3041..d5a78717d 100644 --- a/app/models/issue.rb +++ b/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? diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb index 67e33246d..3291ba737 100644 --- a/app/views/issues/_edit.html.erb +++ b/app/views/issues/_edit.html.erb @@ -38,9 +38,29 @@ <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %> - +
<%= l(:label_attachment_plural) %> -

<%= render :partial => 'attachments/form', :locals => {:container => @issue} %>

+ <% if @issue.attachments.any? && @issue.safe_attribute?('deleted_attachment_ids') %> +
<%= link_to l(:label_edit_attachments), '#', :onclick => "$('#existing-attachments').toggle(); return false;" %>
+
+ <% @issue.attachments.each do |attachment| %> + + <%= text_field_tag '', attachment.filename, :class => "filename", :disabled => true %> + + + <% end %> +
+
+ <% end %> + +
+ <%= render :partial => 'attachments/form', :locals => {:container => @issue} %> +
<% end %> diff --git a/public/javascripts/attachments.js b/public/javascripts/attachments.js index f33524543..c5e6b962a 100644 --- a/public/javascripts/attachments.js +++ b/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(); +}); diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 9816c6508..8c936bd33 100644 --- a/public/stylesheets/application.css +++ b/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: " "; } diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index d571d4bda..d878b9608 100644 --- a/test/functional/issues_controller_test.rb +++ b/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 -- 2.27.0