Merge branch '21992-disable-access-requests-by-default' into 'master'

changelogs/unreleased/21992-disable-access-requests-by-default.yml

+---
+title: Disable "Request Access" functionality by default for new projects and groups
+merge_request: 7425
+author: 

db/migrate/20161020075734_default_request_access_groups.rb

+class DefaultRequestAccessGroups < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  DOWNTIME = false
+
+  def up
+    change_column_default :namespaces, :request_access_enabled, false
+  end
+
+  def down
+    change_column_default :namespaces, :request_access_enabled, true
+  end
+end

db/migrate/20161020075830_default_request_access_projects.rb

+class DefaultRequestAccessProjects < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  DOWNTIME = false
+  
+  def up
+    change_column_default :projects, :request_access_enabled, false
+  end
+
+  def down
+    change_column_default :projects, :request_access_enabled, true
+  end
+end

db/schema.rb

@@ -723,7 +723,7 @@ ActiveRecord::Schema.define(version: 20161109150329) do
     t.string "avatar"
     t.boolean "share_with_group_lock", default: false
     t.integer "visibility_level", default: 20, null: false
-    t.boolean "request_access_enabled", default: true, null: false
+    t.boolean "request_access_enabled", default: false, null: false
     t.datetime "deleted_at"
     t.boolean "lfs_enabled"
     t.text "description_html"
@@ -911,7 +911,7 @@ ActiveRecord::Schema.define(version: 20161109150329) do
     t.boolean "only_allow_merge_if_build_succeeds", default: false, null: false
     t.boolean "has_external_issue_tracker"
     t.string "repository_storage", default: "default", null: false
-    t.boolean "request_access_enabled", default: true, null: false
+    t.boolean "request_access_enabled", default: false, null: false
     t.boolean "has_external_wiki"
     t.boolean "lfs_enabled"
     t.text "description_html"

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

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

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

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

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)

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

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)

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)

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]

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|

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) }
 

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)

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) }

spec/models/group_spec.rb

 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 }

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) }
 

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

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

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

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

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|

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

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) }

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