UI and copywriting improvements

+ Move 'Edit Project/Group' out of membership-related partial
+ Show the access request buttons only to logged-in users
+ Put the request access buttons out of in a more visible button
+ Improve the copy in the #remove_member_message helper
Signed-off-by: Rémy Coutable <remy@rymai.me>

app/assets/stylesheets/pages/groups.scss

@@ -39,3 +39,20 @@
     }
   }
 }
+
+.groups-cover-block {
+
+  .container-fluid {
+    position: relative;
+  }
+
+  .access-request-button {
+    @include btn-gray;
+    position: absolute;
+    right: 16px;
+    bottom: 32px;
+    padding: 3px 10px;
+    text-transform: none;
+    background-color: $background-color;
+  }
+}

app/assets/stylesheets/pages/projects.scss

@@ -229,13 +229,20 @@
     right: 16px;
     bottom: 0;
 
-    .btn {
-      padding: 3px 10px;
-      background-color: $background-color;
+    @media (max-width: $screen-lg-min) {
+      top: 0;
     }
 
-    @media (max-width: 1304px) {
-      top: 0;
+    .access-request-button {
+      position: absolute;
+      right: 0;
+      bottom: 61px;
+
+      @media (max-width: $screen-lg-min) {
+        position: relative;
+        bottom: 0;
+        margin-right: 10px;
+      }
     }
   }
 

app/controllers/concerns/access_request_actions.rb

-module AccessRequestActions
-  extend ActiveSupport::Concern
-
-  def request_access
-    access_requestable_resource.request_access(current_user)
-
-    redirect_to access_requestable_resource_path,
-                notice: 'Your request for access has been queued for review.'
-  end
-
-  def approve_access_request
-    @member = access_requestable_resource.public_send(member_entity_name.pluralize).request.find(params[:id])
-
-    return render_403 unless can?(current_user, :"update_#{member_entity_name}", @member)
-
-    @member.accept_request
-
-    redirect_to access_requestable_resource_members_path
-  end
-
-  protected
-
-  def access_requestable_resource
-    raise NotImplementedError
-  end
-
-  def access_requestable_resource_path
-    access_requestable_resource
-  end
-
-  def access_requestable_resource_members_path
-    [access_requestable_resource, 'members']
-  end
-
-  def member_entity_name
-    "#{access_requestable_resource.class.to_s.underscore}_member"
-  end
-end

app/controllers/concerns/membership_actions.rb

+module MembershipActions
+  extend ActiveSupport::Concern
+  include MembersHelper
+
+  def request_access
+    membershipable.request_access(current_user)
+
+    redirect_to polymorphic_path(membershipable),
+                notice: 'Your request for access has been queued for review.'
+  end
+
+  def approve_access_request
+    @member = membershipable.members.request.find(params[:id])
+
+    return render_403 unless can?(current_user, action_member_permission(:update, @member), @member)
+
+    @member.accept_request
+
+    redirect_to polymorphic_url([membershipable, :members])
+  end
+
+  def leave
+    @member = membershipable.members.find_by(user_id: current_user)
+    return render_403 unless @member
+
+    source_type = @member.real_source_type.humanize(capitalize: false)
+
+    if can?(current_user, action_member_permission(:destroy, @member), @member)
+      notice =
+        if @member.request?
+          "Your access request to the #{source_type} has been withdrawn."
+        else
+          "You left the \"#{@member.source.human_name}\" #{source_type}."
+        end
+      @member.destroy
+
+      redirect_to [:dashboard, @member.real_source_type.tableize], notice: notice
+    else
+      if cannot_leave?
+        alert = "You can not leave the \"#{@member.source.human_name}\" #{source_type}."
+        alert << " Transfer or delete the #{source_type}."
+        redirect_to polymorphic_url(membershipable), alert: alert
+      else
+        render_403
+      end
+    end
+  end
+
+  protected
+
+  def membershipable
+    raise NotImplementedError
+  end
+
+  def cannot_leave?
+    raise NotImplementedError
+  end
+end

app/controllers/groups/group_members_controller.rb

 class Groups::GroupMembersController < Groups::ApplicationController
-  include AccessRequestActions
+  include MembershipActions
 
   # Authorize
   before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access]
@@ -38,7 +38,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
 
     return render_403 unless can?(current_user, :destroy_group_member, @group_member)
 
-    @group_member.request? ? @group_member.decline_request : @group_member.destroy
+    @group_member.destroy
 
     respond_to do |format|
       format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
@@ -60,46 +60,16 @@ class Groups::GroupMembersController < Groups::ApplicationController
     end
   end
 
-  def leave
-    @group_member =
-      @group.group_members.find_by(user_id: current_user.id) ||
-      @group.group_members.find_by(created_by_id: current_user.id)
-
-    if can?(current_user, :destroy_group_member, @group_member)
-      notice =
-        if @group_member.request?
-          'You withdrawn your access request to the group.'
-        else
-          "You left #{@group.name} group."
-        end
-      @group_member.destroy
-
-      redirect_to dashboard_groups_path, notice: notice
-    else
-      if @group.last_owner?(current_user)
-        redirect_to(dashboard_groups_path, alert: "You can not leave #{group.name} group because you're the last owner. Transfer or delete the group.")
-      else
-        return render_403
-      end
-    end
-  end
-
   protected
 
   def member_params
     params.require(:group_member).permit(:access_level, :user_id)
   end
 
-  # AccessRequestActions concern
-  def access_requestable_resource
-    @group
-  end
-
-  def access_requestable_resource_path
-    group_path(@group)
-  end
+  # MembershipActions concern
+  alias_method :membershipable, :group
 
-  def access_requestable_resource_members_path
-    group_group_members_path(@group)
+  def cannot_leave?
+    @group.last_owner?(current_user)
   end
 end

app/controllers/projects/project_members_controller.rb

 class Projects::ProjectMembersController < Projects::ApplicationController
-  include AccessRequestActions
+  include MembershipActions
 
   # Authorize
   before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access]
@@ -52,7 +52,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
     return render_403 unless can?(current_user, :destroy_project_member, @project_member)
 
-    @project_member.request? ? @project_member.decline_request : @project_member.destroy
+    @project_member.destroy
 
     respond_to do |format|
       format.html do
@@ -76,31 +76,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
     end
   end
 
-  def leave
-    @project_member =
-      @project.project_members.find_by(user_id: current_user.id) ||
-      @project.project_members.find_by(created_by_id: current_user.id)
-
-    if can?(current_user, :destroy_project_member, @project_member)
-      notice =
-        if @project_member.request?
-          'You withdrawn your access request to the project.'
-        else
-          'You left the project.'
-        end
-      @project_member.destroy
-
-      redirect_to dashboard_projects_path, notice: notice
-    else
-      if current_user == @project.owner
-        message = 'You can not leave your own project. Transfer or delete the project.'
-        redirect_back_or_default(default: { action: 'index' }, options: { alert: message })
-      else
-        render_403
-      end
-    end
-  end
-
   def apply_import
     source_project = Project.find(params[:source_project_id])
 
@@ -121,16 +96,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController
     params.require(:project_member).permit(:user_id, :access_level)
   end
 
-  # AccessRequestActions concern
-  def access_requestable_resource
-    @project
-  end
-
-  def access_requestable_resource_path
-    namespace_project_path(@project.namespace, @project)
-  end
+  # MembershipActions concern
+  alias_method :membershipable, :project
 
-  def access_requestable_resource_members_path
-    namespace_project_project_members_path(@project.namespace, @project)
+  def cannot_leave?
+    current_user == @project.owner
   end
 end

app/helpers/gitlab_routing_helper.rb

@@ -13,10 +13,23 @@
 #   merge_request_path(merge_request)
 #
 module GitlabRoutingHelper
+  # Project
   def project_path(project, *args)
     namespace_project_path(project.namespace, project, *args)
   end
 
+  def project_url(project, *args)
+    namespace_project_url(project.namespace, project, *args)
+  end
+
+  def edit_project_path(project, *args)
+    edit_namespace_project_path(project.namespace, project, *args)
+  end
+
+  def edit_project_url(project, *args)
+    edit_namespace_project_url(project.namespace, project, *args)
+  end
+
   def project_files_path(project, *args)
     namespace_project_tree_path(project.namespace, project, @ref || project.repository.root_ref)
   end
@@ -41,10 +54,6 @@ module GitlabRoutingHelper
     activity_namespace_project_path(project.namespace, project, *args)
   end
 
-  def edit_project_path(project, *args)
-    edit_namespace_project_path(project.namespace, project, *args)
-  end
-
   def runners_path(project, *args)
     namespace_project_runners_path(project.namespace, project, *args)
   end
@@ -65,14 +74,6 @@ module GitlabRoutingHelper
     namespace_project_milestone_path(entity.project.namespace, entity.project, entity, *args)
   end
 
-  def project_url(project, *args)
-    namespace_project_url(project.namespace, project, *args)
-  end
-
-  def edit_project_url(project, *args)
-    edit_namespace_project_url(project.namespace, project, *args)
-  end
-
   def issue_url(entity, *args)
     namespace_project_issue_url(entity.project.namespace, entity.project, entity, *args)
   end
@@ -92,4 +93,56 @@ module GitlabRoutingHelper
       toggle_subscription_namespace_project_merge_request_path(entity.project.namespace, entity.project, entity)
     end
   end
+
+  ## Members
+  def project_members_url(project, *args)
+    namespace_project_project_members_url(project.namespace, project)
+  end
+
+  def project_member_path(project_member, *args)
+    namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member)
+  end
+
+  def request_access_project_members_path(project, *args)
+    request_access_namespace_project_project_members_path(project.namespace, project)
+  end
+
+  def leave_project_members_path(project, *args)
+    leave_namespace_project_project_members_path(project.namespace, project)
+  end
+
+  def approve_access_request_project_member_path(project_member, *args)
+    approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member)
+  end
+
+  def resend_invite_project_member_path(project_member, *args)
+    resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member)
+  end
+
+  # Groups
+
+  ## Members
+  def group_members_url(group, *args)
+    group_group_members_url(group, *args)
+  end
+
+  def group_member_path(group_member, *args)
+    group_group_member_path(group_member.source, group_member)
+  end
+
+  def request_access_group_members_path(group, *args)
+    request_access_group_group_members_path(group)
+  end
+
+  def leave_group_members_path(group, *args)
+    leave_group_group_members_path(group)
+  end
+
+  def approve_access_request_group_member_path(group_member, *args)
+    approve_access_request_group_group_member_path(group_member.source, group_member)
+  end
+
+  def resend_invite_group_member_path(group_member, *args)
+    resend_invite_group_group_member_path(group_member.source, group_member)
+  end
 end

