From 427450051881415b3ea833dac454a033c86c912d Mon Sep 17 00:00:00 2001 From: ishikawa999 <14245262+ishikawa999@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:01:13 +0900 Subject: [PATCH] Add REST API for documents --- app/controllers/documents_controller.rb | 97 +++-- app/views/documents/index.api.rsb | 15 + app/views/documents/show.api.rsb | 17 + test/functional/documents_controller_test.rb | 21 + test/integration/api_test/documents_test.rb | 399 +++++++++++++++++++ 5 files changed, 525 insertions(+), 24 deletions(-) create mode 100644 app/views/documents/index.api.rsb create mode 100644 app/views/documents/show.api.rsb create mode 100644 test/integration/api_test/documents_test.rb diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb index 4e2c255d5..f4cc78152 100644 --- a/app/controllers/documents_controller.rb +++ b/app/controllers/documents_controller.rb @@ -25,30 +25,50 @@ class DocumentsController < ApplicationController before_action :find_model_object, :except => [:index, :new, :create] before_action :find_project_from_association, :except => [:index, :new, :create] before_action :authorize + accept_api_auth :index, :show, :create, :update, :destroy helper :attachments helper :custom_fields def index - @sort_by = %w(category date title author).include?(params[:sort_by]) ? params[:sort_by] : 'category' - documents = @project.documents.includes(:attachments, :category).to_a - case @sort_by - when 'date' - documents.sort!{|a, b| b.updated_on <=> a.updated_on} - @grouped = documents.group_by {|d| d.updated_on.to_date} - when 'title' - @grouped = documents.group_by {|d| d.title.first.upcase} - when 'author' - @grouped = documents.select{|d| d.attachments.any?}.group_by {|d| d.attachments.last.author} - else - @grouped = documents.group_by(&:category) + respond_to do |format| + format.html do + @sort_by = %w(category date title author).include?(params[:sort_by]) ? params[:sort_by] : 'category' + documents = @project.documents.includes(:attachments, :category).to_a + case @sort_by + when 'date' + documents.sort!{|a, b| b.updated_on <=> a.updated_on} + @grouped = documents.group_by {|d| d.updated_on.to_date} + when 'title' + @grouped = documents.group_by {|d| d.title.first.upcase} + when 'author' + @grouped = documents.select{|d| d.attachments.any?}.group_by {|d| d.attachments.last.author} + else + @grouped = documents.group_by(&:category) + end + @document = @project.documents.build + render :layout => false if request.xhr? + end + format.api do + @offset, @limit = api_offset_and_limit + scope = @project.documents + @document_count = scope.count + @documents = scope.includes(:project, :category, :attachments). + preload(:custom_values => :custom_field). + order("#{Document.table_name}.created_on DESC, #{Document.table_name}.id DESC"). + limit(@limit). + offset(@offset). + to_a + end end - @document = @project.documents.build - render :layout => false if request.xhr? end def show @attachments = @document.attachments.to_a + respond_to do |format| + format.html + format.api + end end def new @@ -59,13 +79,26 @@ class DocumentsController < ApplicationController def create @document = @project.documents.build @document.safe_attributes = params[:document] - @document.save_attachments(params[:attachments]) + attachments = params[:attachments] + attachments ||= params[:document] && params[:document][:uploads] if api_request? + @document.save_attachments(attachments) if @document.save - render_attachment_warning_if_needed(@document) - flash[:notice] = l(:notice_successful_create) - redirect_to project_documents_path(@project) + respond_to do |format| + format.html do + render_attachment_warning_if_needed(@document) + flash[:notice] = l(:notice_successful_create) + redirect_to project_documents_path(@project) + end + format.api do + @attachments = @document.attachments.to_a + render :action => 'show', :status => :created, :location => document_url(@document) + end + end else - render :action => 'new' + respond_to do |format| + format.html {render :action => 'new'} + format.api {render_validation_errors(@document)} + end end end @@ -74,18 +107,34 @@ class DocumentsController < ApplicationController def update @document.safe_attributes = params[:document] + if api_request? + @document.save_attachments(params[:attachments] || (params[:document] && params[:document][:uploads])) + end if @document.save - flash[:notice] = l(:notice_successful_update) - redirect_to document_path(@document) + respond_to do |format| + format.html do + flash[:notice] = l(:notice_successful_update) + redirect_to document_path(@document) + end + format.api {render_api_ok} + end else - render :action => 'edit' + respond_to do |format| + format.html {render :action => 'edit'} + format.api {render_validation_errors(@document)} + end end end def destroy @document.destroy if request.delete? - flash[:notice] = l(:notice_successful_delete) - redirect_to project_documents_path(@project) + respond_to do |format| + format.html do + flash[:notice] = l(:notice_successful_delete) + redirect_to project_documents_path(@project) + end + format.api {render_api_ok} + end end def add_attachment diff --git a/app/views/documents/index.api.rsb b/app/views/documents/index.api.rsb new file mode 100644 index 000000000..c212459c5 --- /dev/null +++ b/app/views/documents/index.api.rsb @@ -0,0 +1,15 @@ +api.array :documents, api_meta(:total_count => @document_count, :offset => @offset, :limit => @limit) do + @documents.each do |document| + api.document do + api.id document.id + api.project(:id => document.project_id, :name => document.project.name) unless document.project.nil? + api.category(:id => document.category_id, :name => document.category.name) unless document.category.nil? + api.title document.title + api.description document.description + api.created_on document.created_on + api.updated_on document.updated_on + + render_api_custom_values document.visible_custom_field_values, api + end + end +end diff --git a/app/views/documents/show.api.rsb b/app/views/documents/show.api.rsb new file mode 100644 index 000000000..7b0fd1afd --- /dev/null +++ b/app/views/documents/show.api.rsb @@ -0,0 +1,17 @@ +api.document do + api.id @document.id + api.project(:id => @document.project_id, :name => @document.project.name) unless @document.project.nil? + api.category(:id => @document.category_id, :name => @document.category.name) unless @document.category.nil? + api.title @document.title + api.description @document.description + api.created_on @document.created_on + api.updated_on @document.updated_on + + render_api_custom_values @document.visible_custom_field_values, api + + api.array :attachments do + @attachments.each do |attachment| + render_api_attachment(attachment, api) + end + end if include_in_api_response?('attachments') +end diff --git a/test/functional/documents_controller_test.rb b/test/functional/documents_controller_test.rb index f82627f03..beff79d64 100644 --- a/test/functional/documents_controller_test.rb +++ b/test/functional/documents_controller_test.rb @@ -237,6 +237,27 @@ class DocumentsControllerTest < Redmine::ControllerTest assert_equal 'test_update', document.title end + def test_update_should_not_add_attachments + @request.session[:user_id] = 2 + set_tmp_attachments_directory + + assert_no_difference 'Attachment.count' do + put( + :update, + :params => { + :id => 1, + :document => {:title => 'test_update'}, + :attachments => { + '1' => { + 'file' => uploaded_test_file('testfile.txt', 'text/plain') + } + } + } + ) + end + assert_redirected_to '/documents/1' + end + def test_update_with_failure @request.session[:user_id] = 2 put( diff --git a/test/integration/api_test/documents_test.rb b/test/integration/api_test/documents_test.rb new file mode 100644 index 000000000..1233cbd84 --- /dev/null +++ b/test/integration/api_test/documents_test.rb @@ -0,0 +1,399 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require_relative '../../test_helper' + +class Redmine::ApiTest::DocumentsTest < Redmine::ApiTest::Base + test "GET /projects/:project_id/documents.xml should return documents" do + get '/projects/ecookbook/documents.xml' + + assert_response :success + assert_equal 'application/xml', response.media_type + assert_select 'documents[type=array][total_count=?]', Project.find(1).documents.count.to_s do + assert_select 'document', 3 + # documents are sorted by descending creation date + assert_select 'document:first-of-type' do + assert_select 'id', :text => '3' + assert_select 'project[id=1][name=eCookbook]' + assert_select 'category[id=3][name="Technical documentation"]' + assert_select 'title', :text => 'An other document 2' + assert_select 'created_on', :text => Document.find(3).created_on.iso8601 + assert_select 'updated_on', :text => Document.find(3).updated_on.iso8601 + end + end + end + + test "GET /projects/:project_id/documents.json should return documents" do + get '/projects/ecookbook/documents.json' + + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert_kind_of Hash, json + assert_kind_of Array, json['documents'] + assert_equal Project.find(1).documents.count, json['total_count'] + # documents are sorted by descending creation date + assert_equal 3, json['documents'].first['id'] + assert_equal({'id' => 3, 'name' => 'Technical documentation'}, json['documents'].first['category']) + end + + test "GET /projects/:project_id/documents.xml should paginate" do + get '/projects/ecookbook/documents.xml', :params => {:limit => 2, :offset => 1} + + assert_response :success + assert_select 'documents[type=array][total_count="3"][offset="1"][limit="2"]' do + assert_select 'document', 2 + assert_select 'document id', :text => '2' + assert_select 'document id', :text => '1' + end + end + + test "GET /projects/:project_id/documents.xml should deny access without permission" do + Role.anonymous.remove_permission! :view_documents + + get '/projects/ecookbook/documents.xml' + assert_response :unauthorized + end + + test "GET /projects/:project_id/documents.xml without permission should return 403" do + Role.find(1).remove_permission! :view_documents + + get '/projects/ecookbook/documents.xml', :headers => credentials('jsmith') + + assert_response :forbidden + end + + test "GET /documents/:id.xml should return the document" do + get '/documents/1.xml' + + assert_response :success + assert_equal 'application/xml', response.media_type + assert_select 'document' do + assert_select 'id', :text => '1' + assert_select 'project[id=1][name=eCookbook]' + assert_select 'category[id=1][name=Uncategorized]' + assert_select 'title', :text => 'Test document' + assert_select 'description', :text => 'Document description' + assert_select 'created_on', :text => Document.find(1).created_on.iso8601 + assert_select 'updated_on', :text => Document.find(1).updated_on.iso8601 + assert_select 'attachments', 0 + end + end + + test "GET /documents/:id.json should return the document" do + get '/documents/1.json' + + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert_kind_of Hash, json['document'] + assert_equal 1, json['document']['id'] + assert_equal 'Test document', json['document']['title'] + end + + test "GET /documents/:id.xml with include=attachments should include attachments" do + get '/documents/1.xml?include=attachments' + + assert_select 'document attachments[type=array]' do + assert_select 'attachment', 2 + assert_select 'attachment id', :text => '2' do + assert_select '~ filename', :text => 'document.txt' + assert_select '~ content_url', + :text => 'http://www.example.com/attachments/download/2/document.txt' + end + end + end + + test "GET /documents/:id.xml should include custom fields" do + field = DocumentCustomField.generate!(:name => 'Author', :field_format => 'string') + document = Document.find(1) + document.custom_field_values = {field.id => 'John Smith'} + document.save! + + get '/documents/1.xml' + + assert_response :success + assert_select 'document custom_fields[type=array]' do + assert_select "custom_field[id=\"#{field.id}\"][name=Author]" do + assert_select 'value', :text => 'John Smith' + end + end + end + + test "GET /documents/:id.xml on a private project without credentials should return 401" do + Document.find(1).project.update_column :is_public, false + + get '/documents/1.xml' + + assert_response :unauthorized + end + + test "POST /projects/:project_id/documents.xml should create a document with the attributes" do + payload = <<~XML + + + API document + This is a document created by the API + 2 + + XML + assert_difference('Document.count') do + post( + '/projects/1/documents.xml', + :params => payload, + :headers => {"CONTENT_TYPE" => 'application/xml'}.merge(credentials('jsmith'))) + end + assert_response :created + assert_equal 'application/xml', response.media_type + + document = Document.order(:id => :desc).first + assert_equal 'API document', document.title + assert_equal 'This is a document created by the API', document.description + assert_equal DocumentCategory.find(2), document.category + assert_equal Project.find(1), document.project + assert_equal document_url(document), response.headers['Location'] + assert_select 'document id', :text => document.id.to_s + end + + test "POST /projects/:project_id/documents.json should create a document with the attributes" do + payload = <<~JSON + { + "document": { + "title": "API document", + "description": "This is a document created by the API", + "category_id": 2 + } + } + JSON + assert_difference('Document.count') do + post( + '/projects/1/documents.json', + :params => payload, + :headers => {"CONTENT_TYPE" => 'application/json'}.merge(credentials('jsmith'))) + end + assert_response :created + + document = Document.order(:id => :desc).first + assert_equal 'API document', document.title + assert_equal 'This is a document created by the API', document.description + + json = ActiveSupport::JSON.decode(response.body) + assert_equal document.id, json['document']['id'] + end + + test "POST /projects/:project_id/documents.json with failure should return errors" do + assert_no_difference('Document.count') do + post( + '/projects/1/documents.json', + :params => {:document => {:title => '', :category_id => 1}}, + :headers => credentials('jsmith')) + end + assert_response :unprocessable_content + json = ActiveSupport::JSON.decode(response.body) + assert json['errors'].include?("Title cannot be blank") + end + + test "POST /projects/:project_id/documents.json with attachment should create a document with attachment" do + token = json_upload('test_create_with_attachment', credentials('jsmith')) + attachment = Attachment.find_by_token(token) + assert_difference 'Document.count' do + post( + '/projects/1/documents.json', + :params => {:document => {:title => 'API document with attachment', + :category_id => 1, + :uploads => [{:token => token, :filename => 'test.txt', + :content_type => 'text/plain'}]}}, + :headers => credentials('jsmith')) + assert_response :created + end + document = Document.order(:id => :desc).first + assert_equal 'API document with attachment', document.title + assert_equal attachment, document.attachments.first + + attachment.reload + assert_equal 'test.txt', attachment.filename + assert_equal 'text/plain', attachment.content_type + assert_equal 'test_create_with_attachment'.size, attachment.filesize + assert_equal 2, attachment.author_id + end + + test "POST /projects/:project_id/documents.xml with attachment should create a document with attachment" do + token = xml_upload('test_create_with_attachment', credentials('jsmith')) + payload = <<~XML + + + API document with attachment + 1 + + + #{token} + test.txt + + + + XML + assert_difference 'Document.count' do + post( + '/projects/1/documents.xml', + :params => payload, + :headers => {"CONTENT_TYPE" => 'application/xml'}.merge(credentials('jsmith'))) + assert_response :created + end + document = Document.order(:id => :desc).first + assert_equal ['test.txt'], document.attachments.map(&:filename) + end + + test "POST /projects/:project_id/documents.xml without permission should return 403" do + Role.find(1).remove_permission! :add_documents + + assert_no_difference 'Document.count' do + post( + '/projects/1/documents.xml', + :params => {:document => {:title => 'API document'}}, + :headers => credentials('jsmith')) + end + assert_response :forbidden + end + + test "PUT /documents/:id.xml should update the document" do + payload = <<~XML + + + Updated title + Updated description + 2 + + XML + put( + '/documents/1.xml', + :params => payload, + :headers => {"CONTENT_TYPE" => 'application/xml'}.merge(credentials('jsmith'))) + + assert_response :no_content + assert_equal '', response.body + document = Document.find(1) + assert_equal 'Updated title', document.title + assert_equal 'Updated description', document.description + assert_equal DocumentCategory.find(2), document.category + end + + test "PUT /documents/:id.xml with failure should return errors" do + put( + '/documents/1.xml', + :params => {:document => {:title => ''}}, + :headers => credentials('jsmith')) + + assert_response :unprocessable_content + assert_select 'errors error', :text => "Title cannot be blank" + end + + test "PUT /documents/:id.json should update the document" do + put( + '/documents/1.json', + :params => {:document => {:title => 'Updated title'}}, + :headers => credentials('jsmith')) + + assert_response :no_content + assert_equal 'Updated title', Document.find(1).title + end + + test "PUT /documents/:id.json with failure should return errors" do + put( + '/documents/1.json', + :params => {:document => {:title => ''}}, + :headers => credentials('jsmith')) + + assert_response :unprocessable_content + json = ActiveSupport::JSON.decode(response.body) + assert json['errors'].include?("Title cannot be blank") + end + + test "PUT /documents/:id.json with attachment should add the attachment" do + token = json_upload('test_update_with_attachment', credentials('jsmith')) + + put( + '/documents/2.json', + :params => {:document => {:uploads => [{:token => token, :filename => 'test.txt', + :content_type => 'text/plain'}]}}, + :headers => credentials('jsmith')) + + assert_response :no_content + assert_equal ['test.txt'], Document.find(2).attachments.map(&:filename) + end + + test "PUT /documents/:id.xml with attachment should add the attachment" do + token = xml_upload('test_update_with_attachment', credentials('jsmith')) + payload = <<~XML + + + + + #{token} + test.txt + + + + XML + put( + '/documents/2.xml', + :params => payload, + :headers => {"CONTENT_TYPE" => 'application/xml'}.merge(credentials('jsmith'))) + + assert_response :no_content + assert_equal ['test.txt'], Document.find(2).attachments.map(&:filename) + end + + test "PUT /documents/:id.xml without permission should return 403" do + Role.find(1).remove_permission! :edit_documents + + put( + '/documents/1.xml', + :params => {:document => {:title => 'Updated title'}}, + :headers => credentials('jsmith')) + + assert_response :forbidden + end + + test "DELETE /documents/:id.xml should delete the document" do + assert_difference('Document.count', -1) do + delete '/documents/1.xml', :headers => credentials('jsmith') + end + + assert_response :no_content + assert_equal '', response.body + assert_nil Document.find_by_id(1) + end + + test "DELETE /documents/:id.json should delete the document" do + assert_difference('Document.count', -1) do + delete '/documents/1.json', :headers => credentials('jsmith') + end + + assert_response :no_content + assert_nil Document.find_by_id(1) + end + + test "DELETE /documents/:id.xml without permission should return 403" do + Role.find(1).remove_permission! :delete_documents + + assert_no_difference 'Document.count' do + delete '/documents/1.xml', :headers => credentials('jsmith') + end + + assert_response :forbidden + end +end -- 2.55.0