Remove unassigned users from group members counter

Add validation that differ between tiers

Remove unused method

Remove unassigned groups from user.rb in ce

Fix groups spec

Add unassigned member to specs

Modify specs to new settings

Fix spec for presenter

Fix specs problems

Fix specs

app/helpers/groups/group_members_helper.rb

@@ -6,11 +6,7 @@ module Groups::GroupMembersHelper
   end
 
   def render_invite_member_for_group(group, default_access_level)
-    render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: access_level_roles(group), default_access_level: default_access_level
-  end
-
-  def access_level_roles(group)
-    GroupMember.access_level_roles
+    render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: GroupMember.access_level_roles, default_access_level: default_access_level
   end
 end
 

app/models/concerns/loaded_in_group_list.rb

@@ -54,6 +54,7 @@ module LoadedInGroupList
         .where(members[:source_type].eq(Namespace.name))
         .where(members[:source_id].eq(namespaces[:id]))
         .where(members[:requested_at].eq(nil))
+        .where(members[:access_level].gt(Gitlab::Access::UNASSIGNED))
     end
   end
 
@@ -70,7 +71,7 @@ module LoadedInGroupList
   end
 
   def member_count
-    @member_count ||= try(:preloaded_member_count) || users.count
+    @member_count ||= try(:preloaded_member_count) || full_access_members.count
   end
 end
 

app/models/group.rb

@@ -22,6 +22,8 @@ class Group < Namespace
 
   has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
   alias_method :members, :group_members
+  has_many :full_access_members, -> { where(requested_at: nil).where.not(access_level: Gitlab::Access::UNASSIGNED) }, dependent: :destroy, as: :source, class_name: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent
+
   has_many :users, through: :group_members
   has_many :owners,
     -> { where(members: { access_level: Gitlab::Access::OWNER }) },
@@ -326,7 +328,7 @@ class Group < Namespace
   # rubocop: enable CodeReuse/ServiceClass
 
   def user_ids_for_project_authorizations
-    members_with_parents.where.not(access_level: Gitlab::Access::UNASSIGNED).pluck(:user_id)
+    members_with_parents.non_unassigned.pluck(:user_id)
   end
 
   def self_and_ancestors_ids

app/models/members/group_member.rb

@@ -13,7 +13,8 @@ class GroupMember < Member
   # Make sure group member points only to group as it source
   default_value_for :source_type, SOURCE_TYPE
   validates :source_type, format: { with: /\ANamespace\z/ }
-  validates :access_level, inclusion: { in: Gitlab::Access.values_with_limited }, presence: true
+  validates :access_level, presence: true
+  validate :access_level_inclusion
 
   default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope
 
@@ -47,6 +48,12 @@ class GroupMember < Member
 
   private
 
+  def access_level_inclusion
+    return if access_level.in?(Gitlab::Access.all_values)
+
+    errors.add(:access_level, "is not included in the list")
+  end
+
   def send_invite
     run_after_commit_or_now { notification_service.invite_group_member(self, @raw_invite_token) }
 

app/models/user.rb

@@ -134,8 +134,6 @@ class User < ApplicationRecord
            -> { where(members: { access_level: [Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER] }) },
            through: :group_members,
            source: :group
-  has_many :unassigned_group_members, -> { where(access_level: [Gitlab::Access::UNASSIGNED]) }, source: 'GroupMember', class_name: 'GroupMember'
-  has_many :unassigned_groups, through: :unassigned_group_members, source: :group
 
   # Projects
   has_many :groups_projects,          through: :groups, source: :projects
@@ -892,7 +890,6 @@ class User < ApplicationRecord
     Group.unscoped do
       Group.from_union([
         groups,
-        unassigned_groups,
         authorized_projects.joins(:namespace).select('namespaces.*')
       ])
     end

app/policies/group_policy.rb

@@ -12,7 +12,6 @@ class GroupPolicy < BasePolicy
 
   condition(:has_access) { access_level != GroupMember::NO_ACCESS }
 
-  condition(:unassigned) { access_level >= GroupMember::UNASSIGNED }
   condition(:guest) { access_level >= GroupMember::GUEST }
   condition(:developer) { access_level >= GroupMember::DEVELOPER }
   condition(:owner) { access_level >= GroupMember::OWNER }

ee/app/helpers/ee/groups/group_members_helper.rb

@@ -7,12 +7,4 @@ module EE::Groups::GroupMembersHelper
   def group_member_select_options
     super.merge(skip_ldap: @group.ldap_synced?)
   end
-
-  override :access_level_roles
-  def access_level_roles(group)
-    levels = GroupMember.access_level_roles
-    return levels if group.unassigned_role_allowed?
-
-    levels.except("Unassigned")
-  end
 end

ee/app/models/ee/group_member.rb

@@ -3,6 +3,7 @@
 module EE
   module GroupMember
     extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
 
     prepended do
       extend ::Gitlab::Utils::Override
@@ -31,11 +32,6 @@ module EE
       def member_of_group?(group, user)
         exists?(group: group, user: user)
       end
-
-      override :access_level_roles
-      def access_level_roles
-        ::Gitlab::Access.options_with_unassigned
-      end
     end
 
     def group_has_domain_limitations?

ee/app/models/ee/user.rb

@@ -44,6 +44,9 @@ module EE
       has_many :approvals,                dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
       has_many :approvers,                dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
 
+      has_many :unassigned_group_members, -> { where(access_level: [Gitlab::Access::UNASSIGNED]) }, source: 'GroupMember', class_name: 'GroupMember'
+      has_many :unassigned_groups, through: :unassigned_group_members, source: :group
+
       has_many :users_ops_dashboard_projects
       has_many :ops_dashboard_projects, through: :users_ops_dashboard_projects, source: :project
       has_many :users_security_dashboard_projects

ee/app/presenters/ee/member_presenter.rb

@@ -15,7 +15,7 @@ module EE
 
     override :valid_level_roles
     def valid_level_roles
-      return super if member.source.is_a?(Project) || source_allows_unassigned_role?(member)
+      return super if member.source.is_a?(Project)
 
       super.except("Unassigned")
     end
@@ -25,9 +25,5 @@ module EE
     def override_member_permission
       raise NotImplementedError
     end
-
-    def source_allows_unassigned_role?(member)
-      member.source.unassigned_role_allowed?
-    end
   end
 end

lib/gitlab/access.rb

@@ -39,10 +39,6 @@ module Gitlab
         options_with_owner.values
       end
 
-      def values_with_limited
-        options_with_unassigned.values
-      end
-
       def options
         {
           "Guest"          => GUEST,
@@ -91,7 +87,7 @@ module Gitlab
       end
 
       def human_access(access)
-        options_with_unassigned.key(access)
+        options_with_owner.key(access)
       end
 
       def human_access_with_none(access)

spec/factories/group_members.rb

@@ -28,5 +28,11 @@ FactoryBot.define do
     trait :blocked do
       after(:build) { |group_member, _| group_member.user.block! }
     end
+
+    trait :unassigned do
+      to_create { |instance| instance.save!(validate: false) }
+
+      access_level { GroupMember::UNASSIGNED }
+    end
   end
 end

spec/finders/group_members_finder_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe GroupMembersFinder, '#execute' do
     member1 = group.add_maintainer(user1)
     member2 = group.add_maintainer(user2)
     member3 = group.add_maintainer(user3)
-    create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: create(:user), group: group)
+    create(:group_member, :unassigned, user: create(:user), group: group)
 
     result = described_class.new(group).execute
 

spec/finders/groups_finder_spec.rb

@@ -107,10 +107,10 @@ RSpec.describe GroupsFinder do
           end
 
           context 'being limited access member of parent group' do
-            it 'returns visible subgroups' do
-              create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: parent_group)
+            it 'do not return group with unassigned access' do
+              create(:group_member, :unassigned, user: user, group: parent_group)
 
-              is_expected.to contain_exactly(parent_group, public_subgroup, internal_subgroup)
+              is_expected.to contain_exactly(public_subgroup, internal_subgroup)
             end
           end
 

spec/finders/members_finder_spec.rb

@@ -45,12 +45,12 @@ RSpec.describe MembersFinder, '#execute' do
     expect(result).to contain_exactly(member1)
   end
 
-  it 'does not return members of parent group with limited access' do
+  it 'does not return members of parent group with unassigned access' do
     nested_group.request_access(user1)
     member1 = group.add_maintainer(user2)
     member2 = nested_group.add_maintainer(user3)
     member3 = project.add_maintainer(user4)
-    create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: create(:user), group: group)
+    create(:group_member, :unassigned, user: create(:user), group: group)
 
     result = described_class.new(project, user2).execute
 

spec/lib/gitlab/project_authorizations_spec.rb

@@ -115,7 +115,7 @@ RSpec.describe Gitlab::ProjectAuthorizations do
     end
   end
 
-  context 'user with limited access to group' do
+  context 'user with unassigned access to group' do
     let_it_be(:group) { create(:group) }
     let_it_be(:user) { create(:user) }
 
@@ -125,7 +125,7 @@ RSpec.describe Gitlab::ProjectAuthorizations do
       let!(:group_project) { create(:project, namespace: group) }
 
       before do
-        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+        create(:group_member, :unassigned, user: user, group: group)
       end
 
       it 'does not create authorization' do
@@ -138,7 +138,7 @@ RSpec.describe Gitlab::ProjectAuthorizations do
       let!(:sub_group_project) { create(:project, namespace: sub_group) }
 
       before do
-        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+        create(:group_member, :unassigned, user: user, group: group)
       end
 
       it 'does not create authorization' do
@@ -152,7 +152,7 @@ RSpec.describe Gitlab::ProjectAuthorizations do
 
       before do
         create(:group_group_link, shared_group: shared_group, shared_with_group: group)
-        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+        create(:group_member, :unassigned, user: user, group: group)
       end
 
       it 'does not create authorization' do
@@ -166,7 +166,7 @@ RSpec.describe Gitlab::ProjectAuthorizations do
 
       before do
         create(:project_group_link, group: group, project: shared_project)
-        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+        create(:group_member, :unassigned, user: user, group: group)
       end
 
       it 'does not create authorization' do

spec/models/group_spec.rb

@@ -692,7 +692,7 @@ RSpec.describe Group do
     before do
       create(:group_member, user: user, group: group_parent, access_level: parent_group_access_level)
       create(:group_member, user: user, group: group, access_level: group_access_level)
-      create(:group_member, user: create(:user), group: group, access_level: Gitlab::Access::UNASSIGNED)
+      create(:group_member, :unassigned, user: create(:user), group: group)
       create(:group_member, user: user, group: group_child, access_level: child_group_access_level)
     end
 

spec/models/member_spec.rb

@@ -149,7 +149,7 @@ RSpec.describe Member do
 
       accepted_request_user = create(:user).tap { |u| project.request_access(u) }
       @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
-      @member_unassigned = create(:group_member, access_level: Gitlab::Access::UNASSIGNED, group: group)
+      @member_unassigned = create(:group_member, :unassigned, group: group)
     end
 
     describe '.access_for_user_ids' do

spec/services/authorized_project_update/project_create_service_spec.rb

@@ -81,7 +81,7 @@ RSpec.describe AuthorizedProjectUpdate::ProjectCreateService do
         before do
           create(:group_member, access_level: Gitlab::Access::REPORTER, group: group, user: group_user)
           create(:group_member, access_level: Gitlab::Access::MAINTAINER, group: shared_with_group, user: group_user)
-          create(:group_member, access_level: Gitlab::Access::UNASSIGNED, group: shared_with_group, user: create(:user))
+          create(:group_member, :unassigned, group: shared_with_group, user: create(:user))
 
           create(:group_group_link, shared_group: group, shared_with_group: shared_with_group, group_access: Gitlab::Access::DEVELOPER)
 
@@ -124,9 +124,9 @@ RSpec.describe AuthorizedProjectUpdate::ProjectCreateService do
       end
     end
 
-    context 'member with limited access' do
+    context 'member with unassigned access' do
       before do
-        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: group_user, group: group)
+        create(:group_member, :unassigned, user: group_user, group: group)
       end
 
       it 'does not create project authorization' do

spec/services/authorized_project_update/project_group_link_create_service_spec.rb

@@ -114,7 +114,7 @@ RSpec.describe AuthorizedProjectUpdate::ProjectGroupLinkCreateService do
 
     context 'unapproved access requests' do
       before do
-        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: group_user, group: group)
+        create(:group_member, :unassigned, user: group_user, group: group)
       end
 
       it 'does not create project authorization' do