From c5665276b7a465f29230341f8800af4f77b3c141 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Mon, 6 Feb 2012 09:47:10 +0000 Subject: [PATCH] REST API for project memberships (#7420). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8798 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/members_controller.rb | 89 +++++--- app/views/members/index.api.rsb | 18 ++ app/views/members/show.api.rsb | 14 ++ config/routes.rb | 2 +- lib/redmine.rb | 2 +- test/integration/api_test/memberships_test.rb | 190 ++++++++++++++++++ test/integration/routing/members_test.rb | 20 ++ 7 files changed, 308 insertions(+), 27 deletions(-) create mode 100644 app/views/members/index.api.rsb create mode 100644 app/views/members/show.api.rsb create mode 100644 test/integration/api_test/memberships_test.rb diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index ae2f57a13..80328aa7f 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -17,29 +17,52 @@ class MembersController < ApplicationController model_object Member - before_filter :find_model_object, :except => [:create, :autocomplete] - before_filter :find_project_from_association, :except => [:create, :autocomplete] - before_filter :find_project_by_project_id, :only => [:create, :autocomplete] + before_filter :find_model_object, :except => [:index, :create, :autocomplete] + before_filter :find_project_from_association, :except => [:index, :create, :autocomplete] + before_filter :find_project_by_project_id, :only => [:index, :create, :autocomplete] before_filter :authorize + accept_api_auth :index, :show, :create, :update, :destroy + + def index + @offset, @limit = api_offset_and_limit + @member_count = @project.member_principals.count + @member_pages = Paginator.new self, @member_count, @limit, params['page'] + @offset ||= @member_pages.current.offset + @members = @project.member_principals.all( + :order => "#{Member.table_name}.id", + :limit => @limit, + :offset => @offset + ) + + respond_to do |format| + format.html { head 406 } + format.api + end + end + + def show + respond_to do |format| + format.html { head 406 } + format.api + end + end def create members = [] - if params[:membership] && request.post? + if params[:membership] && params[:membership][:user_ids] attrs = params[:membership].dup - if (user_ids = attrs.delete(:user_ids)) - user_ids.each do |user_id| - members << Member.new(attrs.merge(:user_id => user_id)) - end - else - members << Member.new(attrs) + user_ids = attrs.delete(:user_ids) + user_ids.each do |user_id| + members << Member.new(attrs.merge(:user_id => user_id)) end - @project.members << members + else + members << Member.new(params[:membership]) end + @project.members << members + respond_to do |format| if members.present? && members.all? {|m| m.valid? } - format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project } - format.js { render(:update) {|page| page.replace_html "tab-content-members", :partial => 'projects/settings/members' @@ -47,8 +70,11 @@ class MembersController < ApplicationController members.each {|member| page.visual_effect(:highlight, "member-#{member.id}") } } } + format.api { + @member = members.first + render :action => 'show', :status => :created, :location => membership_url(@member) + } else - format.js { render(:update) {|page| errors = members.collect {|m| @@ -58,7 +84,7 @@ class MembersController < ApplicationController page.alert(l(:notice_failed_to_save_members, :errors => errors.join(', '))) } } - + format.api { render_validation_errors(members.first) } end end end @@ -67,17 +93,23 @@ class MembersController < ApplicationController if params[:membership] @member.role_ids = params[:membership][:role_ids] end - if request.put? && @member.save - respond_to do |format| - format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project } - format.js { - render(:update) {|page| - page.replace_html "tab-content-members", :partial => 'projects/settings/members' - page << 'hideOnLoad()' - page.visual_effect(:highlight, "member-#{@member.id}") - } + saved = @member.save + respond_to do |format| + format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project } + format.js { + render(:update) {|page| + page.replace_html "tab-content-members", :partial => 'projects/settings/members' + page << 'hideOnLoad()' + page.visual_effect(:highlight, "member-#{@member.id}") } - end + } + format.api { + if saved + head :ok + else + render_validation_errors(@member) + end + } end end @@ -92,6 +124,13 @@ class MembersController < ApplicationController page << 'hideOnLoad()' } } + format.api { + if @member.destroyed? + head :ok + else + head :unprocessable_entity + end + } end end diff --git a/app/views/members/index.api.rsb b/app/views/members/index.api.rsb new file mode 100644 index 000000000..dd591ea7c --- /dev/null +++ b/app/views/members/index.api.rsb @@ -0,0 +1,18 @@ +api.array :memberships, api_meta(:total_count => @member_count, :offset => @offset, :limit => @limit) do + @members.each do |membership| + api.membership do + api.id membership.id + api.project :id => membership.project.id, :name => membership.project.name + api.__send__ membership.principal.class.name.underscore, :id => membership.principal.id, :name => membership.principal.name + api.array :roles do + membership.member_roles.each do |member_role| + if member_role.role + attrs = {:id => member_role.role.id, :name => member_role.role.name} + attrs.merge!(:inherited => true) if member_role.inherited_from.present? + api.role attrs + end + end + end + end + end +end diff --git a/app/views/members/show.api.rsb b/app/views/members/show.api.rsb new file mode 100644 index 000000000..55def9eab --- /dev/null +++ b/app/views/members/show.api.rsb @@ -0,0 +1,14 @@ +api.membership do + api.id @member.id + api.project :id => @member.project.id, :name => @member.project.name + api.__send__ @member.principal.class.name.underscore, :id => @member.principal.id, :name => @member.principal.name + api.array :roles do + @member.member_roles.each do |member_role| + if member_role.role + attrs = {:id => member_role.role.id, :name => member_role.role.name} + attrs.merge!(:inherited => true) if member_role.inherited_from.present? + api.role attrs + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 66f0182be..583eff755 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -170,7 +170,7 @@ ActionController::Routing::Routes.draw do |map| project.resources :repositories, :shallow => true, :except => [:index, :show], :member => {:committers => [:get, :post]} project.resources :memberships, :shallow => true, :controller => 'members', - :only => [:create, :update, :destroy], + :only => [:index, :show, :create, :update, :destroy], :collection => {:autocomplete => :get} project.wiki_start_page 'wiki', :controller => 'wiki', :action => 'show', :conditions => {:method => :get} diff --git a/lib/redmine.rb b/lib/redmine.rb index 2a55a5ddb..408f1be82 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -52,7 +52,7 @@ Redmine::AccessControl.map do |map| map.permission :add_project, {:projects => [:new, :create]}, :require => :loggedin map.permission :edit_project, {:projects => [:settings, :edit, :update]}, :require => :member map.permission :select_project_modules, {:projects => :modules}, :require => :member - map.permission :manage_members, {:projects => :settings, :members => [:create, :update, :destroy, :autocomplete]}, :require => :member + map.permission :manage_members, {:projects => :settings, :members => [:index, :show, :create, :update, :destroy, :autocomplete]}, :require => :member map.permission :manage_versions, {:projects => :settings, :versions => [:new, :create, :edit, :update, :close_completed, :destroy]}, :require => :member map.permission :add_subprojects, {:projects => [:new, :create]}, :require => :member diff --git a/test/integration/api_test/memberships_test.rb b/test/integration/api_test/memberships_test.rb new file mode 100644 index 000000000..04696962a --- /dev/null +++ b/test/integration/api_test/memberships_test.rb @@ -0,0 +1,190 @@ +# Redmine - project management software +# Copyright (C) 2006-2012 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::MembershipsTest < ActionController::IntegrationTest + fixtures :projects, :users, :roles, :members, :member_roles + + def setup + Setting.rest_api_enabled = '1' + end + + context "/projects/:project_id/memberships" do + context "GET" do + context "xml" do + should "return memberships" do + get '/projects/1/memberships.xml', {}, credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.content_type + assert_tag :tag => 'memberships', + :attributes => {:type => 'array'}, + :child => { + :tag => 'membership', + :child => { + :tag => 'id', + :content => '2', + :sibling => { + :tag => 'user', + :attributes => {:id => '3', :name => 'Dave Lopper'}, + :sibling => { + :tag => 'roles', + :child => { + :tag => 'role', + :attributes => {:id => '2', :name => 'Developer'} + } + } + } + } + } + end + end + + context "json" do + should "return memberships" do + get '/projects/1/memberships.json', {}, credentials('jsmith') + + assert_response :success + assert_equal 'application/json', @response.content_type + json = ActiveSupport::JSON.decode(response.body) + assert_equal({ + "memberships" => + [{"id"=>1, + "project" => {"name"=>"eCookbook", "id"=>1}, + "roles" => [{"name"=>"Manager", "id"=>1}], + "user" => {"name"=>"John Smith", "id"=>2}}, + {"id"=>2, + "project" => {"name"=>"eCookbook", "id"=>1}, + "roles" => [{"name"=>"Developer", "id"=>2}], + "user" => {"name"=>"Dave Lopper", "id"=>3}}], + "limit" => 25, + "total_count" => 2, + "offset" => 0}, + json) + end + end + end + + context "POST" do + context "xml" do + should "create membership" do + assert_difference 'Member.count' do + post '/projects/1/memberships.xml', {:membership => {:user_id => 7, :role_ids => [2,3]}}, credentials('jsmith') + + assert_response :created + end + end + + should "return errors on failure" do + assert_no_difference 'Member.count' do + post '/projects/1/memberships.xml', {:membership => {:role_ids => [2,3]}}, credentials('jsmith') + + assert_response :unprocessable_entity + assert_equal 'application/xml', @response.content_type + assert_tag 'errors', :child => {:tag => 'error', :content => "Principal can't be blank"} + end + end + end + end + end + + context "/memberships/:id" do + context "GET" do + context "xml" do + should "return the membership" do + get '/memberships/2.xml', {}, credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.content_type + assert_tag :tag => 'membership', + :child => { + :tag => 'id', + :content => '2', + :sibling => { + :tag => 'user', + :attributes => {:id => '3', :name => 'Dave Lopper'}, + :sibling => { + :tag => 'roles', + :child => { + :tag => 'role', + :attributes => {:id => '2', :name => 'Developer'} + } + } + } + } + end + end + + context "json" do + should "return the membership" do + get '/memberships/2.json', {}, credentials('jsmith') + + assert_response :success + assert_equal 'application/json', @response.content_type + json = ActiveSupport::JSON.decode(response.body) + assert_equal( + {"membership" => { + "id" => 2, + "project" => {"name"=>"eCookbook", "id"=>1}, + "roles" => [{"name"=>"Developer", "id"=>2}], + "user" => {"name"=>"Dave Lopper", "id"=>3}} + }, + json) + end + end + end + + context "PUT" do + context "xml" do + should "update membership" do + assert_not_equal [1,2], Member.find(2).role_ids.sort + assert_no_difference 'Member.count' do + put '/memberships/2.xml', {:membership => {:user_id => 3, :role_ids => [1,266]}}, credentials('jsmith') + + assert_response :ok + end + member = Member.find(2) + assert_equal [1,2], member.role_ids.sort + end + end + end + + context "DELETE" do + context "xml" do + should "destroy membership" do + assert_difference 'Member.count', -1 do + delete '/memberships/2.xml', {}, credentials('jsmith') + + assert_response :ok + end + assert_nil Member.find_by_id(2) + end + + should "respond with 422 on failure" do + assert_no_difference 'Member.count' do + # A membership with an inherited role can't be deleted + Member.find(2).member_roles.first.update_attribute :inherited_from, 99 + delete '/memberships/2.xml', {}, credentials('jsmith') + + assert_response :unprocessable_entity + end + end + end + end + end +end diff --git a/test/integration/routing/members_test.rb b/test/integration/routing/members_test.rb index 4c78b15c7..87181b089 100644 --- a/test/integration/routing/members_test.rb +++ b/test/integration/routing/members_test.rb @@ -19,18 +19,38 @@ require File.expand_path('../../../test_helper', __FILE__) class RoutingMembersTest < ActionController::IntegrationTest def test_members + assert_routing( + { :method => 'get', :path => "/projects/5234/memberships.xml" }, + { :controller => 'members', :action => 'index', :project_id => '5234', :format => 'xml' } + ) + assert_routing( + { :method => 'get', :path => "/memberships/5234.xml" }, + { :controller => 'members', :action => 'show', :id => '5234', :format => 'xml' } + ) assert_routing( { :method => 'post', :path => "/projects/5234/memberships" }, { :controller => 'members', :action => 'create', :project_id => '5234' } ) + assert_routing( + { :method => 'post', :path => "/projects/5234/memberships.xml" }, + { :controller => 'members', :action => 'create', :project_id => '5234', :format => 'xml' } + ) assert_routing( { :method => 'put', :path => "/memberships/5234" }, { :controller => 'members', :action => 'update', :id => '5234' } ) + assert_routing( + { :method => 'put', :path => "/memberships/5234.xml" }, + { :controller => 'members', :action => 'update', :id => '5234', :format => 'xml' } + ) assert_routing( { :method => 'delete', :path => "/memberships/5234" }, { :controller => 'members', :action => 'destroy', :id => '5234' } ) + assert_routing( + { :method => 'delete', :path => "/memberships/5234.xml" }, + { :controller => 'members', :action => 'destroy', :id => '5234', :format => 'xml' } + ) assert_routing( { :method => 'get', :path => "/projects/5234/memberships/autocomplete" }, { :controller => 'members', :action => 'autocomplete', :project_id => '5234' }