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

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

diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index f002c907cd861175f63e1d0ab544f92df945d1e0..ad0b1e4b888e6828590ab6a2fc1e3c8b3b7db22d 100644 (file)
--- a/app/controllers/attachments_controller.rb
+++ b/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

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 4bc674f8d752dba91a95ebd300b6c45b89baa3ac..c9283aed783202b8c780763bd3747ba02935994f 100644 (file)
--- a/app/models/attachment.rb
+++ b/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

diff --git a/config/routes.rb b/config/routes.rb
index 7fe0ae95c5352de87f19aedf60c0e98e8f9bfd04..5795dc09cfe37f7025436bc4b145bdc1fd9a020b 100644 (file)
--- a/config/routes.rb
+++ b/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
 

diff --git a/test/integration/api_test/api_routing_test.rb b/test/integration/api_test/api_routing_test.rb
index ccdb93307538e255bab4c469e1c1e180a5a35806..5e686a9e2b7239dcc33fd0ef42fc66ac7897b2e9 100644 (file)
--- a/test/integration/api_test/api_routing_test.rb
+++ b/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
 

diff --git a/test/integration/api_test/attachments_test.rb b/test/integration/api_test/attachments_test.rb
index c7d2869ee5690f050d7602cc81e4d92ad0d72f86..3129209229ffe702b2e22527980b646a00688d1a 100644 (file)
--- a/test/integration/api_test/attachments_test.rb
+++ b/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

diff --git a/test/integration/routing/attachments_test.rb b/test/integration/routing/attachments_test.rb
index a29a0073c73ddc479d87ba736f826826c89b3c12..4c46e9d968ae52ea3961b5031404c4d7a925c2fc 100644 (file)
--- a/test/integration/routing/attachments_test.rb
+++ b/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