REST Api for Groups (#8981).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9758 e93f8b46-1217-0410-a6f0-8f06a7374b81

8 files changed, 294 insertions, 39 deletions

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 6053bcc0e..e1c13969d 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -19,7 +19,8 @@ class GroupsController < ApplicationController
   layout 'admin'
 
   before_filter :require_admin
-  before_filter :find_group, :only => [:show, :edit, :update, :destroy, :add_users, :remove_user, :autocomplete_for_user, :edit_membership, :destroy_membership]
+  before_filter :find_group, :except => [:index, :new, :create]
+  accept_api_auth :index, :show, :create, :update, :destroy, :add_users, :remove_user
 
   helper :custom_fields
 
@@ -28,24 +29,19 @@ class GroupsController < ApplicationController
 
     respond_to do |format|
       format.html
-      format.xml  { render :xml => @groups }
+      format.api
     end
   end
 
   def show
     respond_to do |format|
       format.html
-      format.xml  { render :xml => @group }
+      format.api
     end
   end
 
   def new
     @group = Group.new
-
-    respond_to do |format|
-      format.html
-      format.xml  { render :xml => @group }
-    end
   end
 
   def create
@@ -58,10 +54,10 @@ class GroupsController < ApplicationController
           flash[:notice] = l(:notice_successful_create)
           redirect_to(params[:continue] ? new_group_path : groups_path)
         }
-        format.xml  { render :xml => @group, :status => :created, :location => @group }
+        format.api  { render :action => 'show', :status => :created, :location => group_url(@group) }
       else
         format.html { render :action => "new" }
-        format.xml  { render :xml => @group.errors, :status => :unprocessable_entity }
+        format.api  { render_validation_errors(@group) }
       end
     end
   end
@@ -76,10 +72,10 @@ class GroupsController < ApplicationController
       if @group.save
         flash[:notice] = l(:notice_successful_update)
         format.html { redirect_to(groups_path) }
-        format.xml  { head :ok }
+        format.api  { head :ok }
       else
         format.html { render :action => "edit" }
-        format.xml  { render :xml => @group.errors, :status => :unprocessable_entity }
+        format.api  { render_validation_errors(@group) }
       end
     end
   end
@@ -89,12 +85,12 @@ class GroupsController < ApplicationController
 
     respond_to do |format|
       format.html { redirect_to(groups_url) }
-      format.xml  { head :ok }
+      format.api  { head :ok }
     end
   end
 
   def add_users
-    users = User.find_all_by_id(params[:user_ids])
+    users = User.find_all_by_id(params[:user_id] || params[:user_ids])
     @group.users << users if request.post?
     respond_to do |format|
       format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'users' }
@@ -104,6 +100,7 @@ class GroupsController < ApplicationController
           users.each {|user| page.visual_effect(:highlight, "user-#{user.id}") }
         }
       }
+      format.api { head :ok }
     end
   end
 
@@ -112,6 +109,7 @@ class GroupsController < ApplicationController
     respond_to do |format|
       format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'users' }
       format.js { render(:update) {|page| page.replace_html "tab-content-users", :partial => 'groups/users'} }
+      format.api { head :ok }
     end
   end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index a269c5c16..61266d97a 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -30,6 +30,7 @@ class Group < Principal
   before_destroy :remove_references_before_destroy
 
   safe_attributes 'name',
+    'user_ids',
     'custom_field_values',
     'custom_fields',
     :if => lambda {|group, user| user.admin?}
