Add request access for groups

Signed-off-by: Rémy Coutable <remy@rymai.me>

app/views/notify/project_request_access_denied_email.html.haml

-%p
-  Your request to join project
-  #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)}
-  has been denied.

app/views/projects/notes/_note.html.haml

@@ -17,7 +17,7 @@
           %a{ href: "##{dom_id(note)}" }
             = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         .note-actions
-          - access = note.project.team.human_max_access(note.author.id)
+          - access = max_access_level(note.project, note.author)
           - if access
             %span.note-role.hidden-xs= access
           - if current_user

app/views/projects/project_members/_group_members.html.haml

@@ -9,8 +9,13 @@
         = link_to group_group_members_path(@group), class: 'btn' do
           Manage group members
   %ul.content-list
-    - members.limit(20).each do |member|
-      = render 'groups/group_members/group_member', member: member, show_controls: false
-    - if members.count > 20
+    = render partial: 'shared/members/member',
+             collection: members.limit(20),
+             as: :member,
+             locals: { show_controls: false }
+    - if members.size > 20
       %li
-        and #{members.count - 20} more. For full list visit #{link_to 'group members page', group_group_members_path(@group)}
+        and
+        = members.size - 20
+        more. For full list visit
+        = link_to 'group members page', group_group_members_path(@group)

app/views/projects/project_members/_new_project_member.html.haml

@@ -9,7 +9,7 @@
   .form-group
     = f.label :access_level, "Project Access", class: 'control-label'
     .col-sm-10
-      = select_tag :access_level, options_for_select(ProjectMember.access_roles, @project_member.access_level), class: "project-access-select select2"
+      = select_tag :access_level, options_for_select(ProjectMember.access_level_roles, @project_member.access_level), class: "project-access-select select2"
       .help-block
         Read more about role permissions
         %strong= link_to "here", help_page_path("permissions", "permissions"), class: "vlink"

app/views/projects/project_members/_pending.html.haml

