summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/group.rb36
-rw-r--r--app/models/member.rb79
-rw-r--r--app/models/members/group_member.rb16
-rw-r--r--app/models/members/project_member.rb44
-rw-r--r--app/models/project.rb5
-rw-r--r--app/models/project_team.rb14
-rw-r--r--db/fixtures/development/06_teams.rb2
-rw-r--r--lib/api/members.rb17
-rw-r--r--lib/gitlab/access.rb4
-rw-r--r--spec/controllers/projects/templates_controller_spec.rb2
-rw-r--r--spec/factories/project_members.rb23
-rw-r--r--spec/features/projects/members/owner_cannot_leave_project_spec.rb4
-rw-r--r--spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb4
-rw-r--r--spec/features/projects_spec.rb6
-rw-r--r--spec/finders/joined_groups_finder_spec.rb2
-rw-r--r--spec/finders/projects_finder_spec.rb2
-rw-r--r--spec/lib/gitlab/template/issue_template_spec.rb6
-rw-r--r--spec/lib/gitlab/template/merge_request_template_spec.rb6
-rw-r--r--spec/mailers/notify_spec.rb50
-rw-r--r--spec/models/issue_spec.rb2
-rw-r--r--spec/models/member_spec.rb243
-rw-r--r--spec/models/members/group_member_spec.rb27
-rw-r--r--spec/models/members/project_member_spec.rb50
-rw-r--r--spec/models/project_spec.rb2
-rw-r--r--spec/requests/api/merge_requests_spec.rb4
25 files changed, 431 insertions, 219 deletions
diff --git a/app/models/group.rb b/app/models/group.rb
index aefb94b2ada..a2f88cca828 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -102,40 +102,44 @@ class Group < Namespace
self[:lfs_enabled]
end
- def add_users(user_ids, access_level, current_user: nil, expires_at: nil)
- user_ids.each do |user_id|
- Member.add_user(
- self.group_members,
- user_id,
- access_level,
- current_user: current_user,
- expires_at: expires_at
- )
- end
+ def add_users(users, access_level, current_user: nil, expires_at: nil)
+ GroupMember.add_users_to_group(
+ self,
+ users,
+ access_level,
+ current_user: current_user,
+ expires_at: expires_at
+ )
end
def add_user(user, access_level, current_user: nil, expires_at: nil)
- add_users([user], access_level, current_user: current_user, expires_at: expires_at)
+ GroupMember.add_user(
+ self,
+ user,
+ access_level,
+ current_user: current_user,
+ expires_at: expires_at
+ )
end
def add_guest(user, current_user = nil)
- add_user(user, Gitlab::Access::GUEST, current_user: current_user)
+ add_user(user, :guest, current_user: current_user)
end
def add_reporter(user, current_user = nil)
- add_user(user, Gitlab::Access::REPORTER, current_user: current_user)
+ add_user(user, :reporter, current_user: current_user)
end
def add_developer(user, current_user = nil)
- add_user(user, Gitlab::Access::DEVELOPER, current_user: current_user)
+ add_user(user, :developer, current_user: current_user)
end
def add_master(user, current_user = nil)
- add_user(user, Gitlab::Access::MASTER, current_user: current_user)
+ add_user(user, :master, current_user: current_user)
end
def add_owner(user, current_user = nil)
- add_user(user, Gitlab::Access::OWNER, current_user: current_user)
+ add_user(user, :owner, current_user: current_user)
end
def has_owner?(user)
diff --git a/app/models/member.rb b/app/models/member.rb
index 69406379948..265c11ca113 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -80,49 +80,70 @@ class Member < ActiveRecord::Base
find_by(invite_token: invite_token)
end
- # This method is used to find users that have been entered into the "Add members" field.
- # These can be the User objects directly, their IDs, their emails, or new emails to be invited.
- def user_for_id(user_id)
- return user_id if user_id.is_a?(User)
-
- user = User.find_by(id: user_id)
- user ||= User.find_by(email: user_id)
- user ||= user_id
- user
- end
-
- def add_user(members, user_id, access_level, current_user: nil, expires_at: nil)
- user = user_for_id(user_id)
+ def add_user(source, user, access_level, current_user: nil, expires_at: nil)
+ user = retrieve_user(user)
+ access_level = retrieve_access_level(access_level)
# `user` can be either a User object or an email to be invited
- if user.is_a?(User)
- member = members.find_or_initialize_by(user_id: user.id)
+ member =
+ if user.is_a?(User)
+ source.members.find_by(user_id: user.id) ||
+ source.requesters.find_by(user_id: user.id) ||
+ source.members.build(user_id: user.id)
+ else
+ source.members.build(invite_email: user)
+ end
+
+ return member unless can_update_member?(current_user, member)
+
+ member.attributes = {
+ created_by: member.created_by || current_user,
+ access_level: access_level,
+ expires_at: expires_at
+ }
+
+ if member.request?
+ member.accept_request
else
- member = members.build
- member.invite_email = user
+ member.save
end
- if can_update_member?(current_user, member) || project_creator?(member, access_level)
- member.created_by ||= current_user
- member.access_level = access_level
- member.expires_at = expires_at
+ member
+ end
- member.save
- end
+ def access_levels
+ Gitlab::Access.sym_options
end
private
+ # This method is used to find users that have been entered into the "Add members" field.
+ # These can be the User objects directly, their IDs, their emails, or new emails to be invited.
+ def retrieve_user(user)
+ return user if user.is_a?(User)
+
+ User.find_by(id: user) || User.find_by(email: user) || user
+ end
+
+ def retrieve_access_level(access_level)
+ access_levels.fetch(access_level) { access_level.to_i }
+ end
+
def can_update_member?(current_user, member)
# There is no current user for bulk actions, in which case anything is allowed
- !current_user ||
- current_user.can?(:update_group_member, member) ||
- current_user.can?(:update_project_member, member)
+ !current_user || current_user.can?(:"update_#{member.type.underscore}", member)
end
- def project_creator?(member, access_level)
- member.new_record? && member.owner? &&
- access_level.to_i == ProjectMember::MASTER
+ def add_users_to_source(source, users, access_level, current_user: nil, expires_at: nil)
+ users.each do |user|
+ add_user(
+ source,
+ user,
+ access_level,
+ current_user: current_user,
+ expires_at: expires_at
+ )
+ end
end
end
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 2f13d339c89..1b54a85d064 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -12,6 +12,22 @@ class GroupMember < Member
Gitlab::Access.options_with_owner
end
+ def self.access_levels
+ Gitlab::Access.sym_options_with_owner
+ end
+
+ def self.add_users_to_group(group, users, access_level, current_user: nil, expires_at: nil)
+ self.transaction do
+ add_users_to_source(
+ group,
+ users,
+ access_level,
+ current_user: current_user,
+ expires_at: expires_at
+ )
+ end
+ end
+
def group
source
end
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index ec2d40eb11c..125f26369d7 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -34,36 +34,20 @@ class ProjectMember < Member
# :master
# )
#
- def add_users_to_projects(project_ids, user_ids, access, current_user: nil, expires_at: nil)
- access_level = if roles_hash.has_key?(access)
- roles_hash[access]
- elsif roles_hash.values.include?(access.to_i)
- access
- else
- raise "Non valid access"
- end
-
- users = user_ids.map { |user_id| Member.user_for_id(user_id) }
-
- ProjectMember.transaction do
+ def add_users_to_projects(project_ids, users, access_level, current_user: nil, expires_at: nil)
+ self.transaction do
project_ids.each do |project_id|
project = Project.find(project_id)
- users.each do |user|
- Member.add_user(
- project.project_members,
- user,
- access_level,
- current_user: current_user,
- expires_at: expires_at
- )
- end
+ add_users_to_source(
+ project,
+ users,
+ access_level,
+ current_user: current_user,
+ expires_at: expires_at
+ )
end
end
-
- true
- rescue
- false
end
def truncate_teams(project_ids)
@@ -84,13 +68,15 @@ class ProjectMember < Member
truncate_teams [project.id]
end
- def roles_hash
- Gitlab::Access.sym_options
- end
-
def access_level_roles
Gitlab::Access.options
end
+
+ private
+
+ def can_update_member?(current_user, member)
+ super || (member.owner? && member.new_record?)
+ end
end
def access_field
diff --git a/app/models/project.rb b/app/models/project.rb
index 7265cb55594..507228606df 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -146,6 +146,7 @@ class Project < ActiveRecord::Base
delegate :name, to: :owner, allow_nil: true, prefix: true
delegate :members, to: :team, prefix: true
+ delegate :add_user, to: :team
# Validations
validates :creator, presence: true, on: :create
@@ -1016,10 +1017,6 @@ class Project < ActiveRecord::Base
project_members.find_by(user_id: user)
end
- def add_user(user, access_level, current_user: nil, expires_at: nil)
- team.add_user(user, access_level, current_user: current_user, expires_at: expires_at)
- end
-
def default_branch
@default_branch ||= repository.root_ref if repository.exists?
end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index d9ce5088903..79d041d2775 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -33,18 +33,24 @@ class ProjectTeam
member
end
- def add_users(users, access, current_user: nil, expires_at: nil)
+ def add_users(users, access_level, current_user: nil, expires_at: nil)
ProjectMember.add_users_to_projects(
[project.id],
users,
- access,
+ access_level,
current_user: current_user,
expires_at: expires_at
)
end
- def add_user(user, access, current_user: nil, expires_at: nil)
- add_users([user], access, current_user: current_user, expires_at: expires_at)
+ def add_user(user, access_level, current_user: nil, expires_at: nil)
+ ProjectMember.add_user(
+ project,
+ user,
+ access_level,
+ current_user: current_user,
+ expires_at: expires_at
+ )
end
# Remove all users from project team
diff --git a/db/fixtures/development/06_teams.rb b/db/fixtures/development/06_teams.rb
index 3e8cdcd67b4..9739a5ac8d5 100644
--- a/db/fixtures/development/06_teams.rb
+++ b/db/fixtures/development/06_teams.rb
@@ -1,7 +1,7 @@
Gitlab::Seeder.quiet do
Group.all.each do |group|
User.all.sample(4).each do |user|
- if group.add_users([user.id], Gitlab::Access.values.sample)
+ if group.add_user(user, Gitlab::Access.values.sample).persisted?
print '.'
else
print 'F'
diff --git a/lib/api/members.rb b/lib/api/members.rb
index 37f0a6512f4..a18ce769e29 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -59,13 +59,6 @@ module API
authorize_admin_source!(source_type, source)
required_attributes! [:user_id, :access_level]
- access_requester = source.requesters.find_by(user_id: params[:user_id])
- if access_requester
- # We pass current_user = access_requester so that the requester doesn't
- # receive a "access denied" email
- ::Members::DestroyService.new(access_requester, access_requester.user).execute
- end
-
member = source.members.find_by(user_id: params[:user_id])
# This is to ensure back-compatibility but 409 behavior should be used
@@ -73,18 +66,12 @@ module API
conflict!('Member already exists') if source_type == 'group' && member
unless member
- source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at])
- member = source.members.find_by(user_id: params[:user_id])
+ member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at])
end
- if member
+ if member.persisted? && member.valid?
present member.user, with: Entities::Member, member: member
else
- # Since `source.add_user` doesn't return a member object, we have to
- # build a new one and populate its errors in order to render them.
- member = source.members.build(attributes_for_keys([:user_id, :access_level, :expires_at]))
- member.valid? # populate the errors
-
# This is to ensure back-compatibility but 400 behavior should be used
# for all validation errors in 9.0!
render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level)
diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb
index a533bac2692..9b484a2ecfd 100644
--- a/lib/gitlab/access.rb
+++ b/lib/gitlab/access.rb
@@ -53,6 +53,10 @@ module Gitlab
}
end
+ def sym_options_with_owner
+ sym_options.merge(owner: OWNER)
+ end
+
def protection_options
{
"Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE,
diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb
index 7b3a26d7ca7..19a152bcb05 100644
--- a/spec/controllers/projects/templates_controller_spec.rb
+++ b/spec/controllers/projects/templates_controller_spec.rb
@@ -13,7 +13,7 @@ describe Projects::TemplatesController do
end
before do
- project.team.add_user(user, Gitlab::Access::MASTER)
+ project.add_user(user, Gitlab::Access::MASTER)
project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false)
end
diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb
index cf3659ba275..1ddb305a8af 100644
--- a/spec/factories/project_members.rb
+++ b/spec/factories/project_members.rb
@@ -4,24 +4,9 @@ FactoryGirl.define do
project
master
- trait :guest do
- access_level ProjectMember::GUEST
- end
-
- trait :reporter do
- access_level ProjectMember::REPORTER
- end
-
- trait :developer do
- access_level ProjectMember::DEVELOPER
- end
-
- trait :master do
- access_level ProjectMember::MASTER
- end
-
- trait :owner do
- access_level ProjectMember::OWNER
- end
+ trait(:guest) { access_level ProjectMember::GUEST }
+ trait(:reporter) { access_level ProjectMember::REPORTER }
+ trait(:developer) { access_level ProjectMember::DEVELOPER }
+ trait(:master) { access_level ProjectMember::MASTER }
end
end
diff --git a/spec/features/projects/members/owner_cannot_leave_project_spec.rb b/spec/features/projects/members/owner_cannot_leave_project_spec.rb
index 67811b1048e..6e948b7a616 100644
--- a/spec/features/projects/members/owner_cannot_leave_project_spec.rb
+++ b/spec/features/projects/members/owner_cannot_leave_project_spec.rb
@@ -1,12 +1,10 @@
require 'spec_helper'
feature 'Projects > Members > Owner cannot leave project', feature: true do
- let(:owner) { create(:user) }
let(:project) { create(:project) }
background do
- project.team << [owner, :owner]
- login_as(owner)
+ login_as(project.owner)
visit namespace_project_path(project.namespace, project)
end
diff --git a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
index 0e54c4fdf20..4ca9272b9c1 100644
--- a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
+++ b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
@@ -1,12 +1,10 @@
require 'spec_helper'
feature 'Projects > Members > Owner cannot request access to his project', feature: true do
- let(:owner) { create(:user) }
let(:project) { create(:project) }
background do
- project.team << [owner, :owner]
- login_as(owner)
+ login_as(project.owner)
visit namespace_project_path(project.namespace, project)
end
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index 2242cb6236a..c30d38b6508 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -82,7 +82,7 @@ feature 'Project', feature: true do
before do
login_with(user)
- project.team.add_user(user, Gitlab::Access::MASTER)
+ project.add_user(user, Gitlab::Access::MASTER)
visit namespace_project_path(project.namespace, project)
end
@@ -101,8 +101,8 @@ feature 'Project', feature: true do
context 'on issues page', js: true do
before do
login_with(user)
- project.team.add_user(user, Gitlab::Access::MASTER)
- project2.team.add_user(user, Gitlab::Access::MASTER)
+ project.add_user(user, Gitlab::Access::MASTER)
+ project2.add_user(user, Gitlab::Access::MASTER)
visit namespace_project_issue_path(project.namespace, project, issue)
end
diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb
index f90a8e007c8..29a47e005a6 100644
--- a/spec/finders/joined_groups_finder_spec.rb
+++ b/spec/finders/joined_groups_finder_spec.rb
@@ -43,7 +43,7 @@ describe JoinedGroupsFinder do
context 'if profile visitor is in one of the private group projects' do
before do
project = create(:project, :private, group: private_group, name: 'B', path: 'B')
- project.team.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
+ project.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
end
it 'shows group' do
diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb
index 7a3a74335e8..13bda5f7c5a 100644
--- a/spec/finders/projects_finder_spec.rb
+++ b/spec/finders/projects_finder_spec.rb
@@ -38,7 +38,7 @@ describe ProjectsFinder do
describe 'with private projects' do
before do
- private_project.team.add_user(user, Gitlab::Access::MASTER)
+ private_project.add_user(user, Gitlab::Access::MASTER)
end
it do
diff --git a/spec/lib/gitlab/template/issue_template_spec.rb b/spec/lib/gitlab/template/issue_template_spec.rb
index f770857e958..d2d334e6413 100644
--- a/spec/lib/gitlab/template/issue_template_spec.rb
+++ b/spec/lib/gitlab/template/issue_template_spec.rb
@@ -10,7 +10,7 @@ describe Gitlab::Template::IssueTemplate do
let(:file_path_3) { '.gitlab/issue_templates/feature_proposal.md' }
before do
- project.team.add_user(user, Gitlab::Access::MASTER)
+ project.add_user(user, Gitlab::Access::MASTER)
project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false)
project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false)
project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
@@ -53,7 +53,7 @@ describe Gitlab::Template::IssueTemplate do
context 'when repo is bare or empty' do
let(:empty_project) { create(:empty_project) }
- before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+ before { empty_project.add_user(user, Gitlab::Access::MASTER) }
it "returns empty array" do
templates = subject.by_category('', empty_project)
@@ -78,7 +78,7 @@ describe Gitlab::Template::IssueTemplate do
context "when repo is empty" do
let(:empty_project) { create(:empty_project) }
- before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+ before { empty_project.add_user(user, Gitlab::Access::MASTER) }
it "raises file not found" do
issue_template = subject.new('.gitlab/issue_templates/not_existent.md', empty_project)
diff --git a/spec/lib/gitlab/template/merge_request_template_spec.rb b/spec/lib/gitlab/template/merge_request_template_spec.rb
index bb0f68043fa..ddf68c4cf78 100644
--- a/spec/lib/gitlab/template/merge_request_template_spec.rb
+++ b/spec/lib/gitlab/template/merge_request_template_spec.rb
@@ -10,7 +10,7 @@ describe Gitlab::Template::MergeRequestTemplate do
let(:file_path_3) { '.gitlab/merge_request_templates/feature_proposal.md' }
before do
- project.team.add_user(user, Gitlab::Access::MASTER)
+ project.add_user(user, Gitlab::Access::MASTER)
project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false)
project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false)
project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
@@ -53,7 +53,7 @@ describe Gitlab::Template::MergeRequestTemplate do
context 'when repo is bare or empty' do
let(:empty_project) { create(:empty_project) }
- before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+ before { empty_project.add_user(user, Gitlab::Access::MASTER) }
it "returns empty array" do
templates = subject.by_category('', empty_project)
@@ -78,7 +78,7 @@ describe Gitlab::Template::MergeRequestTemplate do
context "when repo is empty" do
let(:empty_project) { create(:empty_project) }
- before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+ before { empty_project.add_user(user, Gitlab::Access::MASTER) }
it "raises file not found" do
issue_template = subject.new('.gitlab/merge_request_templates/not_existent.md', empty_project)
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 0363bc74939..0f69119e82e 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -492,21 +492,22 @@ describe Notify do
end
end
- def invite_to_project(project:, email:, inviter:)
- Member.add_user(
- project.project_members,
- 'toto@example.com',
- Gitlab::Access::DEVELOPER,
- current_user: inviter
+ def invite_to_project(project, inviter:)
+ create(
+ :project_member,
+ :developer,
+ project: project,
+ invite_token: '1234',
+ invite_email: 'toto@example.com',
+ user: nil,
+ created_by: inviter
)
-
- project.project_members.invite.last
end
describe 'project invitation' do
let(:project) { create(:project) }
let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
- let(:project_member) { invite_to_project(project: project, email: 'toto@example.com', inviter: master) }
+ let(:project_member) { invite_to_project(project, inviter: master) }
subject { Notify.member_invited_email('project', project_member.id, project_member.invite_token) }
@@ -525,10 +526,10 @@ describe Notify do
describe 'project invitation accepted' do
let(:project) { create(:project) }
- let(:invited_user) { create(:user) }
+ let(:invited_user) { create(:user, name: 'invited user') }
let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
let(:project_member) do
- invitee = invite_to_project(project: project, email: 'toto@example.com', inviter: master)
+ invitee = invite_to_project(project, inviter: master)
invitee.accept_invite!(invited_user)
invitee
end
@@ -552,7 +553,7 @@ describe Notify do
let(:project) { create(:project) }
let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
let(:project_member) do
- invitee = invite_to_project(project: project, email: 'toto@example.com', inviter: master)
+ invitee = invite_to_project(project, inviter: master)
invitee.decline_invite!
invitee
end
@@ -744,21 +745,22 @@ describe Notify do
end
end
- def invite_to_group(group:, email:, inviter:)
- Member.add_user(
- group.group_members,
- 'toto@example.com',
- Gitlab::Access::DEVELOPER,
- current_user: inviter
+ def invite_to_group(group, inviter:)
+ create(
+ :group_member,
+ :developer,
+ group: group,
+ invite_token: '1234',
+ invite_email: 'toto@example.com',
+ user: nil,
+ created_by: inviter
)
-
- group.group_members.invite.last
end
describe 'group invitation' do
let(:group) { create(:group) }
let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
- let(:group_member) { invite_to_group(group: group, email: 'toto@example.com', inviter: owner) }
+ let(:group_member) { invite_to_group(group, inviter: owner) }
subject { Notify.member_invited_email('group', group_member.id, group_member.invite_token) }
@@ -777,10 +779,10 @@ describe Notify do
describe 'group invitation accepted' do
let(:group) { create(:group) }
- let(:invited_user) { create(:user) }
+ let(:invited_user) { create(:user, name: 'invited user') }
let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
let(:group_member) do
- invitee = invite_to_group(group: group, email: 'toto@example.com', inviter: owner)
+ invitee = invite_to_group(group, inviter: owner)
invitee.accept_invite!(invited_user)
invitee
end
@@ -804,7 +806,7 @@ describe Notify do
let(:group) { create(:group) }
let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
let(:group_member) do
- invitee = invite_to_group(group: group, email: 'toto@example.com', inviter: owner)
+ invitee = invite_to_group(group, inviter: owner)
invitee.decline_invite!
invitee
end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 3259f795296..3b8b743af2d 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -494,7 +494,7 @@ describe Issue, models: true do
context 'with an admin user' do
let(:project) { create(:empty_project) }
- let(:user) { create(:user, admin: true) }
+ let(:user) { create(:admin) }
it 'returns true for a regular issue' do
issue = build(:issue, project: project)
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 0b1634f654a..a9d3fcc6587 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -74,22 +74,17 @@ describe Member, models: true do
@blocked_master = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::MASTER)
@blocked_developer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::DEVELOPER)
- Member.add_user(
- project.members,
- 'toto1@example.com',
- Gitlab::Access::DEVELOPER,
- current_user: @master_user
- )
- @invited_member = project.members.invite.find_by_invite_email('toto1@example.com')
+ @invited_member = create(:project_member, :developer,
+ project: project,
+ invite_token: '1234',
+ invite_email: 'toto1@example.com')
accepted_invite_user = build(:user, state: :active)
- Member.add_user(
- project.members,
- 'toto2@example.com',
- Gitlab::Access::DEVELOPER,
- current_user: @master_user
- )
- @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
+ @accepted_invite_member = create(:project_member, :developer,
+ project: project,
+ invite_token: '1234',
+ 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.requesters.find_by(user_id: requested_user.id)
@@ -176,39 +171,209 @@ describe Member, models: true do
it { is_expected.to respond_to(:user_email) }
end
- describe ".add_user" do
- let!(:user) { create(:user) }
- let(:project) { create(:project) }
+ describe '.add_user' do
+ %w[project group].each do |source_type|
+ context "when source is a #{source_type}" do
+ let!(:source) { create(source_type) }
+ let!(:user) { create(:user) }
+ let!(:admin) { create(:admin) }
- context "when called with a user id" do
- it "adds the user as a member" do
- Member.add_user(project.project_members, user.id, ProjectMember::MASTER)
+ it 'returns a <Source>Member object' do
+ member = described_class.add_user(source, user, :master)
- expect(project.users).to include(user)
- end
- end
+ expect(member).to be_a "#{source_type.classify}Member".constantize
+ expect(member).to be_persisted
+ end
- context "when called with a user object" do
- it "adds the user as a member" do
- Member.add_user(project.project_members, user, ProjectMember::MASTER)
+ it 'sets members.created_by to the given current_user' do
+ member = described_class.add_user(source, user, :master, current_user: admin)
- expect(project.users).to include(user)
- end
- end
+ expect(member.created_by).to eq(admin)
+ end
- context "when called with a known user email" do
- it "adds the user as a member" do
- Member.add_user(project.project_members, user.email, ProjectMember::MASTER)
+ it 'sets members.expires_at to the given expires_at' do
+ member = described_class.add_user(source, user, :master, expires_at: Date.new(2016, 9, 22))
- expect(project.users).to include(user)
- end
- end
+ expect(member.expires_at).to eq(Date.new(2016, 9, 22))
+ end
+
+ described_class.access_levels.each do |sym_key, int_access_level|
+ it "accepts the :#{sym_key} symbol as access level" do
+ expect(source.users).not_to include(user)
+
+ member = described_class.add_user(source, user.id, sym_key)
+
+ expect(member.access_level).to eq(int_access_level)
+ expect(source.users.reload).to include(user)
+ end
+
+ it "accepts the #{int_access_level} integer as access level" do
+ expect(source.users).not_to include(user)
+
+ member = described_class.add_user(source, user.id, int_access_level)
+
+ expect(member.access_level).to eq(int_access_level)
+ expect(source.users.reload).to include(user)
+ end
+ end
+
+ context 'with no current_user' do
+ context 'when called with a known user id' do
+ it 'adds the user as a member' do
+ expect(source.users).not_to include(user)
+
+ described_class.add_user(source, user.id, :master)
+
+ expect(source.users.reload).to include(user)
+ end
+ end
+
+ context 'when called with an unknown user id' do
+ it 'adds the user as a member' do
+ expect(source.users).not_to include(user)
+
+ described_class.add_user(source, 42, :master)
+
+ expect(source.users.reload).not_to include(user)
+ end
+ end
+
+ context 'when called with a user object' do
+ it 'adds the user as a member' do
+ expect(source.users).not_to include(user)
+
+ described_class.add_user(source, user, :master)
+
+ expect(source.users.reload).to include(user)
+ end
+ end
+
+ context 'when called with a requester user object' do
+ before do
+ source.request_access(user)
+ end
+
+ it 'adds the requester as a member' do
+ expect(source.users).not_to include(user)
+ expect(source.requesters.exists?(user_id: user)).to be_truthy
+
+ expect { described_class.add_user(source, user, :master) }.
+ to raise_error(Gitlab::Access::AccessDeniedError)
+
+ expect(source.users.reload).not_to include(user)
+ expect(source.requesters.reload.exists?(user_id: user)).to be_truthy
+ end
+ end
+
+ context 'when called with a known user email' do
+ it 'adds the user as a member' do
+ expect(source.users).not_to include(user)
+
+ described_class.add_user(source, user.email, :master)
+
+ expect(source.users.reload).to include(user)
+ end
+ end
+
+ context 'when called with an unknown user email' do
+ it 'creates an invited member' do
+ expect(source.users).not_to include(user)
+
+ described_class.add_user(source, 'user@example.com', :master)
+
+ expect(source.members.invite.pluck(:invite_email)).to include('user@example.com')
+ end
+ end
+ end
+
+ context 'when current_user can update member' do
+ it 'creates the member' do
+ expect(source.users).not_to include(user)
+
+ described_class.add_user(source, user, :master, current_user: admin)
+
+ expect(source.users.reload).to include(user)
+ end
+
+ context 'when called with a requester user object' do
+ before do
+ source.request_access(user)
+ end
+
+ it 'adds the requester as a member' do
+ expect(source.users).not_to include(user)
+ expect(source.requesters.exists?(user_id: user)).to be_truthy
+
+ described_class.add_user(source, user, :master, current_user: admin)
+
+ expect(source.users.reload).to include(user)
+ expect(source.requesters.reload.exists?(user_id: user)).to be_falsy
+ end
+ end
+ end
+
+ context 'when current_user cannot update member' do
+ it 'does not create the member' do
+ expect(source.users).not_to include(user)
+
+ member = described_class.add_user(source, user, :master, current_user: user)
+
+ expect(source.users.reload).not_to include(user)
+ expect(member).not_to be_persisted
+ end
+
+ context 'when called with a requester user object' do
+ before do
+ source.request_access(user)
+ end
+
+ it 'does not destroy the requester' do
+ expect(source.users).not_to include(user)
+ expect(source.requesters.exists?(user_id: user)).to be_truthy
+
+ described_class.add_user(source, user, :master, current_user: user)
+
+ expect(source.users.reload).not_to include(user)
+ expect(source.requesters.exists?(user_id: user)).to be_truthy
+ end
+ end
+ end
+
+ context 'when member already exists' do
+ before do
+ source.add_user(user, :developer)
+ end
+
+ context 'with no current_user' do
+ it 'updates the member' do
+ expect(source.users).to include(user)
+
+ described_class.add_user(source, user, :master)
+
+ expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MASTER)
+ end
+ end
+
+ context 'when current_user can update member' do
+ it 'updates the member' do
+ expect(source.users).to include(user)
+
+ described_class.add_user(source, user, :master, current_user: admin)
+
+ expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MASTER)
+ end
+ end
+
+ context 'when current_user cannot update member' do
+ it 'does not update the member' do
+ expect(source.users).to include(user)
- context "when called with an unknown user email" do
- it "adds a member invite" do
- Member.add_user(project.project_members, "user@example.com", ProjectMember::MASTER)
+ described_class.add_user(source, user, :master, current_user: user)
- expect(project.project_members.invite.pluck(:invite_email)).to include("user@example.com")
+ expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER)
+ end
+ end
+ end
end
end
end
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index 56fa7fa6134..370aeb9e0a9 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -1,6 +1,33 @@
require 'spec_helper'
describe GroupMember, models: true do
+ describe '.access_level_roles' do
+ it 'returns Gitlab::Access.options_with_owner' do
+ expect(described_class.access_level_roles).to eq(Gitlab::Access.options_with_owner)
+ end
+ end
+
+ describe '.access_levels' do
+ it 'returns Gitlab::Access.options_with_owner' do
+ expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner)
+ end
+ end
+
+ describe '.add_users_to_group' do
+ it 'adds the given users to the given group' do
+ group = create(:group)
+ users = create_list(:user, 2)
+
+ described_class.add_users_to_group(
+ group,
+ [users.first.id, users.second],
+ described_class::MASTER
+ )
+
+ expect(group.users).to include(users.first, users.second)
+ end
+ end
+
describe 'notifications' do
describe "#after_create" do
it "sends email to user" do
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index 805c15a4e5e..d85a1c1e3b2 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -15,6 +15,26 @@ describe ProjectMember, models: true do
it { is_expected.to include_module(Gitlab::ShellAdapter) }
end
+ describe '.access_level_roles' do
+ it 'returns Gitlab::Access.options' do
+ expect(described_class.access_level_roles).to eq(Gitlab::Access.options)
+ end
+ end
+
+ describe '.add_user' do
+ context 'when called with the project owner' do
+ it 'adds the user as a member' do
+ project = create(:empty_project)
+
+ expect(project.users).not_to include(project.owner)
+
+ described_class.add_user(project, project.owner, :master, current_user: project.owner)
+
+ expect(project.users.reload).to include(project.owner)
+ end
+ end
+ end
+
describe '#real_source_type' do
subject { create(:project_member).real_source_type }
@@ -50,7 +70,7 @@ describe ProjectMember, models: true do
end
end
- describe :import_team do
+ describe '.import_team' do
before do
@project_1 = create :project
@project_2 = create :project
@@ -81,25 +101,21 @@ describe ProjectMember, models: true do
end
describe '.add_users_to_projects' do
- before do
- @project_1 = create :project
- @project_2 = create :project
+ it 'adds the given users to the given projects' do
+ projects = create_list(:empty_project, 2)
+ users = create_list(:user, 2)
- @user_1 = create :user
- @user_2 = create :user
-
- ProjectMember.add_users_to_projects(
- [@project_1.id, @project_2.id],
- [@user_1.id, @user_2.id],
- ProjectMember::MASTER
- )
- end
+ described_class.add_users_to_projects(
+ [projects.first.id, projects.second],
+ [users.first.id, users.second],
+ described_class::MASTER)
- it { expect(@project_1.users).to include(@user_1) }
- it { expect(@project_1.users).to include(@user_2) }
+ expect(projects.first.users).to include(users.first)
+ expect(projects.first.users).to include(users.second)
- it { expect(@project_2.users).to include(@user_1) }
- it { expect(@project_2.users).to include(@user_2) }
+ expect(projects.second.users).to include(users.first)
+ expect(projects.second.users).to include(users.second)
+ end
end
describe '.truncate_teams' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 83f61f0af0a..50423d027ac 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -836,7 +836,7 @@ describe Project, models: true do
describe 'when a user has access to a project' do
before do
- project.team.add_user(user, Gitlab::Access::MASTER)
+ project.add_user(user, Gitlab::Access::MASTER)
end
it { is_expected.to eq([project]) }
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index a7930c59df9..b813ee967f8 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -15,7 +15,7 @@ describe API::API, api: true do
let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
before do
- project.team << [user, :reporters]
+ project.team << [user, :reporter]
end
describe "GET /projects/:id/merge_requests" do
@@ -299,7 +299,7 @@ describe API::API, api: true do
let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) }
before :each do |each|
- fork_project.team << [user2, :reporters]
+ fork_project.team << [user2, :reporter]
end
it "returns merge_request" do