Make access request specs explicitly enable or disable access requests as required

24 files changed, 96 insertions, 76 deletions

diff --git a/changelogs/unreleased/21992-disable-access-requests-by-default.yml b/changelogs/unreleased/21992-disable-access-requests-by-default.yml
new file mode 100644
index 00000000000..ddcb2169407
--- /dev/null
+++ b/changelogs/unreleased/21992-disable-access-requests-by-default.yml
@@ -0,0 +1,4 @@
+---
+title: Disable "Request Access" functionality by default for new projects and groups
+merge_request: 7425
+author: 
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index c7db84dd5f9..60db0192dfd 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Groups::GroupMembersController do
   let(:user)  { create(:user) }
-  let(:group) { create(:group, :public) }
+  let(:group) { create(:group, :public, :access_requestable) }
 
   describe 'GET index' do
     it 'renders index with 200 status code' do
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 2a7523c6512..b52137fbe7e 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -2,7 +2,7 @@ require('spec_helper')
 
 describe Projects::ProjectMembersController do
   let(:user) { create(:user) }
-  let(:project) { create(:project, :public) }
+  let(:project) { create(:empty_project, :public, :access_requestable) }
 
   describe 'GET index' do
     it 'renders index with 200 status code' do
diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb
index 2d47a6f6c4c..ebd3595ea64 100644
--- a/spec/factories/groups.rb
+++ b/spec/factories/groups.rb
@@ -15,5 +15,9 @@ FactoryGirl.define do
     trait :private do
       visibility_level Gitlab::VisibilityLevel::PRIVATE
     end
+
+    trait :access_requestable do
+      request_access_enabled true
+    end
   end
 end
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index bfd88a254f1..1166498ddff 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -24,6 +24,10 @@ FactoryGirl.define do
       visibility_level Gitlab::VisibilityLevel::PRIVATE
     end
 
+    trait :access_requestable do
+      request_access_enabled true
+    end
+
     trait :empty_repo do
       after(:create) do |project|
         project.create_repository
diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb
index d811b05b0c3..dbe150823ba 100644
--- a/spec/features/groups/members/owner_manages_access_requests_spec.rb
+++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'Groups > Members > Owner manages access requests', feature: true do
   let(:user) { create(:user) }
   let(:owner) { create(:user) }
-  let(:group) { create(:group, :public) }
+  let(:group) { create(:group, :public, :access_requestable) }
 
   background do
     group.request_access(user)
diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb
index b3baa2ab57c..d8c9c487996 100644
--- a/spec/features/groups/members/user_requests_access_spec.rb
+++ b/spec/features/groups/members/user_requests_access_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'Groups > Members > User requests access', feature: true do
   let(:user) { create(:user) }
   let(:owner) { create(:user) }
-  let(:group) { create(:group, :public) }
+  let(:group) { create(:group, :public, :access_requestable) }
   let!(:project) { create(:project, :private, namespace: group) }
 
   background do
diff --git a/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb b/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb
index c4ed92d2780..4973e0aee85 100644
--- a/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb
+++ b/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb
@@ -3,8 +3,8 @@ require 'spec_helper'
 feature 'Projects > Members > Group requester cannot request access to project', feature: true do
   let(:user) { create(:user) }
   let(:owner) { create(:user) }
-  let(:group) { create(:group, :public) }
-  let(:project) { create(:project, :public, namespace: group) }
+  let(:group) { create(:group, :public, :access_requestable) }
+  let(:project) { create(:project, :public, :access_requestable, namespace: group) }
 
   background do
     group.add_owner(owner)
diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb
index d15376931c3..143390b71cd 100644
--- a/spec/features/projects/members/master_manages_access_requests_spec.rb
+++ b/spec/features/projects/members/master_manages_access_requests_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'Projects > Members > Master manages access requests', feature: true do
   let(:user) { create(:user) }
   let(:master) { create(:user) }
-  let(:project) { create(:project, :public) }
+  let(:project) { create(:empty_project, :public, :access_requestable) }
 
   background do
     project.request_access(user)
diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb
index 56ede8eb5be..97c42bd7f01 100644
--- a/spec/features/projects/members/user_requests_access_spec.rb
+++ b/spec/features/projects/members/user_requests_access_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'Projects > Members > User requests access', feature: true do
   let(:user) { create(:user) }
   let(:master) { create(:user) }
