From 696c51085246058408c5f709927df8db07b90d54 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 1 Oct 2016 13:22:35 +0000 Subject: [PATCH] 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 | 22 +++++++++++++++++- app/models/attachment.rb | 3 +++ config/routes.rb | 2 +- test/integration/api_test/api_routing_test.rb | 1 + test/integration/api_test/attachments_test.rb | 23 +++++++++++++++++++ test/integration/routing/attachments_test.rb | 4 ++-- 6 files changed, 51 insertions(+), 4 deletions(-) diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index f002c907c..ad0b1e4b8 100644 --- 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 4bc674f8d..c9283aed7 100644 --- 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 7fe0ae95c..5795dc09c 100644 --- 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 ccdb93307..5e686a9e2 100644 --- 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 c7d2869ee..312920922 100644 --- 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 a29a0073c..4c46e9d96 100644 --- 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 -- 2.39.5