Merged r22122 from trunk to 5.0-stable (#38297).

git-svn-id: https://svn.redmine.org/redmine/branches/5.0-stable@22123 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/issues_controller.rb

@@ -190,8 +190,16 @@ class IssuesController < ApplicationController
   def update
     return unless update_issue_from_params
 
-    @issue.save_attachments(params[:attachments] ||
-                             (params[:issue] && params[:issue][:uploads]))
+    attachments = params[:attachments] || params.dig(:issue, :uploads)
+    if @issue.attachments_addable?
+      @issue.save_attachments(attachments)
+    else
+      attachments = attachments.to_unsafe_hash if attachments.respond_to?(:to_unsafe_hash)
+      if [Hash, Array].any? { |klass| attachments.is_a?(klass) } && attachments.any?
+        flash[:warning] = l(:warning_attachments_not_saved, attachments.size)
+      end
+    end
+
     saved = false
     begin
       saved = save_issue_with_child_records

app/models/issue.rb

@@ -195,6 +195,10 @@ class Issue < ActiveRecord::Base
     )
   end
 
+  def attachments_addable?(user=User.current)
+    attributes_editable?(user) || notes_addable?(user)
+  end
+
   # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_editable?
   def attachments_editable?(user=User.current)
     attributes_editable?(user)

app/views/issues/_edit.html.erb

@@ -42,7 +42,8 @@
 
       <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %>
       </fieldset>
-
+    <% end %>
+    <% if @issue.attachments_addable? %>
       <fieldset id="add_attachments"><legend><%= l(:label_attachment_plural) %></legend>
         <% 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>

test/integration/issues_test.rb

@@ -139,6 +139,80 @@ class IssuesTest < Redmine::IntegrationTest
     assert_equal 0, Issue.find(1).attachments.length
   end
 
+  def test_edit_add_attachment_form
+    log_user('jsmith', 'jsmith')
+    role = Role.find(1)
+
+    role.add_permission! :edit_issues
+    role.remove_permission! :edit_own_issues
+    role.remove_permission! :add_issue_notes
+
+    get '/issues/1'
+    assert_response :success
+    assert_select 'div#new-attachments', 1
+
+    get '/issues/1/edit'
+    assert_response :success
+    assert_select 'div#new-attachments', 1
+
+    role.remove_permission! :edit_issues
+    role.add_permission! :edit_own_issues
+    role.remove_permission! :add_issue_notes
+
+    get '/issues/1'
+    assert_response :success
+    assert_select 'div#new-attachments', 1
+
+    get '/issues/1/edit'
+    assert_response :success
+    assert_select 'div#new-attachments', 1
+
+    role.remove_permission! :edit_issues
+    role.remove_permission! :edit_own_issues
+    role.add_permission! :add_issue_notes
+
+    get '/issues/1'
+    assert_response :success
+    assert_select 'div#new-attachments', 1
+
+    get '/issues/1/edit'
+    assert_response :success
+    assert_select 'div#new-attachments', 1
+  end
+
+  def test_edit_check_permission_for_add_attachment
+    log_user('jsmith', 'jsmith')
+    role = Role.find(1)
+
+    role.remove_permission! :edit_issues
+    role.remove_permission! :edit_own_issues
+    role.add_permission! :add_issue_notes
+
+    role.permissions_all_trackers = {'view_issues' => '0', 'add_issue_notes' => '0' }
+    role.permissions_tracker_ids = {'view_issues' => ['1'], 'add_issue_notes' => ['2'] }
+    role.save!
+
+    assert_no_difference 'Attachment.count' do
+      put(
+        '/issues/1',
+        :params => {
+          :issue => {:notes => 'Some notes'},
+          :attachments => {
+            '1' => {
+              'file' => uploaded_test_file('testfile.txt', 'text/plain'),
+              'description' => 'This is an attachment'
+            }
+          }
+        }
+      )
+    end
+    assert_redirected_to '/issues/1'
+
+    follow_redirect!
+    assert_response :success
+    assert_select '.flash', '1 file(s) could not be saved.'
+  end
+
   def test_next_and_previous_links_should_be_displayed_after_query_grouped_and_sorted_by_version
     with_settings :default_language => 'en' do
       get '/projects/ecookbook/issues?set_filter=1&group_by=fixed_version&sort=priority:desc,fixed_version,id'