Merge pull request #8723 from jubianchi/api-groups-path

Access groups using path

CHANGELOG

@@ -53,7 +53,7 @@ v 7.8.0
   - Add a new API function that retrieves all issues assigned to a single milestone (Justin Whear and Hannes Rosenögger)
   - 
   - 
-  - 
+  - API: Access groups with their path (Julien Bianchi)
   - 
   - 
   - 

doc/api/groups.md

@@ -32,7 +32,7 @@ GET /groups/:id
 
 Parameters:
 
-- `id` (required) - The ID of a group
+- `id` (required) - The ID or path of a group
 
 ## New group
 
@@ -58,7 +58,7 @@ POST  /groups/:id/projects/:project_id
 
 Parameters:
 
-- `id` (required) - The ID of a group
+- `id` (required) - The ID or path of a group
 - `project_id` (required) - The ID of a project
 
 ## Remove group
@@ -71,7 +71,7 @@ DELETE /groups/:id
 
 Parameters:
 
-- `id` (required) - The ID of a user group
+- `id` (required) - The ID or path of a user group
 
 ## Search for group
 
@@ -148,7 +148,7 @@ POST /groups/:id/members
 
 Parameters:
 
-- `id` (required) - The ID of a group
+- `id` (required) - The ID or path of a group
 - `user_id` (required) - The ID of a user to add
 - `access_level` (required) - Project access level
 
@@ -162,5 +162,5 @@ DELETE /groups/:id/members/:user_id
 
 Parameters:
 
-- `id` (required) - The ID of a user group
+- `id` (required) - The ID or path of a user group
 - `user_id` (required) - The ID of a group member

lib/api/group_members.rb

@@ -3,22 +3,6 @@ class GroupMembers < Grape::API
     before { authenticate! }
 
     resource :groups do
-      helpers do
-        def find_group(id)
-          group = Group.find(id)
-
-          if can?(current_user, :read_group, group)
-            group
-          else
-            render_api_error!("403 Forbidden - #{current_user.username} lacks sufficient access to #{group.name}", 403)
-          end
-        end
-
-        def validate_access_level?(level)
-          Gitlab::Access.options_with_owner.values.include? level.to_i
-        end
-      end
-
       # Get a list of group members viewable by the authenticated user.
       #
       # Example Request:

lib/api/groups.rb

@@ -4,22 +4,6 @@ class Groups < Grape::API
     before { authenticate! }
 
     resource :groups do
-      helpers do
-        def find_group(id)
-          group = Group.find(id)
-
-          if can?(current_user, :read_group, group)
-            group
-          else
-            render_api_error!("403 Forbidden - #{current_user.username} lacks sufficient access to #{group.name}", 403)
-          end
-        end
-
-        def validate_access_level?(level)
-          Gitlab::Access.options_with_owner.values.include? level.to_i
-        end
-      end
-
       # Get a groups list
       #
       # Example Request:

lib/api/helpers.rb

@@ -55,6 +55,21 @@ def find_project(id)
       end
     end
 
+    def find_group(id)
+      begin
+        group = Group.find(id)
+      rescue ActiveRecord::RecordNotFound
+        group = Group.find_by!(path: id)
+      end
+
+      if can?(current_user, :read_group, group)
+        group
+      else
+        forbidden!("#{current_user.username} lacks sufficient "\
+        "access to #{group.name}")
+      end
+    end
+
     def paginate(relation)
       per_page  = params[:per_page].to_i
       paginated = relation.page(params[:page]).per(per_page)
@@ -135,10 +150,16 @@ def validate_label_params(params)
       errors
     end
 
+    def validate_access_level?(level)
+      Gitlab::Access.options_with_owner.values.include? level.to_i
+    end
+
     # error helpers
 
-    def forbidden!
-      render_api_error!('403 Forbidden', 403)
+    def forbidden!(reason = nil)
+      message = ['403 Forbidden']
+      message << " - #{reason}" if reason
+      render_api_error!(message.join(' '), 403)
     end
 
     def bad_request!(attribute)

spec/requests/api/groups_spec.rb

@@ -73,6 +73,24 @@
         response.status.should == 404
       end
     end
+
+    context 'when using group path in URL' do
+      it 'should return any existing group' do
+        get api("/groups/#{group1.path}", admin)
+        response.status.should == 200
+        json_response['name'] == group2.name
+      end
+
+      it 'should not return a non existing group' do
+        get api('/groups/unknown', admin)
+        response.status.should == 404
+      end
+
+      it 'should not return a group not attached to user1' do
+        get api("/groups/#{group2.path}", user1)
+        response.status.should == 403
+      end
+    end
   end
 
   describe "POST /groups" do