From 6acd45570b2e32ad6cab1a11419ffa58585f0e24 Mon Sep 17 00:00:00 2001 From: Angelo Gulina Date: Tue, 22 Feb 2022 10:06:44 +0100 Subject: [PATCH 1/3] Add seat count usage alerts to group pages When reaching the threshold limits for seat count, the user will be notified. The notification will appear based on seat count values and the subscripton's seat reconciliation status. Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79563/ EE: true --- .../javascripts/persistent_user_callouts.js | 2 +- app/views/layouts/_page.html.haml | 2 +- .../gitlab_subscriptions/seat_count_alert.rb | 14 ++ ee/app/controllers/ee/groups_controller.rb | 5 + .../ee/projects/issues_controller.rb | 5 + ee/app/controllers/ee/projects_controller.rb | 5 + .../controllers/groups/billings_controller.rb | 6 + .../groups/usage_quotas_controller.rb | 5 + ee/app/helpers/seat_count_alert_helper.rb | 19 ++ ee/app/helpers/seats_count_alert_helper.rb | 72 ------- .../header/_seat_count_alert.html.haml | 15 ++ .../header/_seats_count_alert.html.haml | 13 -- .../seat_count_alert_spec.rb | 82 ++++++++ .../controllers/ee/groups_controller_spec.rb | 8 + .../groups/billings_controller_spec.rb | 6 + .../groups/usage_quotas_controller_spec.rb | 8 + .../controllers/projects_controller_spec.rb | 2 + .../seat_count_alert_spec.rb | 117 +++++++++++ .../seats_count_alert_spec.rb | 56 ------ .../helpers/seat_count_alert_helper_spec.rb | 66 ++++++ .../helpers/seats_count_alert_helper_spec.rb | 188 ------------------ .../projects/issues_controller_spec.rb | 6 + .../seat_count_alert_shared_examples.rb | 29 +++ locale/gitlab.pot | 9 +- 24 files changed, 407 insertions(+), 333 deletions(-) create mode 100644 ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb create mode 100644 ee/app/helpers/seat_count_alert_helper.rb delete mode 100644 ee/app/helpers/seats_count_alert_helper.rb create mode 100644 ee/app/views/layouts/header/_seat_count_alert.html.haml delete mode 100644 ee/app/views/layouts/header/_seats_count_alert.html.haml create mode 100644 ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb create mode 100644 ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb delete mode 100644 ee/spec/features/gitlab_subscriptions/seats_count_alert_spec.rb create mode 100644 ee/spec/helpers/seat_count_alert_helper_spec.rb delete mode 100644 ee/spec/helpers/seats_count_alert_helper_spec.rb create mode 100644 ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb diff --git a/app/assets/javascripts/persistent_user_callouts.js b/app/assets/javascripts/persistent_user_callouts.js index 100ffc0664b473..31e80107fd8c0f 100644 --- a/app/assets/javascripts/persistent_user_callouts.js +++ b/app/assets/javascripts/persistent_user_callouts.js @@ -10,7 +10,7 @@ const PERSISTENT_USER_CALLOUTS = [ '.js-new-user-signups-cap-reached', '.js-eoa-bronze-plan-banner', '.js-security-newsletter-callout', - '.js-approaching-seats-count-threshold', + '.js-approaching-seat-count-threshold', '.js-storage-enforcement-banner', '.js-user-over-limit-free-plan-alert', '.js-minute-limit-banner', diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 185ed228d92e60..b7cf7b7468fe49 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -16,7 +16,7 @@ = yield :flash_message = dispensable_render "shared/service_ping_consent" = dispensable_render_if_exists "layouts/header/ee_subscribable_banner" - = dispensable_render_if_exists "layouts/header/seats_count_alert" + = dispensable_render_if_exists "layouts/header/seat_count_alert" = dispensable_render_if_exists "shared/namespace_storage_limit_alert" = dispensable_render_if_exists "shared/namespace_user_cap_reached_alert" = dispensable_render_if_exists "shared/new_user_signups_cap_reached_alert" diff --git a/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb b/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb new file mode 100644 index 00000000000000..ea452b3d362ae2 --- /dev/null +++ b/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module GitlabSubscriptions + module SeatCountAlert + def generate_seat_count_alert_data(namespace) + return unless current_user && (root_ancestor = namespace&.root_ancestor) + + GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService.new( + namespace: root_ancestor, + user: current_user + ).execute + end + end +end diff --git a/ee/app/controllers/ee/groups_controller.rb b/ee/app/controllers/ee/groups_controller.rb index 2fe797b1b913fe..249e5772ef6f7b 100644 --- a/ee/app/controllers/ee/groups_controller.rb +++ b/ee/app/controllers/ee/groups_controller.rb @@ -9,6 +9,7 @@ module GroupsController prepended do include GeoInstrumentation + include GitlabSubscriptions::SeatCountAlert alias_method :ee_authorize_admin_group!, :authorize_admin_group! @@ -20,6 +21,10 @@ module GroupsController push_force_frontend_feature_flag(:iteration_cadences, @group&.iteration_cadences_feature_flag_enabled?) end + before_action only: :show do + @seat_count_data = generate_seat_count_alert_data(@group) + end + feature_category :subgroups, [:restore] end diff --git a/ee/app/controllers/ee/projects/issues_controller.rb b/ee/app/controllers/ee/projects/issues_controller.rb index dba106abe5d13c..afed9e89409ed9 100644 --- a/ee/app/controllers/ee/projects/issues_controller.rb +++ b/ee/app/controllers/ee/projects/issues_controller.rb @@ -9,6 +9,7 @@ module IssuesController prepended do include DescriptionDiffActions include GeoInstrumentation + include GitlabSubscriptions::SeatCountAlert before_action :disable_query_limiting_ee, only: [:update] before_action only: [:new, :create] do @@ -28,6 +29,10 @@ module IssuesController ) end + before_action only: %i[show index] do + @seat_count_data = generate_seat_count_alert_data(@project) + end + feature_category :team_planning, [:delete_description_version, :description_diff] urgency :low, [:delete_description_version, :description_diff] end diff --git a/ee/app/controllers/ee/projects_controller.rb b/ee/app/controllers/ee/projects_controller.rb index 927663fb20301b..082a2e6c3513dd 100644 --- a/ee/app/controllers/ee/projects_controller.rb +++ b/ee/app/controllers/ee/projects_controller.rb @@ -7,6 +7,7 @@ module ProjectsController prepended do include GeoInstrumentation + include GitlabSubscriptions::SeatCountAlert before_action :log_download_export_audit_event, only: [:download_export] before_action :log_archive_audit_event, only: [:archive] @@ -16,6 +17,10 @@ module ProjectsController push_frontend_feature_flag(:permit_all_shared_groups_for_approval, project) end + before_action only: :show do + @seat_count_data = generate_seat_count_alert_data(@project) + end + feature_category :projects, [:restore] end diff --git a/ee/app/controllers/groups/billings_controller.rb b/ee/app/controllers/groups/billings_controller.rb index 013871be9911c5..ee89f10b76f48b 100644 --- a/ee/app/controllers/groups/billings_controller.rb +++ b/ee/app/controllers/groups/billings_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Groups::BillingsController < Groups::ApplicationController + include GitlabSubscriptions::SeatCountAlert + before_action :authorize_admin_group! before_action :verify_namespace_plan_check_enabled @@ -8,6 +10,10 @@ class Groups::BillingsController < Groups::ApplicationController push_frontend_feature_flag(:refresh_billings_seats, type: :ops) end + before_action only: :index do + @seat_count_data = generate_seat_count_alert_data(@group) + end + layout 'group_settings' feature_category :purchase diff --git a/ee/app/controllers/groups/usage_quotas_controller.rb b/ee/app/controllers/groups/usage_quotas_controller.rb index 4c05f1b1d1805e..292585fcb6eff9 100644 --- a/ee/app/controllers/groups/usage_quotas_controller.rb +++ b/ee/app/controllers/groups/usage_quotas_controller.rb @@ -2,6 +2,7 @@ class Groups::UsageQuotasController < Groups::ApplicationController include OneTrustCSP + include GitlabSubscriptions::SeatCountAlert before_action :authorize_admin_group! before_action :verify_usage_quotas_enabled! @@ -9,6 +10,10 @@ class Groups::UsageQuotasController < Groups::ApplicationController before_action :push_update_storage_usage_design, only: :index before_action :push_usage_quotas_pipelines_vue, only: :index + before_action only: [:index] do + @seat_count_data = generate_seat_count_alert_data(@group) + end + layout 'group_settings' feature_category :purchase diff --git a/ee/app/helpers/seat_count_alert_helper.rb b/ee/app/helpers/seat_count_alert_helper.rb new file mode 100644 index 00000000000000..3f92bf4de1cc1e --- /dev/null +++ b/ee/app/helpers/seat_count_alert_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module SeatCountAlertHelper + def show_seat_count_alert? + @seat_count_data.present? && @seat_count_data[:namespace].present? + end + + def remaining_seat_count + @seat_count_data[:remaining_seat_count] + end + + def total_seat_count + @seat_count_data[:total_seat_count] + end + + def namespace + @seat_count_data[:namespace] + end +end diff --git a/ee/app/helpers/seats_count_alert_helper.rb b/ee/app/helpers/seats_count_alert_helper.rb deleted file mode 100644 index 8b58f163de7431..00000000000000 --- a/ee/app/helpers/seats_count_alert_helper.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -module SeatsCountAlertHelper - def display_seats_count_alert! - @display_seats_count_alert = true - end - - def learn_more_link - link_to _('Learn more.'), help_page_path('subscriptions/quarterly_reconciliation'), target: '_blank', rel: 'noopener noreferrer' - end - - def group_name - root_namespace&.name - end - - def remaining_seats_count - return unless total_seats_count && seats_in_use - - total_seats_count - seats_in_use - end - - def seats_usage_link - return unless root_namespace - - link_to _('View seat usage'), current_usage_quotas_path, class: 'btn gl-alert-action btn-info btn-md gl-button' - end - - def show_seats_count_alert? - return false unless ::Gitlab.com? && group_with_owner? && current_subscription - return false if user_dismissed_alert? - - !!@display_seats_count_alert - end - - def total_seats_count - current_subscription&.seats - end - - private - - def user_dismissed_alert? - current_user.dismissed_callout_for_group?( - feature_name: Users::GroupCalloutsHelper::APPROACHING_SEAT_COUNT_THRESHOLD, - group: root_namespace, - ignore_dismissal_earlier_than: last_member_added_at - ) - end - - def last_member_added_at - root_namespace&.last_billed_user_created_at - end - - def group_with_owner? - root_namespace&.group_namespace? && root_namespace&.has_owner?(current_user) - end - - def root_namespace - @project&.root_ancestor || @group&.root_ancestor - end - - def current_subscription - root_namespace&.gitlab_subscription - end - - def seats_in_use - current_subscription&.seats_in_use - end - - def current_usage_quotas_path - usage_quotas_path(root_namespace, anchor: 'seats-quota-tab') - end -end diff --git a/ee/app/views/layouts/header/_seat_count_alert.html.haml b/ee/app/views/layouts/header/_seat_count_alert.html.haml new file mode 100644 index 00000000000000..721c49ef0296d4 --- /dev/null +++ b/ee/app/views/layouts/header/_seat_count_alert.html.haml @@ -0,0 +1,15 @@ +- return unless show_seat_count_alert? + +.container.container-limited.pt-3 + = render Pajamas::AlertComponent.new(alert_class: 'js-approaching-seat-count-threshold', + alert_data: { dismiss_endpoint: group_callouts_path, + feature_id: Users::GroupCalloutsHelper::APPROACHING_SEAT_COUNT_THRESHOLD, + group_id: namespace.id }, + title: _('%{group_name} is approaching the limit of available seats') % { group_name: namespace.name }, + close_button_data: { testid: 'approaching-seat-count-threshold-alert-dismiss' }) do |c| + = c.body do + = n_('Your subscription has %{remaining_seat_count} out of %{total_seat_count} seat remaining.', 'Your subscription has %{remaining_seat_count} out of %{total_seat_count} seats remaining.', total_seat_count) % { remaining_seat_count: remaining_seat_count, total_seat_count: total_seat_count } + = _('Even if you reach the number of seats in your subscription, you can continue to add users, and GitLab will bill you for the overage.') + = link_to _('Learn more.'), help_page_path('subscriptions/quarterly_reconciliation'), target: '_blank', rel: 'noopener noreferrer' + = c.actions do + = link_to _('View seat usage'), usage_quotas_path(namespace, anchor: 'seats-quota-tab'), class: 'btn gl-alert-action btn-info btn-md gl-button' diff --git a/ee/app/views/layouts/header/_seats_count_alert.html.haml b/ee/app/views/layouts/header/_seats_count_alert.html.haml deleted file mode 100644 index db4cb0fbb33500..00000000000000 --- a/ee/app/views/layouts/header/_seats_count_alert.html.haml +++ /dev/null @@ -1,13 +0,0 @@ -- return unless show_seats_count_alert? -.container.container-limited.pt-3 - = render Pajamas::AlertComponent.new(alert_class: 'js-approaching-seats-count-threshold', - alert_data: { dismiss_endpoint: group_callouts_path, - feature_id: Users::GroupCalloutsHelper::APPROACHING_SEAT_COUNT_THRESHOLD, - group_id: root_namespace.id }, - title: _('%{group_name} is approaching the limit of available seats') % { group_name: group_name }, - close_button_data: { testid: 'approaching-seats-count-threshold-alert-dismiss' }) do |c| - = c.body do - = _('Your subscription has %{remaining_seats_count} out of %{total_seats_count} seats remaining. Even if you reach the number of seats in your subscription, you can continue to add users, and GitLab will bill you for the overage.') % { remaining_seats_count: remaining_seats_count, total_seats_count: total_seats_count } - = learn_more_link - = c.actions do - = seats_usage_link diff --git a/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb b/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb new file mode 100644 index 00000000000000..528098a0b5b878 --- /dev/null +++ b/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatCountAlert do + controller(ActionController::Base) do + include GitlabSubscriptions::SeatCountAlert + end + + let_it_be(:user) { create(:user) } + let_it_be(:root_ancestor) { create(:group) } + + describe '#generate_seat_count_alert_data' do + let(:response_data) do + { namespace: root_ancestor, remaining_seat_count: 5, seats_in_use: 5, total_seat_count: 10 } + end + + context 'when the user is not authenticated' do + it 'does not set the seat count data' do + expect(controller.generate_seat_count_alert_data(root_ancestor)).to be_nil + end + end + + context 'when the user is authenticated' do + before do + sign_in(user) + end + + context 'when the namespace is nil' do + it 'does not set the seat count data' do + expect(controller.generate_seat_count_alert_data(nil)).to be_nil + end + end + + context 'when supplied a project' do + it 'sets the data based on the root ancestor' do + project = build(:project, namespace: root_ancestor) + + expect_next_instance_of( + GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService, + namespace: root_ancestor, + user: user + ) do |service| + expect(service).to receive(:execute).and_return(response_data) + end + + expect(controller.generate_seat_count_alert_data(project)).to eq response_data + end + end + + context 'when supplied a top level group' do + it 'sets the data based on that group' do + expect_next_instance_of( + GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService, + namespace: root_ancestor, + user: user + ) do |service| + expect(service).to receive(:execute).and_return(response_data) + end + + expect(controller.generate_seat_count_alert_data(root_ancestor)).to eq response_data + end + end + + context 'when supplied a subgroup' do + it 'sets the data based on the root ancestor' do + subgroup = build(:group, parent: root_ancestor) + + expect_next_instance_of( + GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService, + namespace: root_ancestor, + user: user + ) do |service| + expect(service).to receive(:execute).and_return(response_data) + end + + expect(controller.generate_seat_count_alert_data(subgroup)).to eq response_data + end + end + end + end +end diff --git a/ee/spec/controllers/ee/groups_controller_spec.rb b/ee/spec/controllers/ee/groups_controller_spec.rb index 6509ae8e7316dc..ee631cc9af5d8f 100644 --- a/ee/spec/controllers/ee/groups_controller_spec.rb +++ b/ee/spec/controllers/ee/groups_controller_spec.rb @@ -17,7 +17,15 @@ subject { get :show, params: { id: group.to_param } } + before do + namespace.add_owner(user) + + sign_in(user) + end + it_behaves_like 'namespace storage limit alert' + + it_behaves_like 'seat count alert' end describe 'GET #activity' do diff --git a/ee/spec/controllers/groups/billings_controller_spec.rb b/ee/spec/controllers/groups/billings_controller_spec.rb index 70b46ea626268c..71bbddcfa5d581 100644 --- a/ee/spec/controllers/groups/billings_controller_spec.rb +++ b/ee/spec/controllers/groups/billings_controller_spec.rb @@ -50,6 +50,12 @@ def get_index expect(assigns(:plans_data)).to eq(data) end + it_behaves_like 'seat count alert' do + subject { get_index } + + let(:namespace) { group } + end + context 'when CustomersDot is unavailable' do before do allow_next_instance_of(GitlabSubscriptions::FetchSubscriptionPlansService) do |instance| diff --git a/ee/spec/controllers/groups/usage_quotas_controller_spec.rb b/ee/spec/controllers/groups/usage_quotas_controller_spec.rb index f0a4fba0f8ffa5..afb3e64e46773e 100644 --- a/ee/spec/controllers/groups/usage_quotas_controller_spec.rb +++ b/ee/spec/controllers/groups/usage_quotas_controller_spec.rb @@ -31,6 +31,14 @@ end end + describe 'GET #index' do + it_behaves_like 'seat count alert' do + subject { get :index, params: { group_id: group } } + + let(:namespace) { group } + end + end + describe 'GET #pending_members' do let(:feature_available) { true } diff --git a/ee/spec/controllers/projects_controller_spec.rb b/ee/spec/controllers/projects_controller_spec.rb index 225cee28534e69..0a685533523ed5 100644 --- a/ee/spec/controllers/projects_controller_spec.rb +++ b/ee/spec/controllers/projects_controller_spec.rb @@ -104,6 +104,8 @@ end it_behaves_like 'namespace storage limit alert' + + it_behaves_like 'seat count alert' end end diff --git a/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb b/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb new file mode 100644 index 00000000000000..06477a84290c48 --- /dev/null +++ b/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'approaching seat count threshold alert', :saas, :js do + include SubscriptionPortalHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } + + let_it_be(:gitlab_subscription) do + create( + :gitlab_subscription, + namespace: group, + plan_code: Plan::ULTIMATE, + seats: 20, + max_seats_used: 18, + max_seats_used_changed_at: 1.day.ago + ) + end + + before do + stub_ee_application_setting(should_check_namespace_plan: true) + + stub_subscription_request_seat_usage(true) + end + + shared_examples 'a hidden alert' do + it 'does not appear on the group page' do + visit group_path(group) + + expect_alert_to_be_hidden + end + + it 'does not appear on the project page' do + visit project_path(project) + + expect_alert_to_be_hidden + end + end + + context 'when the user is not authenticated' do + before do + group.add_owner(user) + end + + it_behaves_like 'a hidden alert' + end + + context 'user is not eligible for the alert' do + before do + group.add_developer(user) + + sign_in(user) + end + + it_behaves_like 'a hidden alert' + end + + context 'when the user is eligible for the alert' do + before do + group.add_owner(user) + + sign_in(user) + end + + it 'shows the dismissible alert on the group page' do + visit group_path(group) + + expect(page).to have_content("#{group.name} is approaching the limit of available seats") + expect(page) + .to have_content( + "Your subscription has #{gitlab_subscription.seats - gitlab_subscription.max_seats_used} out of" \ + " #{gitlab_subscription.seats} seats remaining." + ) + expect(page).to have_link('View seat usage', href: usage_quotas_path(group, anchor: 'seats-quota-tab')) + + find('[data-testid="approaching-seat-count-threshold-alert-dismiss"]').click + + expect_alert_to_be_hidden + + wait_for_requests + # reload the page to ensure it stays dismissed + visit group_path(group) + + expect_alert_to_be_hidden + end + + it 'shows the dismissible alert on the project page' do + visit project_path(project) + + expect(page).to have_content("#{group.name} is approaching the limit of available seats") + expect(page) + .to have_content( + "Your subscription has #{gitlab_subscription.seats - gitlab_subscription.max_seats_used} out of" \ + " #{gitlab_subscription.seats} seats remaining." + ) + expect(page).to have_link('View seat usage', href: usage_quotas_path(group, anchor: 'seats-quota-tab')) + + find('[data-testid="approaching-seat-count-threshold-alert-dismiss"]').click + + expect_alert_to_be_hidden + + wait_for_requests + # reload the page to ensure it stays dismissed + visit project_path(project) + + expect_alert_to_be_hidden + end + end + + def expect_alert_to_be_hidden + expect(page).not_to have_content("#{group.name} is approaching the limit of available seats") + expect(page).not_to have_link('View seat usage', href: usage_quotas_path(group, anchor: 'seats-quota-tab')) + end +end diff --git a/ee/spec/features/gitlab_subscriptions/seats_count_alert_spec.rb b/ee/spec/features/gitlab_subscriptions/seats_count_alert_spec.rb deleted file mode 100644 index d10a89a43d35f3..00000000000000 --- a/ee/spec/features/gitlab_subscriptions/seats_count_alert_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Display approaching seats count threshold alert', :saas, :js do - let_it_be(:user) { create(:user) } - - shared_examples_for 'a hidden alert' do - it 'does not show the alert' do - visit visit_path - - expect(page).not_to have_content("#{group.name} is approaching the limit of available seats") - expect(page).not_to have_link('View seat usage', href: usage_quotas_path(group, anchor: 'seats-quota-tab')) - end - end - - shared_examples_for 'a visible alert' do - it 'shows the alert' do - visit visit_path - - expect(page).to have_content("#{group.name} is approaching the limit of available seats") - expect(page).to have_content("Your subscription has #{gitlab_subscription.seats - gitlab_subscription.seats_in_use} out of #{gitlab_subscription.seats} seats remaining. Even if you reach the number of seats in your subscription, you can continue to add users, and GitLab will bill you for the overage.") - expect(page).to have_link('View seat usage', href: usage_quotas_path(group, anchor: 'seats-quota-tab')) - end - end - - shared_examples_for 'a dismissed alert' do - context 'when alert was dismissed' do - before do - visit visit_path - - find('body.page-initialised [data-testid="approaching-seats-count-threshold-alert-dismiss"]').click - end - - it_behaves_like 'a hidden alert' - end - end - - context 'when conditions not met' do - let_it_be(:group) { create(:group) } - let_it_be(:visit_path) { group_path(group) } - - context 'when logged out' do - it_behaves_like 'a hidden alert' - end - - context 'when logged in owner' do - before do - group.add_owner(user) - sign_in(user) - end - - it_behaves_like 'a hidden alert' - end - end -end diff --git a/ee/spec/helpers/seat_count_alert_helper_spec.rb b/ee/spec/helpers/seat_count_alert_helper_spec.rb new file mode 100644 index 00000000000000..15991eafc255ce --- /dev/null +++ b/ee/spec/helpers/seat_count_alert_helper_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SeatCountAlertHelper, :saas do + let(:user) { create(:user) } + + let(:seat_count_data) do + { + namespace: namespace, + remaining_seat_count: 15 - 14, + seats_in_use: 14, + total_seat_count: 15 + } + end + + before do + assign(:seat_count_data, seat_count_data) + + allow(helper).to receive(:current_user).and_return(user) + end + + describe '#remaining_seat_count' do + let(:namespace) { create(:group) } + + it 'sets remaining seats count to the correct number' do + expect(helper.remaining_seat_count).to eq(1) + end + end + + describe '#show_seat_count_alert?' do + context 'with no seat count data' do + let(:seat_count_data) { nil } + + it 'does not show the alert' do + expect(helper.show_seat_count_alert?).to be false + end + end + + context 'with seat count data' do + let(:namespace) { create(:group) } + + it 'does show the alert' do + expect(helper.show_seat_count_alert?).to be true + end + end + end + + describe '#total_seat_count' do + context 'when the namespace is nil' do + let(:seat_count_data) { { namespace: nil } } + + it 'returns nil' do + expect(helper.total_seat_count).to be_nil + end + end + + context 'when the namespace is present' do + let(:namespace) { create(:group) } + + it 'sets total seats count to the correct number' do + expect(helper.total_seat_count).to eq(15) + end + end + end +end diff --git a/ee/spec/helpers/seats_count_alert_helper_spec.rb b/ee/spec/helpers/seats_count_alert_helper_spec.rb deleted file mode 100644 index 009918a254182f..00000000000000 --- a/ee/spec/helpers/seats_count_alert_helper_spec.rb +++ /dev/null @@ -1,188 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe SeatsCountAlertHelper, :saas do - include Devise::Test::ControllerHelpers - - let_it_be(:group) { nil } - let_it_be(:project) { nil } - let_it_be(:user) { create(:user) } - - before do - assign(:project, project) - assign(:group, group) - allow(helper).to receive(:current_user).and_return(user) - end - - shared_examples 'learn more link is built' do - it 'builds the correct link' do - expect(helper.learn_more_link).to match %r{.+}m - end - end - - shared_examples 'seats info are not populated' do - it 'sets remaining seats count to nil' do - expect(helper.remaining_seats_count).to be_nil - end - - it 'sets total seats count to nil' do - expect(helper.total_seats_count).to be_nil - end - end - - shared_examples 'seats info are populated' do - it 'sets remaining seats count to the correct number' do - expect(helper.remaining_seats_count).to eq(14) - end - - it 'sets total seats count to the correct number' do - expect(helper.total_seats_count).to eq(15) - end - end - - shared_examples 'group info are populated' do - it 'builds the correct link' do - expect(helper.seats_usage_link).to match %r{.+}m - end - - it 'has a group name' do - expect(helper.group_name).to eq(context.name) - end - end - - shared_examples 'group info are not populated' do - it 'does not build the correct link' do - expect(helper.seats_usage_link).to be_nil - end - - it 'does not have a group name' do - expect(helper.group_name).to be_nil - end - end - - shared_examples 'alert is not displayed while some info are' do - it_behaves_like 'learn more link is built' - - it_behaves_like 'seats info are not populated' - - it_behaves_like 'group info are not populated' - - it 'does not show the alert' do - expect(helper.show_seats_count_alert?).to be false - end - end - - shared_examples 'alert is displayed' do - include_examples 'learn more link is built' - - include_examples 'seats info are populated' - - include_examples 'group info are populated' - - it 'does show the alert' do - expect(helper.show_seats_count_alert?).to be true - end - end - - shared_examples 'alert is not displayed' do - include_examples 'learn more link is built' - - include_examples 'seats info are populated' - - include_examples 'group info are populated' - - it 'does not show the alert' do - expect(helper.show_seats_count_alert?).to be false - end - end - - shared_examples 'common cases for users' do - let_it_be(:gitlab_subscription) do - create(:gitlab_subscription, namespace: context, plan_code: Plan::ULTIMATE, seats: 15, seats_in_use: 1) - end - - describe 'without a owner' do - before do - context.add_user(user, GroupMember::DEVELOPER) - helper.display_seats_count_alert! - end - - include_examples 'alert is not displayed' - end - - describe 'with a owner' do - before do - context.add_owner(user) - end - - context 'without display seats count' do - include_examples 'alert is not displayed' - end - - context 'with display seats count' do - before do - helper.display_seats_count_alert! - end - - include_examples 'alert is displayed' - end - end - end - - it 'sets @display_seats_count_alert to true' do - expect(helper.instance_variable_get(:@display_seats_count_alert)).to be nil - - helper.display_seats_count_alert! - - expect(helper.instance_variable_get(:@display_seats_count_alert)).to be true - end - - describe 'with no subscription' do - include_examples 'alert is not displayed while some info are' - end - - describe 'outside a group or project context' do - before do - helper.display_seats_count_alert! - end - - include_examples 'alert is not displayed while some info are' - end - - describe 'within a group context' do - let_it_be(:group) { create(:group) } - let_it_be(:context) { group } - let_it_be(:project) { nil } - - include_examples 'common cases for users' - end - - describe 'within a subgroup context' do - let_it_be(:context) { create(:group) } - let_it_be(:group) { create(:group, parent: context) } - let_it_be(:project) { nil } - - include_examples 'common cases for users' - end - - describe 'within a project context' do - let_it_be(:group) { nil } - let_it_be(:context) { create(:group) } - let_it_be(:project) { create(:project, namespace: context) } - - include_examples 'common cases for users' - end - - describe 'within a user namespace context' do - let_it_be(:project) { create(:project) } - - before do - helper.display_seats_count_alert! - end - - it 'does show the alert' do - expect(helper.show_seats_count_alert?).to be false - end - end -end diff --git a/ee/spec/requests/projects/issues_controller_spec.rb b/ee/spec/requests/projects/issues_controller_spec.rb index f4bb415f20af97..690415396a5a5d 100644 --- a/ee/spec/requests/projects/issues_controller_spec.rb +++ b/ee/spec/requests/projects/issues_controller_spec.rb @@ -75,6 +75,12 @@ def get_issues get_issues # Warm the cache end + it_behaves_like 'seat count alert' do + subject { get_issues } + + let(:namespace) { group } + end + it 'does not cause extra queries when there are other subepic issues' do create(:epic_issue, issue: issue, epic: epic) diff --git a/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb b/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb new file mode 100644 index 00000000000000..703d822883077d --- /dev/null +++ b/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'seat count alert' do + context 'when the namespace qualifies for the alert' do + it 'sets the seat_count_data' do + seat_count_data = { namespace: namespace, remaining_seat_count: 1, seats_in_use: 9, total_seat_count: 10 } + + allow_next_instance_of(GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService) do |service| + allow(service).to receive(:execute).and_return(seat_count_data) + end + + subject + + expect(assigns(:seat_count_data)).to eq seat_count_data + end + end + + context 'when the namespace does not qualify for the alert' do + it 'sets the seat_count_data to nil' do + allow_next_instance_of(GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService) do |service| + allow(service).to receive(:execute).and_return(nil) + end + + subject + + expect(assigns(:seat_count_data)).to be_nil + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5689a6ed071cc2..abe806e45e29ea 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15019,6 +15019,9 @@ msgstr "" msgid "Estimated" msgstr "" +msgid "Even if you reach the number of seats in your subscription, you can continue to add users, and GitLab will bill you for the overage." +msgstr "" + msgid "EventFilterBy|Filter by all" msgstr "" @@ -44152,8 +44155,10 @@ msgstr "" msgid "Your subscription expired!" msgstr "" -msgid "Your subscription has %{remaining_seats_count} out of %{total_seats_count} seats remaining. Even if you reach the number of seats in your subscription, you can continue to add users, and GitLab will bill you for the overage." -msgstr "" +msgid "Your subscription has %{remaining_seat_count} out of %{total_seat_count} seat remaining." +msgid_plural "Your subscription has %{remaining_seat_count} out of %{total_seat_count} seats remaining." +msgstr[0] "" +msgstr[1] "" msgid "Your subscription is now expired. To renew, export your license usage file and email it to %{renewal_service_email}. A new license will be emailed to the email address registered in the %{customers_dot}. You can add this license to your instance. To use Free tier, remove your current license." msgstr "" -- GitLab From 453c2172dc11a257948e0b2dd241662588bf70c9 Mon Sep 17 00:00:00 2001 From: Josianne Hyson Date: Wed, 11 May 2022 18:37:36 +1200 Subject: [PATCH 2/3] Add feature flag to control Seat Count Alert We want to release the seat count alert with the ability to quickly roll it back if the alert causes issues or impacts performance. Add the seat usage alert feature flag to control this. Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/348481 MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79563 --- .../development/seat_count_alerts.yml | 8 ++++++++ .../gitlab_subscriptions/seat_count_alert.rb | 1 + .../seat_count_alert_spec.rb | 8 ++++++++ .../seat_count_alert_spec.rb | 11 +++++++++++ .../concerns/seat_count_alert_shared_examples.rb | 16 ++++++++++++++++ 5 files changed, 44 insertions(+) create mode 100644 config/feature_flags/development/seat_count_alerts.yml diff --git a/config/feature_flags/development/seat_count_alerts.yml b/config/feature_flags/development/seat_count_alerts.yml new file mode 100644 index 00000000000000..3b05f391cbce96 --- /dev/null +++ b/config/feature_flags/development/seat_count_alerts.yml @@ -0,0 +1,8 @@ +--- +name: seat_count_alerts +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79563 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362041 +milestone: '15.1' +type: development +group: group::purchase +default_enabled: false diff --git a/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb b/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb index ea452b3d362ae2..e3eb77351a1bf8 100644 --- a/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb +++ b/ee/app/controllers/concerns/gitlab_subscriptions/seat_count_alert.rb @@ -4,6 +4,7 @@ module GitlabSubscriptions module SeatCountAlert def generate_seat_count_alert_data(namespace) return unless current_user && (root_ancestor = namespace&.root_ancestor) + return if Feature.disabled?(:seat_count_alerts, root_ancestor) GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService.new( namespace: root_ancestor, diff --git a/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb b/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb index 528098a0b5b878..f304ff26945c0d 100644 --- a/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb +++ b/ee/spec/controllers/concerns/gitlab_subscriptions/seat_count_alert_spec.rb @@ -32,6 +32,14 @@ end end + context 'when the feature flag is disabled' do + it 'does not set the seat count data' do + stub_feature_flags(seat_count_alerts: false) + + expect(controller.generate_seat_count_alert_data(root_ancestor)).to be_nil + end + end + context 'when supplied a project' do it 'sets the data based on the root ancestor' do project = build(:project, namespace: root_ancestor) diff --git a/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb b/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb index 06477a84290c48..b87f862a2e5497 100644 --- a/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb +++ b/ee/spec/features/gitlab_subscriptions/seat_count_alert_spec.rb @@ -58,6 +58,17 @@ it_behaves_like 'a hidden alert' end + context 'when the feature flag is disabled' do + before do + group.add_owner(user) + sign_in(user) + + stub_feature_flags(seat_count_alerts: false) + end + + it_behaves_like 'a hidden alert' + end + context 'when the user is eligible for the alert' do before do group.add_owner(user) diff --git a/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb b/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb index 703d822883077d..c7f6fad43d10e1 100644 --- a/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb +++ b/ee/spec/support/shared_examples/controllers/concerns/seat_count_alert_shared_examples.rb @@ -15,6 +15,22 @@ end end + context 'when the feature flag is disabled' do + it 'sets the seat_count_data to nil' do + stub_feature_flags(seat_count_alerts: false) + + seat_count_data = { namespace: namespace, remaining_seat_count: 1, seats_in_use: 9, total_seat_count: 10 } + + allow_next_instance_of(GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService) do |service| + allow(service).to receive(:execute).and_return(seat_count_data) + end + + subject + + expect(assigns(:seat_count_data)).to be_nil + end + end + context 'when the namespace does not qualify for the alert' do it 'sets the seat_count_data to nil' do allow_next_instance_of(GitlabSubscriptions::Reconciliations::CalculateSeatCountDataService) do |service| -- GitLab From 13ce6040279d9918c6e7f6fa0cb0282c61db4fc9 Mon Sep 17 00:00:00 2001 From: Josianne Hyson Date: Wed, 18 May 2022 15:25:52 +1200 Subject: [PATCH 3/3] Hide seat usage alert unless seat count changed We don't want to show the seat count alert unless the subscription has an associated `max_seats_used_changed_at` date. If we don't have this date, the user will be unable to dismiss the alert. Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/348481 MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79563 --- .../calculate_seat_count_data_service.rb | 2 +- .../calculate_seat_count_data_service_spec.rb | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ee/app/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service.rb b/ee/app/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service.rb index 0ecefdf6ec053b..750f4714209ea4 100644 --- a/ee/app/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service.rb +++ b/ee/app/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service.rb @@ -26,7 +26,7 @@ def initialize(namespace:, user:) def execute return unless owner_of_paid_group? && seat_count_threshold_reached? - return if user_dismissed_alert? + return if max_seats_used_changed_at.nil? || user_dismissed_alert? return unless alert_user_overage? { diff --git a/ee/spec/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service_spec.rb b/ee/spec/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service_spec.rb index 1d2e47f1dcc086..7005a58f1049fb 100644 --- a/ee/spec/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/reconciliations/calculate_seat_count_data_service_spec.rb @@ -31,6 +31,16 @@ it { is_expected.to be nil } end + context 'when the max_seats_used has not been updated on the subscription' do + let(:root_ancestor) { create(:group) } + + it 'returns nil' do + create(:gitlab_subscription, namespace: root_ancestor, plan_code: Plan::ULTIMATE, seats: 10, max_seats_used: 9) + + expect(subject).to be_nil + end + end + context 'when conditions are not met' do let(:max_seats_used) { 9 } @@ -40,7 +50,8 @@ namespace: root_ancestor, plan_code: Plan::ULTIMATE, seats: 10, - max_seats_used: max_seats_used + max_seats_used: max_seats_used, + max_seats_used_changed_at: 1.day.ago ) end @@ -120,7 +131,8 @@ namespace: root_ancestor, plan_code: Plan::ULTIMATE, seats: seats, - max_seats_used: max_seats_used + max_seats_used: max_seats_used, + max_seats_used_changed_at: 1.day.ago ) root_ancestor.add_owner(user) -- GitLab