diff --git a/app/helpers/subscribable_banner_helper.rb b/app/helpers/subscribable_banner_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..c9d4370f8ad53a96fb867c0c3ebc49d9a2c0a145 --- /dev/null +++ b/app/helpers/subscribable_banner_helper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module SubscribableBannerHelper + # Overridden in EE + def display_subscription_banner! + end +end + +SubscribableBannerHelper.prepend_if_ee('EE::SubscribableBannerHelper') diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 032766327ca2fad497208a07c79e40a0a733fcbd..7e5bf6ddde17a4fb3c20e07c09dcaa684257b060 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -1,10 +1,6 @@ - breadcrumb_title _("Details") - @content_class = "limit-container-width" unless fluid_layout -= content_for :flash_message do - - if Feature.enabled?(:subscribable_banner_subscription) - = render_if_exists "layouts/header/ee_subscribable_banner", subscription: true - = content_for :meta_tags do = auto_discovery_link_tag(:atom, group_url(@group, rss_url_options), title: "#{@group.name} activity") diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 57445424b26c11e42cccdc5658c2a60b610826f3..d1cf83b2a9f8e28a0c1a1d13dddacaf1ad3a2ccf 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -6,8 +6,6 @@ .alert-wrapper = render 'shared/outdated_browser' = render_if_exists 'layouts/header/users_over_license_banner' - - if Feature.enabled?(:subscribable_banner_license, default_enabled: true) - = render_if_exists "layouts/header/ee_subscribable_banner" = render_if_exists "layouts/header/licensed_user_count_threshold" = render "layouts/broadcast" = render "layouts/header/read_only_banner" @@ -15,6 +13,7 @@ = yield :flash_message = render "shared/ping_consent" = render_account_recovery_regular_check + = render_if_exists "layouts/header/ee_subscribable_banner" - unless @hide_breadcrumbs = render "layouts/nav/breadcrumbs" .d-flex diff --git a/app/views/layouts/group.html.haml b/app/views/layouts/group.html.haml index 49de821f1c2eeaf3a393aeed420f75ad1f02f9ce..36b664e5888b8a86f1864f54a1ab4c0a925670cf 100644 --- a/app/views/layouts/group.html.haml +++ b/app/views/layouts/group.html.haml @@ -2,6 +2,7 @@ - page_description @group.description unless page_description - header_title group_title(@group) unless header_title - nav "group" +- display_subscription_banner! - @left_sidebar = true - content_for :page_specific_javascripts do diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index b8ef38272fc7316a1c1dfbd12da2f84353080923..820cb9eea47ae32443a98c5960335a90bcdb3122 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -2,7 +2,8 @@ - page_description @project.description unless page_description - header_title project_title(@project) unless header_title - nav "project" -- @left_sidebar = true +- display_subscription_banner! +- @left_sidebar = true - content_for :project_javascripts do - project = @target_project || @project diff --git a/app/views/projects/_flash_messages.html.haml b/app/views/projects/_flash_messages.html.haml index 8217608db4ea9927316eb3a47fd2ba0820c87ab0..f9222387e97f2d31183f8d12cfc9c0fc5c42595a 100644 --- a/app/views/projects/_flash_messages.html.haml +++ b/app/views/projects/_flash_messages.html.haml @@ -8,6 +8,4 @@ - unless project.empty_repo? = render 'shared/auto_devops_implicitly_enabled_banner', project: project = render_if_exists 'projects/above_size_limit_warning', project: project - - if Feature.enabled?(:subscribable_banner_subscription) - = render_if_exists "layouts/header/ee_subscribable_banner", subscription: true = render_if_exists 'shared/shared_runners_minutes_limit', project: project, classes: [container_class, ("limit-container-width" unless fluid_layout)] diff --git a/ee/app/helpers/ee/subscribable_banner_helper.rb b/ee/app/helpers/ee/subscribable_banner_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..9602fbbdf0b99def55f74cdaaa6c6fcaf517f02e --- /dev/null +++ b/ee/app/helpers/ee/subscribable_banner_helper.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module EE + module SubscribableBannerHelper + extend ::Gitlab::Utils::Override + + def gitlab_subscription_or_license + return decorated_subscription if display_subscription_banner? + + License.current if display_license_banner? + end + + def gitlab_subscription_message_or_license_message + return subscription_message if display_subscription_banner? + + license_message if display_license_banner? + end + + override :display_subscription_banner! + def display_subscription_banner! + @display_subscription_banner = true + end + + private + + def license_message(signed_in: signed_in?, is_admin: current_user&.admin?, license: License.current) + ::Gitlab::ExpiringSubscriptionMessage.new( + subscribable: license, + signed_in: signed_in, + is_admin: is_admin + ).message + end + + def subscription_message + entity = @project || @group + namespace = @project&.namespace || @group + + ::Gitlab::ExpiringSubscriptionMessage.new( + subscribable: decorated_subscription, + signed_in: signed_in?, + is_admin: can?(current_user, :owner_access, entity), + namespace: namespace + ).message + end + + def decorated_subscription + entity = @project || @group + subscription = entity&.closest_gitlab_subscription + + return unless subscription + + ::SubscriptionPresenter.new(subscription) + end + + def display_license_banner? + ::Feature.enabled?(:subscribable_license_banner, default_enabled: true) + end + + def display_subscription_banner? + @display_subscription_banner && + ::Gitlab::CurrentSettings.should_check_namespace_plan? && + ::Feature.enabled?(:subscribable_subscription_banner, default_enabled: false) + end + end +end diff --git a/ee/app/helpers/license_helper.rb b/ee/app/helpers/license_helper.rb index 2d73f2cfb2f01e76f8303bb4e9f155c89374339b..8c44e136e391d0543f88cf80f5d47d81e634713b 100644 --- a/ee/app/helpers/license_helper.rb +++ b/ee/app/helpers/license_helper.rb @@ -18,14 +18,6 @@ def maximum_user_count License.current&.maximum_user_count || 0 end - def license_message(signed_in: signed_in?, is_admin: current_user&.admin?, license: License.current) - Gitlab::ExpiringSubscriptionMessage.new( - subscribable: license, - signed_in: signed_in, - is_admin: is_admin - ).message - end - def seats_calculation_message(license) return unless license.present? return unless license.exclude_guests_from_active_count? diff --git a/ee/app/helpers/subscriptions_helper.rb b/ee/app/helpers/subscriptions_helper.rb index 12665a5fc3cf375cdc76e77d2bac0466edb78583..d3060f813692107851680af59d66edc1b2a7888a 100644 --- a/ee/app/helpers/subscriptions_helper.rb +++ b/ee/app/helpers/subscriptions_helper.rb @@ -3,29 +3,6 @@ module SubscriptionsHelper include ::Gitlab::Utils::StrongMemoize - def subscription_message - return unless ::Gitlab.com? - - entity = @project || @group - namespace = @project&.namespace || @group - - ::Gitlab::ExpiringSubscriptionMessage.new( - subscribable: decorated_subscription, - signed_in: signed_in?, - is_admin: can?(current_user, :owner_access, entity), - namespace: namespace - ).message - end - - def decorated_subscription - entity = @project || @group - subscription = entity&.closest_gitlab_subscription - - return unless subscription - - SubscriptionPresenter.new(subscription) - end - def subscription_data { setup_for_company: (current_user.setup_for_company == true).to_s, diff --git a/ee/app/presenters/subscription_presenter.rb b/ee/app/presenters/subscription_presenter.rb index 05092cb64d2a2769c6f1df8959e84fef16b840b0..734877e6ae093a50e1c504f62819c9fa3576095f 100644 --- a/ee/app/presenters/subscription_presenter.rb +++ b/ee/app/presenters/subscription_presenter.rb @@ -8,7 +8,7 @@ def block_changes? end def plan - namespace.try(:actual_plan_name) + hosted_plan.name end def notify_admins? diff --git a/ee/app/views/layouts/header/_ee_subscribable_banner.html.haml b/ee/app/views/layouts/header/_ee_subscribable_banner.html.haml index 6c2f34dc0ab30455bcec0a2e457e5fe37c232840..48aa3e6fed66ac2e878026a94fa975358befd193 100644 --- a/ee/app/views/layouts/header/_ee_subscribable_banner.html.haml +++ b/ee/app/views/layouts/header/_ee_subscribable_banner.html.haml @@ -1,9 +1,5 @@ -- if local_assigns[:subscription] - - subscribable = decorated_subscription - - message = subscription_message -- else - - subscribable = License.current - - message = license_message(license: subscribable) +- subscribable = gitlab_subscription_or_license +- message = gitlab_subscription_message_or_license_message - if message.present? && subscribable.present? .container-fluid.container-limited.pt-3 diff --git a/ee/changelogs/unreleased/jswain_add_subscribable_banner_to_all_group_pages.yml b/ee/changelogs/unreleased/jswain_add_subscribable_banner_to_all_group_pages.yml new file mode 100644 index 0000000000000000000000000000000000000000..5b213839843b68c59583f302aed29beee321f8ed --- /dev/null +++ b/ee/changelogs/unreleased/jswain_add_subscribable_banner_to_all_group_pages.yml @@ -0,0 +1,5 @@ +--- +title: Display expiring subscription banner on more pages +merge_request: 31497 +author: +type: changed diff --git a/ee/spec/helpers/ee/subscribable_banner_helper_spec.rb b/ee/spec/helpers/ee/subscribable_banner_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..278c4c2bb1a7ecf9a6954150420b90babe626e3f --- /dev/null +++ b/ee/spec/helpers/ee/subscribable_banner_helper_spec.rb @@ -0,0 +1,201 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe EE::SubscribableBannerHelper do + describe '#gitlab_subscription_or_license' do + subject { helper.gitlab_subscription_or_license } + + shared_examples 'when a subscription exists' do + let(:gitlab_subscription) { build_stubbed(:gitlab_subscription) } + + it 'returns a decorator' do + allow(entity).to receive(:closest_gitlab_subscription).and_return(gitlab_subscription) + + expect(subject).to be_a(SubscriptionPresenter) + end + end + + context 'when feature flag is enabled' do + let(:license) { double(:license) } + + before do + stub_feature_flags(subscribable_banner: true) + end + + context 'when instance variable true' do + before do + assign(:display_subscription_banner, true) + end + + context 'when should_check_namespace_plan is true' do + before do + allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(true) + end + + context 'when a project exists' do + let(:entity) { create(:project) } + + before do + assign(:project, entity) + end + + it_behaves_like 'when a subscription exists' + end + + context 'when a group exists' do + let(:entity) { create(:group) } + + before do + assign(:group, entity) + end + + it_behaves_like 'when a subscription exists' + end + end + + context 'when should_check_namespace_plan is false' do + before do + allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(false) + end + + it 'returns the current license' do + expect(License).to receive(:current).and_return(license) + expect(subject).to eq(license) + end + end + end + + context 'when instance variable false' do + before do + assign(:display_subscription_banner, false) + allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(true) + end + + it 'returns the current license' do + expect(License).to receive(:current).and_return(license) + expect(subject).to eq(license) + end + end + end + end + + describe '#gitlab_subscription_message_or_license_message' do + subject { helper.gitlab_subscription_message_or_license_message } + + let(:message) { double(:message) } + + context 'when feature flag is enabled' do + before do + stub_feature_flags(subscribable_banner: true) + end + + context 'when instance variable true' do + before do + assign(:display_subscription_banner, true) + end + + context 'when should_check_namespace_plan is true' do + before do + allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(true) + end + + let(:gitlab_subscription) { entity.closest_gitlab_subscription } + let(:decorated_mock) { double(:decorated_mock) } + let(:message_mock) { double(:message_mock) } + let(:user) { double(:user_mock) } + + shared_examples 'subscription message' do + it 'calls Gitlab::ExpiringSubscriptionMessage and SubscriptionPresenter if is Gitlab.com?' do + allow(helper).to receive(:signed_in?).and_return(true) + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).with(user, :owner_access, entity).and_return(true) + + expect(SubscriptionPresenter).to receive(:new).with(gitlab_subscription).and_return(decorated_mock) + expect(::Gitlab::ExpiringSubscriptionMessage).to receive(:new).with( + subscribable: decorated_mock, + signed_in: true, + is_admin: true, + namespace: namespace + ).and_return(message_mock) + expect(message_mock).to receive(:message).and_return('hey yay yay yay') + + expect(subject).to eq('hey yay yay yay') + end + end + + context 'when a project is present' do + let(:entity) { create(:project, namespace: namespace) } + let(:namespace) { create(:namespace_with_plan) } + + before do + assign(:project, entity) + end + + it_behaves_like 'subscription message' + end + + context 'when a group is present' do + let(:entity) { create(:group_with_plan) } + let(:namespace) { entity } + + before do + assign(:project, nil) + assign(:group, entity) + end + + it_behaves_like 'subscription message' + end + end + + context 'when should_check_namespace_plan is false' do + let(:license) { double(:license) } + let(:message_mock) { double(:message_mock) } + let(:user) { double(:user) } + + before do + allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(false) + allow(License).to receive(:current).and_return(license) + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:signed_in?).and_return(true) + allow(user).to receive(:admin?).and_return(false) + end + + it 'calls Gitlab::ExpiringSubscriptionMessage to get expiring message' do + expect(Gitlab::ExpiringSubscriptionMessage).to receive(:new).with( + subscribable: license, + signed_in: true, + is_admin: false + ).and_return(message_mock) + + expect(message_mock).to receive(:message) + + subject + end + end + end + + context 'when instance variable false' do + before do + assign(:display_subscription_banner, false) + allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(true) + end + + it 'returns the license message' do + expect(helper).to receive(:license_message).and_return(message) + expect(subject).to eq(message) + end + end + end + end + + describe '#display_subscription_banner!' do + it 'sets @display_subscription_banner to true' do + expect(helper.instance_variable_get(:@display_subscription_banner)).to be nil + + helper.display_subscription_banner! + + expect(helper.instance_variable_get(:@display_subscription_banner)).to be true + end + end +end diff --git a/ee/spec/helpers/license_helper_spec.rb b/ee/spec/helpers/license_helper_spec.rb index 1917b6da2f30820956dcefe9afe010b96f7b3dc3..d692e15cb8a9d74c83d3c06caa0f66ae8c688f63 100644 --- a/ee/spec/helpers/license_helper_spec.rb +++ b/ee/spec/helpers/license_helper_spec.rb @@ -8,27 +8,6 @@ def stub_default_url_options(host: "localhost", protocol: "http", port: nil, scr allow(Rails.application.routes).to receive(:default_url_options).and_return(url_options) end - describe '#license_message' do - let(:license) { double(:license) } - let(:message_mock) { double(:message_mock) } - - before do - allow(License).to receive(:current).and_return(license) - end - - it 'calls Gitlab::ExpiringSubscriptionMessage to get expiring message' do - expect(Gitlab::ExpiringSubscriptionMessage).to receive(:new).with( - subscribable: license, - signed_in: true, - is_admin: false - ).and_return(message_mock) - - expect(message_mock).to receive(:message) - - license_message(signed_in: true, is_admin: false) - end - end - describe '#api_license_url' do it 'returns license API url' do stub_default_url_options diff --git a/ee/spec/helpers/subscriptions_helper_spec.rb b/ee/spec/helpers/subscriptions_helper_spec.rb index aa26cbf08f731747231e97a1fb58f5649db922a1..6155129cef6cb751f8462c9962e0f806449ce71d 100644 --- a/ee/spec/helpers/subscriptions_helper_spec.rb +++ b/ee/spec/helpers/subscriptions_helper_spec.rb @@ -87,108 +87,4 @@ it { is_expected.to eq(nil) } end end - - describe '#subscription_message' do - let(:gitlab_subscription) { entity.closest_gitlab_subscription } - let(:decorated_mock) { double(:decorated_mock) } - let(:message_mock) { double(:message_mock) } - let(:user) { double(:user_mock) } - - it 'if it is not Gitlab.com? it returns nil' do - allow(Gitlab).to receive(:com?).and_return(false) - - expect(helper.subscription_message).to be_nil - end - - shared_examples 'subscription message' do - it 'calls Gitlab::ExpiringSubscriptionMessage and SubscriptionPresenter if is Gitlab.com?' do - allow(Gitlab).to receive(:com?).and_return(true) - allow(helper).to receive(:signed_in?).and_return(true) - allow(helper).to receive(:current_user).and_return(user) - allow(helper).to receive(:can?).with(user, :owner_access, entity).and_return(true) - - expect(SubscriptionPresenter).to receive(:new).with(gitlab_subscription).and_return(decorated_mock) - expect(::Gitlab::ExpiringSubscriptionMessage).to receive(:new).with( - subscribable: decorated_mock, - signed_in: true, - is_admin: true, - namespace: namespace - ).and_return(message_mock) - expect(message_mock).to receive(:message).and_return('hey yay yay yay') - - expect(helper.subscription_message).to eq('hey yay yay yay') - end - end - - context 'when a project is present' do - let(:entity) { create(:project, namespace: namespace) } - let(:namespace) { create(:namespace_with_plan) } - - before do - assign(:project, entity) - end - - it_behaves_like 'subscription message' - end - - context 'when a group is present' do - let(:entity) { create(:group_with_plan) } - let(:namespace) { entity } - - before do - assign(:project, nil) - assign(:group, entity) - end - - it_behaves_like 'subscription message' - end - end - - describe '#decorated_subscription' do - subject { helper.decorated_subscription } - - shared_examples 'when a subscription exists' do - let(:gitlab_subscription) { build_stubbed(:gitlab_subscription) } - - it 'returns a decorator' do - allow(entity).to receive(:closest_gitlab_subscription).and_return(gitlab_subscription) - - expect(subject).to be_a(SubscriptionPresenter) - end - end - - context 'when a project exists' do - let(:entity) { create(:project) } - - before do - assign(:project, entity) - end - - it_behaves_like 'when a subscription exists' - end - - context 'when a group exists' do - let(:entity) { create(:group) } - - before do - assign(:group, entity) - end - - it_behaves_like 'when a subscription exists' - end - - context 'when no subscription exists' do - let(:entity) { create(:project) } - - before do - assign(:project, entity) - end - - it 'returns a nil object' do - allow(entity).to receive(:closest_gitlab_subscription).and_return(nil) - - expect(subject).to be_nil - end - end - end end diff --git a/spec/helpers/subscribable_banner_helper_spec.rb b/spec/helpers/subscribable_banner_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..75f2e32d7d818d288bcd6e9b9fd670f8fcb05344 --- /dev/null +++ b/spec/helpers/subscribable_banner_helper_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SubscribableBannerHelper do + describe '#display_subscription_banner!' do + it 'is over-written in EE' do + expect { helper.display_subscription_banner! }.not_to raise_error + end + end +end