Merge branch '221063-dismiss-user-notifcation-dot' into 'master'

Dismiss user notification dot upon acknowledgement

See merge request gitlab-org/gitlab!34397

app/assets/javascripts/persistent_user_callout.js

@@ -18,17 +18,21 @@ export default class PersistentUserCallout {
 
   init() {
     const closeButton = this.container.querySelector('.js-close');
+    const followLink = this.container.querySelector('.js-follow-link');
 
-    if (!closeButton) {
-      return;
+    if (closeButton) {
+      this.handleCloseButtonCallout(closeButton);
+    } else if (followLink) {
+      this.handleFollowLinkCallout(followLink);
     }
+  }
 
+  handleCloseButtonCallout(closeButton) {
     closeButton.addEventListener('click', event => this.dismiss(event));
 
     if (this.deferLinks) {
       this.container.addEventListener('click', event => {
         const isDeferredLink = event.target.classList.contains(DEFERRED_LINK_CLASS);
-
         if (isDeferredLink) {
           const { href, target } = event.target;
 
@@ -38,6 +42,10 @@ export default class PersistentUserCallout {
     }
   }
 
+  handleFollowLinkCallout(followLink) {
+    followLink.addEventListener('click', event => this.registerCalloutWithLink(event));
+  }
+
   dismiss(event, deferredLinkOptions = null) {
     event.preventDefault();
 
@@ -58,6 +66,27 @@ export default class PersistentUserCallout {
       });
   }
 
+  registerCalloutWithLink(event) {
+    event.preventDefault();
+
+    const { href } = event.currentTarget;
+
+    axios
+      .post(this.dismissEndpoint, {
+        feature_name: this.featureId,
+      })
+      .then(() => {
+        window.location.assign(href);
+      })
+      .catch(() => {
+        Flash(
+          __(
+            'An error occurred while acknowledging the notification. Refresh the page and try again.',
+          ),
+        );
+      });
+  }
+
   static factory(container, options) {
     if (!container) {
       return undefined;

app/assets/javascripts/persistent_user_callouts.js

@@ -4,6 +4,7 @@ const PERSISTENT_USER_CALLOUTS = [
   '.js-recovery-settings-callout',
   '.js-users-over-license-callout',
   '.js-admin-licensed-user-count-threshold',
+  '.js-buy-pipeline-minutes-notification-callout',
 ];
 
 const initCallouts = () => {

ee/app/helpers/ee/runners_helper.rb

@@ -3,6 +3,8 @@ module EE
   module RunnersHelper
     include ::Gitlab::Utils::StrongMemoize
 
+    BUY_PIPELINE_MINUTES_NOTIFICATION_DOT = 'buy_pipeline_minutes_notification_dot'
+
     def show_buy_pipeline_minutes?(project, namespace)
       return false unless experiment_enabled?(:ci_notification_dot) || experiment_enabled?(:buy_ci_minutes_version_a)
 
@@ -11,12 +13,26 @@ module EE
 
     def show_pipeline_minutes_notification_dot?(project, namespace)
       return false unless experiment_enabled?(:ci_notification_dot)
+      return false if notification_dot_acknowledged?
+
+      show_out_of_pipeline_minutes_notification?(project, namespace)
+    end
+
+    def show_buy_pipeline_with_subtext?(project, namespace)
+      return false unless experiment_enabled?(:ci_notification_dot)
+      return false unless notification_dot_acknowledged?
 
       show_out_of_pipeline_minutes_notification?(project, namespace)
     end
 
     private
 
+    def notification_dot_acknowledged?
+      strong_memoize(:notification_dot_acknowledged) do
+        user_dismissed?(BUY_PIPELINE_MINUTES_NOTIFICATION_DOT)
+      end
+    end
+
     def show_out_of_pipeline_minutes_notification?(project, namespace)
       strong_memoize(:show_out_of_pipeline_minutes_notification) do
         next unless project&.persisted? || namespace&.persisted?

ee/app/models/ee/user_callout_enums.rb

@@ -19,7 +19,8 @@ module EE
           account_recovery_regular_check: 12,
           users_over_license_banner: 16,
           standalone_vulnerabilities_introduction_banner: 17,
-          active_user_count_threshold: 18
+          active_user_count_threshold: 18,
+          buy_pipeline_minutes_notification_dot: 19
         )
       end
     end

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

@@ -2,17 +2,30 @@
 
 - link_text = s_("CurrentUser|Buy Pipeline minutes")
 - link_emoji = emoji_icon('clock9', 'aria-hidden': true)
+- link_class = 'ci-minutes-emoji js-buy-pipeline-minutes-link'
+- data_attributes = { 'track-event': 'click_buy_ci_minutes', 'track-label': current_user.namespace.actual_plan_name, 'track-property': 'user_dropdown' }
+- path = profile_usage_quotas_path
 
-%li
-  = link_to profile_usage_quotas_path,
-    class: 'ci-minutes-emoji js-buy-pipeline-minutes-link',
-    data: { 'track-event': 'click_buy_ci_minutes', 'track-label': current_user.namespace.actual_plan_name, 'track-property': 'user_dropdown' } do
-    - if show_pipeline_minutes_notification_dot?(project, namespace)
-      .gl-pb-2
-        = link_text
-        = link_emoji
-      %span.small.gl-pb-3.gl-text-orange-800
-        = s_("CurrentUser|One of your groups is running out")
-    - else
+- content_for :buy_pipeline_with_subtext do
+  .gl-pb-2
+    = link_text
+    = link_emoji
+  %span.small.gl-pb-3.gl-text-orange-800
+    = s_("CurrentUser|One of your groups is running out")
+
+- if show_pipeline_minutes_notification_dot?(project, namespace)
+  - link_class << ' js-follow-link'
+
+  %li.js-buy-pipeline-minutes-notification-callout{ data: { feature_id: RunnersHelper::BUY_PIPELINE_MINUTES_NOTIFICATION_DOT,
+    dismiss_endpoint: user_callouts_path } }
+    = link_to path, class: link_class, data: data_attributes do
+      = yield :buy_pipeline_with_subtext
+- elsif show_buy_pipeline_with_subtext?(project, namespace)
+  %li
+    = link_to path, class: link_class, data: data_attributes do
+      = yield :buy_pipeline_with_subtext
+- else
+  %li
+    = link_to path, class: link_class, data: data_attributes do
       = link_text
       = link_emoji

ee/spec/helpers/ee/runners_helper_spec.rb

@@ -4,14 +4,14 @@ require "spec_helper"
 
 RSpec.describe EE::RunnersHelper do
   let_it_be(:user) { create(:user) }
+  let_it_be(:namespace) { create(:namespace, owner: user) }
+  let_it_be(:project) { create(:project, namespace: namespace) }
 
   before do
     allow(helper).to receive(:current_user).and_return(user)
   end
 
   shared_examples_for 'minutes notification' do
-    let_it_be(:namespace) { create(:namespace, owner: user) }
-    let_it_be(:project) { create(:project, namespace: namespace) }
     let(:show_warning) { true }
     let(:context_level) { project }
     let(:threshold) { double('Ci::Minutes::Notification', show?: show_warning) }
@@ -109,10 +109,52 @@ RSpec.describe EE::RunnersHelper do
     describe '.show_pipeline_minutes_notification_dot?' do
       subject { helper.show_pipeline_minutes_notification_dot?(project, namespace) }
 
-      it_behaves_like 'minutes notification' do
+      before do
+        allow(helper).to receive(:experiment_enabled?).with(:ci_notification_dot).and_return(experiment_status)
+      end
+
+      it_behaves_like 'minutes notification'
+
+      context 'when the notification dot has been acknowledged' do
+        before do
+          create(:user_callout, user: user, feature_name: described_class::BUY_PIPELINE_MINUTES_NOTIFICATION_DOT)
+          expect(helper).not_to receive(:show_out_of_pipeline_minutes_notification?)
+        end
+
+        it { is_expected.to be_falsy }
+      end
+
+      context 'when the notification dot has not been acknowledged' do
+        before do
+          expect(helper).to receive(:show_out_of_pipeline_minutes_notification?).and_return(true)
+        end
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    describe '.show_buy_pipeline_with_subtext?' do
+      subject { helper.show_buy_pipeline_with_subtext?(project, namespace) }
+
+      before do
+        allow(helper).to receive(:experiment_enabled?).with(:ci_notification_dot).and_return(experiment_status)
+      end
+
+      context 'when the notification dot has not been acknowledged' do
         before do
-          allow(helper).to receive(:experiment_enabled?).with(:ci_notification_dot).and_return(experiment_status)
+          expect(helper).not_to receive(:show_out_of_pipeline_minutes_notification?)
         end
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when the notification dot has been acknowledged' do
+        before do
+          create(:user_callout, user: user, feature_name: described_class::BUY_PIPELINE_MINUTES_NOTIFICATION_DOT)
+          expect(helper).to receive(:show_out_of_pipeline_minutes_notification?).and_return(true)
+        end
+
+        it { is_expected.to be_truthy }
       end
     end
   end

ee/spec/views/layouts/header/_current_user_dropdown.html.haml_spec.rb

@@ -8,19 +8,21 @@ RSpec.describe 'layouts/header/_current_user_dropdown' do
   describe 'Buy Pipeline Minutes link in user dropdown' do
     let(:need_minutes) { true }
     let(:show_notification_dot) { false }
+    let(:show_subtext) { false }
 
     before do
       allow(view).to receive(:current_user).and_return(user)
       allow(view).to receive(:show_upgrade_link?).and_return(false)
       allow(view).to receive(:show_buy_pipeline_minutes?).and_return(need_minutes)
       allow(view).to receive(:show_pipeline_minutes_notification_dot?).and_return(show_notification_dot)
+      allow(view).to receive(:show_buy_pipeline_with_subtext?).and_return(show_subtext)
 
       render
     end
 
     subject { rendered }
 
-    context 'when pipeline minutes need bought' do
+    context 'when pipeline minutes need bought without notification dot' do
       it 'has "Buy Pipeline minutes" link with correct data properties', :aggregate_failures do
         expect(subject).to have_selector('[data-track-event="click_buy_ci_minutes"]')
         expect(subject).to have_selector("[data-track-label='#{user.namespace.actual_plan_name}']")
@@ -36,6 +38,21 @@ RSpec.describe 'layouts/header/_current_user_dropdown' do
       it 'has "Buy Pipeline minutes" link with correct text', :aggregate_failures do
         expect(subject).to have_link('Buy Pipeline minutes')
         expect(subject).to have_content('One of your groups is running out')
+        expect(subject).to have_selector('.js-follow-link')
+        expect(subject).to have_selector("[data-feature-id='#{RunnersHelper::BUY_PIPELINE_MINUTES_NOTIFICATION_DOT}']")
+        expect(subject).to have_selector("[data-dismiss-endpoint='#{user_callouts_path}']")
+      end
+    end
+
+    context 'when pipeline minutes need bought and notification dot has been acknowledged' do
+      let(:show_subtext) { true }
+
+      it 'has "Buy Pipeline minutes" link with correct text', :aggregate_failures do
+        expect(subject).to have_link('Buy Pipeline minutes')
+        expect(subject).to have_content('One of your groups is running out')
+        expect(subject).not_to have_selector('.js-follow-link')
+        expect(subject).not_to have_selector("[data-feature-id='#{RunnersHelper::BUY_PIPELINE_MINUTES_NOTIFICATION_DOT}']")
+        expect(subject).not_to have_selector("[data-dismiss-endpoint='#{user_callouts_path}']")
       end
     end
 

locale/gitlab.pot

@@ -2257,6 +2257,9 @@ msgstr ""
 msgid "An error occurred when updating the issue weight"
 msgstr ""
 
+msgid "An error occurred while acknowledging the notification. Refresh the page and try again."
+msgstr ""
+
 msgid "An error occurred while adding formatted title for epic"
 msgstr ""
 

spec/frontend/persistent_user_callout_spec.js

@@ -43,6 +43,23 @@ describe('PersistentUserCallout', () => {
     return fixture;
   }
 
+  function createFollowLinkFixture() {
+    const fixture = document.createElement('div');
+    fixture.innerHTML = `
+      <ul>
+        <li
+          class="container"
+          data-dismiss-endpoint="${dismissEndpoint}"
+          data-feature-id="${featureName}"
+        >
+          <a class="js-follow-link" href="/somewhere-pleasant">A Link</a>
+        </li>
+      </ul>
+  `;
+
+    return fixture;
+  }
+
   describe('dismiss', () => {
     let button;
     let mockAxios;
@@ -144,6 +161,55 @@ describe('PersistentUserCallout', () => {
     });
   });
 
+  describe('follow links', () => {
+    let link;
+    let mockAxios;
+    let persistentUserCallout;
+
+    beforeEach(() => {
+      const fixture = createFollowLinkFixture();
+      const container = fixture.querySelector('.container');
+      link = fixture.querySelector('.js-follow-link');
+      mockAxios = new MockAdapter(axios);
+
+      persistentUserCallout = new PersistentUserCallout(container);
+      jest.spyOn(persistentUserCallout.container, 'remove').mockImplementation(() => {});
+
+      delete window.location;
+      window.location = { assign: jest.fn() };
+    });
+
+    afterEach(() => {
+      mockAxios.restore();
+    });
+
+    it('uses a link to trigger callout and defers following until callout is finished', () => {
+      const { href } = link;
+      mockAxios.onPost(dismissEndpoint).replyOnce(200);
+
+      link.click();
+
+      return waitForPromises().then(() => {
+        expect(window.location.assign).toBeCalledWith(href);
+        expect(mockAxios.history.post[0].data).toBe(JSON.stringify({ feature_name: featureName }));
+        expect(persistentUserCallout.container.remove).not.toHaveBeenCalled();
+      });
+    });
+
+    it('invokes Flash when the dismiss request fails', () => {
+      mockAxios.onPost(dismissEndpoint).replyOnce(500);
+
+      link.click();
+
+      return waitForPromises().then(() => {
+        expect(window.location.assign).not.toHaveBeenCalled();
+        expect(Flash).toHaveBeenCalledWith(
+          'An error occurred while acknowledging the notification. Refresh the page and try again.',
+        );
+      });
+    });
+  });
+
   describe('factory', () => {
     it('returns an instance of PersistentUserCallout with the provided container property', () => {
       const fixture = createFixture();