From 5ad0ea815d8768c3763a2e640b56dea09bf7b3e9 Mon Sep 17 00:00:00 2001 From: Mohamed Hamda <mhamda@gitlab.com> Date: Wed, 6 Dec 2023 13:23:06 +0000 Subject: [PATCH] Change to use the helper Guard SM APIs with the code_suggestions_available? Introduce SubscriptionHelper Remove code_suggestions and use gitlab_saas_subscriptions Adjust specs --- .../user_add_on_assignments/create.rb | 9 +--- .../user_add_on_assignments/remove.rb | 3 +- .../add_on_eligible_users_resolver.rb | 2 +- .../code_suggestions_helper.rb | 8 +-- .../subscription_helper.rb | 13 +++++ ee/app/models/ee/user.rb | 3 +- ...ions.yml => gitlab_saas_subscriptions.yml} | 4 +- ee/lib/ee/gitlab/saas.rb | 2 +- .../code_suggestions_helper_spec.rb | 4 +- .../subscription_helper_spec.rb | 49 +++++++++++++++++++ ee/spec/models/ee/user_spec.rb | 4 +- .../add_on_eligible_users_spec.rb | 2 +- .../add_on_eligible_users_spec.rb | 6 +-- .../user_add_on_assignments/create_spec.rb | 29 +++++++---- .../user_add_on_assignments/remove_spec.rb | 20 ++++++-- .../saas/create_service_spec.rb | 2 +- .../self_managed/create_service_spec.rb | 2 +- 17 files changed, 119 insertions(+), 43 deletions(-) create mode 100644 ee/app/helpers/gitlab_subscriptions/subscription_helper.rb rename ee/config/saas_features/{code_suggestions.yml => gitlab_saas_subscriptions.yml} (69%) create mode 100644 ee/spec/helpers/gitlab_subscriptions/subscription_helper_spec.rb diff --git a/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/create.rb b/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/create.rb index 26bb423e41477..5dd9f666326c0 100644 --- a/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/create.rb +++ b/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/create.rb @@ -5,6 +5,7 @@ module GitlabSubscriptions module UserAddOnAssignments class Create < BaseMutation graphql_name 'UserAddOnAssignmentCreate' + include ::GitlabSubscriptions::CodeSuggestionsHelper argument :add_on_purchase_id, ::Types::GlobalIDType[::GitlabSubscriptions::AddOnPurchase], required: true, description: 'Global ID of AddOnPurchase to be assigned to.' @@ -50,9 +51,7 @@ def ready?(add_on_purchase_id:, user_id:) attr_reader :add_on_purchase, :user_to_be_assigned def feature_enabled? - Feature.enabled?(:hamilton_seat_management, add_on_purchase&.namespace) - # Once the FF for SM is merged we should use it for SM instead of hamilton_seat_management - # https://gitlab.com/gitlab-org/gitlab/-/issues/433011 + code_suggestions_available?(add_on_purchase&.namespace) end def create_user_add_on_service @@ -64,10 +63,6 @@ def create_user_add_on_service service_class.new(add_on_purchase: add_on_purchase, user: user_to_be_assigned) end - - def gitlab_saas? - ::Gitlab::Saas.feature_available?(:code_suggestions) - end end end end diff --git a/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/remove.rb b/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/remove.rb index c9c42ac50ce3d..91a2073df7337 100644 --- a/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/remove.rb +++ b/ee/app/graphql/mutations/gitlab_subscriptions/user_add_on_assignments/remove.rb @@ -5,6 +5,7 @@ module GitlabSubscriptions module UserAddOnAssignments class Remove < BaseMutation graphql_name 'UserAddOnAssignmentRemove' + include ::GitlabSubscriptions::CodeSuggestionsHelper argument :add_on_purchase_id, ::Types::GlobalIDType[::GitlabSubscriptions::AddOnPurchase], required: true, description: 'Global ID of AddOnPurchase assignment belongs to.' @@ -54,7 +55,7 @@ def ready?(add_on_purchase_id:, user_id:) attr_reader :add_on_purchase, :user_to_be_removed def feature_enabled? - Feature.enabled?(:hamilton_seat_management, add_on_purchase&.namespace) + code_suggestions_available?(add_on_purchase&.namespace) end def log_event diff --git a/ee/app/graphql/resolvers/gitlab_subscriptions/self_managed/add_on_eligible_users_resolver.rb b/ee/app/graphql/resolvers/gitlab_subscriptions/self_managed/add_on_eligible_users_resolver.rb index 7492215224216..40900647953b5 100644 --- a/ee/app/graphql/resolvers/gitlab_subscriptions/self_managed/add_on_eligible_users_resolver.rb +++ b/ee/app/graphql/resolvers/gitlab_subscriptions/self_managed/add_on_eligible_users_resolver.rb @@ -33,7 +33,7 @@ def resolve(add_on_type:, search: nil) private def authorize! - return unless Gitlab::Saas.feature_available?(:code_suggestions) || !current_user.can_admin_all_resources? + return unless gitlab_saas? || !current_user.can_admin_all_resources? raise_resource_not_available_error! end diff --git a/ee/app/helpers/gitlab_subscriptions/code_suggestions_helper.rb b/ee/app/helpers/gitlab_subscriptions/code_suggestions_helper.rb index 458ab4f2775e2..a91f00cbbe8e7 100644 --- a/ee/app/helpers/gitlab_subscriptions/code_suggestions_helper.rb +++ b/ee/app/helpers/gitlab_subscriptions/code_suggestions_helper.rb @@ -2,6 +2,8 @@ module GitlabSubscriptions module CodeSuggestionsHelper + include GitlabSubscriptions::SubscriptionHelper + def code_suggestions_available?(namespace = nil) if gitlab_saas? Feature.enabled?(:hamilton_seat_management, namespace) @@ -9,11 +11,5 @@ def code_suggestions_available?(namespace = nil) Feature.enabled?(:self_managed_code_suggestions) end end - - private - - def gitlab_saas? - ::Gitlab::Saas.feature_available?(:code_suggestions) - end end end diff --git a/ee/app/helpers/gitlab_subscriptions/subscription_helper.rb b/ee/app/helpers/gitlab_subscriptions/subscription_helper.rb new file mode 100644 index 0000000000000..c369ddef8c5dc --- /dev/null +++ b/ee/app/helpers/gitlab_subscriptions/subscription_helper.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module GitlabSubscriptions + module SubscriptionHelper + def gitlab_saas? + ::Gitlab::Saas.feature_available?(:gitlab_saas_subscriptions) + end + + def gitlab_sm? + !gitlab_saas? + end + end +end diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 39f87aee7a529..ebe12a71d1131 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -11,6 +11,7 @@ module User include ::Gitlab::Utils::StrongMemoize include AuditorUserHelper + include GitlabSubscriptions::SubscriptionHelper DEFAULT_ROADMAP_LAYOUT = 'months' DEFAULT_GROUP_VIEW = 'details' @@ -584,7 +585,7 @@ def code_suggestions_add_on_available? end def eligible_for_self_managed_code_suggestions? - return false if ::Gitlab::Saas.feature_available?(:code_suggestions) + return false if gitlab_saas? active? && !bot? && !ghost? end diff --git a/ee/config/saas_features/code_suggestions.yml b/ee/config/saas_features/gitlab_saas_subscriptions.yml similarity index 69% rename from ee/config/saas_features/code_suggestions.yml rename to ee/config/saas_features/gitlab_saas_subscriptions.yml index 771e062a45fa6..a97edc596b6e8 100644 --- a/ee/config/saas_features/code_suggestions.yml +++ b/ee/config/saas_features/gitlab_saas_subscriptions.yml @@ -1,5 +1,5 @@ --- -name: code_suggestions -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137534 +name: gitlab_saas_subscriptions +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138644 milestone: '16.7' group: group::purchase diff --git a/ee/lib/ee/gitlab/saas.rb b/ee/lib/ee/gitlab/saas.rb index 336b8a19e44d9..a369439146ad4 100644 --- a/ee/lib/ee/gitlab/saas.rb +++ b/ee/lib/ee/gitlab/saas.rb @@ -18,7 +18,7 @@ module Saas search_indexing_status subscriptions_trials group_custom_roles - code_suggestions + gitlab_saas_subscriptions ].freeze CONFIG_FILE_ROOT = 'ee/config/saas_features' diff --git a/ee/spec/helpers/gitlab_subscriptions/code_suggestions_helper_spec.rb b/ee/spec/helpers/gitlab_subscriptions/code_suggestions_helper_spec.rb index 8bf12f8dcf23b..4c37d1b139a4f 100644 --- a/ee/spec/helpers/gitlab_subscriptions/code_suggestions_helper_spec.rb +++ b/ee/spec/helpers/gitlab_subscriptions/code_suggestions_helper_spec.rb @@ -8,7 +8,7 @@ let_it_be(:namespace) { build_stubbed(:group) } before do - stub_saas_features(code_suggestions: true) + stub_saas_features(gitlab_saas_subscriptions: true) end context 'when SaaS feature flag is globally enabled' do @@ -40,7 +40,7 @@ context 'when GitLab is self-managed' do before do - stub_saas_features(code_suggestions: false) + stub_saas_features(gitlab_saas_subscriptions: false) end context 'when self-managed feature flag is enabled' do diff --git a/ee/spec/helpers/gitlab_subscriptions/subscription_helper_spec.rb b/ee/spec/helpers/gitlab_subscriptions/subscription_helper_spec.rb new file mode 100644 index 0000000000000..e4b67e53d070e --- /dev/null +++ b/ee/spec/helpers/gitlab_subscriptions/subscription_helper_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SubscriptionHelper, feature_category: :seat_cost_management do + describe '#gitlab_saas?' do + context 'when GitLab is SaaS' do + before do + stub_saas_features(gitlab_saas_subscriptions: true) + end + + it 'returns true' do + expect(helper.gitlab_saas?).to be_truthy + end + end + + context 'when GitLab is not SaaS' do + before do + stub_saas_features(gitlab_saas_subscriptions: false) + end + + it 'returns false' do + expect(helper.gitlab_saas?).to be_falsy + end + end + end + + describe '#gitlab_sm?' do + context 'when GitLab is self-managed' do + before do + stub_saas_features(gitlab_saas_subscriptions: false) + end + + it 'returns true' do + expect(helper.gitlab_sm?).to be_truthy + end + end + + context 'when GitLab is not self-managed' do + before do + stub_saas_features(gitlab_saas_subscriptions: true) + end + + it 'returns false' do + expect(helper.gitlab_sm?).to be_falsy + end + end + end +end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index 1fcedc47814cd..fad5950d909f7 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -2929,7 +2929,7 @@ context 'when on Saas/Gitlab.com' do before do - stub_saas_features(code_suggestions: true) + stub_saas_features(gitlab_saas_subscriptions: true) end it 'returns false by default' do @@ -2939,7 +2939,7 @@ context 'when on self managed' do before do - stub_saas_features(code_suggestions: false) + stub_saas_features(gitlab_saas_subscriptions: false) stub_licensed_features(code_suggestions: true) end diff --git a/ee/spec/requests/api/graphql/gitlab_subscriptions/add_on_eligible_users_spec.rb b/ee/spec/requests/api/graphql/gitlab_subscriptions/add_on_eligible_users_spec.rb index 4b81f3f9d40d8..cea122635cdf9 100644 --- a/ee/spec/requests/api/graphql/gitlab_subscriptions/add_on_eligible_users_spec.rb +++ b/ee/spec/requests/api/graphql/gitlab_subscriptions/add_on_eligible_users_spec.rb @@ -23,7 +23,7 @@ end before do - stub_saas_features(code_suggestions: true) + stub_saas_features(gitlab_saas_subscriptions: true) end context 'when the user is not eligible to admin add-on purchases on the namespace' do diff --git a/ee/spec/requests/api/graphql/gitlab_subscriptions/self_managed/add_on_eligible_users_spec.rb b/ee/spec/requests/api/graphql/gitlab_subscriptions/self_managed/add_on_eligible_users_spec.rb index e880ca4d06ead..95110716180b0 100644 --- a/ee/spec/requests/api/graphql/gitlab_subscriptions/self_managed/add_on_eligible_users_spec.rb +++ b/ee/spec/requests/api/graphql/gitlab_subscriptions/self_managed/add_on_eligible_users_spec.rb @@ -43,7 +43,7 @@ describe 'authorization' do context 'on a self-managed instance' do before do - stub_saas_features(code_suggestions: false) # !Gitlab.com? + stub_saas_features(gitlab_saas_subscriptions: false) end context 'with owner access' do @@ -64,7 +64,7 @@ let_it_be(:current_user) { create(:admin) } before do - stub_saas_features(code_suggestions: true) # Gitlab.com? + stub_saas_features(gitlab_saas_subscriptions: true) end include_examples 'not authorized' @@ -79,7 +79,7 @@ let_it_be(:active_user_2) { create(:user, name: 'GitlabX') } before do - stub_saas_features(code_suggestions: false) + stub_saas_features(gitlab_saas_subscriptions: false) end before_all do diff --git a/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/create_spec.rb b/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/create_spec.rb index 8f3f2f4ad72f4..ad5eac8f94274 100644 --- a/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/create_spec.rb +++ b/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/create_spec.rb @@ -55,14 +55,6 @@ end shared_examples 'validates the query' do - context 'when feature flag hamilton_seat_management is disabled' do - before do - stub_feature_flags(hamilton_seat_management: false) - end - - it_behaves_like 'empty response' - end - context 'when current_user is admin' do let(:current_user) { create(:admin) } @@ -179,7 +171,7 @@ context 'on Gitlab.com/Saas' do before do - stub_saas_features(code_suggestions: true) + stub_saas_features(gitlab_saas_subscriptions: true) end let_it_be(:current_user) { create(:user) } @@ -190,6 +182,15 @@ namespace.add_owner(current_user) namespace.add_developer(assignee_user) end + + context 'with feature flag disabled' do + before do + stub_feature_flags(hamilton_seat_management: false) + end + + it_behaves_like 'empty response' + end + it_behaves_like 'validates the query' it_behaves_like 'success response' @@ -287,7 +288,7 @@ context 'on self managed instances' do before do - stub_saas_features(code_suggestions: false) + stub_saas_features(gitlab_saas_subscriptions: false) end let(:current_user) { create(:admin) } @@ -296,6 +297,14 @@ it_behaves_like 'validates the query' it_behaves_like 'success response' + context 'with feature flag disabled' do + before do + stub_feature_flags(self_managed_code_suggestions: false) + end + + it_behaves_like 'empty response' + end + context 'when current_user is not an admin' do let(:current_user) { create(:user) } diff --git a/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/remove_spec.rb b/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/remove_spec.rb index 6f2db0a8fce28..95723e2955229 100644 --- a/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/remove_spec.rb +++ b/ee/spec/requests/api/graphql/gitlab_subscriptions/user_add_on_assignments/remove_spec.rb @@ -108,12 +108,24 @@ it_behaves_like 'success response' - context 'when feature flag hamilton_seat_management is disabled' do - before do - stub_feature_flags(hamilton_seat_management: false) + context 'when feature flag is disabled' do + context 'when SaaS' do + before do + stub_saas_features(gitlab_saas_subscriptions: true) + stub_feature_flags(hamilton_seat_management: false) + end + + it_behaves_like 'empty response' end - it_behaves_like 'empty response' + context 'when self-managed' do + before do + stub_saas_features(gitlab_saas_subscriptions: false) + stub_feature_flags(self_managed_code_suggestions: false) + end + + it_behaves_like 'empty response' + end end context 'when current_user is admin' do diff --git a/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/saas/create_service_spec.rb b/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/saas/create_service_spec.rb index b2ab310031aa5..b13f5964730e5 100644 --- a/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/saas/create_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/saas/create_service_spec.rb @@ -17,7 +17,7 @@ end before do - stub_saas_features(code_suggestions: true) + stub_saas_features(gitlab_saas_subscriptions: true) end describe '#execute' do diff --git a/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/self_managed/create_service_spec.rb b/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/self_managed/create_service_spec.rb index 71a401a745849..2e3144a023bbc 100644 --- a/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/self_managed/create_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/user_add_on_assignments/self_managed/create_service_spec.rb @@ -13,7 +13,7 @@ end before do - stub_saas_features(code_suggestions: false) + stub_saas_features(gitlab_saas_subscriptions: false) end describe '#execute' do -- GitLab