app/helpers/members_helper.rb

 module MembersHelper
-  def member_class(member)
-    "#{member.source.class.to_s}Member".constantize
-  end
-
-  def members_association(entity)
-    "#{entity.class.to_s.underscore}_members".to_sym
-  end
-
+  # Returns a `<action>_<source>_member` association, e.g.:
+  # - admin_project_member, update_project_member, destroy_project_member
+  # - admin_group_member, update_group_member, destroy_group_member
   def action_member_permission(action, member)
-    "#{action}_#{member.source.class.to_s.underscore}_member".to_sym
+    "#{action}_#{member.type.underscore}".to_sym
   end
 
-  def can_see_entity_roles?(user, entity)
+  def can_see_member_roles?(source:, user: nil)
     return false unless user
 
-    user.is_admin? || entity.send(members_association(entity)).exists?(user_id: user.id)
-  end
-
-  def member_path(member)
-    case member.source
-    when Project
-      namespace_project_project_member_path(member.source.namespace, member.source, member)
-    when Group
-      group_group_member_path(member.source, member)
-    else
-      raise ArgumentError.new('Unknown object class')
-    end
+    user.is_admin? || source.members.exists?(user_id: user.id)
   end
 
-  def resend_invite_member_path(member)
-    case member.source
-    when Project
-      resend_invite_namespace_project_project_member_path(member.source.namespace, member.source, member)
-    when Group
-      resend_invite_group_group_member_path(member.source, member)
-    else
-      raise ArgumentError.new('Unknown object class')
-    end
-  end
+  def remove_member_message(member, user: nil)
+    user = current_user if defined?(current_user)
 
-  def request_access_path(entity)
-    case entity
-    when Project
-      request_access_namespace_project_project_members_path(entity.namespace, entity)
-    when Group
-      request_access_group_group_members_path(entity)
-    else
-      raise ArgumentError.new('Unknown object class')
-    end
-  end
-
-  def approve_request_member_path(member)
-    case member.source
-    when Project
-      approve_access_request_namespace_project_project_member_path(member.source.namespace, member.source, member)
-    when Group
-      approve_access_request_group_group_member_path(member.source, member)
-    else
-      raise ArgumentError.new('Unknown object class')
-    end
-  end
+    text = 'Are you sure you want to '
+    action =
+      if member.request?
+        if member.user == user
+          'withdraw your access request for'
+        else
+          "deny #{member.user.name}'s request to join"
+        end
+      elsif member.invite?
+        "revoke the invitation for #{member.invite_email} to join"
+      else
+        "remove #{member.user.name} from"
+      end
 
-  def leave_path(entity)
-    case entity
-    when Project
-      leave_namespace_project_project_members_path(entity.namespace, entity)
-    when Group
-      leave_group_group_members_path(entity)
-    else
-      raise ArgumentError.new('Unknown object class')
-    end
-  end
-
-  def withdraw_request_message(entity)
-    "Are you sure you want to withdraw your access request for the \"#{entity_name(entity)}\" #{entity_type(entity)}?"
-  end
-
-  def remove_member_message(member)
-    entity = member.source
-    entity_type = entity_type(entity)
-    entity_name = entity_name(entity)
-
-    if member.request?
-      "You are going to deny #{member.created_by.name}'s request to join the #{entity_name} #{entity_type}. Are you sure?"
-    elsif member.invite?
-      "You are going to revoke the invitation for #{member.invite_email} to join the #{entity_name} #{entity_type}. Are you sure?"
-    else
-      "You are going to remove #{member.user.name} from the #{entity_name} #{entity_type}. Are you sure?"
-    end
+    text << action << " the #{member.source.human_name} #{member.real_source_type.humanize(capitalize: false)}?"
   end
 
   def remove_member_title(member)
-    member.request? ? 'Deny access request' : 'Remove user'
-  end
-
-  def leave_confirmation_message(entity)
-    "Are you sure you want to leave \"#{entity_name(entity)}\" #{entity_type(entity)}?"
-  end
-
-  private
+    text = " from #{member.real_source_type.humanize(capitalize: false)}"
 
-  def entity_type(entity)
-    entity.class.to_s.underscore
+    text.prepend(member.request? ? 'Deny access request' : 'Remove user')
   end
 
-  def entity_name(entity)
-    case entity
-    when Project
-      entity.name_with_namespace
-    when Group
-      entity.name
-    else
-      raise ArgumentError.new('Unknown object class')
-    end
+  def leave_confirmation_message(member_source)
+    "Are you sure you want to leave the " \
+    "\"#{member_source.human_name}\" #{member_source.class.to_s.humanize(capitalize: false)}?"
   end
 end

app/helpers/projects_helper.rb

 module ProjectsHelper
-  def max_access_level(project, user)
-    Gitlab::Access.options_with_owner.key(project.team.max_member_access(user.id))
-  end
-
   def link_to_project(project)
     link_to [project.namespace.becomes(Namespace), project], title: h(project.name) do
       title = content_tag(:span, project.name, class: 'project-name')

app/mailers/emails/members.rb

 module Emails
   module Members
     extend ActiveSupport::Concern
+    include MembersHelper
 
     included do
-      attr_reader :member_target_type
-      helper_method :member, :access_requester, :member_target_type, :member_target_name, :member_target_url
+      helper_method :member_source, :member
     end
 
-    def member_access_requested_email(member_target_type, member_id)
-      @member_target_type = member_target_type
+    def member_access_requested_email(member_source_type, member_id)
+      @member_source_type = member_source_type
       @member_id = member_id
 
-      admins = User.where(id: target.public_send(members_association).admins.pluck(:user_id)).pluck(:notification_email)
+      admins = member_source.members.owners_and_masters.includes(:user).pluck(:notification_email)
 
       mail(to: admins,
-           subject: subject("Request to join the #{member_target_name} #{member_target_type}"))
+           subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}"))
     end
 
-    def member_access_granted_email(member_target_type, member_id)
-      @member_target_type = member_target_type
+    def member_access_granted_email(member_source_type, member_id)
+      @member_source_type = member_source_type
       @member_id = member_id
 
       mail(to: member.user.notification_email,
-           subject: subject("Access to the #{member_target_name} #{member_target_type} was granted"))
+           subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
     end
 
-    def member_access_denied_email(member_target_type, target_id, user_id)
-      @member_target_type = member_target_type
-      @target = target_class.find(target_id)
+    def member_access_denied_email(member_source_type, source_id, user_id)
+      @member_source_type = member_source_type
+      @member_source = member_source_class.find(source_id)
+      requester = User.find(user_id)
 
-      mail(to: User.find(user_id).notification_email,
-           subject: subject("Access to the #{member_target_name} #{member_target_type} was denied"))
+      mail(to: requester.notification_email,
+           subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied"))
     end
 
-    def member_invited_email(member_target_type, member_id, token)
-      @member_target_type = member_target_type
+    def member_invited_email(member_source_type, member_id, token)
+      @member_source_type = member_source_type
       @member_id = member_id
       @token = token
 
       mail(to: member.invite_email,
-           subject: "Invitation to join the #{member_target_name} #{member_target_type}")
+           subject: "Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}")
     end
 
-    def member_invite_accepted_email(member_target_type, member_id)
-      @member_target_type = member_target_type
+    def member_invite_accepted_email(member_source_type, member_id)
+      @member_source_type = member_source_type
       @member_id = member_id
-      return if access_requester.nil?
+      return unless member.created_by
 