-  let(:project) { create(:project, :public) }
+  let(:project) { create(:project, :public, :access_requestable) }
 
   background do
     project.team << [master, :master]
diff --git a/spec/finders/access_requests_finder_spec.rb b/spec/finders/access_requests_finder_spec.rb
index 8cfea9659cb..c7278e971ae 100644
--- a/spec/finders/access_requests_finder_spec.rb
+++ b/spec/finders/access_requests_finder_spec.rb
@@ -3,12 +3,17 @@ require 'spec_helper'
 describe AccessRequestsFinder, services: true do
   let(:user) { create(:user) }
   let(:access_requester) { create(:user) }
-  let(:project) { create(:project, :public) }
-  let(:group) { create(:group, :public) }
 
-  before do
-    project.request_access(access_requester)
-    group.request_access(access_requester)
+  let(:project) do
+    create(:empty_project, :public, :access_requestable) do |project|
+      project.request_access(access_requester)
+    end
+  end
+
+  let(:group) do
+    create(:group, :public, :access_requestable) do |group|
+      group.request_access(access_requester)
+    end
   end
 
   shared_examples 'a finder returning access requesters' do |method_name|
diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index 6703d88e357..ffca1c94da1 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -11,11 +11,11 @@ describe MembersHelper do
 
   describe '#remove_member_message' do
     let(:requester) { build(:user) }
-    let(:project) { create(:empty_project, :public) }
+    let(:project) { create(:empty_project, :public, :access_requestable) }
     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_request) { project.request_access(requester) }
-    let(:group) { create(:group) }
+    let(:group) { create(:group, :access_requestable) }
     let(:group_member) { build(:group_member, group: group) }
     let(:group_member_invite) { build(:group_member, group: group).tap { |m| m.generate_invite_token! } }
     let(:group_member_request) { group.request_access(requester) }
@@ -32,10 +32,10 @@ describe MembersHelper do
 
   describe '#remove_member_title' do
     let(:requester) { build(:user) }
-    let(:project) { create(:empty_project, :public) }
+    let(:project) { create(:empty_project, :public, :access_requestable) }
     let(:project_member) { build(:project_member, project: project) }
     let(:project_member_request) { project.request_access(requester) }
-    let(:group) { create(:group) }
+    let(:group) { create(:group, :access_requestable) }
     let(:group_member) { build(:group_member, group: group) }
     let(:group_member_request) { group.request_access(requester) }
 
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index f5f3f58613d..932a5dc4862 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -401,7 +401,12 @@ describe Notify do
 
     describe 'project access requested' do
       context 'for a project in a user namespace' do
-        let(:project) { create(:project, :public).tap { |p| p.team << [p.owner, :master, p.owner] } }
+        let(:project) do
+          create(:empty_project, :public, :access_requestable) do |project|
+            project.team << [project.owner, :master, project.owner]
+          end
+        end
+
         let(:user) { create(:user) }
         let(:project_member) do
           project.request_access(user)
@@ -428,7 +433,7 @@ describe Notify do
       context 'for a project in a group' do
         let(:group_owner) { create(:user) }
         let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } }
-        let(:project) { create(:project, :public, namespace: group) }
+        let(:project) { create(:empty_project, :public, :access_requestable, namespace: group) }
         let(:user) { create(:user) }
         let(:project_member) do
           project.request_access(user)
@@ -454,7 +459,7 @@ describe Notify do
     end
 
     describe 'project access denied' do
