From 21b37187445f03c9395f5fc36b5bd1a42d99d980 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Mon, 4 Jul 2011 17:03:04 +0000 Subject: [PATCH] Adds REST API for issue relations (#7366). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@6176 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issue_relations_controller.rb | 43 +++++++--- app/models/issue.rb | 5 ++ app/views/issue_relations/show.api.rsb | 7 ++ app/views/issues/_relations.rhtml | 4 +- app/views/issues/show.api.rsb | 2 +- config/routes.rb | 7 +- lib/redmine.rb | 2 +- .../issue_relations_controller_test.rb | 39 ++++++--- .../api_test/issue_relations_test.rb | 83 +++++++++++++++++++ test/integration/routing_test.rb | 13 ++- 10 files changed, 172 insertions(+), 33 deletions(-) create mode 100644 app/views/issue_relations/show.api.rsb create mode 100644 test/integration/api_test/issue_relations_test.rb diff --git a/app/controllers/issue_relations_controller.rb b/app/controllers/issue_relations_controller.rb index b095df37a..094036768 100644 --- a/app/controllers/issue_relations_controller.rb +++ b/app/controllers/issue_relations_controller.rb @@ -1,5 +1,5 @@ -# redMine - project management software -# Copyright (C) 2006-2007 Jean-Philippe Lang +# Redmine - project management software +# Copyright (C) 2006-2011 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 @@ -17,14 +17,28 @@ class IssueRelationsController < ApplicationController before_filter :find_issue, :find_project_from_association, :authorize + accept_key_auth :show, :create, :destroy - def new + def show + @relation = @issue.find_relation(params[:id]) + + respond_to do |format| + format.html { render :nothing => true } + format.api + end + rescue ActiveRecord::RecordNotFound + render_404 + end + + verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed } + def create @relation = IssueRelation.new(params[:relation]) @relation.issue_from = @issue if params[:relation] && m = params[:relation][:issue_to_id].to_s.match(/^#?(\d+)$/) @relation.issue_to = Issue.visible.find_by_id(m[1].to_i) end - @relation.save if request.post? + saved = @relation.save + respond_to do |format| format.html { redirect_to :controller => 'issues', :action => 'show', :id => @issue } format.js do @@ -37,22 +51,31 @@ class IssueRelationsController < ApplicationController end end end + format.api { + if saved + render :action => 'show', :status => :created, :location => issue_relation_url(@issue, @relation) + else + render_validation_errors(@relation) + end + } end end + verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy - relation = IssueRelation.find(params[:id]) - if request.post? && @issue.relations.include?(relation) - relation.destroy - @issue.reload - end + relation = @issue.find_relation(params[:id]) + relation.destroy + respond_to do |format| format.html { redirect_to :controller => 'issues', :action => 'show', :id => @issue } format.js { - @relations = @issue.relations.select {|r| r.other_issue(@issue) && r.other_issue(@issue).visible? } + @relations = @issue.reload.relations.select {|r| r.other_issue(@issue) && r.other_issue(@issue).visible? } render(:update) {|page| page.replace_html "relations", :partial => 'issues/relations'} } + format.api { head :ok } end + rescue ActiveRecord::RecordNotFound + render_404 end private diff --git a/app/models/issue.rb b/app/models/issue.rb index 79c491547..5ac166270 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -501,6 +501,11 @@ class Issue < ActiveRecord::Base def relations (relations_from + relations_to).sort end + + # Finds an issue relation given its id. + def find_relation(relation_id) + IssueRelation.find(relation_id, :conditions => ["issue_to_id = ? OR issue_from_id = ?", id, id]) + end def all_dependent_issues(except=[]) except << self diff --git a/app/views/issue_relations/show.api.rsb b/app/views/issue_relations/show.api.rsb new file mode 100644 index 000000000..0a3e2918a --- /dev/null +++ b/app/views/issue_relations/show.api.rsb @@ -0,0 +1,7 @@ +api.relation do + api.id @relation.id + api.issue_id @relation.issue_from_id + api.issue_to_id @relation.issue_to_id + api.relation_type @relation.relation_type_for(@issue) + api.delay @relation.delay +end diff --git a/app/views/issues/_relations.rhtml b/app/views/issues/_relations.rhtml index 19ae08b04..065f8da9b 100644 --- a/app/views/issues/_relations.rhtml +++ b/app/views/issues/_relations.rhtml @@ -20,7 +20,7 @@ <%= format_date(relation.other_issue(@issue).start_date) %> <%= format_date(relation.other_issue(@issue).due_date) %> <%= link_to_remote(image_tag('link_break.png'), { :url => {:controller => 'issue_relations', :action => 'destroy', :issue_id => @issue, :id => relation}, - :method => :post + :method => :delete }, :title => l(:label_relation_delete)) if authorize_for('issue_relations', 'destroy') %> <% end %> @@ -29,7 +29,7 @@ <% end %> <% remote_form_for(:relation, @relation, - :url => {:controller => 'issue_relations', :action => 'new', :issue_id => @issue}, + :url => {:controller => 'issue_relations', :action => 'create', :issue_id => @issue}, :method => :post, :complete => "Form.Element.focus('relation_issue_to_id');", :html => {:id => 'new-relation-form', :style => (@relation ? '' : 'display: none;')}) do |f| %> diff --git a/app/views/issues/show.api.rsb b/app/views/issues/show.api.rsb index 170aeb375..3493a29cd 100644 --- a/app/views/issues/show.api.rsb +++ b/app/views/issues/show.api.rsb @@ -27,7 +27,7 @@ api.issue do api.array :relations do @relations.each do |relation| - api.relation(:id => relation.id, :issue_id => relation.other_issue(@issue).id, :relation_type => relation.relation_type_for(@issue), :delay => relation.delay) + api.relation(:id => relation.id, :issue_id => relation.issue_from_id, :issue_to_id => relation.issue_to_id, :relation_type => relation.relation_type_for(@issue), :delay => relation.delay) end end if include_in_api_response?('relations') && @relations.present? diff --git a/config/routes.rb b/config/routes.rb index a019881a8..7bb6b0b53 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,17 +110,13 @@ ActionController::Routing::Routes.draw do |map| map.resources :issues, :member => { :edit => :post }, :collection => {} do |issues| issues.resources :time_entries, :controller => 'timelog' + issues.resources :relations, :controller => 'issue_relations', :only => [:show, :create, :destroy] end map.resources :issues, :path_prefix => '/projects/:project_id', :collection => { :create => :post } do |issues| issues.resources :time_entries, :controller => 'timelog' end - map.with_options :controller => 'issue_relations', :conditions => {:method => :post} do |relations| - relations.connect 'issues/:issue_id/relations/:id', :action => 'new' - relations.connect 'issues/:issue_id/relations/:id/destroy', :action => 'destroy' - end - map.connect 'projects/:id/members/new', :controller => 'members', :action => 'new' map.with_options :controller => 'users' do |users| @@ -235,7 +231,6 @@ ActionController::Routing::Routes.draw do |map| map.connect 'projects/:project_id/boards/:action/:id', :controller => 'boards' map.connect 'boards/:board_id/topics/:action/:id', :controller => 'messages' map.connect 'wiki/:id/:page/:action', :page => nil, :controller => 'wiki' - map.connect 'issues/:issue_id/relations/:action/:id', :controller => 'issue_relations' map.connect 'projects/:project_id/news/:action', :controller => 'news' map.connect 'projects/:project_id/timelog/:action/:id', :controller => 'timelog', :project_id => /.+/ map.with_options :controller => 'repositories' do |omap| diff --git a/lib/redmine.rb b/lib/redmine.rb index 283f0250f..e08a5eb01 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -69,7 +69,7 @@ Redmine::AccessControl.map do |map| :reports => [:issue_report, :issue_report_details]} map.permission :add_issues, {:issues => [:new, :create, :update_form]} map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new]} - map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]} + map.permission :manage_issue_relations, {:issue_relations => [:show, :create, :destroy]} map.permission :manage_subtasks, {} map.permission :set_issues_private, {} map.permission :set_own_issues_private, {}, :require => :loggedin diff --git a/test/functional/issue_relations_controller_test.rb b/test/functional/issue_relations_controller_test.rb index 72f13eb9c..3f1408785 100644 --- a/test/functional/issue_relations_controller_test.rb +++ b/test/functional/issue_relations_controller_test.rb @@ -1,3 +1,20 @@ +# Redmine - project management software +# Copyright (C) 2006-2011 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 File.expand_path('../../test_helper', __FILE__) require 'issue_relations_controller' @@ -25,18 +42,18 @@ class IssueRelationsControllerTest < ActionController::TestCase User.current = nil end - def test_new + def test_create assert_difference 'IssueRelation.count' do @request.session[:user_id] = 3 - post :new, :issue_id => 1, + post :create, :issue_id => 1, :relation => {:issue_to_id => '2', :relation_type => 'relates', :delay => ''} end end - def test_new_xhr + def test_create_xhr assert_difference 'IssueRelation.count' do @request.session[:user_id] = 3 - xhr :post, :new, + xhr :post, :create, :issue_id => 3, :relation => {:issue_to_id => '1', :relation_type => 'relates', :delay => ''} assert_select_rjs 'relations' do @@ -46,19 +63,19 @@ class IssueRelationsControllerTest < ActionController::TestCase end end - def test_new_should_accept_id_with_hash + def test_create_should_accept_id_with_hash assert_difference 'IssueRelation.count' do @request.session[:user_id] = 3 - post :new, :issue_id => 1, + post :create, :issue_id => 1, :relation => {:issue_to_id => '#2', :relation_type => 'relates', :delay => ''} end end - def test_new_should_not_break_with_non_numerical_id + def test_create_should_not_break_with_non_numerical_id assert_no_difference 'IssueRelation.count' do assert_nothing_raised do @request.session[:user_id] = 3 - post :new, :issue_id => 1, + post :create, :issue_id => 1, :relation => {:issue_to_id => 'foo', :relation_type => 'relates', :delay => ''} end end @@ -70,7 +87,7 @@ class IssueRelationsControllerTest < ActionController::TestCase assert_no_difference 'IssueRelation.count' do @request.session[:user_id] = 3 - post :new, :issue_id => 1, + post :create, :issue_id => 1, :relation => {:issue_to_id => '4', :relation_type => 'relates', :delay => ''} end end @@ -80,7 +97,7 @@ class IssueRelationsControllerTest < ActionController::TestCase def test_destroy assert_difference 'IssueRelation.count', -1 do @request.session[:user_id] = 3 - post :destroy, :id => '2', :issue_id => '3' + delete :destroy, :id => '2', :issue_id => '3' end end @@ -92,7 +109,7 @@ class IssueRelationsControllerTest < ActionController::TestCase assert_difference 'IssueRelation.count', -1 do @request.session[:user_id] = 3 - xhr :post, :destroy, :id => '2', :issue_id => '3' + xhr :delete, :destroy, :id => '2', :issue_id => '3' assert_select_rjs 'relations' do assert_select 'table', 1 assert_select 'tr', 1 # relation left diff --git a/test/integration/api_test/issue_relations_test.rb b/test/integration/api_test/issue_relations_test.rb new file mode 100644 index 000000000..13ac7d5c8 --- /dev/null +++ b/test/integration/api_test/issue_relations_test.rb @@ -0,0 +1,83 @@ +# Redmine - project management software +# Copyright (C) 2006-2011 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 File.expand_path('../../../test_helper', __FILE__) + +class ApiTest::IssueRelationsTest < ActionController::IntegrationTest + fixtures :all + + def setup + Setting.rest_api_enabled = '1' + end + + context "/issues/:issue_id/relations" do + context "POST" do + should "create a relation" do + assert_difference('IssueRelation.count') do + post '/issues/2/relations.xml', {:relation => {:issue_to_id => 7, :relation_type => 'relates'}}, :authorization => credentials('jsmith') + end + + relation = IssueRelation.first(:order => 'id DESC') + assert_equal 2, relation.issue_from_id + assert_equal 7, relation.issue_to_id + assert_equal 'relates', relation.relation_type + + assert_response :created + assert_equal 'application/xml', @response.content_type + assert_tag 'relation', :child => {:tag => 'id', :content => relation.id.to_s} + end + + context "with failure" do + should "return the errors" do + assert_no_difference('IssueRelation.count') do + post '/issues/2/relations.xml', {:relation => {:issue_to_id => 7, :relation_type => 'foo'}}, :authorization => credentials('jsmith') + end + + assert_response :unprocessable_entity + assert_tag :errors, :child => {:tag => 'error', :content => 'relation_type is not included in the list'} + end + end + end + end + + context "/issues/:issue_id/relations/:id" do + context "GET" do + should "return the relation" do + get '/issues/3/relations/2.xml', {}, :authorization => credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.content_type + assert_tag 'relation', :child => {:tag => 'id', :content => '2'} + end + end + + context "DELETE" do + should "delete the relation" do + assert_difference('IssueRelation.count', -1) do + delete '/issues/3/relations/2.xml', {}, :authorization => credentials('jsmith') + end + + assert_response :ok + assert_nil IssueRelation.find_by_id(2) + end + end + end + + def credentials(user, password=nil) + ActionController::HttpAuthentication::Basic.encode_credentials(user, password || user) + end +end diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 705c56282..c12900aef 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -118,8 +118,17 @@ class RoutingTest < ActionController::IntegrationTest end context "issue relations" do - should_route :post, "/issues/1/relations", :controller => 'issue_relations', :action => 'new', :issue_id => '1' - should_route :post, "/issues/1/relations/23/destroy", :controller => 'issue_relations', :action => 'destroy', :issue_id => '1', :id => '23' + should_route :post, "/issues/1/relations", :controller => 'issue_relations', :action => 'create', :issue_id => '1' + should_route :post, "/issues/1/relations.xml", :controller => 'issue_relations', :action => 'create', :issue_id => '1', :format => 'xml' + should_route :post, "/issues/1/relations.json", :controller => 'issue_relations', :action => 'create', :issue_id => '1', :format => 'json' + + should_route :get, "/issues/1/relations/23", :controller => 'issue_relations', :action => 'show', :issue_id => '1', :id => '23' + should_route :get, "/issues/1/relations/23.xml", :controller => 'issue_relations', :action => 'show', :issue_id => '1', :id => '23', :format => 'xml' + should_route :get, "/issues/1/relations/23.json", :controller => 'issue_relations', :action => 'show', :issue_id => '1', :id => '23', :format => 'json' + + should_route :delete, "/issues/1/relations/23", :controller => 'issue_relations', :action => 'destroy', :issue_id => '1', :id => '23' + should_route :delete, "/issues/1/relations/23.xml", :controller => 'issue_relations', :action => 'destroy', :issue_id => '1', :id => '23', :format => 'xml' + should_route :delete, "/issues/1/relations/23.json", :controller => 'issue_relations', :action => 'destroy', :issue_id => '1', :id => '23', :format => 'json' end context "issue reports" do