diff --git a/app/views/groups/index.api.rsb b/app/views/groups/index.api.rsb
new file mode 100644
index 000000000..6a4efeaf7
--- /dev/null
+++ b/app/views/groups/index.api.rsb
@@ -0,0 +1,10 @@
+api.array :groups do
+  @groups.each do |group|
+    api.group do
+      api.id         group.id
+      api.name       group.lastname
+
+      render_api_custom_values group.visible_custom_field_values, api
+    end
+  end
+end
diff --git a/app/views/groups/show.api.rsb b/app/views/groups/show.api.rsb
new file mode 100644
index 000000000..764bdceda
--- /dev/null
+++ b/app/views/groups/show.api.rsb
@@ -0,0 +1,30 @@
+api.group do
+  api.id         @group.id
+  api.name       @group.lastname
+
+  render_api_custom_values @group.visible_custom_field_values, api
+
+  api.array :users do
+    @group.users.each do |user|
+      api.user :id => user.id, :name => user.name
+    end
+  end
+
+  api.array :memberships do
+    @group.memberships.each do |membership|
+      api.membership do
+        api.id membership.id
+        api.project :id => membership.project.id, :name => membership.project.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 if membership.project
+    end
+  end if include_in_api_response?('memberships')
+end
diff --git a/app/views/users/show.api.rsb b/app/views/users/show.api.rsb
index b7d23ca97..ec7e5030f 100644
--- a/app/views/users/show.api.rsb
+++ b/app/views/users/show.api.rsb
@@ -9,6 +9,12 @@ api.user do
 
   render_api_custom_values @user.visible_custom_field_values, api
 
+  api.array :groups do |groups|
+    @user.groups.each do |group|
+      api.group :id => group.id, :name => group.name
+    end
+  end if User.current.admin? && include_in_api_response?('groups')
+
   api.array :memberships do
     @memberships.each do |membership|
       api.membership do
