Commit 076e0406 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'rc-new-members-request-access-service' into 'master'

New `Members::RequestAccessService`

Part of #21979.

See merge request !6265
parents c9352aa6 76232d54
...@@ -8,9 +8,6 @@ module AccessRequestable ...@@ -8,9 +8,6 @@ module AccessRequestable
extend ActiveSupport::Concern extend ActiveSupport::Concern
def request_access(user) def request_access(user)
members.create( Members::RequestAccessService.new(self, user).execute
access_level: Gitlab::Access::DEVELOPER,
user: user,
requested_at: Time.now.utc)
end end
end end
module Members
class RequestAccessService < BaseService
attr_accessor :source
def initialize(source, current_user)
@source = source
@current_user = current_user
end
def execute
raise Gitlab::Access::AccessDeniedError unless can_request_access?(source)
source.members.create(
access_level: Gitlab::Access::DEVELOPER,
user: current_user,
requested_at: Time.now.utc)
end
private
def can_request_access?(source)
source && can?(current_user, :request_access, source)
end
end
end
...@@ -11,7 +11,7 @@ describe MembersHelper do ...@@ -11,7 +11,7 @@ describe MembersHelper do
describe '#remove_member_message' do describe '#remove_member_message' do
let(:requester) { build(:user) } let(:requester) { build(:user) }
let(:project) { create(:project) } let(:project) { create(:empty_project, :public) }
let(:project_member) { build(:project_member, project: project) } let(:project_member) { build(:project_member, project: project) }
let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } } let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } }
let(:project_member_request) { project.request_access(requester) } let(:project_member_request) { project.request_access(requester) }
...@@ -32,7 +32,7 @@ describe MembersHelper do ...@@ -32,7 +32,7 @@ describe MembersHelper do
describe '#remove_member_title' do describe '#remove_member_title' do
let(:requester) { build(:user) } let(:requester) { build(:user) }
let(:project) { create(:project) } let(:project) { create(:empty_project, :public) }
let(:project_member) { build(:project_member, project: project) } let(:project_member) { build(:project_member, project: project) }
let(:project_member_request) { project.request_access(requester) } let(:project_member_request) { project.request_access(requester) }
let(:group) { create(:group) } let(:group) { create(:group) }
......
...@@ -402,7 +402,7 @@ describe Notify do ...@@ -402,7 +402,7 @@ describe Notify do
describe 'project access requested' do describe 'project access requested' do
context 'for a project in a user namespace' do context 'for a project in a user namespace' do
let(:project) { create(:project).tap { |p| p.team << [p.owner, :master, p.owner] } } let(:project) { create(:project, :public).tap { |p| p.team << [p.owner, :master, p.owner] } }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project_member) do let(:project_member) do
project.request_access(user) project.request_access(user)
...@@ -429,7 +429,7 @@ describe Notify do ...@@ -429,7 +429,7 @@ describe Notify do
context 'for a project in a group' do context 'for a project in a group' do
let(:group_owner) { create(:user) } let(:group_owner) { create(:user) }
let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } } let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } }
let(:project) { create(:project, namespace: group) } let(:project) { create(:project, :public, namespace: group) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project_member) do let(:project_member) do
project.request_access(user) project.request_access(user)
......
...@@ -57,7 +57,7 @@ describe Member, models: true do ...@@ -57,7 +57,7 @@ describe Member, models: true do
describe 'Scopes & finders' do describe 'Scopes & finders' do
before do before do
project = create(:empty_project) project = create(:empty_project, :public)
group = create(:group) group = create(:group)
@owner_user = create(:user).tap { |u| group.add_owner(u) } @owner_user = create(:user).tap { |u| group.add_owner(u) }
@owner = group.members.find_by(user_id: @owner_user.id) @owner = group.members.find_by(user_id: @owner_user.id)
......
...@@ -68,7 +68,7 @@ describe Project, models: true do ...@@ -68,7 +68,7 @@ describe Project, models: true do
it { is_expected.to have_many(:forks).through(:forked_project_links) } it { is_expected.to have_many(:forks).through(:forked_project_links) }
describe '#members & #requesters' do describe '#members & #requesters' do
let(:project) { create(:project) } let(:project) { create(:project, :public) }
let(:requester) { create(:user) } let(:requester) { create(:user) }
let(:developer) { create(:user) } let(:developer) { create(:user) }
before do before do
......
...@@ -137,7 +137,7 @@ describe ProjectTeam, models: true do ...@@ -137,7 +137,7 @@ describe ProjectTeam, models: true do
describe '#find_member' do describe '#find_member' do
context 'personal project' do context 'personal project' do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project, :public) }
let(:requester) { create(:user) } let(:requester) { create(:user) }
before do before do
...@@ -200,7 +200,7 @@ describe ProjectTeam, models: true do ...@@ -200,7 +200,7 @@ describe ProjectTeam, models: true do
let(:requester) { create(:user) } let(:requester) { create(:user) }
context 'personal project' do context 'personal project' do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project, :public) }
context 'when project is not shared with group' do context 'when project is not shared with group' do
before do before do
......
...@@ -64,12 +64,12 @@ describe API::AccessRequests, api: true do ...@@ -64,12 +64,12 @@ describe API::AccessRequests, api: true do
context 'when authenticated as a member' do context 'when authenticated as a member' do
%i[developer master].each do |type| %i[developer master].each do |type|
context "as a #{type}" do context "as a #{type}" do
it 'returns 400' do it 'returns 403' do
expect do expect do
user = public_send(type) user = public_send(type)
post api("/#{source_type.pluralize}/#{source.id}/access_requests", user) post api("/#{source_type.pluralize}/#{source.id}/access_requests", user)
expect(response).to have_http_status(400) expect(response).to have_http_status(403)
end.not_to change { source.requesters.count } end.not_to change { source.requesters.count }
end end
end end
...@@ -87,6 +87,20 @@ describe API::AccessRequests, api: true do ...@@ -87,6 +87,20 @@ describe API::AccessRequests, api: true do
end end
context 'when authenticated as a stranger' do context 'when authenticated as a stranger' do
context "when access request is disabled for the #{source_type}" do
before do
source.update(request_access_enabled: false)
end
it 'returns 403' do
expect do
post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger)
expect(response).to have_http_status(403)
end.not_to change { source.requesters.count }
end
end
it 'returns 201' do it 'returns 201' do
expect do expect do
post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger)
......
require 'spec_helper'
describe Members::RequestAccessService, services: true do
let(:user) { create(:user) }
let(:project) { create(:project, :private) }
let(:group) { create(:group, :private) }
shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
it 'raises Gitlab::Access::AccessDeniedError' do
expect { described_class.new(source, user).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
shared_examples 'a service creating a access request' do
it 'succeeds' do
expect { described_class.new(source, user).execute }.to change { source.requesters.count }.by(1)
end
it 'returns a <Source>Member' do
member = described_class.new(source, user).execute
expect(member).to be_a "#{source.class.to_s}Member".constantize
expect(member.requested_at).to be_present
end
end
context 'when source is nil' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { nil }
end
end
context 'when current user cannot request access to the project' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { project }
end
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { group }
end
end
context 'when current user can request access to the project' do
before do
project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
group.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
end
it_behaves_like 'a service creating a access request' do
let(:source) { project }
end
it_behaves_like 'a service creating a access request' do
let(:source) { group }
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment