diff --git a/ee/app/finders/member_roles/roles_finder.rb b/ee/app/finders/member_roles/roles_finder.rb index f261416308308fa603cb163911c4ebbe7a837ef4..013f7e9791fdde20ded1cbffe46fb6d56710cbef 100644 --- a/ee/app/finders/member_roles/roles_finder.rb +++ b/ee/app/finders/member_roles/roles_finder.rb @@ -4,6 +4,8 @@ module MemberRoles class RolesFinder + include ::GitlabSubscriptions::SubscriptionHelper + attr_reader :current_user, :params VALID_PARAMS = [:parent, :id, :instance_roles].freeze @@ -80,7 +82,7 @@ def root_ancestor end def can_read_instance_roles? - return false if saas? + return false if gitlab_com_subscription? Ability.allowed?(current_user, :admin_member_role) end @@ -94,9 +96,5 @@ def allowed_read_member_role?(group) can_read_instance_roles? end - - def saas? - Gitlab::Saas.feature_available?(:group_custom_roles) - end end end diff --git a/ee/app/graphql/mutations/member_roles/create.rb b/ee/app/graphql/mutations/member_roles/create.rb index 49924de1819d1c55cb8b1671e576fb149464284d..8b04c299378be4690b2c23c4aee5a31dc74f4ca5 100644 --- a/ee/app/graphql/mutations/member_roles/create.rb +++ b/ee/app/graphql/mutations/member_roles/create.rb @@ -5,6 +5,7 @@ module MemberRoles class Create < Base graphql_name 'MemberRoleCreate' + include ::GitlabSubscriptions::SubscriptionHelper include Mutations::ResolvesNamespace argument :base_access_level, @@ -49,22 +50,18 @@ def authorize_admin_roles!(group) end def authorize_group_member_roles!(group) - raise_resource_not_available_error! if restrict_member_roles? && !saas? + raise_resource_not_available_error! if restrict_member_roles? && !gitlab_com_subscription? raise_resource_not_available_error! unless Ability.allowed?(current_user, :admin_member_role, group) raise_resource_not_available_error! unless group.custom_roles_enabled? end def authorize_instance_member_roles! raise_resource_not_available_error! unless Ability.allowed?(current_user, :admin_member_role) - raise_resource_not_available_error! if saas? - end - - def saas? - Gitlab::Saas.feature_available?(:group_custom_roles) + raise_resource_not_available_error! if gitlab_com_subscription? end def missing_group_path?(args) - return false unless saas? + return false unless gitlab_com_subscription? args[:group_path].blank? end diff --git a/ee/app/helpers/member_roles_helper.rb b/ee/app/helpers/member_roles_helper.rb index 65b00ea886a6147bc42615b0f2e197557b41314c..5e74616208955e7fc4f502e53e5758b51b96a9db 100644 --- a/ee/app/helpers/member_roles_helper.rb +++ b/ee/app/helpers/member_roles_helper.rb @@ -5,7 +5,7 @@ def manage_member_roles_path(source) root_group = source&.root_ancestor return unless root_group&.custom_roles_enabled? - if ::Gitlab::Saas.feature_available?(:group_custom_roles) && can?(current_user, :admin_group_member, root_group) + if gitlab_com_subscription? && can?(current_user, :admin_group_member, root_group) group_settings_roles_and_permissions_path(root_group) elsif current_user&.can_admin_all_resources? admin_application_settings_roles_and_permissions_path diff --git a/ee/app/models/members/member_role.rb b/ee/app/models/members/member_role.rb index aa058a44b02846792e0364d15e8be2456697cba9..6a56077431ff0be7395b6c606100e781549d99ec 100644 --- a/ee/app/models/members/member_role.rb +++ b/ee/app/models/members/member_role.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class MemberRole < ApplicationRecord # rubocop:disable Gitlab/NamespacedClass + include GitlabSubscriptions::SubscriptionHelper + MAX_COUNT_PER_GROUP_HIERARCHY = 10 NON_PERMISSION_COLUMNS = [ @@ -19,7 +21,7 @@ class MemberRole < ApplicationRecord # rubocop:disable Gitlab/NamespacedClass has_many :saml_group_links belongs_to :namespace - validates :namespace, presence: true, if: :group_role_required? + validates :namespace, presence: true, if: :gitlab_com_subscription? validates :name, presence: true validates :base_access_level, presence: true, inclusion: { in: LEVELS } validate :belongs_to_top_level_namespace @@ -154,8 +156,4 @@ def prevent_delete_after_member_associated throw :abort # rubocop:disable Cop/BanCatchThrow end - - def group_role_required? - Gitlab::Saas.feature_available?(:group_custom_roles) - end end diff --git a/ee/app/services/member_roles/create_service.rb b/ee/app/services/member_roles/create_service.rb index 9ff93603bf4afc5f4133487f0f7d9580847e0ec1..ea6c3207ff5ea66fe1a9367f91d3b6cb5bf2a7dc 100644 --- a/ee/app/services/member_roles/create_service.rb +++ b/ee/app/services/member_roles/create_service.rb @@ -2,6 +2,8 @@ module MemberRoles class CreateService < BaseService + include ::GitlabSubscriptions::SubscriptionHelper + def execute return authorized_error unless allowed? return root_group_error if subgroup? @@ -22,7 +24,7 @@ def allowed_for_group? end def allowed_for_instance? - return false if saas? + return false if gitlab_com_subscription? can?(current_user, :admin_member_role) end @@ -33,15 +35,11 @@ def root_group_error end def subgroup? - return false unless saas? + return false unless gitlab_com_subscription? !group.root? end - def saas? - Gitlab::Saas.feature_available?(:group_custom_roles) - end - def create_member_role member_role = MemberRole.new(params.merge(namespace: group)) diff --git a/ee/config/saas_features/group_custom_roles.yml b/ee/config/saas_features/group_custom_roles.yml deleted file mode 100644 index 3df9f7ab2fe3fe7ed7c33019bd70383622ad8967..0000000000000000000000000000000000000000 --- a/ee/config/saas_features/group_custom_roles.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -name: group_custom_roles -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135633 -milestone: '16.6' -group: group::authorization diff --git a/ee/lib/ee/gitlab/saas.rb b/ee/lib/ee/gitlab/saas.rb index 9b2de2b217f02fc937cec64880b1d667a06a7afa..5debfdeda7f2ab5aff3b0f89f97bcf45f35a5e5e 100644 --- a/ee/lib/ee/gitlab/saas.rb +++ b/ee/lib/ee/gitlab/saas.rb @@ -17,7 +17,6 @@ module Saas purchases_additional_minutes search_indexing_status subscriptions_trials - group_custom_roles gitlab_com_subscriptions duo_chat_categorize_question google_artifact_registry diff --git a/ee/spec/features/admin/member_roles_spec.rb b/ee/spec/features/admin/member_roles_spec.rb index 22527be7765b3b621161635ccaef5c5213445ebd..5948c20b94622822380a2c01371b402735c8d1ed 100644 --- a/ee/spec/features/admin/member_roles_spec.rb +++ b/ee/spec/features/admin/member_roles_spec.rb @@ -54,7 +54,7 @@ def created_role(name, id, access_level, permissions) context 'when on self-managed' do before do - stub_saas_features(group_custom_roles: false) + stub_saas_features(gitlab_com_subscriptions: false) visit admin_application_settings_roles_and_permissions_path end @@ -62,8 +62,10 @@ def created_role(name, id, access_level, permissions) it_behaves_like 'creates a new custom role' end - context 'when on SaaS', :saas do + context 'when on SaaS' do before do + stub_saas_features(gitlab_com_subscriptions: true) + visit admin_application_settings_roles_and_permissions_path end diff --git a/ee/spec/features/groups/member_roles_spec.rb b/ee/spec/features/groups/member_roles_spec.rb index a499ddbb16d983c579c9c3ba6a61776cfb335363..ec3a63c67dc6b6dbdb1c2604b9b6ecbbc89d10ba 100644 --- a/ee/spec/features/groups/member_roles_spec.rb +++ b/ee/spec/features/groups/member_roles_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Member Roles', :saas, :js, feature_category: :permissions do +RSpec.describe 'Member Roles', :js, feature_category: :permissions do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } @@ -59,6 +59,8 @@ def created_role(name, id, access_level, permissions) context 'when on SaaS' do before do + stub_saas_features(gitlab_com_subscriptions: true) + visit group_settings_roles_and_permissions_path(group) end @@ -67,7 +69,7 @@ def created_role(name, id, access_level, permissions) context 'when on self-managed' do before do - stub_saas_features(group_custom_roles: false) + stub_saas_features(gitlab_com_subscriptions: false) end context 'when restrict_member_roles feature-flag is disabled' do diff --git a/ee/spec/finders/member_roles/roles_finder_spec.rb b/ee/spec/finders/member_roles/roles_finder_spec.rb index b42c39fd49f89fda23df80ab2e940308cb10fd62..bbe745ec318eda1900de6fee34b6ed2a26301687 100644 --- a/ee/spec/finders/member_roles/roles_finder_spec.rb +++ b/ee/spec/finders/member_roles/roles_finder_spec.rb @@ -153,7 +153,11 @@ end end - context 'when on SaaS', :saas do + context 'when on SaaS' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + it 'returns an error' do expect { find_member_roles }.to raise_error(ArgumentError) end @@ -174,12 +178,20 @@ let(:current_user) { admin } context 'when on self-managed' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + it 'returns instance member roles' do expect(find_member_roles).to eq([member_role_instance]) end end - context 'when on SaaS', :saas do + context 'when on SaaS' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + it 'returns an empty array' do expect(find_member_roles).to be_empty end @@ -194,6 +206,10 @@ let(:current_user) { admin } context 'when on self-managed' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + it 'returns both instance member roles and group member roles' do expect(find_member_roles).to eq([member_role_instance, member_role_2, member_role_1]) end diff --git a/ee/spec/helpers/member_roles_helper_spec.rb b/ee/spec/helpers/member_roles_helper_spec.rb index 3da1b9cafe7ab4498e223682aca855a3c6f5c963..fa5523f1affcc705b3ff83a671964ed220283bee 100644 --- a/ee/spec/helpers/member_roles_helper_spec.rb +++ b/ee/spec/helpers/member_roles_helper_spec.rb @@ -15,7 +15,11 @@ describe '#manage_member_roles_path' do subject { helper.manage_member_roles_path(source) } - context 'when on saas', :saas do + context 'when on SaaS' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + it { is_expected.to be_nil } context 'as owner' do @@ -36,6 +40,10 @@ end context 'when in admin mode', :enable_admin_mode do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + it { is_expected.to be_nil } context 'as admin' do diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index 3a5e18cd4849efc19742c76b7348351bcffed118..42f5f95704214e83835cc0e1d67cf11e2885ecce 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -17,11 +17,19 @@ it { is_expected.to validate_presence_of(:base_access_level) } it { is_expected.to validate_inclusion_of(:base_access_level).in_array(described_class::LEVELS) } - context 'when running on Gitlab.com', :saas do + context 'when running on Gitlab.com' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + it { is_expected.to validate_presence_of(:namespace) } end context 'when running on self-managed' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + it { is_expected.not_to validate_presence_of(:namespace) } end diff --git a/ee/spec/presenters/member_presenter_spec.rb b/ee/spec/presenters/member_presenter_spec.rb index d5fc654c193dde7b199a30eaaa086f1d330595c0..92f31d0f9bb4747208159724378d11a06377b824 100644 --- a/ee/spec/presenters/member_presenter_spec.rb +++ b/ee/spec/presenters/member_presenter_spec.rb @@ -58,7 +58,6 @@ before do stub_licensed_features(custom_roles: true) - stub_saas_features(group_custom_roles: false) end context 'when the user has permissions to manage group roles' do diff --git a/ee/spec/requests/api/graphql/mutations/member_role/create_member_role_spec.rb b/ee/spec/requests/api/graphql/mutations/member_role/create_member_role_spec.rb index 443f2fdc958139b2ad31f4fb36b0238b3d3d55b5..aecd8ee4c798f120844517d758453af567e3a9a0 100644 --- a/ee/spec/requests/api/graphql/mutations/member_role/create_member_role_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/member_role/create_member_role_spec.rb @@ -113,7 +113,11 @@ end end - context 'when on SaaS', :saas do + context 'when on SaaS' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + context 'with valid arguments' do it_behaves_like 'a mutation that creates a member role' end @@ -144,6 +148,10 @@ end context 'when creating an instance level member role' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + let(:input) do { base_access_level: 'GUEST', @@ -182,8 +190,9 @@ end end - context 'when on SaaS', :saas do + context 'when on SaaS' do before do + stub_saas_features(gitlab_com_subscriptions: true) stub_feature_flags(restrict_member_roles: false) end