Add support for updating attachments over REST API (#22356).

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

app/controllers/attachments_controller.rb

@@ -16,9 +16,10 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class AttachmentsController < ApplicationController
-  before_action :find_attachment, :only => [:show, :download, :thumbnail, :destroy]
+  before_action :find_attachment, :only => [:show, :download, :thumbnail, :update, :destroy]
   before_action :find_editable_attachments, :only => [:edit_all, :update_all]
   before_action :file_readable, :read_authorize, :only => [:show, :download, :thumbnail]
+  before_action :update_authorize, :only => :update
   before_action :delete_authorize, :only => :destroy
   before_action :authorize_global, :only => :upload
 
@@ -122,6 +123,21 @@ class AttachmentsController < ApplicationController
     render :action => 'edit_all'
   end
 
+  def update
+    @attachment.safe_attributes = params[:attachment]
+    saved = @attachment.save
+
+    respond_to do |format|
+      format.api {
+        if saved
+          render_api_ok
+        else
+          render_validation_errors(@attachment)
+        end
+      }
+    end
+  end
+
   def destroy
     if @attachment.container.respond_to?(:init_journal)
       @attachment.container.init_journal(User.current)
@@ -186,6 +202,10 @@ class AttachmentsController < ApplicationController
     @attachment.visible? ? true : deny_access
   end
 
+  def update_authorize
+    @attachment.editable? ? true : deny_access
+  end
+
   def delete_authorize
     @attachment.deletable? ? true : deny_access
   end

app/models/attachment.rb

@@ -19,6 +19,7 @@ require "digest/md5"
 require "fileutils"
 
 class Attachment < ActiveRecord::Base
+  include Redmine::SafeAttributes
   belongs_to :container, :polymorphic => true
   belongs_to :author, :class_name => "User"
 
@@ -56,6 +57,8 @@ class Attachment < ActiveRecord::Base
   after_rollback :delete_from_disk, :on => :create
   after_commit :delete_from_disk, :on => :destroy
 
+  safe_attributes 'filename', 'content_type', 'description'
+
   # Returns an unsaved copy of the attachment
   def copy(attributes=nil)
     copy = self.class.new

config/routes.rb

@@ -293,7 +293,7 @@ Rails.application.routes.draw do
   get 'attachments/download/:id/:filename', :to => 'attachments#download', :id => /\d+/, :filename => /.*/, :as => 'download_named_attachment'
   get 'attachments/download/:id', :to => 'attachments#download', :id => /\d+/
   get 'attachments/thumbnail/:id(/:size)', :to => 'attachments#thumbnail', :id => /\d+/, :size => /\d+/, :as => 'thumbnail'
-  resources :attachments, :only => [:show, :destroy]
+  resources :attachments, :only => [:show, :update, :destroy]
   get 'attachments/:object_type/:object_id/edit', :to => 'attachments#edit_all', :as => :object_attachments_edit
   patch 'attachments/:object_type/:object_id', :to => 'attachments#update_all', :as => :object_attachments
 

test/integration/api_test/api_routing_test.rb

@@ -21,6 +21,7 @@ class Redmine::ApiTest::ApiRoutingTest < Redmine::ApiTest::Routing
 
   def test_attachments
     should_route 'GET /attachments/1' => 'attachments#show', :id => '1'
+    should_route 'PATCH /attachments/1' => 'attachments#update', :id => '1'
     should_route 'POST /uploads' => 'attachments#upload'
   end
 

test/integration/api_test/attachments_test.rb

@@ -99,6 +99,29 @@ class Redmine::ApiTest::AttachmentsTest < Redmine::ApiTest::Base
     assert_nil Attachment.find_by_id(7)
   end
 
+  test "PATCH /attachments/:id.json should update the attachment" do
+    patch '/attachments/7.json',
+      {:attachment => {:filename => 'renamed.zip', :description => 'updated'}},
+      credentials('jsmith')
+
+    assert_response :ok
+    assert_equal 'application/json', response.content_type
+    attachment = Attachment.find(7)
+    assert_equal 'renamed.zip', attachment.filename
+    assert_equal 'updated', attachment.description
+  end
+
+  test "PATCH /attachments/:id.json with failure should return the errors" do
+    patch '/attachments/7.json',
+      {:attachment => {:filename => '', :description => 'updated'}},
+      credentials('jsmith')
+
+    assert_response 422
+    assert_equal 'application/json', response.content_type
+    json = ActiveSupport::JSON.decode(response.body)
+    assert_include "File cannot be blank", json['errors']
+  end
+
   test "POST /uploads.xml should return the token" do
     set_tmp_attachments_directory
     assert_difference 'Attachment.count' do

test/integration/routing/attachments_test.rb

@@ -30,7 +30,7 @@ class RoutingAttachmentsTest < Redmine::RoutingTest
 
     should_route 'DELETE /attachments/1' => 'attachments#destroy', :id => '1'
 
-    should_route 'GET /attachments/issues/1/edit' => 'attachments#edit', :object_type => 'issues', :object_id => '1'
-    should_route 'PATCH /attachments/issues/1' => 'attachments#update', :object_type => 'issues', :object_id => '1'
+    should_route 'GET /attachments/issues/1/edit' => 'attachments#edit_all', :object_type => 'issues', :object_id => '1'
+    should_route 'PATCH /attachments/issues/1' => 'attachments#update_all', :object_type => 'issues', :object_id => '1'
   end
 end