diff --git a/test/integration/api_test/groups_test.rb b/test/integration/api_test/groups_test.rb
new file mode 100644
index 000000000..fdc7983dd
--- /dev/null
+++ b/test/integration/api_test/groups_test.rb
@@ -0,0 +1,199 @@
+# 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::GroupsTest < ActionController::IntegrationTest
+  fixtures :users, :groups_users
+
+  def setup
+    Setting.rest_api_enabled = '1'
+  end
+
+  context "GET /groups" do
+    context ".xml" do
+      should "require authentication" do
+        get '/groups.xml'
+        assert_response 401
+      end
+
+      should "return groups" do
+        get '/groups.xml', {}, credentials('admin')
+        assert_response :success
+        assert_equal 'application/xml', response.content_type
+
+        assert_select 'groups' do
+          assert_select 'group' do
+            assert_select 'name', :text => 'A Team'
+            assert_select 'id', :text => '10'
+          end
+        end
+      end
+    end
+
+    context ".json" do
+      should "require authentication" do
+        get '/groups.json'
+        assert_response 401
+      end
+
+      should "return groups" do
+        get '/groups.json', {}, credentials('admin')
+        assert_response :success
+        assert_equal 'application/json', response.content_type
+
+        json = MultiJson.load(response.body)
+        groups = json['groups']
+        assert_kind_of Array, groups
+        group = groups.detect {|g| g['name'] == 'A Team'}
+        assert_not_nil group
+        assert_equal({'id' => 10, 'name' => 'A Team'}, group)
+      end
+    end
+  end
+
+  context "GET /groups/:id" do
+    context ".xml" do
+      should "return the group with its users" do
+        get '/groups/10.xml', {}, credentials('admin')
+        assert_response :success
+        assert_equal 'application/xml', response.content_type
+
+        assert_select 'group' do
+          assert_select 'name', :text => 'A Team'
+          assert_select 'id', :text => '10'
+          assert_select 'users' do
+            assert_select 'user', Group.find(10).users.count
+            assert_select 'user[id=8]'
+          end
+        end
+      end
+
+      should "include memberships if requested" do
+        get '/groups/10.xml?include=memberships', {}, credentials('admin')
+        assert_response :success
+        assert_equal 'application/xml', response.content_type
+
+        assert_select 'group' do
+          assert_select 'memberships'
+        end
+      end
+    end
+  end
+
+  context "POST /groups" do
+    context "with valid parameters" do
+      context ".xml" do
+        should "create groups" do
+          assert_difference('Group.count') do
+            post '/groups.xml', {:group => {:name => 'Test', :user_ids => [2, 3]}}, credentials('admin')
+            assert_response :created
+            assert_equal 'application/xml', response.content_type
+          end
+  
+          group = Group.order('id DESC').first
+          assert_equal 'Test', group.name
+          assert_equal [2, 3], group.users.map(&:id).sort
+
+          assert_select 'group' do
+            assert_select 'name', :text => 'Test'
+          end
+        end
+      end
+    end
+
+    context "with invalid parameters" do
+      context ".xml" do
+        should "return errors" do
+          assert_no_difference('Group.count') do
+            post '/groups.xml', {:group => {:name => ''}}, credentials('admin')
+          end
+          assert_response :unprocessable_entity
+          assert_equal 'application/xml', response.content_type
+
+          assert_select 'errors' do
+            assert_select 'error', :text => /Name can't be blank/
+          end
+        end
+      end
+    end
+  end
+
+  context "PUT /groups/:id" do
+    context "with valid parameters" do
+      context ".xml" do
+        should "update the group" do
+          put '/groups/10.xml', {:group => {:name => 'New name', :user_ids => [2, 3]}}, credentials('admin')
+          assert_response :ok
+  
+          group = Group.find(10)
+          assert_equal 'New name', group.name
+          assert_equal [2, 3], group.users.map(&:id).sort
+        end
+      end
+    end
+
+    context "with invalid parameters" do
+      context ".xml" do
+        should "return errors" do
+          put '/groups/10.xml', {:group => {:name => ''}}, credentials('admin')
+          assert_response :unprocessable_entity
+          assert_equal 'application/xml', response.content_type
+
+          assert_select 'errors' do
+            assert_select 'error', :text => /Name can't be blank/
+          end
+        end
+      end
+    end
+  end
+
+  context "DELETE /groups/:id" do
+    context ".xml" do
+      should "delete the group" do
+        assert_difference 'Group.count', -1 do
+          delete '/groups/10.xml', {}, credentials('admin')
+          assert_response :ok
+        end
+      end
+    end
+  end
+
+  context "POST /groups/:id/users" do
+    context ".xml" do
+      should "add user to the group" do
+        assert_difference 'Group.find(10).users.count' do
+          post '/groups/10/users.xml', {:user_id => 5}, credentials('admin')
+          assert_response :ok
+        end
+        assert_include User.find(5), Group.find(10).users
+      end
+    end
+  end
+
+  context "DELETE /groups/:id/users/:user_id" do
+    context ".xml" do
+      should "remove user from the group" do
+        assert_difference 'Group.find(10).users.count', -1 do
+          delete '/groups/10/users/8.xml', {}, credentials('admin')
+          assert_response :ok
+        end
+        assert_not_include User.find(8), Group.find(10).users
+      end
+    end
+  end
+end
diff --git a/test/integration/routing/groups_test.rb b/test/integration/routing/groups_test.rb
index f22c80b06..6495201dd 100644
--- a/test/integration/routing/groups_test.rb
+++ b/test/integration/routing/groups_test.rb
@@ -40,47 +40,36 @@ class RoutingGroupsTest < ActionController::IntegrationTest
         { :controller => 'groups', :action => 'new' }
       )
     assert_routing(
-        { :method => 'get', :path => "/groups/new.xml" },
-        { :controller => 'groups', :action => 'new', :format => 'xml' }
-      )
-    assert_routing(
         { :method => 'get', :path => "/groups/1/edit" },
         { :controller => 'groups', :action => 'edit', :id => '1' }
       )
     assert_routing(
         { :method => 'get', :path => "/groups/1/autocomplete_for_user" },
-        { :controller => 'groups', :action => 'autocomplete_for_user',
-          :id => '1' }
+        { :controller => 'groups', :action => 'autocomplete_for_user', :id => '1' }
       )
     assert_routing(
         { :method => 'get', :path => "/groups/1" },
-        { :controller => 'groups', :action => 'show',
-          :id => '1' }
+        { :controller => 'groups', :action => 'show', :id => '1' }
       )
     assert_routing(
         { :method => 'get', :path => "/groups/1.xml" },
-        { :controller => 'groups', :action => 'show',
-          :format => 'xml', :id => '1' }
+        { :controller => 'groups', :action => 'show', :id => '1', :format => 'xml' }
       )
     assert_routing(
         { :method => 'put', :path => "/groups/1" },
-        { :controller => 'groups', :action => 'update',
-          :id => '1' }
+        { :controller => 'groups', :action => 'update', :id => '1' }
       )
     assert_routing(
         { :method => 'put', :path => "/groups/1.xml" },
-        { :controller => 'groups', :action => 'update',
-          :format => 'xml', :id => '1' }
+        { :controller => 'groups', :action => 'update', :id => '1', :format => 'xml' }
       )
     assert_routing(
         { :method => 'delete', :path => "/groups/1" },
-        { :controller => 'groups', :action => 'destroy',
-          :id => '1' }
+        { :controller => 'groups', :action => 'destroy', :id => '1' }
       )
     assert_routing(
         { :method => 'delete', :path => "/groups/1.xml" },
-        { :controller => 'groups', :action => 'destroy',
-          :format => 'xml', :id => '1' }
+        { :controller => 'groups', :action => 'destroy', :id => '1', :format => 'xml' }
       )
   end
 