-      mail(to: access_requester.notification_email,
+      mail(to: member.created_by.notification_email,
            subject: subject('Invitation accepted'))
     end
 
-    def member_invite_declined_email(member_target_type, target_id, invite_email, created_by_id)
-      return if created_by_id.nil?
+    def member_invite_declined_email(member_source_type, source_id, invite_email, created_by_id)
+      return unless created_by_id
 
-      @member_target_type = member_target_type
-      @target = target_class.find(target_id)
+      @member_source_type = member_source_type
+      @member_source = member_source_class.find(source_id)
       @invite_email = invite_email
+      inviter = User.find(created_by_id)
 
-      mail(to: User.find(created_by_id).notification_email,
+      mail(to: inviter.notification_email,
            subject: subject('Invitation declined'))
     end
 
     def member
-      @member ||= member_class.find(@member_id)
+      @member ||= Member.find(@member_id)
     end
 
-    def access_requester
-      @access_requester ||= member.created_by
-    end
-
-    def member_target_name
-      case member_target_type
-      when 'project'
-        target.name_with_namespace
-      else
-        target.name
-      end
-    end
-
-    def member_target_url
-      @member_target_url ||= target.web_url
+    def member_source
+      @member_source ||= member.source
     end
 
     private
 
-    def target
-      @target ||= member.public_send(member_target_type)
-    end
-
-    def target_class
-      @target_class ||= member_target_type.classify.constantize
-    end
-
-    def member_class
-      @member_class ||= "#{member_target_type.classify}Member".constantize
-    end
-
-    def members_association
-      @members_association ||= member_class.to_s.tableize
+    def member_source_class
+      @member_source_type.classify.constantize
     end
   end
 end

app/mailers/notify.rb

@@ -13,6 +13,8 @@ class Notify < BaseMailer
   add_template_helper DiffHelper
   add_template_helper BlobHelper
   add_template_helper EmailsHelper
+  add_template_helper MembersHelper
+  add_template_helper GitlabRoutingHelper
 
   def test_email(recipient_email, subject, body)
     mail(to: recipient_email,

app/models/ability.rb

@@ -460,8 +460,6 @@ class Ability
           rules << :destroy_group_member
         elsif user == target_user
           rules << :destroy_group_member
-        elsif subject.request? && user == subject.created_by
-          rules << :destroy_group_member
         end
       end
 
@@ -481,8 +479,6 @@ class Ability
           rules << :destroy_project_member
         elsif user == target_user
           rules << :destroy_project_member
-        elsif subject.request? && user == subject.created_by
-          rules << :destroy_project_member
         end
       end
 

app/models/concerns/access_requestable.rb

@@ -10,18 +10,7 @@ module AccessRequestable
   def request_access(user)
     members.create(
       access_level: Gitlab::Access::DEVELOPER,
-      created_by: user,
+      user: user,
       requested_at: Time.now.utc)
   end
-
-  def access_requested?(user)
-    members.where(created_by_id: user.id).where.not(requested_at: nil).any?
-  end
-
-  private
-
-  # Returns a `<entities>_members` association, e.g.: project_members, group_members
-  def members
-    @members ||= send("#{self.class.to_s.underscore}_members".to_sym)
-  end
 end

app/models/group.rb

@@ -8,7 +8,7 @@ class Group < Namespace
 
   has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
   alias_method :members, :group_members
-  has_many :users, through: :group_members
+  has_many :users, -> { where(members: { requested_at: nil }) }, through: :group_members
   has_many :project_group_links, dependent: :destroy
   has_many :shared_projects, through: :project_group_links, source: :project
   has_many :notification_settings, dependent: :destroy, as: :source

app/models/member.rb

@@ -8,7 +8,7 @@ class Member < ActiveRecord::Base
   belongs_to :user
   belongs_to :source, polymorphic: true
 
-  validates :user, presence: true, unless: :pending?
+  validates :user, presence: true, unless: :invite?
   validates :source, presence: true
   validates :user_id, uniqueness: { scope: [:source_type, :source_id],
                                     message: "already exists in source",
@@ -27,16 +27,17 @@ class Member < ActiveRecord::Base
     }
 
   scope :invite, -> { where.not(invite_token: nil) }
+  scope :non_invite, -> { where(invite_token: nil) }
   scope :request, -> { where.not(requested_at: nil) }
   scope :non_request, -> { where(requested_at: nil) }
-  scope :non_pending, -> { where.not(user_id: nil) }
+  scope :non_pending, -> { non_request.non_invite }
 
   scope :guests, -> { where(access_level: GUEST) }
   scope :reporters, -> { where(access_level: REPORTER) }
   scope :developers, -> { where(access_level: DEVELOPER) }
   scope :masters,  -> { where(access_level: MASTER) }
   scope :owners,  -> { where(access_level: OWNER) }
-  scope :admins,  -> { where(access_level: [OWNER, MASTER]) }
+  scope :owners_and_masters,  -> { where(access_level: [OWNER, MASTER]) }
 
   before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? }
 
@@ -46,6 +47,7 @@ class Member < ActiveRecord::Base
   after_create :post_create_hook, unless: :pending?
   after_update :post_update_hook, unless: :pending?
   after_destroy :post_destroy_hook, unless: :pending?
+  after_destroy :post_decline_request, if: :request?
 
   delegate :name, :username, :email, to: :user, prefix: true
 
@@ -102,36 +104,31 @@ class Member < ActiveRecord::Base
     end
   end
 
-  def pending?
-    request? || invite?
+  def real_source_type
+    source_type
+  end
+
+  def invite?
+    self.invite_token.present?
   end
 
   def request?
-    user.nil? && created_by.present? && requested_at.present?
+    requested_at.present?
   end
 
-  def invite?
-    self.invite_token.present?
+  def pending?
+    invite? || request?
   end
 
   def accept_request
     return false unless request?
 
-    updated = self.update(user: created_by, requested_at: nil)
+    updated = self.update(requested_at: nil)
     after_accept_request if updated
 
     updated
   end
 
-  def decline_request
-    return false unless request?
-
-    self.destroy
-    after_decline_request if destroyed?
-
-    destroyed?
-  end
-
   def accept_invite!(new_user)
     return false unless invite?
 
@@ -217,7 +214,7 @@ class Member < ActiveRecord::Base
     post_create_hook
   end
 
-  def after_decline_request
+  def post_decline_request
     # override in subclass
   end
 

app/models/members/group_member.rb

@@ -20,6 +20,11 @@ class GroupMember < Member
     access_level
   end
 
+  # Because source_type is `Namespace`...
+  def real_source_type
+    'Group'
+  end
+
   private
 
   def send_invite
@@ -60,8 +65,8 @@ class GroupMember < Member
     super
   end
 
-  def after_decline_request
-    notification_service.decline_group_access_request(group, created_by)
+  def post_decline_request
+    notification_service.decline_group_access_request(self)
 
     super
   end

app/models/members/project_member.rb

@@ -152,8 +152,8 @@ class ProjectMember < Member
     super
   end
 
-  def after_decline_request
-    notification_service.decline_project_access_request(project, created_by)
+  def post_decline_request
+    notification_service.decline_project_access_request(self)
 
     super
   end

app/models/project.rb

@@ -104,7 +104,8 @@ class Project < ActiveRecord::Base
   has_many :hooks,              dependent: :destroy, class_name: 'ProjectHook'
   has_many :protected_branches, dependent: :destroy
   has_many :project_members,    dependent: :destroy, as: :source, class_name: 'ProjectMember'
-  has_many :users, through: :project_members
+  alias_method :members, :project_members
+  has_many :users, -> { where(members: { requested_at: nil }) }, through: :project_members
   has_many :deploy_keys_projects, dependent: :destroy
   has_many :deploy_keys, through: :deploy_keys_projects
   has_many :users_star_projects, dependent: :destroy
@@ -690,6 +691,7 @@ class Project < ActiveRecord::Base
                                end
                              end
   end
+  alias_method :human_name, :name_with_namespace
 
   def path_with_namespace
     if namespace

app/models/project_team.rb

@@ -22,12 +22,12 @@ class ProjectTeam
   end
 
   def find_member(user_id)
-    member = project.project_members.find_by(user_id: user_id)
+    member = project.members.non_request.find_by(user_id: user_id)
 
     # If user is not in project members
     # we should check for group membership
     if group && !member
-      member = group.group_members.find_by(user_id: user_id)
+      member = group.members.non_request.find_by(user_id: user_id)
     end
 
     member
@@ -128,12 +128,16 @@ class ProjectTeam
     end
   end
 
+  def human_max_access(user_id)
+    Gitlab::Access.options_with_owner.key(max_member_access(user_id))
+  end
+
   # This method assumes project and group members are eager loaded for optimal
   # performance.
   def max_member_access(user_id)
     access = []
 
-    project.project_members.each do |member|
+    project.members.non_request.each do |member|
       if member.user_id == user_id
         access << member.access_field if member.access_field
         break
@@ -141,7 +145,7 @@ class ProjectTeam
     end
 
     if group
-      group.group_members.each do |member|
+      group.members.non_request.each do |member|
         if member.user_id == user_id
           access << member.access_field if member.access_field
           break
@@ -174,14 +178,14 @@ class ProjectTeam
   end
 
   def fetch_members(level = nil)
-    project_members = project.project_members
-    group_members = group ? group.group_members : []
+    project_members = project.members.non_request
+    group_members = group ? group.members.non_request : []
     invited_members = []
 
     if project.invited_groups.any? && project.allowed_to_share_with_group?
       project.project_group_links.each do |group_link|
         invited_group = group_link.group
-        im = invited_group.group_members
+        im = invited_group.members.non_request
 
         if level
           int_level = GroupMember.access_level_roles[level.to_s.singularize.titleize]

app/services/notification_service.rb

@@ -175,24 +175,24 @@ class NotificationService
 
   # Project access request
   def new_project_access_request(project_member)
-    mailer.member_access_requested_email('project', project_member.id).deliver_later
+    mailer.member_access_requested_email(project_member.real_source_type, project_member.id).deliver_later
   end
 
-  def decline_project_access_request(project, user)
-    mailer.member_access_denied_email('project', project.id, user.id).deliver_later
+  def decline_project_access_request(project_member)
+    mailer.member_access_denied_email(project_member.real_source_type, project_member.project.id, project_member.user.id).deliver_later
   end
 
   def invite_project_member(project_member, token)
-    mailer.member_invited_email('project', project_member.id, token).deliver_later
+    mailer.member_invited_email(project_member.real_source_type, project_member.id, token).deliver_later
   end
 
   def accept_project_invite(project_member)
-    mailer.member_invite_accepted_email('project', project_member.id).deliver_later
+    mailer.member_invite_accepted_email(project_member.real_source_type, project_member.id).deliver_later
   end
 
   def decline_project_invite(project_member)
     mailer.member_invite_declined_email(
-      'project',
+      project_member.real_source_type,
       project_member.project.id,
       project_member.invite_email,
       project_member.access_level,
@@ -201,24 +201,24 @@ class NotificationService
   end
 
   def new_project_member(project_member)
-    mailer.member_access_granted_email('project', project_member.id).deliver_later
+    mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
   end
 
   def update_project_member(project_member)
-    mailer.member_access_granted_email('project', project_member.id).deliver_later
+    mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
   end
 
   # Group access request
   def new_group_access_request(group_member)
-    mailer.member_access_requested_email('group', group_member.id).deliver_later
+    mailer.member_access_requested_email(group_member.real_source_type, group_member.id).deliver_later
   end
 
-  def decline_group_access_request(group, user)
-    mailer.member_access_denied_email('group', group.id, user.id).deliver_later
+  def decline_group_access_request(group_member)
+    mailer.member_access_denied_email(group_member.real_source_type, group_member.group.id, group_member.user.id).deliver_later
   end
 
   def invite_group_member(group_member, token)
-    mailer.member_invited_email('group', group_member.id, token).deliver_later
+    mailer.member_invited_email(group_member.real_source_type, group_member.id, token).deliver_later
   end
 
   def accept_group_invite(group_member)
@@ -227,7 +227,7 @@ class NotificationService
 
   def decline_group_invite(group_member)
     mailer.member_invite_declined_email(
-      'group',
+      group_member.real_source_type,
       group_member.group.id,
       group_member.invite_email,
       group_member.access_level,
@@ -236,11 +236,11 @@ class NotificationService
   end
 
   def new_group_member(group_member)
-    mailer.member_access_granted_email('group', group_member.id).deliver_later
+    mailer.member_access_granted_email(group_member.real_source_type, group_member.id).deliver_later
   end
 
   def update_group_member(group_member)
-    mailer.member_access_granted_email('group', group_member.id).deliver_later
+    mailer.member_access_granted_email(group_member.real_source_type, group_member.id).deliver_later
   end
 
   def project_was_moved(project, old_path_with_namespace)

app/views/groups/group_members/index.html.haml

@@ -11,7 +11,7 @@
         .new-group-member-holder
           = render "new_group_member"
 
-    = render "shared/members/requests", entity: @group, members: @members
+    = render 'shared/members/requests', membership_source: @group, members: @members.request
 
   .panel.panel-default
     .panel-heading

app/views/groups/show.html.haml

@@ -19,6 +19,9 @@
         .cover-desc.description
           = markdown(@group.description, pipeline: :description)
 
+    - if current_user
+      = render 'shared/members/access_request_buttons', source: @group
+
 %div{ class: container_class }
   .top-area
     %ul.nav-links

app/views/layouts/nav/_group_settings.html.haml

 - if current_user
-  .controls
-    = render 'shared/group_or_project_home_dropdown', entity: @group
+  - if access = @group.users.find_by(id: current_user.id)
+    .controls
+      .dropdown.group-settings-dropdown
+        %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'}
+          = icon('cog')
+          = icon('caret-down')
+        %ul.dropdown-menu.dropdown-menu-align-right
+          - if can?(current_user, :admin_group, @group)
+            = nav_link(path: 'groups#projects') do
+              = link_to projects_group_path(@group), title: 'Projects' do
+                Projects
+            %li.divider
+            %li
+              = link_to edit_group_path(@group) do
+                Edit Group

app/views/layouts/nav/_project.html.haml

 - if current_user
   .controls
-    - access = user_max_access_in_project(current_user.id, @project)
-    - can_edit = can?(current_user, :admin_project, @project)
     .dropdown.project-settings-dropdown
       %a.dropdown-new.btn.btn-default#project-settings-button{href: '#', 'data-toggle' => 'dropdown'}
         = icon('cog')
         = icon('caret-down')
       %ul.dropdown-menu.dropdown-menu-align-right
         = render 'layouts/nav/project_settings'
-        %li.divider
-        - if can_edit
-          %li
-            = link_to edit_project_path(@project) do
-              Edit Project
-        - if access
-          %li
-            = link_to leave_path(@project),
-              data: { confirm: leave_confirmation_message(@project) }, method: :delete do
-              Leave Project
-        - elsif @project.access_requested?(current_user)
-          %li
-            = link_to leave_path(@project),
-              data: { confirm: withdraw_request_message(@project) }, method: :delete do
-              Withdraw Request
-        - else
-          %li
-            = link_to request_access_path(@project),
-              class: 'btn btn-gray', style: 'margin-left: 10px', method: :post do
-              Request Access
+
+        - access = @project.team.max_member_access(current_user.id)
+        - can_edit = can?(current_user, :admin_project, @project)
+        - if can_edit || access
+          %li.divider
+          - if can_edit
+            %li
+              = link_to edit_project_path(@project) do
+                Edit Project
+          - if access
+            %li
+              = link_to polymorphic_path([:leave, @project, :members]),
+                data: { confirm: leave_confirmation_message(@project) }, method: :delete do
+                Leave Project
 
 %div{ class: nav_control_class }
   %ul.nav-links.scrolling-tabs

app/views/notify/group_access_granted_email.html.haml

-%p
-  You have been granted #{@group_member.human_access} access to group
-  #{link_to @group.name, @target_url}.

app/views/notify/group_access_granted_email.text.erb

-You have been granted <%= @group_member.human_access %> access to group <%= @group.name %>.
-
-<%= @target_url %>

app/views/notify/group_invite_accepted_email.html.haml

-%p
-  #{@group_member.invite_email}, now known as
-  #{link_to @group_member.user.name, user_url(@group_member.user)},
-  has accepted your invitation to join group
-  #{link_to @group.name, group_url(@group)}.
-

app/views/notify/group_invite_accepted_email.text.erb

-<%= @group_member.invite_email %>, now known as <%= @group_member.user.name %>, has accepted your invitation to join group <%= @group.name %>.
-
-<%= group_url(@group) %>

app/views/notify/group_invite_declined_email.html.haml

-%p
-  #{@invite_email}
-  has declined your invitation to join group
-  #{link_to @group.name, group_url(@group)}.
-

app/views/notify/group_invite_declined_email.text.erb

-<%= @invite_email %> has declined your invitation to join group <%= @group.name %>.
-
-<%= group_url(@group) %>

app/views/notify/group_member_invited_email.html.haml

-%p
-  You have been invited
-  - if inviter = @group_member.created_by
-    by
-    = link_to inviter.name, user_url(inviter)
-  to join group
-  = link_to @group.name, group_url(@group)
-  as #{@group_member.human_access}.
-
-%p
-  = link_to 'Accept invitation', invite_url(@token)
-  or
-  = link_to 'decline', decline_invite_url(@token)
-

app/views/notify/group_member_invited_email.text.erb

-You have been invited <%= "by #{@group_member.created_by.name} " if @group_member.created_by %>to join group <%= @group.name %> as <%= @group_member.human_access %>.
-
-Accept invitation: <%= invite_url(@token) %>
-Decline invitation: <%= decline_invite_url(@token) %>

app/views/notify/member_access_denied_email.html.haml

 %p
   Your request to join the
-  #{link_to member_target_name, member_target_url} #{member_target_type}
+  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}
   has been denied.

app/views/notify/member_access_denied_email.text.erb

-Your request to join the <%= member_target_name %> <%= member_target_type %> has been denied.
+Your request to join the <%= member_source.human_name %> <%= member_source.model_name.singular %> has been denied.
 
-<%= member_target_url %>
+<%= member_source.web_url %>

app/views/notify/member_access_granted_email.html.haml

 %p
   You have been granted #{member.human_access} access to the
-  #{link_to member_target_name, member_target_url} #{member_target_type}.
+  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.

app/views/notify/member_access_granted_email.text.erb

-You have been granted <%= member.human_access %> access to the <%= member_target_name %> <%= member_target_type %>.
+You have been granted <%= member.human_access %> access to the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
 
-<%= member_target_url %>
+<%= member_source.web_url %>

app/views/notify/member_access_requested_email.html.haml

 %p
-  #{link_to access_requester.name, access_requester} requested #{member.human_access}
-  access to the #{link_to member_target_name, member_target_url} #{member_target_type}.
+  #{link_to member.user.name, member.user} requested #{member.human_access}
+  access to the #{link_to member_source.human_name, polymorphic_url([member_source, :members])} #{member_source.model_name.singular}.

app/views/notify/member_access_requested_email.text.erb

-<%= access_requester.name %> (<%= user_url(access_requester) %>) requested <%= member.human_access %> access to the <%= member_target_name %> <%= member_target_type %>.
+<%= member.user.name %> (<%= user_url(member.user) %>) requested <%= member.human_access %> access to the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
 
-<%= member_target_url %>
+<%= polymorphic_url([member_source, :members]) %>

app/views/notify/member_invite_accepted_email.html.haml

@@ -2,4 +2,4 @@
   #{member.invite_email}, now known as
   #{link_to member.user.name, user_url(member.user)},
   has accepted your invitation to join the
-  #{link_to member_target_name, member_target_url} #{member_target_type}.
+  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.

app/views/notify/member_invite_accepted_email.text.erb

-<%= member.invite_email %>, now known as <%= member.user.name %>, has accepted your invitation to join the <%= member_target_name %> <%= member_target_type %>.
+<%= member.invite_email %>, now known as <%= member.user.name %>, has accepted your invitation to join the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
 
-<%= member_target_url %>
+<%= member_source.web_url %>

app/views/notify/member_invite_declined_email.html.haml

 %p
   #{@invite_email}
   has declined your invitation to join the
-  #{link_to member_target_name, member_target_url} #{member_target_type}.
+  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.

app/views/notify/member_invite_declined_email.text.erb

-<%= @invite_email %> has declined your invitation to join the <%= member_target_name %> <%= member_target_type %>.
+<%= @invite_email %> has declined your invitation to join the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
 
-<%= member_target_url %>
+<%= member_source.web_url %>

app/views/notify/member_invited_email.html.haml

 %p
   You have been invited
-  - if access_requester
+  - if member.created_by
     by
-    = link_to access_requester.name, user_url(access_requester)
+    = link_to member.created_by.name, user_url(member.created_by)
   to join the
-  = link_to member_target_name, member_target_url
-  #{member_target_type} as #{member.human_access}.
+  = link_to member_source.human_name, member_source.web_url
+  #{member_source.model_name.singular} as #{member.human_access}.
 
 %p
   = link_to 'Accept invitation', invite_url(@token)

app/views/notify/member_invited_email.text.erb

-You have been invited <%= "by #{access_requester.name} " if access_requester %>to join the <%= member_target_name %> <%= member_target_type %> as <%= member.human_access %>.
+You have been invited <%= "by #{member.created_by.name} " if member.created_by %>to join the <%= member_source.human_name %> <%= member_source.model_name.singular %> as <%= member.human_access %>.
 
 Accept invitation: <%= invite_url(@token) %>
 Decline invitation: <%= decline_invite_url(@token) %>

app/views/projects/_home_panel.html.haml

@@ -29,10 +29,13 @@
           .project-clone-holder
             = render "shared/clone_panel"
 
-      .project-repo-buttons.btn-group.project-right-buttons
-        = render "projects/buttons/download"
-        = render 'projects/buttons/dropdown'
-        = render 'projects/buttons/notifications'
+      .project-repo-buttons.project-right-buttons
+        - if current_user
+          = render 'shared/members/access_request_buttons', source: @project
+        .btn-group
+          = render "projects/buttons/download"
+          = render 'projects/buttons/dropdown'
+          = render 'projects/buttons/notifications'
 
 :javascript
   new Star();

app/views/projects/buttons/_notifications.html.haml

 - if @notification_setting
   = form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), method: :patch, remote: true, html: { class: 'inline', id: 'notification-form' } do |f|
     = f.hidden_field :level
-    .dropdown
+    .dropdown.hidden-sm
       %button.btn.btn-default.notifications-btn#notifications-button{ data: { toggle: "dropdown" }, aria: { haspopup: "true", expanded: "false" } }
         = icon('bell')
         = notification_title(@notification_setting.level)

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 = max_access_level(note.project, note.author)
+          - access = note.project.team.human_max_access(note.author.id)
           - if access
             %span.note-role.hidden-xs= access
           - if current_user

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

@@ -6,8 +6,9 @@
       (#{members.count})
     - if can?(current_user, :admin_group_member, @group)
       .controls
-        = link_to group_group_members_path(@group), class: 'btn' do
-          Manage group members
+        = link_to 'Manage group members',
+                  group_group_members_path(@group),
+                  class: 'btn'
   %ul.content-list
     = render partial: 'shared/members/member',
              collection: members.limit(20),
@@ -15,7 +16,4 @@
              locals: { show_controls: false }
     - if members.size > 20
       %li
-        and
-        = members.size - 20
-        more. For full list visit
-        = link_to 'group members page', group_group_members_path(@group)
+        and #{members.count - 20} more. For full list visit #{link_to 'group members page', group_group_members_path(@group)}

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

@@ -13,9 +13,9 @@
           Users with access to this project are listed below.
         = render "new_project_member"
 
-    = render "shared/members/requests", entity: @project, members: @project_members
+    = render 'shared/members/requests', membership_source: @project, members: @project_members.request
 
-  = render "team", members: @project_members.non_request
+  = render 'team', members: @project_members.non_request
 
   - if @group
     = render "group_members", members: @group_members

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/members/_access_request_buttons.html.haml

+- member = source.members.find_by(user_id: current_user.id)
+
+- if member
+  - if member.request?
+    = link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]),
+              method: :delete,
+              data: { confirm: remove_member_message(member) },
+              class: 'btn access-request-button hidden-xs'
+- else
+  = link_to 'Request Access', polymorphic_path([:request_access, source, :members]),
+            method: :post,
+            class: 'btn access-request-button hidden-xs'

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

 - show_roles = local_assigns.fetch(:show_roles, true)
 - show_controls = local_assigns.fetch(:show_controls, true)
-- user = member.request? ? member.created_by : member.user
+- user = member.user
 
 %li.js-toggle-container{ class: dom_class(member), id: dom_id(member) }
   %span{ class: ("list-item-name" if show_controls) }
@@ -18,25 +18,25 @@
           %strong Blocked
 
       - if member.request?
-        %small
+        %span.cgray
           – 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
       %span.cgray
-        invited
+        – 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 show_controls && can?(current_user, action_member_permission(:admin, member), member.source)
-        = link_to 'Resend invite', resend_invite_member_path(member),
+        = link_to 'Resend invite', polymorphic_path([:resend_invite, member]),
                   method: :post,
                   class: 'btn-xs btn'
 
-  - if show_roles && can_see_entity_roles?(current_user, member.source)
+  - if show_roles && can_see_member_roles?(source: member.source, user: current_user)
     %span.pull-right
       %strong= member.human_access
       - if show_controls
@@ -48,30 +48,30 @@
 
           - if member.request?
              
-            = link_to icon('check inverse'), approve_request_member_path(member),
+            = link_to icon('check inverse'), polymorphic_path([:approve_access_request, 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_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 icon('trash'), member_path(member),
+            = link_to icon('sign-out', text: 'Leave'), polymorphic_path([:leave, member.source, :members]),
                       method: :delete,
+                      data: { confirm: leave_confirmation_message(member.source) },
+                      class: 'btn-xs btn btn-remove'
+          - else
+            = link_to icon('trash'), member,
                       remote: true,
+                      method: :delete,
                       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 member_path(member), as: "#{member.source.class.to_s.underscore}_member".to_sym, remote: true do |f|
+      = form_for member, remote: true do |f|
         .prepend-top-10
-          = f.select :access_level, options_for_select(member_class(member).access_level_roles, member.access_level), {}, class: 'form-control'
+          = f.select :access_level, options_for_select(member.class.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?
+- if members.any?
   .panel.panel-default
     .panel-heading
-      %strong= entity.name
+      %strong= membership_source.name
       access requests
-      %small= "(#{requesters.size})"
+      %small= "(#{members.size})"
     %ul.content-list
-      = render partial: 'shared/members/member', collection: requesters, as: :member
+      = render partial: 'shared/members/member', collection: members, as: :member

features/steps/dashboard/group.rb

@@ -62,6 +62,6 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps
   end
 
   step 'I should see the "Can not leave message"' do
-    expect(page).to have_content "You can not leave Owned group because you're the last owner"
+    expect(page).to have_content "You can not leave the \"Owned\" group."
   end
 end

features/steps/group/members.rb

@@ -53,7 +53,7 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
   step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do
     page.within '.content-list' do
       expect(page).to have_content('sjobs@apple.com')
-      expect(page).to have_content('invited')
+      expect(page).to have_content('Invited')
       expect(page).to have_content('Reporter')
     end
   end
@@ -116,11 +116,9 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
     member = mary_jane_member
 
     page.within "#group_member_#{member.id}" do
-      find(".js-toggle-button").click
-      page.within "#edit_group_member_#{member.id}" do
-        select 'Developer', from: 'group_member_access_level'
-        click_on 'Save'
-      end
+      click_button "Edit access level"
+      select 'Developer', from: 'group_member_access_level'
+      click_on 'Save'
     end
   end
 

features/steps/project/team_management.rb

@@ -26,8 +26,11 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
   end
 
   step 'I should see "Mike" in team list as "Reporter"' do
-    page.within ".access-reporter" do
+    user = User.find_by(name: 'Mike')
+    project_member = project.project_members.find_by(user_id: user.id)
+    page.within "#project_member_#{project_member.id}" do
       expect(page).to have_content('Mike')
+      expect(page).to have_content('Reporter')
     end
   end
 
@@ -40,16 +43,20 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
   end
 
   step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do
-    page.within ".access-reporter" do
+    project_member = project.project_members.find_by(invite_email: 'sjobs@apple.com')
+    page.within "#project_member_#{project_member.id}" do
       expect(page).to have_content('sjobs@apple.com')
-      expect(page).to have_content('invited')
+      expect(page).to have_content('Invited')
       expect(page).to have_content('Reporter')
     end
   end
 
   step 'I should see "Dmitriy" in team list as "Developer"' do
-    page.within ".access-developer" do
+    user = User.find_by(name: 'Dmitriy')
+    project_member = project.project_members.find_by(user_id: user.id)
+    page.within "#project_member_#{project_member.id}" do
       expect(page).to have_content('Dmitriy')
+      expect(page).to have_content('Developer')
     end
   end
 
@@ -65,15 +72,14 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
   end
 
   step 'I should see "Dmitriy" in team list as "Reporter"' do
-    page.within ".access-reporter" do
+    user = User.find_by(name: 'Dmitriy')
+    project_member = project.project_members.find_by(user_id: user.id)
+    page.within "#project_member_#{project_member.id}" do
       expect(page).to have_content('Dmitriy')
+      expect(page).to have_content('Reporter')
     end
   end
 
-  step 'I click link "Remove from team"' do
-    click_link "Remove from team"
-  end
-
   step 'I should not see "Dmitriy" in team list' do
     user = User.find_by(name: "Dmitriy")
     expect(page).not_to have_content(user.name)
@@ -120,7 +126,7 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
     user = User.find_by(name: 'Dmitriy')
     project_member = project.project_members.find_by(user_id: user.id)
     page.within "#project_member_#{project_member.id}" do
-      click_link('Remove user from team')
+      click_link('Remove user from project')
     end
   end
 

lib/api/entities.rb

@@ -88,10 +88,7 @@ module API
     class Group < Grape::Entity
       expose :id, :name, :path, :description, :visibility_level
       expose :avatar_url
-
-      expose :web_url do |group, options|
-        Gitlab::Routing.url_helpers.group_url(group)
-      end
+      expose :web_url
     end
 
     class GroupDetail < Group

spec/controllers/groups/group_members_controller_spec.rb

@@ -35,7 +35,7 @@ describe Groups::GroupMembersController do
       let(:group_user) { create(:user) }
       let(:member) do
         group.add_developer(group_user)
-        group.group_members.find_by(user_id: group_user.id)
+        group.members.find_by(user_id: group_user)
       end
 
       context 'when user does not have enough rights' do
@@ -103,7 +103,7 @@ describe Groups::GroupMembersController do
         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 set_flash.to "You left the \"#{group.name}\" group."
           expect(response).to redirect_to(dashboard_groups_path)
           expect(group.users).not_to include user
         end
@@ -118,8 +118,8 @@ describe Groups::GroupMembersController do
         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(response).to redirect_to(group_path(group))
+          expect(response).to set_flash[:alert].to "You can not leave the \"#{group.name}\" group. Transfer or delete the group."
           expect(group.users).to include user
         end
       end
@@ -133,9 +133,9 @@ describe Groups::GroupMembersController do
         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 set_flash.to 'Your access request to the group has been withdrawn.'
           expect(response).to redirect_to(dashboard_groups_path)
-          expect(group.group_members.request).to be_empty
+          expect(group.members.request).to be_empty
           expect(group.users).not_to include user
         end
       end
@@ -155,18 +155,18 @@ describe Groups::GroupMembersController do
 
       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.members.request.exists?(user_id: user)).to be_truthy
       expect(group.users).not_to include user
     end
   end
 
-  describe '#approve' do
+  describe '#approve_access_request' do
     let(:group) { create(:group, :public) }
 
     context 'when member is not found' do
       it 'returns 403' do
         post :approve_access_request, group_id: group,
-                       id: 42
+                                      id: 42
 
         expect(response.status).to eq(403)
       end
@@ -177,7 +177,7 @@ describe Groups::GroupMembersController do
       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)
+        group.members.request.find_by(user_id: group_requester)
       end
 
       context 'when user does not have enough rights' do
@@ -188,7 +188,7 @@ describe Groups::GroupMembersController do
 
         it 'returns 403' do
           post :approve_access_request, group_id: group,
-                         id: member
+                                        id: member
 
           expect(response.status).to eq(403)
           expect(group.users).not_to include group_requester
@@ -203,7 +203,7 @@ describe Groups::GroupMembersController do
 
         it 'adds user to members' do
           post :approve_access_request, group_id: group,
-                         id: member
+                                        id: member
 
           expect(response).to redirect_to(group_group_members_path(group))
           expect(group.users).to include group_requester

spec/controllers/projects/project_members_controller_spec.rb

@@ -80,7 +80,7 @@ describe Projects::ProjectMembersController do
       let(:team_user) { create(:user) }
       let(:member) do
         project.team << [team_user, :developer]
-        project.project_members.find_by(user_id: team_user.id)
+        project.members.find_by(user_id: team_user.id)
       end
 
       context 'when user does not have enough rights' do
@@ -154,7 +154,7 @@ describe Projects::ProjectMembersController do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 
-          expect(response).to set_flash.to 'You left the project.'
+          expect(response).to set_flash.to "You left the \"#{project.human_name}\" project."
           expect(response).to redirect_to(dashboard_projects_path)
           expect(project.users).not_to include user
         end
@@ -167,14 +167,14 @@ describe Projects::ProjectMembersController do
           sign_in(user)
         end
 
-        it 'cannot removes himself from the project' do
+        it 'cannot remove himself from the project' do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 
           expect(response).to redirect_to(
-            namespace_project_project_members_path(project.namespace, project)
+            namespace_project_path(project.namespace, project)
           )
-          expect(response).to set_flash[:alert].to 'You can not leave your own project. Transfer or delete the project.'
+          expect(response).to set_flash[:alert].to "You can not leave the \"#{project.human_name}\" project. Transfer or delete the project."
           expect(project.users).to include user
         end
       end
@@ -189,9 +189,9 @@ describe Projects::ProjectMembersController do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 
-          expect(response).to set_flash.to 'You withdrawn your access request to the project.'
+          expect(response).to set_flash.to 'Your access request to the project has been withdrawn.'
           expect(response).to redirect_to(dashboard_projects_path)
-          expect(project.project_members.request).to be_empty
+          expect(project.members.request).to be_empty
           expect(project.users).not_to include user
         end
       end
@@ -214,7 +214,7 @@ describe Projects::ProjectMembersController do
       expect(response).to redirect_to(
         namespace_project_path(project.namespace, project)
       )
-      expect(project.project_members.request.find_by(created_by_id: user.id).created_by).to eq user
+      expect(project.members.request.exists?(user_id: user)).to be_truthy
       expect(project.users).not_to include user
     end
   end
@@ -225,8 +225,8 @@ describe Projects::ProjectMembersController do
     context 'when member is not found' do
       it 'returns 404' do
         post :approve_access_request, namespace_id: project.namespace,
-                       project_id: project,
-                       id: 42
+                                      project_id: project,
+                                      id: 42
 
         expect(response.status).to eq(404)
       end
@@ -237,7 +237,7 @@ describe Projects::ProjectMembersController do
       let(:team_requester) { create(:user) }
       let(:member) do
         project.request_access(team_requester)
-        project.project_members.request.find_by(created_by_id: team_requester.id)
+        project.members.request.find_by(user_id: team_requester.id)
       end
 
       context 'when user does not have enough rights' do
@@ -248,8 +248,8 @@ describe Projects::ProjectMembersController do
 
         it 'returns 404' do
           post :approve_access_request, namespace_id: project.namespace,
-                         project_id: project,
-                         id: member
+                                        project_id: project,
+                                        id: member
 
           expect(response.status).to eq(404)
           expect(project.users).not_to include team_requester
@@ -264,8 +264,8 @@ describe Projects::ProjectMembersController do
 
         it 'adds user to members' do
           post :approve_access_request, namespace_id: project.namespace,
-                         project_id: project,
-                         id: member
+                                        project_id: project,
+                                        id: member
 
           expect(response).to redirect_to(
             namespace_project_project_members_path(project.namespace, project)

spec/features/groups/members/owner_manages_access_requests_spec.rb

@@ -22,12 +22,10 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
 
     expect_visible_access_request(group, user)
 
-    perform_enqueued_jobs do
-      click_on 'Grant access'
-    end
+    perform_enqueued_jobs { click_on 'Grant access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was granted/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was granted"
   end
 
   scenario 'master can deny access' do
@@ -35,17 +33,15 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
 
     expect_visible_access_request(group, user)
 
-    perform_enqueued_jobs do
-      click_on 'Deny access'
-    end
+    perform_enqueued_jobs { click_on 'Deny access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was denied/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was denied"
   end
 
 
   def expect_visible_access_request(group, user)
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content "#{group.name} access requests (1)"
     expect(page).to have_content user.name
   end

spec/features/groups/members/user_requests_access_spec.rb

@@ -8,47 +8,41 @@ feature 'Groups > Members > User requests access', feature: true do
   background do
     group.add_owner(owner)
     login_as(user)
+    visit group_path(group)
   end
 
   scenario 'user can request access to a group' do
-    visit group_path(group)
-
-    perform_enqueued_jobs do
-      click_link 'Request Access'
-    end
+    perform_enqueued_jobs { click_link 'Request Access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Request to join #{group.name} group/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group"
 
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content 'Your request for access has been queued for review.'
-    expect(page).to have_content 'Withdraw Request'
+
+    expect(page).to have_content 'Withdraw Access Request'
   end
 
   scenario 'user is not listed in the group members page' do
-    visit group_path(group)
-
     click_link 'Request Access'
 
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
 
     click_link 'Members'
 
-    visit group_group_members_path(group)
     page.within('.content') do
       expect(page).not_to have_content(user.name)
     end
   end
 
   scenario 'user can withdraw its request for access' do
-    visit group_path(group)
     click_link 'Request Access'
 
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
 
-    click_link 'Withdraw Request'
+    click_link 'Withdraw Access Request'
 
-    expect(group.access_requested?(user)).to be_falsey
-    expect(page).to have_content 'You withdrawn your access request to the group.'
+    expect(group.members.request.exists?(user_id: user)).to be_falsey
+    expect(page).to have_content 'Your access request to the group has been withdrawn.'
   end
 end

spec/features/projects/members/master_manages_access_requests_spec.rb

@@ -22,12 +22,10 @@ feature 'Projects > Members > Master manages access requests', feature: true do
 
     expect_visible_access_request(project, user)
 
-    perform_enqueued_jobs do
-      click_on 'Grant access'
-    end
+    perform_enqueued_jobs { click_on 'Grant access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was granted/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.name_with_namespace} project was granted"
   end
 
   scenario 'master can deny access' do
@@ -35,16 +33,14 @@ feature 'Projects > Members > Master manages access requests', feature: true do
 
     expect_visible_access_request(project, user)
 
-    perform_enqueued_jobs do
-      click_on 'Deny access'
-    end
+    perform_enqueued_jobs { click_on 'Deny access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was denied/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.name_with_namespace} project was denied"
   end
 
   def expect_visible_access_request(project, user)
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content "#{project.name} access requests (1)"
     expect(page).to have_content user.name
   end

spec/features/projects/members/user_requests_access_spec.rb

@@ -8,30 +8,27 @@ feature 'Projects > Members > User requests access', feature: true do
   background do
     project.team << [master, :master]
     login_as(user)
+    visit namespace_project_path(project.namespace, project)
   end
 
   scenario 'user can request access to a project' do
-    visit namespace_project_path(project.namespace, project)
-
-    perform_enqueued_jobs do
-      click_link 'Request Access'
-    end
+    perform_enqueued_jobs { click_link 'Request Access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [master.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Request to join #{project.name_with_namespace} project/
+    expect(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.name_with_namespace} project"
 
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content 'Your request for access has been queued for review.'
-    expect(page).to have_content 'Withdraw Request'
+
+    expect(page).to have_content 'Withdraw Access Request'
   end
 
   scenario 'user is not listed in the project members page' do
-    visit namespace_project_path(project.namespace, project)
-
     click_link 'Request Access'
 
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
 
+    open_project_settings_menu
     click_link 'Members'
 
     visit namespace_project_project_members_path(project.namespace, project)
@@ -41,14 +38,17 @@ feature 'Projects > Members > User requests access', feature: true do
   end
 
   scenario 'user can withdraw its request for access' do
-    visit namespace_project_path(project.namespace, project)
     click_link 'Request Access'
 
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
 
-    click_link 'Withdraw Request'
+    click_link 'Withdraw Access Request'
+
+    expect(project.members.request.exists?(user_id: user)).to be_falsey
+    expect(page).to have_content 'Your access request to the project has been withdrawn.'
+  end
 
-    expect(project.access_requested?(user)).to be_falsey
-    expect(page).to have_content 'You withdrawn your access request to the project.'
+  def open_project_settings_menu
+    find('#project-settings-button').click
   end
 end

spec/helpers/gitlab_routing_helper_spec.rb

+require 'spec_helper'
+
+describe GitlabRoutingHelper do
+  describe 'Project URL helpers' do
+    describe '#project_members_url' do
+      let(:project) { build_stubbed(:empty_project) }
+
+      it { expect(project_members_url(project)).to eq namespace_project_project_members_url(project.namespace, project) }
+    end
+
+    describe '#project_member_path' do
+      let(:project_member) { create(:project_member) }
+
+      it { expect(project_member_path(project_member)).to eq namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+    end
+
+    describe '#request_access_project_members_path' do
+      let(:project) { build_stubbed(:empty_project) }
+
+      it { expect(request_access_project_members_path(project)).to eq request_access_namespace_project_project_members_path(project.namespace, project) }
+    end
+
+    describe '#leave_project_members_path' do
+      let(:project) { build_stubbed(:empty_project) }
+
+      it { expect(leave_project_members_path(project)).to eq leave_namespace_project_project_members_path(project.namespace, project) }
+    end
+
+    describe '#approve_access_request_project_member_path' do
+      let(:project_member) { create(:project_member) }
+
+      it { expect(approve_access_request_project_member_path(project_member)).to eq approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+    end
+
+    describe '#resend_invite_project_member_path' do
+      let(:project_member) { create(:project_member) }
+
+      it { expect(resend_invite_project_member_path(project_member)).to eq resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+    end
+  end
+
+  describe 'Group URL helpers' do
+    describe '#group_members_url' do
+      let(:group) { build_stubbed(:group) }
+
+      it { expect(group_members_url(group)).to eq group_group_members_url(group) }
+    end
+
+    describe '#group_member_path' do
+      let(:group_member) { create(:group_member) }
+
+      it { expect(group_member_path(group_member)).to eq group_group_member_path(group_member.source, group_member) }
+    end
+
+    describe '#request_access_group_members_path' do
+      let(:group) { build_stubbed(:group) }
+
+      it { expect(request_access_group_members_path(group)).to eq request_access_group_group_members_path(group) }
+    end
+
+    describe '#leave_group_members_path' do
+      let(:group) { build_stubbed(:group) }
+
+      it { expect(leave_group_members_path(group)).to eq leave_group_group_members_path(group) }
+    end
+
+    describe '#approve_access_request_group_member_path' do
+      let(:group_member) { create(:group_member) }
+
+      it { expect(approve_access_request_group_member_path(group_member)).to eq approve_access_request_group_group_member_path(group_member.source, group_member) }
+    end
+
+    describe '#resend_invite_group_member_path' do
+      let(:group_member) { create(:group_member) }
+
+      it { expect(resend_invite_group_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
+    end
+  end
+end

spec/helpers/members_helper_spec.rb

 require 'spec_helper'
 
 describe MembersHelper do
-  describe '#member_class' do
-    let(:project_member) { build(:project_member) }
-    let(:group_member) { build(:group_member) }
-
-    it { expect(member_class(project_member)).to eq ProjectMember }
-    it { expect(member_class(group_member)).to eq GroupMember }
-  end
-
-  describe '#members_association' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(members_association(project)).to eq :project_members }
-    it { expect(members_association(group)).to eq :group_members }
-  end
-
   describe '#action_member_permission' do
     let(:project_member) { build(:project_member) }
     let(:group_member) { build(:group_member) }
@@ -25,73 +9,20 @@ describe MembersHelper do
     it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member }
   end
 
-  describe '#can_see_entity_roles?' do
-    let(:project) { create(:project) }
+  describe '#can_see_member_roles?' do
+    let(:project) { create(:empty_project) }
     let(:group) { create(:group) }
     let(:user) { build(:user) }
     let(:admin) { build(:user, :admin) }
     let(:project_member) { create(:project_member, project: project) }
     let(:group_member) { create(:group_member, group: group) }
 
-    it { expect(can_see_entity_roles?(nil, project)).to be_falsy }
-    it { expect(can_see_entity_roles?(nil, group)).to be_falsy }
-    it { expect(can_see_entity_roles?(admin, project)).to be_truthy }
-    it { expect(can_see_entity_roles?(admin, group)).to be_truthy }
-    it { expect(can_see_entity_roles?(project_member.user, project)).to be_truthy }
-    it { expect(can_see_entity_roles?(group_member.user, group)).to be_truthy }
-  end
-
-  describe '#member_path' do
-    let(:project_member) { create(:project_member) }
-    let(:group_member) { create(:group_member) }
-
-    it { expect(member_path(project_member)).to eq namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
-    it { expect(member_path(group_member)).to eq group_group_member_path(group_member.source, group_member) }
-    it { expect { member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#resend_invite_member_path' do
-    let(:project_member) { create(:project_member) }
-    let(:group_member) { create(:group_member) }
-
-    it { expect(resend_invite_member_path(project_member)).to eq resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
-    it { expect(resend_invite_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
-    it { expect { resend_invite_member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#request_access_path' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(request_access_path(project)).to eq request_access_namespace_project_project_members_path(project.namespace, project) }
-    it { expect(request_access_path(group)).to eq request_access_group_group_members_path(group) }
-    it { expect { request_access_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#approve_request_member_path' do
-    let(:project_member) { create(:project_member) }
-    let(:group_member) { create(:group_member) }
-
-    it { expect(approve_request_member_path(project_member)).to eq approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
-    it { expect(approve_request_member_path(group_member)).to eq approve_access_request_group_group_member_path(group_member.source, group_member) }
-    it { expect { approve_request_member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#leave_path' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(leave_path(project)).to eq leave_namespace_project_project_members_path(project.namespace, project) }
-    it { expect(leave_path(group)).to eq leave_group_group_members_path(group) }
-    it { expect { leave_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#withdraw_request_message' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(withdraw_request_message(project)).to eq "Are you sure you want to withdraw your access request for the \"#{project.name_with_namespace}\" project?" }
-    it { expect(withdraw_request_message(group)).to eq "Are you sure you want to withdraw your access request for the \"#{group.name}\" group?" }
+    it { expect(can_see_member_roles?(source: project, user: nil)).to be_falsy }
+    it { expect(can_see_member_roles?(source: group, user: nil)).to be_falsy }
+    it { expect(can_see_member_roles?(source: project, user: admin)).to be_truthy }
+    it { expect(can_see_member_roles?(source: group, user: admin)).to be_truthy }
+    it { expect(can_see_member_roles?(source: project, user: project_member.user)).to be_truthy }
+    it { expect(can_see_member_roles?(source: group, user: group_member.user)).to be_truthy }
   end
 
   describe '#remove_member_message' do
@@ -105,12 +36,14 @@ describe MembersHelper do
     let(:group_member_invite) { build(:group_member, group: group).tap { |m| m.generate_invite_token! } }
     let(:group_member_request) { group.request_access(requester) }
 
-    it { expect(remove_member_message(project_member)).to eq "You are going to remove #{project_member.user.name} from the #{project.name_with_namespace} project. Are you sure?" }
-    it { expect(remove_member_message(project_member_invite)).to eq "You are going to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.name_with_namespace} project. Are you sure?" }
-    it { expect(remove_member_message(project_member_request)).to eq "You are going to deny #{requester.name}'s request to join the #{project.name_with_namespace} project. Are you sure?" }
-    it { expect(remove_member_message(group_member)).to eq "You are going to remove #{group_member.user.name} from the #{group.name} group. Are you sure?" }
-    it { expect(remove_member_message(group_member_invite)).to eq "You are going to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group. Are you sure?" }
-    it { expect(remove_member_message(group_member_request)).to eq "You are going to deny #{requester.name}'s request to join the #{group.name} group. Are you sure?" }
+    it { expect(remove_member_message(project_member)).to eq "Are you sure you want to remove #{project_member.user.name} from the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(project_member_invite)).to eq "Are you sure you want to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(project_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(project_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(group_member)).to eq "Are you sure you want to remove #{group_member.user.name} from the #{group.name} group?" }
+    it { expect(remove_member_message(group_member_invite)).to eq "Are you sure you want to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group?" }
+    it { expect(remove_member_message(group_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{group.name} group?" }
+    it { expect(remove_member_message(group_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{group.name} group?" }
   end
 
   describe '#remove_member_title' do
@@ -122,10 +55,10 @@ describe MembersHelper do
     let(:group_member) { build(:group_member, group: group) }
     let(:group_member_request) { group.request_access(requester) }
 
-    it { expect(remove_member_title(project_member)).to eq 'Remove user' }
-    it { expect(remove_member_title(project_member_request)).to eq 'Deny access request' }
-    it { expect(remove_member_title(group_member)).to eq 'Remove user' }
-    it { expect(remove_member_title(group_member_request)).to eq 'Deny access request' }
+    it { expect(remove_member_title(project_member)).to eq 'Remove user from project' }
+    it { expect(remove_member_title(project_member_request)).to eq 'Deny access request from project' }
+    it { expect(remove_member_title(group_member)).to eq 'Remove user from group' }
+    it { expect(remove_member_title(group_member_request)).to eq 'Deny access request from group' }
   end
 
   describe '#leave_confirmation_message' do
@@ -133,7 +66,7 @@ describe MembersHelper do
     let(:group) { build_stubbed(:group) }
     let(:user) { build_stubbed(:user) }
 
-    it { expect(leave_confirmation_message(project)).to eq "Are you sure you want to leave \"#{project.name_with_namespace}\" project?" }
-    it { expect(leave_confirmation_message(group)).to eq "Are you sure you want to leave \"#{group.name}\" group?" }
+    it { expect(leave_confirmation_message(project)).to eq "Are you sure you want to leave the \"#{project.name_with_namespace}\" project?" }
+    it { expect(leave_confirmation_message(group)).to eq "Are you sure you want to leave the \"#{group.name}\" group?" }
   end
 end

spec/helpers/projects_helper_spec.rb

 require 'spec_helper'
 
 describe ProjectsHelper do
-  describe '#max_access_level' do
-    let(:master) { create(:user) }
-    let(:owner) { create(:user) }
-    let(:reporter) { create(:user) }
-    let(:group) { create(:group) }
-    let(:project) { build_stubbed(:empty_project, namespace: group) }
-
-    before do
-      group.add_master(master)
-      group.add_owner(owner)
-      group.add_reporter(reporter)
-    end
-
-    it { expect(max_access_level(project, master)).to eq 'Master' }
-    it { expect(max_access_level(project, owner)).to eq 'Owner' }
-    it { expect(max_access_level(project, reporter)).to eq 'Reporter' }
-    it { expect(max_access_level(project, build_stubbed(:user))).to be_nil }
-  end
-
   describe "#project_status_css_class" do
     it "returns appropriate class" do
       expect(project_status_css_class("started")).to eq("active")

spec/models/concerns/access_requestable_spec.rb

@@ -7,8 +7,7 @@ describe AccessRequestable do
       let(:user) { create(:user) }
 
       it { expect(group.request_access(user)).to be_a(GroupMember) }
-      it { expect(group.request_access(user).user).to be_nil }
-      it { expect(group.request_access(user).created_by).to eq(user) }
+      it { expect(group.request_access(user).user).to eq(user) }
     end
 
     describe '#access_requested?' do
@@ -17,7 +16,7 @@ describe AccessRequestable do
 
       before { group.request_access(user) }
 
-      it { expect(group.access_requested?(user)).to be_truthy }
+      it { expect(group.members.request.exists?(user_id: user)).to be_truthy }
     end
   end
 
@@ -35,7 +34,7 @@ describe AccessRequestable do
 
       before { project.request_access(user) }
 
-      it { expect(project.access_requested?(user)).to be_truthy }
+      it { expect(project.members.request.exists?(user_id: user)).to be_truthy }
     end
   end
 end

spec/models/group_spec.rb

@@ -10,17 +10,6 @@ describe Group, models: true do
     it { is_expected.to have_many(:project_group_links).dependent(:destroy) }
     it { is_expected.to have_many(:shared_projects).through(:project_group_links) }
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
-
-    describe '#group_members' do
-      let(:user) { create(:user) }
-      let(:group) { create(:group) }
-
-      before { group.request_access(user) }
-
-      it 'does not includes membership requests' do
-        expect(user.group_members).to be_empty
-      end
-    end
   end
 
   describe 'modules' do

spec/models/member_spec.rb

@@ -55,45 +55,78 @@ describe Member, models: true do
     end
   end
 
-  describe 'Scopes' do
+  describe 'Scopes & finders' do
     before do
       project = create(:project)
-      @invited_member = build(:project_member, user: nil).tap { |m| m.generate_invite_token! }
-      @accepted_invite_member = build(:project_member, user: nil).tap { |m| m.generate_invite_token! && m.accept_invite!(build(:user)) }
+      group = create(:group)
+      @owner_user = create(:user).tap { |u| group.add_owner(u) }
+      @owner = group.members.find_by(user_id: @owner_user.id)
+
+      @master_user = create(:user).tap { |u| project.team << [u, :master] }
+      @master = project.members.find_by(user_id: @master_user.id)
+
+      ProjectMember.add_user(project.members, 'toto1@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      @invited_member = project.members.invite.find_by_invite_email('toto1@example.com')
+
+      accepted_invite_user = build(:user)
+      ProjectMember.add_user(project.members, 'toto2@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
 
       requested_user = create(:user).tap { |u| project.request_access(u) }
-      @requested_member = project.project_members.find_by(created_by_id: requested_user.id)
+      @requested_member = project.members.request.find_by(user_id: requested_user.id)
+
       accepted_request_user = create(:user).tap { |u| project.request_access(u) }
-      @accepted_request_member = project.project_members.find_by(created_by_id: accepted_request_user.id).tap { |m| m.accept_request }
+      @accepted_request_member = project.members.request.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
     end
 
-    describe '#invite' do
+    describe '.invite' do
+      it { expect(described_class.invite).not_to include @master }
       it { expect(described_class.invite).to include @invited_member }
       it { expect(described_class.invite).not_to include @accepted_invite_member }
       it { expect(described_class.invite).not_to include @requested_member }
       it { expect(described_class.invite).not_to include @accepted_request_member }
     end
 
-    describe '#request' do
+    describe '.non_invite' do
+      it { expect(described_class.non_invite).to include @master }
+      it { expect(described_class.non_invite).not_to include @invited_member }
+      it { expect(described_class.non_invite).to include @accepted_invite_member }
+      it { expect(described_class.non_invite).to include @requested_member }
+      it { expect(described_class.non_invite).to include @accepted_request_member }
+    end
+
+    describe '.request' do
+      it { expect(described_class.request).not_to include @master }
       it { expect(described_class.request).not_to include @invited_member }
       it { expect(described_class.request).not_to include @accepted_invite_member }
       it { expect(described_class.request).to include @requested_member }
       it { expect(described_class.request).not_to include @accepted_request_member }
     end
 
-    describe '#non_request' do
+    describe '.non_request' do
+      it { expect(described_class.non_request).to include @master }
       it { expect(described_class.non_request).to include @invited_member }
       it { expect(described_class.non_request).to include @accepted_invite_member }
       it { expect(described_class.non_request).not_to include @requested_member }
       it { expect(described_class.non_request).to include @accepted_request_member }
     end
 
-    describe '#non_pending' do
+    describe '.non_pending' do
+      it { expect(described_class.non_pending).to include @master }
       it { expect(described_class.non_pending).not_to include @invited_member }
       it { expect(described_class.non_pending).to include @accepted_invite_member }
       it { expect(described_class.non_pending).not_to include @requested_member }
       it { expect(described_class.non_pending).to include @accepted_request_member }
     end
+
+    describe '.owners_and_masters' do
+      it { expect(described_class.owners_and_masters).to include @owner }
+      it { expect(described_class.owners_and_masters).to include @master }
+      it { expect(described_class.owners_and_masters).not_to include @invited_member }
+      it { expect(described_class.owners_and_masters).not_to include @accepted_invite_member }
+      it { expect(described_class.owners_and_masters).not_to include @requested_member }
+      it { expect(described_class.owners_and_masters).not_to include @accepted_request_member }
+    end
   end
 
   describe "Delegate methods" do
@@ -101,6 +134,18 @@ describe Member, models: true do
     it { is_expected.to respond_to(:user_email) }
   end
 
+  describe 'Callbacks' do
+    describe 'after_destroy :post_decline_request, if: :request?' do
+      let(:member) { create(:project_member, requested_at: Time.now.utc) }
+
+      it 'calls #post_decline_request' do
+        expect(member).to receive(:post_decline_request)
+
+        member.destroy
+      end
+    end
+  end
+
   describe ".add_user" do
     let!(:user)    { create(:user) }
     let(:project) { create(:project) }
@@ -139,18 +184,9 @@ describe Member, models: true do
   end
 
   describe '#accept_request' do
-    let(:user) { create(:user) }
-    let(:member) { create(:project_member, requested_at: Time.now.utc, user: nil, created_by: user) }
-
-    it 'returns true' do
-      expect(member.accept_request).to be_truthy
-    end
+    let(:member) { create(:project_member, requested_at: Time.now.utc) }
 
-    it 'sets the user' do
-      member.accept_request
-
-      expect(member.user).to eq(user)
-    end
+    it { expect(member.accept_request).to be_truthy }
 
     it 'clears requested_at' do
       member.accept_request
@@ -165,25 +201,24 @@ describe Member, models: true do
     end
   end
 
-  describe '#decline_request' do
-    let(:user) { create(:user) }
-    let(:member) { create(:project_member, requested_at: Time.now.utc, user: nil, created_by: user) }
+  describe '#invite?' do
+    subject { create(:project_member, invite_email: "user@example.com", user: nil) }
 
-    it 'returns true' do
-      expect(member.decline_request).to be_truthy
-    end
+    it { is_expected.to be_invite }
+  end
 
-    it 'destroys the member' do
-      member.decline_request
+  describe '#request?' do
+    subject { create(:project_member, requested_at: Time.now.utc) }
 
-      expect(member).to be_destroyed
-    end
+    it { is_expected.to be_request }
+  end
 
-    it 'calls #after_decline_request' do
-      expect(member).to receive(:after_decline_request)
+  describe '#pending?' do
+    let(:invited_member) { create(:project_member, invite_email: "user@example.com", user: nil) }
+    let(:requester) { create(:project_member, requested_at: Time.now.utc) }
 
-      member.decline_request
-    end
+    it { expect(invited_member).to be_invite }
+    it { expect(requester).to be_pending }
   end
 
   describe "#accept_invite!" do

spec/models/members/group_member_spec.rb

@@ -51,24 +51,30 @@ describe GroupMember, models: true do
       end
     end
 
-    describe 'after accept_request' do
-      let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#after_accept_request' do
+      it 'calls NotificationService.accept_group_access_request' do
+        member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it "calls #accept_group_access_request" do
         expect_any_instance_of(NotificationService).to receive(:new_group_member)
 
-        member.accept_request
+        member.__send__(:after_accept_request)
       end
     end
 
-    describe 'after decline_request' do
-      let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#post_decline_request' do
+      it 'calls NotificationService.decline_group_access_request' do
+        member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it "calls #decline_group_access_request" do
         expect_any_instance_of(NotificationService).to receive(:decline_group_access_request)
 
-        member.decline_request
+        member.__send__(:post_decline_request)
       end
     end
+
+    describe '#real_source_type' do
+      subject { create(:group_member).real_source_type }
+
+      it { is_expected.to eq 'Group' }
+    end
   end
 end

spec/models/members/project_member_spec.rb

@@ -33,6 +33,12 @@ describe ProjectMember, models: true do
     it { is_expected.to include_module(Gitlab::ShellAdapter) }
   end
 
+  describe '#real_source_type' do
+    subject { create(:project_member).real_source_type }
+
+    it { is_expected.to eq 'Project' }
+  end
+
   describe "#destroy" do
     let(:owner)   { create(:project_member, access_level: ProjectMember::OWNER) }
     let(:project) { owner.project }
@@ -137,23 +143,23 @@ describe ProjectMember, models: true do
   end
 
   describe 'notifications' do
-    describe 'after accept_request' do
-      let(:member) { create(:project_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#after_accept_request' do
+      it 'calls NotificationService.new_project_member' do
+        member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it 'calls #accept_project_access_request' do
         expect_any_instance_of(NotificationService).to receive(:new_project_member)
 
-        member.accept_request
+        member.__send__(:after_accept_request)
       end
     end
 
-    describe 'after decline_request' do
-      let(:member) { create(:project_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#post_decline_request' do
+      it 'calls NotificationService.decline_project_access_request' do
+        member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it 'calls #decline_project_access_request' do
         expect_any_instance_of(NotificationService).to receive(:decline_project_access_request)
 
-        member.decline_request
+        member.__send__(:post_decline_request)
       end
     end
   end

spec/models/project_spec.rb

@@ -29,17 +29,6 @@ describe Project, models: true do
     it { is_expected.to have_many(:variables) }
     it { is_expected.to have_many(:triggers) }
     it { is_expected.to have_many(:todos).dependent(:destroy) }
-
-    describe '#project_members' do
-      let(:user) { create(:user) }
-      let(:project) { create(:project) }
-
-      before { project.request_access(user) }
-
-      it 'does not includes membership requests' do
-        expect(user.project_members).to be_empty
-      end
-    end
   end
 
   describe 'modules' do
@@ -100,11 +89,17 @@ describe Project, models: true do
     it { is_expected.to respond_to(:repo_exists?) }
     it { is_expected.to respond_to(:update_merge_requests) }
     it { is_expected.to respond_to(:execute_hooks) }
-    it { is_expected.to respond_to(:name_with_namespace) }
     it { is_expected.to respond_to(:owner) }
     it { is_expected.to respond_to(:path_with_namespace) }
   end
 
+  describe '#name_with_namespace' do
+    let(:project) { build_stubbed(:empty_project) }
+
+    it { expect(project.name_with_namespace).to eq "#{project.namespace.human_name} / #{project.name}" }
+    it { expect(project.human_name).to eq project.name_with_namespace }
+  end
+
   describe '#to_reference' do
     let(:project) { create(:empty_project) }
 

spec/models/project_team_spec.rb

@@ -112,6 +112,28 @@ describe ProjectTeam, models: true do
     end
   end
 
+  describe "#human_max_access" do
+    it 'returns Master role' do
+      user = create(:user)
+      group = create(:group)
+      group.add_master(user)
+
+      project = build_stubbed(:empty_project, namespace: group)
+
+      expect(project.team.human_max_access(user.id)).to eq 'Master'
+    end
+
+    it 'returns Owner role' do
+      user = create(:user)
+      group = create(:group)
+      group.add_owner(user)
+
+      project = build_stubbed(:empty_project, namespace: group)
+
+      expect(project.team.human_max_access(user.id)).to eq 'Owner'
+    end
+  end
+
   describe '#max_member_access' do
     let(:requester) { create(:user) }