Move EE-specific ProjectTeam code to a EE module and remove EE-specific code in Members API

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

app/models/project_team.rb

 class ProjectTeam
   include BulkMemberAccessLoad
 
+  prepend EE::ProjectTeam
+
   attr_accessor :project
 
   def initialize(project)
@@ -40,8 +42,6 @@ class ProjectTeam
   end
 
   def add_users(users, access_level, current_user: nil, expires_at: nil)
-    return false if group_member_lock
-
     ProjectMember.add_users(
       project,
       users,
@@ -173,12 +173,4 @@ class ProjectTeam
   def group
     project.group
   end
-
-  def group_member_lock
-    group && group.membership_lock
-  end
-
-  def merge_max!(first_hash, second_hash)
-    first_hash.merge!(second_hash) { |_key, old, new| old > new ? old : new }
-  end
 end

ee/app/models/ee/project_team.rb

+module EE
+  module ProjectTeam
+    extend ActiveSupport::Concern
+
+    def add_users(users, access_level, current_user: nil, expires_at: nil)
+      raise NotImplementedError unless defined?(super)
+      return false if group_member_lock
+
+      super
+    end
+
+    def add_user(user, access_level, current_user: nil, expires_at: nil)
+      raise NotImplementedError unless defined?(super)
+      return false if group_member_lock
+
+      super
+    end
+
+    private
+
+    def group_member_lock
+      group && group.membership_lock
+    end
+  end
+end

lib/api/members.rb

@@ -54,12 +54,6 @@ module API
           source = find_source(source_type, params[:id])
           authorize_admin_source!(source_type, source)
 
-          ## EE specific
-          if source_type == 'project' && source.group && source.group.membership_lock
-            not_allowed!
-          end
-          ## EE specific
-
           member = source.members.find_by(user_id: params[:user_id])
           conflict!('Member already exists') if member
 

spec/ee/models/project_team_spec.rb

+require "spec_helper"
+
+describe ProjectTeam do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+
+  describe '#add_users' do
+    let(:user1) { create(:user) }
+    let(:user2) { create(:user) }
+
+    context 'when group membership is locked' do
+      before do
+        group.update_attribute(:membership_lock, true)
+      end
+
+      it 'does not add the given users to the team' do
+        project.team.add_users([user1, user2], :reporter)
+
+        expect(project.team.reporter?(user1)).to be(false)
+        expect(project.team.reporter?(user2)).to be(false)
+      end
+    end
+  end
+
+  describe '#add_user' do
+    let(:user) { create(:user) }
+
+    context 'when group membership is locked' do
+      before do
+        group.update_attribute(:membership_lock, true)
+      end
+
+      it 'does not add the given user to the team' do
+        project.team.add_user(user, :reporter)
+
+        expect(project.team.reporter?(user)).to be(false)
+      end
+    end
+  end
+end

spec/models/project_team_spec.rb

@@ -178,6 +178,30 @@ describe ProjectTeam do
     end
   end
 
+  describe '#add_users' do
+    let(:user1) { create(:user) }
+    let(:user2) { create(:user) }
+    let(:project) { create(:project) }
+
+    it 'add the given users to the team' do
+      project.team.add_users([user1, user2], :reporter)
+
+      expect(project.team.reporter?(user1)).to be(true)
+      expect(project.team.reporter?(user2)).to be(true)
+    end
+  end
+
+  describe '#add_user' do
+    let(:user) { create(:user) }
+    let(:project) { create(:project) }
+
+    it 'add the given user to the team' do
+      project.team.add_user(user, :reporter)
+
+      expect(project.team.reporter?(user)).to be(true)
+    end
+  end
+
   describe "#human_max_access" do
     it 'returns Master role' do
       user = create(:user)