Add request access for groups

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

9 files changed, 86 insertions, 91 deletions

diff --git a/app/models/ability.rb b/app/models/ability.rb
index b3db26f989e..90156bf130c 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -153,7 +153,7 @@ class Ability
 
       RequestStore.store[key] ||= begin
         # Push abilities on the users team role
-        rules.push(*project_team_rules(project.team, user)) unless project.team.pending?(user)
+        rules.push(*project_team_rules(project.team, user))
 
         if project.owner == user ||
           (project.group && project.group.has_owner?(user)) ||
@@ -187,6 +187,8 @@ class Ability
         project_report_rules
       elsif team.guest?(user)
         project_guest_rules
+      else
+        []
       end
     end
 
@@ -458,6 +460,8 @@ 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
 
@@ -477,6 +481,8 @@ 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
 
diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb
new file mode 100644
index 00000000000..cf37284e31a
--- /dev/null
+++ b/app/models/concerns/access_requestable.rb
@@ -0,0 +1,27 @@
+# == AccessRequestable concern
+#
+# Contains functionality related to objects that can receive request for access.
+#
+# Used by Project, and Group.
+#
+module AccessRequestable
+  extend ActiveSupport::Concern
+
+  def request_access(user)
+    members.create(
+      access_level: Gitlab::Access::DEVELOPER,
+      created_by: 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
diff --git a/app/models/group.rb b/app/models/group.rb
index aec92e335e6..b6929112cba 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -3,6 +3,7 @@ require 'carrierwave/orm/activerecord'
 class Group < Namespace
   include Gitlab::ConfigHelper
   include Gitlab::VisibilityLevel
+  include AccessRequestable
   include Referable
 
   has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
diff --git a/app/models/member.rb b/app/models/member.rb
index 2210e7dd66a..5c3a5eab406 100644
--- a/app/models/member.rb
+++ b/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: :invite?
+  validates :user, presence: true, unless: :pending?
   validates :source, presence: true
   validates :user_id, uniqueness: { scope: [:source_type, :source_id],
                                     message: "already exists in source",
@@ -26,29 +26,25 @@ class Member < ActiveRecord::Base
       allow_nil: true
     }
 
-  scope :invite, -> { where(user_id: nil) }
-  scope :non_invite, -> { where('user_id IS NOT NULL') }
-  scope :request, -> { where(requested: true) }
-  scope :non_request, -> { where(requested: nil) }
-  scope :pending, -> { where("user_id IS NULL OR requested") }
-  scope :non_pending, -> { self.non_invite.non_request }
+  scope :invite, -> { where.not(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 :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]) }
 
   before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? }
 
   after_create :send_invite, if: :invite?
-  after_create :send_request_access, if: :request?
-
+  after_create :send_request, if: :request?
   after_create :create_notification_setting, unless: :pending?
   after_create :post_create_hook, unless: :pending?
-
   after_update :post_update_hook, unless: :pending?
-
   after_destroy :post_destroy_hook, unless: :pending?
 
   delegate :name, :username, :email, to: :user, prefix: true
@@ -111,31 +107,29 @@ class Member < ActiveRecord::Base
   end
 
   def request?
-    self.requested
+    user.nil? && created_by.present? && requested_at.present?
   end
 
   def invite?
     self.invite_token.present?
   end
 
-  def accept_request_access!
+  def accept_request
     return false unless request?
 
-    self.request = false
-    saved = self.save
+    updated = self.update(user: created_by, requested_at: nil)
+    after_accept_request if updated
 
-    after_accept_request_access if saved
-
-    saved
+    updated
   end
 
-  def decline_request_access!
+  def decline_request
     return false unless request?
 
-    destroyed = self.destroy
-    after_decline_request_access if destroyed
+    self.destroy
+    after_decline_request if destroyed?
 
-    destroyed
+    destroyed?
   end
 
   def accept_invite!(new_user)
@@ -191,11 +185,11 @@ class Member < ActiveRecord::Base
 
   private
 
-  def send_request_access
+  def send_invite
     # override in subclass
   end
 
-  def send_invite
+  def send_request
     # override in subclass
   end
 
@@ -211,19 +205,19 @@ class Member < ActiveRecord::Base
     system_hook_service.execute_hooks_for(self, :destroy)
   end
 
-  def after_accept_request_access
+  def after_accept_invite
     post_create_hook
   end
 
-  def after_decline_request_access
+  def after_decline_invite
     # override in subclass
   end
 
-  def after_accept_invite
+  def after_accept_request
     post_create_hook
   end
 
-  def after_decline_invite
+  def after_decline_request
     # override in subclass
   end
 
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index f63a0debf1a..476b4816b90 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -8,9 +8,6 @@ class GroupMember < Member
   validates_format_of :source_type, with: /\ANamespace\z/
   default_scope { where(source_type: SOURCE_TYPE) }
 
-  scope :with_group, ->(group) { where(source_id: group.id) }
-  scope :with_user, ->(user) { where(user_id: user.id) }
-
   def self.access_level_roles
     Gitlab::Access.options_with_owner
   end
@@ -31,6 +28,12 @@ class GroupMember < Member
     super
   end
 
+  def send_request
+    notification_service.new_group_access_request(self)
+
+    super
+  end
+
   def post_create_hook
     notification_service.new_group_member(self)
 
@@ -56,4 +59,10 @@ class GroupMember < Member
 
     super
   end
+
+  def after_decline_request
+    notification_service.decline_group_access_request(group, created_by)
+
+    super
+  end
 end
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index 9db8db8450d..c6fd1a5c3d1 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -11,8 +11,6 @@ class ProjectMember < Member
   default_scope { where(source_type: SOURCE_TYPE) }
 
   scope :in_project, ->(project) { where(source_id: project.id) }
-  scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) }
-  scope :with_user, ->(user) { where(user_id: user.id) }
 
   before_destroy :delete_member_todos
 
@@ -84,7 +82,7 @@ class ProjectMember < Member
       Gitlab::Access.sym_options
     end
 
-    def access_roles
+    def access_level_roles
       Gitlab::Access.options
     end
   end
@@ -107,14 +105,14 @@ class ProjectMember < Member
     user.todos.where(project_id: source_id).destroy_all if user
   end
 
-  def send_request_access
-    notification_service.request_access_project_member(self)
+  def send_invite
+    notification_service.invite_project_member(self, @raw_invite_token)
 
     super
   end
 
-  def send_invite
-    notification_service.invite_project_member(self, @raw_invite_token)
+  def send_request
+    notification_service.new_project_access_request(self)
 
     super
   end
@@ -142,18 +140,6 @@ class ProjectMember < Member
     super
   end
 
-  def after_accept_request_access
-    notification_service.accept_project_request_access(self)
-
-    super
-  end
-
-  def after_decline_request_access
-    notification_service.decline_project_request_access(self)
-
-    super
-  end
-
   def after_accept_invite
     notification_service.accept_project_invite(self)
 
@@ -166,6 +152,12 @@ class ProjectMember < Member
     super
   end
 
+  def after_decline_request
+    notification_service.decline_project_access_request(project, created_by)
+
+    super
+  end
+
   def event_service
     EventCreateService.new
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index dfa99fe0df2..ef665373495 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -5,6 +5,7 @@ class Project < ActiveRecord::Base
   include Gitlab::ShellAdapter
   include Gitlab::VisibilityLevel
   include Gitlab::CurrentSettings
+  include AccessRequestable
   include Referable
   include Sortable
   include AfterCommitQueue
@@ -102,7 +103,7 @@ class Project < ActiveRecord::Base
   has_many :snippets,           dependent: :destroy, class_name: 'ProjectSnippet'
   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 :project_members,    dependent: :destroy, as: :source, class_name: 'ProjectMember'
   has_many :users, through: :project_members
   has_many :deploy_keys_projects, dependent: :destroy
   has_many :deploy_keys, through: :deploy_keys_projects
@@ -680,16 +681,6 @@ class Project < ActiveRecord::Base
     end
   end
 
-  def project_member_by_name_or_email(name = nil, email = nil)
-    user = users.find_by('name like ? or email like ?', name, email)
-    project_members.where(user: user) if user
-  end
-
-  # Get Team Member record by user id
-  def project_member_by_id(user_id)
-    project_members.find_by(user_id: user_id)
-  end
-
   def name_with_namespace
     @name_with_namespace ||= begin
                                if namespace
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 769b73666ce..7fb17df0e96 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -21,16 +21,6 @@ class ProjectTeam
     end
   end
 
-  def find(user_id)
-    user = project.users.find_by(id: user_id)
-
-    if group
-      user ||= group.users.find_by(id: user_id)
-    end
-
-    user
-  end
-
   def find_member(user_id)
     member = project.project_members.find_by(user_id: user_id)
 
@@ -61,13 +51,10 @@ class ProjectTeam
     ProjectMember.truncate_team(project)
   end
 
-  def users
-    members
-  end
-
   def members
     @members ||= fetch_members
   end
+  alias_method :users, :members
 
   def guests
     @guests ||= fetch_members(:guests)
@@ -115,12 +102,6 @@ class ProjectTeam
     false
   end
 
-  def pending?(user)
-    project.project_members.each do |member|
-      return member.pending? if member.user_id == user.id
-    end
-  end
-
   def guest?(user)
     max_member_access(user.id) == Gitlab::Access::GUEST
   end
@@ -147,10 +128,6 @@ 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)
@@ -179,6 +156,7 @@ class ProjectTeam
     access.compact.max
   end
 
+  private
 
   def max_invited_level(user_id)
     project.project_group_links.map do |group_link|
@@ -195,8 +173,6 @@ class ProjectTeam
     end.compact.max
   end
 
-  private
-
   def fetch_members(level = nil)
     project_members = project.project_members
     group_members = group ? group.group_members : []
diff --git a/app/models/user.rb b/app/models/user.rb
index a5b3c8afe51..8d0427da5ab 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -56,8 +56,7 @@ class User < ActiveRecord::Base
 
   # Groups
   has_many :members, dependent: :destroy
-  has_many :project_members, source: 'ProjectMember'
-  has_many :group_members, source: 'GroupMember'
+  has_many :group_members, dependent: :destroy, source: 'GroupMember'
   has_many :groups, through: :group_members
   has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group
   has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group
@@ -65,13 +64,13 @@ class User < ActiveRecord::Base
   # Projects
   has_many :groups_projects,          through: :groups, source: :projects
   has_many :personal_projects,        through: :namespace, source: :projects
+  has_many :project_members,          dependent: :destroy, class_name: 'ProjectMember'
   has_many :projects,                 through: :project_members
   has_many :created_projects,         foreign_key: :creator_id, class_name: 'Project'
   has_many :users_star_projects, dependent: :destroy
   has_many :starred_projects, through: :users_star_projects, source: :project
 
   has_many :snippets,                 dependent: :destroy, foreign_key: :author_id, class_name: "Snippet"
-  has_many :project_members,          dependent: :destroy, class_name: 'ProjectMember'
   has_many :issues,                   dependent: :destroy, foreign_key: :author_id
   has_many :notes,                    dependent: :destroy, foreign_key: :author_id
   has_many :merge_requests,           dependent: :destroy, foreign_key: :author_id