-      let(:project) { create(:project) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
       let(:user) { create(:user) }
       let(:project_member) do
         project.request_access(user)
@@ -474,7 +479,7 @@ describe Notify do
     end
 
     describe 'project access changed' do
-      let(:project) { create(:project) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
       let(:user) { create(:user) }
       let(:project_member) { create(:project_member, project: project, user: user) }
       subject { Notify.member_access_granted_email('project', project_member.id) }
@@ -685,7 +690,7 @@ describe Notify do
 
   context 'for a group' do
     describe 'group access requested' do
-      let(:group) { create(:group) }
+      let(:group) { create(:group, :public, :access_requestable) }
       let(:user) { create(:user) }
       let(:group_member) do
         group.request_access(user)
diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb
index 96eee0e8bdd..4829ef17a20 100644
--- a/spec/models/concerns/access_requestable_spec.rb
+++ b/spec/models/concerns/access_requestable_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe AccessRequestable do
   describe 'Group' do
     describe '#request_access' do
-      let(:group) { create(:group, :public) }
+      let(:group) { create(:group, :public, :access_requestable) }
       let(:user) { create(:user) }
 
       it { expect(group.request_access(user)).to be_a(GroupMember) }
@@ -11,7 +11,7 @@ describe AccessRequestable do
     end
 
     describe '#access_requested?' do
-      let(:group) { create(:group, :public) }
+      let(:group) { create(:group, :public, :access_requestable) }
       let(:user) { create(:user) }
 
       before { group.request_access(user) }
@@ -22,14 +22,14 @@ describe AccessRequestable do
 
   describe 'Project' do
     describe '#request_access' do
-      let(:project) { create(:empty_project, :public) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
       let(:user) { create(:user) }
 
       it { expect(project.request_access(user)).to be_a(ProjectMember) }
     end
 
     describe '#access_requested?' do
-      let(:project) { create(:empty_project, :public) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
       let(:user) { create(:user) }
 
       before { project.request_access(user) }
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 47f89f744cb..1613a586a2c 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Group, models: true do
-  let!(:group) { create(:group) }
+  let!(:group) { create(:group, :access_requestable) }
 
   describe 'associations' do
     it { is_expected.to have_many :projects }
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 485121701af..12419d6fd5a 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -57,7 +57,7 @@ describe Member, models: true do
 
   describe 'Scopes & finders' do
     before do
-      project = create(:empty_project, :public)
+      project = create(:empty_project, :public, :access_requestable)
       group = create(:group)
       @owner_user = create(:user).tap { |u| group.add_owner(u) }
       @owner = group.members.find_by(user_id: @owner_user.id)
@@ -174,7 +174,7 @@ describe Member, models: true do
   describe '.add_user' do
     %w[project group].each do |source_type|
       context "when source is a #{source_type}" do
-        let!(:source) { create(source_type, :public) }
+        let!(:source) { create(source_type, :public, :access_requestable) }
         let!(:user) { create(:user) }
         let!(:admin) { create(:admin) }
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 0810d06b50f..3a0c3ec21cd 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -76,7 +76,7 @@ describe Project, models: true do
     end
 
     describe '#members & #requesters' do
-      let(:project) { create(:project, :public) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
       let(:requester) { create(:user) }
       let(:developer) { create(:user) }
       before do
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index e0f2dadf189..12228425579 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -137,7 +137,7 @@ describe ProjectTeam, models: true do
 
   describe '#find_member' do
     context 'personal project' do
-      let(:project) { create(:empty_project, :public) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
       let(:requester) { create(:user) }
 
       before do
@@ -155,7 +155,7 @@ describe ProjectTeam, models: true do
     end
 
     context 'group project' do
-      let(:group) { create(:group) }
+      let(:group) { create(:group, :access_requestable) }
       let(:project) { create(:empty_project, group: group) }
       let(:requester) { create(:user) }
 
@@ -200,7 +200,7 @@ describe ProjectTeam, models: true do
     let(:requester) { create(:user) }
 
     context 'personal project' do
-      let(:project) { create(:empty_project, :public) }
+      let(:project) { create(:empty_project, :public, :access_requestable) }
 
       context 'when project is not shared with group' do
         before do
@@ -243,7 +243,7 @@ describe ProjectTeam, models: true do
     end
 
     context 'group project' do
-      let(:group) { create(:group) }
+      let(:group) { create(:group, :access_requestable) }
       let(:project) { create(:empty_project, group: group) }
 
       before do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 3b152e15b61..54b23b02003 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -37,7 +37,7 @@ describe User, models: true do
     describe '#group_members' do
       it 'does not include group memberships for which user is a requester' do
         user = create(:user)
-        group = create(:group, :public)
+        group = create(:group, :public, :access_requestable)
         group.request_access(user)
 
         expect(user.group_members).to be_empty
@@ -47,7 +47,7 @@ describe User, models: true do
     describe '#project_members' do
       it 'does not include project memberships for which user is a requester' do
         user = create(:user)
-        project = create(:project, :public)
+        project = create(:project, :public, :access_requestable)
         project.request_access(user)
 
         expect(user.project_members).to be_empty
diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb
index b467890a403..1a771b3c87a 100644
--- a/spec/requests/api/access_requests_spec.rb
+++ b/spec/requests/api/access_requests_spec.rb
@@ -9,19 +9,19 @@ describe API::AccessRequests, api: true  do
   let(:stranger) { create(:user) }
 
   let(:project) do
-    project = create(:project, :public, creator_id: master.id, namespace: master.namespace)
-    project.team << [developer, :developer]
-    project.team << [master, :master]
-    project.request_access(access_requester)
-    project
+    create(:project, :public, :access_requestable, creator_id: master.id, namespace: master.namespace) do |project|
+      project.team << [developer, :developer]
+      project.team << [master, :master]
+      project.request_access(access_requester)
+    end
   end
 
   let(:group) do
-    group = create(:group, :public)
-    group.add_developer(developer)
-    group.add_owner(master)
-    group.request_access(access_requester)
-    group
+    create(:group, :public, :access_requestable) do |group|
+      group.add_developer(developer)
+      group.add_owner(master)
+      group.request_access(access_requester)
+    end
   end
 
   shared_examples 'GET /:sources/:id/access_requests' do |source_type|
@@ -89,7 +89,7 @@ describe API::AccessRequests, api: true  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)
+            source.update_attributes(request_access_enabled: false)
           end
 
           it 'returns 403' do
diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb
index 493c0a893d1..2c94c86ccfa 100644
--- a/spec/requests/api/members_spec.rb
+++ b/spec/requests/api/members_spec.rb
@@ -9,19 +9,19 @@ describe API::Members, api: true  do
   let(:stranger) { create(:user) }
 
   let(:project) do
-    project = create(:project, :public, creator_id: master.id, namespace: master.namespace)
-    project.team << [developer, :developer]
-    project.team << [master, :master]
-    project.request_access(access_requester)
-    project
+    create(:project, :public, :access_requestable, creator_id: master.id, namespace: master.namespace) do |project|
+      project.team << [developer, :developer]
+      project.team << [master, :master]
+      project.request_access(access_requester)
+    end
   end
 
   let!(:group) do
-    group = create(:group, :public)
-    group.add_developer(developer)
-    group.add_owner(master)
-    group.request_access(access_requester)
-    group
+    create(:group, :public, :access_requestable) do |group|
+      group.add_developer(developer)
+      group.add_owner(master)
+      group.request_access(access_requester)
+    end
   end
 
   shared_examples 'GET /:sources/:id/members' do |source_type|
diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb
index 7b090343a3e..7d5a66801db 100644
--- a/spec/services/members/approve_access_request_service_spec.rb
+++ b/spec/services/members/approve_access_request_service_spec.rb
@@ -3,8 +3,8 @@ require 'spec_helper'
 describe Members::ApproveAccessRequestService, services: true do
   let(:user) { create(:user) }
   let(:access_requester) { create(:user) }
-  let(:project) { create(:project, :public) }
-  let(:group) { create(:group, :public) }
+  let(:project) { create(:empty_project, :public, :access_requestable) }
+  let(:group) { create(:group, :public, :access_requestable) }
   let(:opts) { {} }
 
   shared_examples 'a service raising ActiveRecord::RecordNotFound' do
diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb
index 9995f3488af..574df6e0f42 100644
--- a/spec/services/members/destroy_service_spec.rb
+++ b/spec/services/members/destroy_service_spec.rb
@@ -26,6 +26,7 @@ describe Members::DestroyService, services: true do
     context 'when the given member is an access requester' do
       before do
         source.members.find_by(user_id: member_user).destroy
+        source.update_attributes(request_access_enabled: true)
         source.request_access(member_user)
       end
       let(:access_requester) { source.requesters.find_by(user_id: member_user) }
diff --git a/spec/services/members/request_access_service_spec.rb b/spec/services/members/request_access_service_spec.rb
index 0d2d5f03199..853c125dadb 100644
--- a/spec/services/members/request_access_service_spec.rb
+++ b/spec/services/members/request_access_service_spec.rb
@@ -2,8 +2,6 @@ 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
@@ -31,27 +29,26 @@ describe Members::RequestAccessService, services: true do
   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 }
+    %i[project group].each do |source_type|
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+        let(:source) { create(source_type, :private) }
+      end
     end
+  end
 
-    it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
-      let(:source) { group }
+  context 'when access requests are disabled' do
+    %i[project group].each do |source_type|
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+        let(:source) { create(source_type, :public) }
+      end
     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 }
+    %i[project group].each do |source_type|
+      it_behaves_like 'a service creating a access request' do
+        let(:source) { create(source_type, :public, :access_requestable) }
+      end
     end
   end
 end