@@ -90,9 +79,16 @@ class RoutingGroupsTest < ActionController::IntegrationTest
         { :controller => 'groups', :action => 'add_users', :id => '567' }
       )
     assert_routing(
+        { :method => 'post', :path => "/groups/567/users.xml" },
+        { :controller => 'groups', :action => 'add_users', :id => '567', :format => 'xml' }
+      )
+    assert_routing(
         { :method => 'delete', :path => "/groups/567/users/12" },
-        { :controller => 'groups', :action => 'remove_user', :id => '567',
-          :user_id => '12' }
+        { :controller => 'groups', :action => 'remove_user', :id => '567', :user_id => '12' }
+      )
+    assert_routing(
+        { :method => 'delete', :path => "/groups/567/users/12.xml" },
+        { :controller => 'groups', :action => 'remove_user', :id => '567', :user_id => '12', :format => 'xml' }
       )
     assert_routing(
         { :method => 'post', :path => "/groups/destroy_membership/567" },
diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb
index 3d7c2da26..8038ba45c 100644
--- a/test/unit/group_test.rb
+++ b/test/unit/group_test.rb
@@ -53,7 +53,7 @@ class GroupTest < ActiveSupport::TestCase
     assert_include str, g.errors.full_messages
   end
 
-  def test_roles_given_to_new_user
+  def test_group_roles_should_be_given_to_added_user
     group = Group.find(11)
     user = User.find(9)
     project = Project.first
@@ -63,7 +63,7 @@ class GroupTest < ActiveSupport::TestCase
     assert user.member_of?(project)
   end
 
-  def test_roles_given_to_existing_user
+  def test_new_roles_should_be_given_to_existing_user
     group = Group.find(11)
     user = User.find(9)
     project = Project.first
@@ -73,7 +73,22 @@ class GroupTest < ActiveSupport::TestCase
     assert user.member_of?(project)
   end
 
-  def test_roles_updated
+  def test_user_roles_should_updated_when_updating_user_ids
+    group = Group.find(11)
+    user = User.find(9)
+    project = Project.first
+
+    Member.create!(:principal => group, :project => project, :role_ids => [1, 2])
+    group.user_ids = [user.id]
+    group.save!
+    assert User.find(9).member_of?(project)
+
+    group.user_ids = [1]
+    group.save!
+    assert !User.find(9).member_of?(project)
+  end
+
+  def test_user_roles_should_updated_when_updating_group_roles
     group = Group.find(11)
     user = User.find(9)
     project = Project.first
@@ -91,13 +106,13 @@ class GroupTest < ActiveSupport::TestCase
     assert_equal [1], user.reload.roles_for_project(project).collect(&:id).sort
   end
 
-  def test_roles_removed_when_removing_group_membership
+  def test_user_memberships_should_be_removed_when_removing_group_membership
     assert User.find(8).member_of?(Project.find(5))
     Member.find_by_project_id_and_user_id(5, 10).destroy
     assert !User.find(8).member_of?(Project.find(5))
   end
 
-  def test_roles_removed_when_removing_user_from_group
+  def test_user_roles_should_be_removed_when_removing_user_from_group
     assert User.find(8).member_of?(Project.find(5))
     User.find(8).groups.clear
     assert !User.find(8).member_of?(Project.find(5))