-.panel.panel-default
-  .panel-heading
-    %strong #{@project.name}
-    candidates
-    %small
-      (#{members.count})
-    .controls
-      = form_tag namespace_project_project_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form'  do
-        .form-group
-          = search_field_tag :search, params[:search], { placeholder: 'Find existing member by name', class: 'form-control', spellcheck: false }
-        = button_tag class: 'btn', title: 'Search' do
-          = icon("search")
-  %ul.content-list
-    - members.each do |project_member|
-      = render 'project_member', member: project_member
-
-:javascript
-  $('form.member-search-form').on('submit', function (event) {
-    event.preventDefault();
-    Turbolinks.visit(this.action + '?' + $(this).serialize());
-  });

app/views/projects/project_members/_project_member.html.haml

-- user = member.user
-- return unless user || member.invite?
-
-%li{class: "#{dom_class(member)} js-toggle-container project_member_row access-#{member.human_access.downcase}", id: dom_id(member)}
-  %span.list-item-name
-    - if member.user
-      = image_tag avatar_icon(user, 24), class: "avatar s24", alt: ''
-      %strong
-        = link_to user.name, user_path(user)
-      %span.cgray= user.username
-      - if user == current_user
-        %span.label.label-success It's you
-      - if user.blocked?
-        %label.label.label-danger
-          %strong Blocked
-      - if member.request?
-        %span.label.label-info
-          Pending Approval
-    - else
-      = image_tag avatar_icon(member.invite_email, 24), class: "avatar s24", alt: ''
-      %strong
-        = member.invite_email
-      %span.cgray
-        invited
-        - if member.created_by
-          by
-          = link_to member.created_by.name, user_path(member.created_by)
-        = time_ago_with_tooltip(member.created_at)
-
-      - if can?(current_user, :admin_project_member, @project)
-        = link_to resend_invite_namespace_project_project_member_path(@project.namespace, @project, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do
-          Resend invite
-  - if can?(current_user, :admin_project_member, @project)
-    .pull-right
-      %strong= member.human_access
-      - if can?(current_user, :update_project_member, member)
-        = button_tag class: "btn-xs btn-grouped inline btn js-toggle-button",
-                     title: 'Edit access level', type: 'button' do
-          = icon('pencil')
-        - if member.request?
-           
-          = link_to approval_namespace_project_project_member_path(@project.namespace, @project, member),
-                       class: "btn-xs btn btn-success",
-                       title: 'Grant access', type: 'button' do
-            %i.fa.fa-check.fa-inverse
-
-      - if can?(current_user, :destroy_project_member, member)
-         
-        - if member.request?
-          = link_to namespace_project_project_member_path(@project.namespace, @project, member), data: { confirm: remove_from_project_team_message(@project, member) }, method: :delete, class: "btn-xs btn btn-remove", title: 'Deny access' do
-            %i.fa.fa-times.fa-inverse
-        - elsif current_user == user
-          = link_to leave_namespace_project_project_members_path(@project.namespace, @project), data: { confirm: leave_project_message(@project) }, method: :delete, class: "btn-xs btn btn-remove", title: 'Leave project' do
-            = icon("sign-out")
-            Leave
-        - else
-          = link_to namespace_project_project_member_path(@project.namespace, @project, member), data: { confirm: remove_from_project_team_message(@project, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from team' do
-            = icon('trash')
-
-    .edit-member.hide.js-toggle-content
-      %br
-      = form_for member, as: :project_member, url: namespace_project_project_member_path(@project.namespace, @project, member), remote: true do |f|
-        .prepend-top-10
-          = f.select :access_level, options_for_select(ProjectMember.access_roles, member.access_level), {}, class: 'form-control'
-        .prepend-top-10
-          = f.submit 'Save', class: 'btn btn-save'

app/views/projects/project_members/_shared_group_members.html.haml

@@ -14,8 +14,10 @@
             %i.fa.fa-pencil-square-o
             Edit group members
     %ul.content-list
-      - shared_group.group_members.order('access_level DESC').limit(20).each do |member|
-        = render 'groups/group_members/group_member', member: member, show_controls: false, show_roles: false
+      = render partial: 'shared/members/member',
+               collection: shared_group.group_members.order(access_level: :desc).limit(20),
+               as: :member,
+               locals: { show_controls: false, show_roles: false }
       - if shared_group_users_count > 20
         %li
           and #{shared_group_users_count - 20} more. For full list visit #{link_to 'group members page', group_group_members_path(shared_group)}

app/views/projects/project_members/_team.html.haml

@@ -11,8 +11,7 @@
         = button_tag class: 'btn', title: 'Search' do
           = icon("search")
   %ul.content-list
-    - members.each do |project_member|
-      = render 'project_member', member: project_member
+    = render partial: 'shared/members/member', collection: members, as: :member
 :javascript
   $('form.member-search-form').on('submit', function (event) {

app/views/projects/project_members/index.html.haml

@@ -12,7 +12,8 @@
         %p.light
           Users with access to this project are listed below.
         = render "new_project_member"
-    = render "pending", members: @project_members.request
+
+    = render "shared/members/requests", entity: @project, members: @project_members
   = render "team", members: @project_members.non_request

app/views/shared/_group_or_project_home_dropdown.html.haml

+- member = entity.send(members_association(entity)).find_by(user_id: current_user.id)
+- can_edit = can?(current_user, "admin_#{entity.class.to_s.underscore}".to_sym, entity)
+
+- if member || can_edit
+  .dropdown.project-settings-dropdown
+    %a.dropdown-new.btn.btn-gray{ href: '#', id: "#{entity.class.to_s.underscore}-settings-button", data: { toggle: 'dropdown' } }
+      = icon('cog')
+      = icon('caret-down')
+    %ul.dropdown-menu.dropdown-menu-align-right
+      - if can_edit
+        %li
+          = link_to "Edit #{entity.class.to_s}", [:edit, entity]
+
+      - if member
+        %li
+          = link_to "Leave #{entity.class.to_s}",
+                    leave_path(entity),
+                    method: :delete,
+                    data: { confirm: leave_confirmation_message(entity) }
+- elsif entity.access_requested?(current_user)
+  = link_to 'Withdraw Request',
+            leave_path(entity),
+            data: { confirm: withdraw_request_message(entity) },
+            method: :delete,
+            class: 'btn btn-grouped btn-gray'
+- else
+  = link_to 'Request Access',
+            request_access_path(entity),
+            method: :post,
+            class: 'btn btn-grouped btn-gray'

app/views/shared/groups/_group.html.haml

@@ -9,7 +9,7 @@
         = link_to edit_group_path(group), class: "btn" do
           = icon('cogs')
-      = link_to leave_group_group_members_path(group), data: { confirm: leave_group_message(group.name) }, method: :delete, class: "btn", title: 'Leave this group' do
+      = link_to leave_group_group_members_path(group), data: { confirm: leave_confirmation_message(group) }, method: :delete, class: "btn", title: 'Leave this group' do
         = icon('sign-out')
   .stats

app/views/groups/group_members/_group_member.html.haml → app/views/shared/members/_member.html.haml

-- user = member.user
-- return unless user || member.invite?
 - show_roles = local_assigns.fetch(:show_roles, true)
-%li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)}
-  %span{class: ("list-item-name" if show_controls)}
-    - if member.user
+- show_controls = local_assigns.fetch(:show_controls, true)
+- user = member.request? ? member.created_by : member.user
+%li.js-toggle-container{ class: dom_class(member), id: dom_id(member) }
+  %span{ class: ("list-item-name" if show_controls) }
+    - if user
       = image_tag avatar_icon(user, 24), class: "avatar s24", alt: ''
       %strong
         = link_to user.name, user_path(user)
       %span.cgray= user.username
+
       - if user == current_user
         %span.label.label-success It's you
+
       - if user.blocked?
         %label.label.label-danger
           %strong Blocked
+
+      - if member.request?
+        %small
+          – Requested
+          = time_ago_with_tooltip(member.requested_at)
     - else
       = image_tag avatar_icon(member.invite_email, 24), class: "avatar s24", alt: ''
-      %strong
-        = member.invite_email
+      %strong= member.invite_email
       %span.cgray
         invited
         - if member.created_by
@@ -25,33 +31,47 @@
           = link_to member.created_by.name, user_path(member.created_by)
         = time_ago_with_tooltip(member.created_at)
-      - if show_controls && can?(current_user, :admin_group_member, @group)
-        = link_to resend_invite_group_group_member_path(@group, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do
-          Resend invite
-  - if show_roles && should_user_see_group_roles?(current_user, @group)
+      - if show_controls && can?(current_user, action_member_permission(:admin, member), member.source)
+        = link_to 'Resend invite', resend_invite_member_path(member),
+                  method: :post,
+                  class: 'btn-xs btn'
+  - if show_roles && can_see_entity_roles?(current_user, member.source)
     %span.pull-right
-      %strong.member-access-level= member.human_access
+      %strong= member.human_access
       - if show_controls
-        - if can?(current_user, :update_group_member, member)
-          = button_tag class: "btn-xs btn btn-grouped inline js-toggle-button",
-                       title: 'Edit access level', type: 'button' do
-            = icon('pencil')
-        - if can?(current_user, :destroy_group_member, member)
+        - if can?(current_user, action_member_permission(:update, member), member)
+          = button_tag icon('pencil'),
+                       type: 'button',
+                       class: 'btn-xs btn btn-grouped inline js-toggle-button',
+                       title: 'Edit access level'
+
+          - if member.request?
+             
+            = link_to icon('check inverse'), approve_request_member_path(member),
+                      method: :post,
+                      type: 'button',
+                      class: 'btn-xs btn btn-success',
+                      title: 'Grant access'
+        - if can?(current_user, action_member_permission(:destroy, member), member)
            
           - if current_user == user
-            = link_to leave_group_group_members_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from group' do
+            = link_to leave_path(member.source), data: { confirm: leave_confirmation_message(member.source)}, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from group' do
               = icon("sign-out")
               Leave
           - else
-            = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do
-              = icon('trash')
+            = link_to icon('trash'), member_path(member),
+                      method: :delete,
+                      remote: true,
+                      data: { confirm: remove_member_message(member) },
+                      class: 'btn-xs btn btn-remove',
+                      title: remove_member_title(member)
     .edit-member.hide.js-toggle-content
       %br
-      = form_for [@group, member], remote: true do |f|
+      = form_for member_path(member), as: "#{member.source.class.to_s.underscore}_member".to_sym, remote: true do |f|
         .prepend-top-10
-          = f.select :access_level, options_for_select(GroupMember.access_level_roles, member.access_level), {}, class: 'form-control'
+          = f.select :access_level, options_for_select(member_class(member).access_level_roles, member.access_level), {}, class: 'form-control'
         .prepend-top-10
           = f.submit 'Save', class: 'btn btn-save btn-sm'

app/views/shared/members/_requests.html.haml

+- requesters = members.request
+
+- if requesters.any?
+  .panel.panel-default
+    .panel-heading
+      %strong= entity.name
+      access requests
+      %small= "(#{requesters.size})"
+    %ul.content-list
+      = render partial: 'shared/members/member', collection: requesters, as: :member

config/routes.rb

@@ -410,8 +410,15 @@ Rails.application.routes.draw do
     scope module: :groups do
       resources :group_members, only: [:index, :create, :update, :destroy] do
-        post :resend_invite, on: :member
-        delete :leave, on: :collection
+        collection do
+          delete :leave
+          post :request_access
+        end
+
+        member do
+          post :resend_invite
+          post :approve
+        end
       end
       resource :avatar, only: [:destroy]
@@ -778,7 +785,7 @@ Rails.application.routes.draw do
           member do
             post :resend_invite
-            post :approval
+            post :approve
           end
         end

db/migrate/20160314114439_add_membership_request.rb

-class AddMembershipRequest < ActiveRecord::Migration
-  def change
-    add_column :members, :requested, :boolean
-  end
-end

db/migrate/20160314114439_add_requested_at_to_members.rb

+class AddRequestedAtToMembers < ActiveRecord::Migration
+  def change
+    add_column :members, :requested_at, :datetime
+  end
+end

db/schema.rb

@@ -536,7 +536,7 @@ ActiveRecord::Schema.define(version: 20160610301627) do
     t.string   "invite_email"
     t.string   "invite_token"
     t.datetime "invite_accepted_at"
-    t.boolean  "requested"
+    t.datetime "requested_at"
   end
   add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree

features/steps/group/members.rb

@@ -128,9 +128,7 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
     member = mary_jane_member
     page.within "#group_member_#{member.id}" do
-      page.within '.member-access-level' do
-        expect(page).to have_content "Developer"
-      end
+      expect(page).to have_content "Developer"
     end
   end

lib/api/project_members.rb

@@ -46,7 +46,7 @@ module API
         required_attributes! [:user_id, :access_level]
         # either the user is already a team member or a new one
-        project_member = user_project.project_member_by_id(params[:user_id])
+        project_member = user_project.project_member(params[:user_id])
         if project_member.nil?
           project_member = user_project.project_members.new(
             user_id: params[:user_id],

spec/controllers/groups/group_members_controller_spec.rb

@@ -4,17 +4,211 @@ describe Groups::GroupMembersController do
   let(:user)  { create(:user) }
   let(:group) { create(:group) }
-  context "index" do
+  describe '#index' do
     before do
       group.add_owner(user)
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
     end
     it 'renders index with group members' do
-      get :index, group_id: group.path
+      get :index, group_id: group
       expect(response.status).to eq(200)
       expect(response).to render_template(:index)
     end
   end
+
+  describe '#destroy' do
+    let(:group) { create(:group, :public) }
+
+    context 'when member is not found' do
+      it 'returns 403' do
+        delete :destroy, group_id: group,
+                         id: 42
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'when member is found' do
+      let(:user) { create(:user) }
+      let(:group_user) { create(:user) }
+      let(:member) do
+        group.add_developer(group_user)
+        group.group_members.find_by(user_id: group_user.id)
+      end
+
+      context 'when user does not have enough rights' do
+        before do
+          group.add_developer(user)
+          sign_in(user)
+        end
+
+        it 'returns 403' do
+          delete :destroy, group_id: group,
+                           id: member
+
+          expect(response.status).to eq(403)
+          expect(group.users).to include group_user
+        end
+      end
+
+      context 'when user has enough rights' do
+        before do
+          group.add_owner(user)
+          sign_in(user)
+        end
+
+        it '[HTML] removes user from members' do
+          delete :destroy, group_id: group,
+                           id: member
+
+          expect(response).to set_flash.to 'User was successfully removed from group.'
+          expect(response).to redirect_to(group_group_members_path(group))
+          expect(group.users).not_to include group_user
+        end
+
+        it '[JS] removes user from members' do
+          xhr :delete, :destroy, group_id: group,
+                                 id: member
+
+          expect(response).to be_success
+          expect(group.users).not_to include group_user
+        end
+      end
+    end
+  end
+
+  describe '#leave' do
+    let(:group) { create(:group, :public) }
+    let(:user) { create(:user) }
+
+    context 'when member is not found' do
+      before { sign_in(user) }
+
+      it 'returns 403' do
+        delete :leave, group_id: group
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'when member is found' do
+      context 'and is not an owner' do
+        before do
+          group.add_developer(user)
+          sign_in(user)
+        end
+
+        it 'removes user from members' do
+          delete :leave, group_id: group
+
+          expect(response).to set_flash.to "You left #{group.name} group."
+          expect(response).to redirect_to(dashboard_groups_path)
+          expect(group.users).not_to include user
+        end
+      end
+
+      context 'and is an owner' do
+        before do
+          group.add_owner(user)
+          sign_in(user)
+        end
+
+        it 'cannot removes himself from the group' do
+          delete :leave, group_id: group
+
+          expect(response).to redirect_to(dashboard_groups_path)
+          expect(response).to set_flash[:alert].to "You can not leave #{group.name} group because you're the last owner. Transfer or delete the group."
+          expect(group.users).to include user
+        end
+      end
+
+      context 'and is a requester' do
+        before do
+          group.request_access(user)
+          sign_in(user)
+        end
+
+        it 'removes user from members' do
+          delete :leave, group_id: group
+
+          expect(response).to set_flash.to 'You withdrawn your access request to the group.'
+          expect(response).to redirect_to(dashboard_groups_path)
+          expect(group.group_members.request).to be_empty
+          expect(group.users).not_to include user
+        end
+      end
+    end
+  end
+
+  describe '#request_access' do
+    let(:group) { create(:group, :public) }
+    let(:user) { create(:user) }
+
+    before do
+      sign_in(user)
+    end
+
+    it 'creates a new GroupMember that is not a team member' do
+      post :request_access, group_id: group
+
+      expect(response).to set_flash.to 'Your request for access has been queued for review.'
+      expect(response).to redirect_to(group_path(group))
+      expect(group.group_members.request.find_by(created_by_id: user.id).created_by).to eq user
+      expect(group.users).not_to include user
+    end
+  end
+
+  describe '#approve' do
+    let(:group) { create(:group, :public) }
+
+    context 'when member is not found' do
+      it 'returns 403' do
+        post :approve, group_id: group,
+                       id: 42
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'when member is found' do
+      let(:user) { create(:user) }
+      let(:group_requester) { create(:user) }
+      let(:member) do
+        group.request_access(group_requester)
+        group.group_members.request.find_by(created_by_id: group_requester.id)
+      end
+
+      context 'when user does not have enough rights' do
+        before do
+          group.add_developer(user)
+          sign_in(user)
+        end
+
+        it 'returns 403' do
+          post :approve, group_id: group,
+                         id: member
+
+          expect(response.status).to eq(403)
+          expect(group.users).not_to include group_requester
+        end
+      end
+
+      context 'when user has enough rights' do
+        before do
+          group.add_owner(user)
+          sign_in(user)
+        end
+
+        it 'adds user to members' do
+          post :approve, group_id: group,
+                         id: member
+
+          expect(response).to redirect_to(group_group_members_path(group))
+          expect(group.users).to include group_requester
+        end
+      end
+    end
+  end
 end