BUGFIX - decorated_subscription returns nil

decorated_subscription is expected to return nil
when there is no subscription. It was previously returning an empty
SubscriptionDecorator object and caused breakage.

This change corrects the behavior, in tune with what the
Gitlab::ExpiringSubscriptionMessage expects.

This is part of: https://gitlab.com/gitlab-org/gitlab/-/issues/214212

ee/app/helpers/ee/projects_helper.rb

@@ -192,7 +192,11 @@ module EE
     end
 
     def decorated_subscription
-      SubscriptionPresenter.new(@project.gitlab_subscription)
+      subscription = @project.gitlab_subscription
+
+      return unless subscription
+
+      SubscriptionPresenter.new(subscription)
     end
 
     override :membership_locked?

ee/app/views/layouts/header/_ee_subscribable_banner.html.haml

@@ -5,7 +5,7 @@
   - subscribable = License.current
   - message = license_message(license: subscribable)
 
-- if message.present?
+- if message.present? && subscribable.present?
   .container-fluid.container-limited.pt-3
     .alert.alert-dismissible.gitlab-ee-license-banner.hidden.js-gitlab-ee-license-banner.pb-5.border-width-1px.border-style-solid.border-color-default.border-radius-default{ role: 'alert', data: { license_expiry: subscribable.expires_at } }
       %button.close.p-2{ type: 'button', 'data-dismiss' => 'alert', 'aria-label' => 'Dismiss banner' }

ee/lib/gitlab/expiring_subscription_message.rb

@@ -17,10 +17,10 @@ module Gitlab
       return unless notifiable?
 
       message = []
-      message << license_message_subject
-      message << expiration_blocking_message
+      message << license_message_subject if license_message_subject.present?
+      message << expiration_blocking_message if expiration_blocking_message.present?
 
-      message.reject { |string| string.blank? }.join(' ').html_safe
+      message.join(' ').html_safe
     end
 
     private
@@ -96,11 +96,11 @@ module Gitlab
     end
 
     def strong
-      "<strong>".html_safe
+      '<strong>'.html_safe
     end
 
     def strong_close
-      "</strong>".html_safe
+      '</strong>'.html_safe
     end
   end
 end

ee/spec/helpers/license_helper_spec.rb

@@ -16,7 +16,7 @@ describe LicenseHelper do
       allow(License).to receive(:current).and_return(license)
     end
 
-    it 'calls another class with args' do
+    it 'calls Gitlab::ExpiringSubscriptionMessage to get expiring message' do
       expect(Gitlab::ExpiringSubscriptionMessage).to receive(:new).with(
         subscribable: license,
         signed_in: true,
@@ -93,23 +93,19 @@ describe LicenseHelper do
 
   describe '#current_license_title' do
     context 'when there is a current license' do
-      context 'and it has a plan associated to it' do
-        it 'returns the plan titleized' do
-          custom_plan = 'custom plan'
-          license = double('License', plan: custom_plan)
-          allow(License).to receive(:current).and_return(license)
+      it 'returns the plan titleized if it has a plan associated to it' do
+        custom_plan = 'custom plan'
+        license = double('License', plan: custom_plan)
+        allow(License).to receive(:current).and_return(license)
 
-          expect(current_license_title).to eq(custom_plan.titleize)
-        end
+        expect(current_license_title).to eq(custom_plan.titleize)
       end
 
-      context 'and it does not have a plan associated to it' do
-        it 'returns the default title' do
-          license = double('License', plan: nil)
-          allow(License).to receive(:current).and_return(license)
+      it 'returns the default title if it does not have a plan associated to it' do
+        license = double('License', plan: nil)
+        allow(License).to receive(:current).and_return(license)
 
-          expect(current_license_title).to eq('Core')
-        end
+        expect(current_license_title).to eq('Core')
       end
     end
 

ee/spec/helpers/projects_helper_spec.rb

@@ -237,7 +237,7 @@ describe ProjectsHelper do
       expect(helper.subscription_message).to be_nil
     end
 
-    it 'calls 2 classes if is Gitlab.com?' 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)
@@ -256,4 +256,26 @@ describe ProjectsHelper do
       expect(helper.subscription_message).to eq('hey yay yay yay')
     end
   end
+
+  describe '#decorated_subscription' do
+    subject { helper.decorated_subscription }
+
+    context 'when a subscription exists' do
+      let(:gitlab_subscription) { build_stubbed(:gitlab_subscription) }
+
+      it 'returns a decorator' do
+        allow(project).to receive(:gitlab_subscription).and_return(gitlab_subscription)
+
+        expect(subject).to be_a(SubscriptionPresenter)
+      end
+    end
+
+    context 'when no subscription exists' do
+      it 'returns a nil object' do
+        allow(project).to receive(:gitlab_subscription).and_return(nil)
+
+        expect(subject).to be_nil
+      end
+    end
+  end
 end