Merge branch...

Merge branch '333606-use-usercallout-model-to-keep-track-of-trial-status-popover-dismissals' into 'master'

Use UserCallout model to keep track of trial status popover dismissals

See merge request gitlab-org/gitlab!64675

app/models/user_callout.rb

@@ -32,7 +32,9 @@ class UserCallout < ApplicationRecord
     pipeline_needs_hover_tip: 30,
     web_ide_ci_environments_guidance: 31,
     security_configuration_upgrade_banner: 32,
-    cloud_licensing_subscription_activation_banner: 33  # EE-only
+    cloud_licensing_subscription_activation_banner: 33, # EE-only
+    trial_status_reminder_d14: 34,             # EE-only
+    trial_status_reminder_d3: 35               # EE-only
   }
 
   validates :user, presence: true

doc/api/graphql/reference/index.md

@@ -15295,6 +15295,8 @@ Name of the feature that the callout is for.
 | <a id="usercalloutfeaturenameenumsuggest_popover_dismissed"></a>`SUGGEST_POPOVER_DISMISSED` | Callout feature name for suggest_popover_dismissed. |
 | <a id="usercalloutfeaturenameenumtabs_position_highlight"></a>`TABS_POSITION_HIGHLIGHT` | Callout feature name for tabs_position_highlight. |
 | <a id="usercalloutfeaturenameenumthreat_monitoring_info"></a>`THREAT_MONITORING_INFO` | Callout feature name for threat_monitoring_info. |
+| <a id="usercalloutfeaturenameenumtrial_status_reminder_d14"></a>`TRIAL_STATUS_REMINDER_D14` | Callout feature name for trial_status_reminder_d14. |
+| <a id="usercalloutfeaturenameenumtrial_status_reminder_d3"></a>`TRIAL_STATUS_REMINDER_D3` | Callout feature name for trial_status_reminder_d3. |
 | <a id="usercalloutfeaturenameenumultimate_trial"></a>`ULTIMATE_TRIAL` | Callout feature name for ultimate_trial. |
 | <a id="usercalloutfeaturenameenumunfinished_tag_cleanup_callout"></a>`UNFINISHED_TAG_CLEANUP_CALLOUT` | Callout feature name for unfinished_tag_cleanup_callout. |
 | <a id="usercalloutfeaturenameenumweb_ide_alert_dismissed"></a>`WEB_IDE_ALERT_DISMISSED` | Callout feature name for web_ide_alert_dismissed. |

ee/app/assets/javascripts/contextual_sidebar/components/trial_status_popover.vue

@@ -2,6 +2,7 @@
 import { GlButton, GlPopover, GlSprintf } from '@gitlab/ui';
 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import { debounce } from 'lodash';
+import axios from '~/lib/utils/axios_utils';
 import { formatDate } from '~/lib/utils/datetime_utility';
 import { sprintf } from '~/locale';
 import Tracking from '~/tracking';
@@ -24,7 +25,7 @@ export default {
   },
   mixins: [trackingMixin],
   inject: {
-    containerId: { default: null },
+    containerId: {},
     groupName: {},
     planName: {},
     plansHref: {},
@@ -32,6 +33,8 @@ export default {
     startInitiallyShown: { default: false },
     targetId: {},
     trialEndDate: {},
+    userCalloutsPath: {},
+    userCalloutsFeatureId: {},
   },
   data() {
     return {
@@ -62,6 +65,7 @@ export default {
       this.forciblyShowing = true;
       this.showCloseButton = true;
       this.show = true;
+      this.onForciblyShown();
     }
   },
   mounted() {
@@ -78,6 +82,18 @@ export default {
       const { action, ...options } = this.$options.trackingEvents.closeBtnClick;
       this.track(action, options);
     },
+    onForciblyShown() {
+      if (this.userCalloutsPath && this.userCalloutsFeatureId) {
+        axios
+          .post(this.userCalloutsPath, {
+            feature_name: this.userCalloutsFeatureId,
+          })
+          .catch((e) => {
+            // eslint-disable-next-line no-console, @gitlab/require-i18n-strings
+            console.error('Failed to dismiss trial status popover.', e);
+          });
+      }
+    },
     onResize() {
       this.updateDisabledState();
     },

ee/app/assets/javascripts/contextual_sidebar/init_trial_status_widget_and_popover.js

@@ -44,6 +44,8 @@ export const initTrialStatusPopover = () => {
     startInitiallyShown,
     targetId,
     trialEndDate,
+    userCalloutsPath,
+    userCalloutsFeatureId,
   } = el.dataset;
 
   return new Vue({
@@ -57,6 +59,8 @@ export const initTrialStatusPopover = () => {
       startInitiallyShown: startInitiallyShown !== undefined,
       targetId,
       trialEndDate: new Date(trialEndDate),
+      userCalloutsPath,
+      userCalloutsFeatureId,
     },
     render: (createElement) => createElement(TrialStatusPopover),
   });

ee/app/helpers/trial_status_widget_helper.rb

@@ -8,6 +8,8 @@
 module TrialStatusWidgetHelper
   D14_CALLOUT_RANGE = (7..14).freeze # between 14 & 7 days remaining
   D3_CALLOUT_RANGE = (0..3).freeze # between 3 & 0 days remaining
+  D14_CALLOUT_ID = 'trial_status_reminder_d14'
+  D3_CALLOUT_ID = 'trial_status_reminder_d3'
 
   # NOTE: We are okay hard-coding the production value for the Ulitmate 1-year
   # SaaS plan ID while this is all part of an active experiment. If & when the
@@ -23,7 +25,9 @@ module TrialStatusWidgetHelper
       purchase_href: ultimate_subscription_path_for_group(group),
       start_initially_shown: force_popover_to_be_shown?(group),
       target_id: base_attrs[:container_id],
-      trial_end_date: group.trial_ends_on
+      trial_end_date: group.trial_ends_on,
+      user_callouts_path: user_callouts_path,
+      user_callouts_feature_id: current_user_callout_feature_id(group.trial_days_remaining)
     )
   end
 
@@ -52,15 +56,20 @@ module TrialStatusWidgetHelper
   def force_popover_to_be_shown?(group)
     experiment(:forcibly_show_trial_status_popover, group: group) do |e|
       e.use { false }
+      e.try { !dismissed_feature_callout?(current_user_callout_feature_id(group.trial_days_remaining)) }
+      e.run
+    end
+  end
 
-      e.try do
-        days_remaining = group.trial_days_remaining
+  def current_user_callout_feature_id(days_remaining)
+    return D14_CALLOUT_ID if D14_CALLOUT_RANGE.cover?(days_remaining)
+    return D3_CALLOUT_ID if D3_CALLOUT_RANGE.cover?(days_remaining)
+  end
 
-        D14_CALLOUT_RANGE.cover?(days_remaining) || D3_CALLOUT_RANGE.cover?(days_remaining)
-      end
+  def dismissed_feature_callout?(feature_name)
+    return true if feature_name.blank?
 
-      e.run
-    end
+    current_user.dismissed_callout?(feature_name: feature_name)
   end
 
   def trial_status_common_data_attrs(group)

ee/spec/frontend/contextual_sidebar/trial_status_popover_spec.js

@@ -6,6 +6,7 @@ import { POPOVER, TRACKING_PROPERTY } from 'ee/contextual_sidebar/components/con
 import TrialStatusPopover from 'ee/contextual_sidebar/components/trial_status_popover.vue';
 import { mockTracking, unmockTracking } from 'helpers/tracking_helper';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
+import axios from '~/lib/utils/axios_utils';
 
 Vue.config.ignoredElements = ['gl-emoji'];
 
@@ -28,12 +29,16 @@ describe('TrialStatusPopover component', () => {
     return extendedWrapper(
       mountFn(TrialStatusPopover, {
         provide: {
+          containerId: undefined,
           groupName: 'Some Test Group',
           planName: 'Ultimate',
           plansHref: 'billing/path-for/group',
           purchaseHref: 'transactions/new',
+          startInitiallyShown: undefined,
           targetId: 'target-element-identifier',
           trialEndDate: new Date('2021-02-28'),
+          userCalloutsPath: undefined,
+          userCalloutsFeatureId: undefined,
           ...providers,
         },
       }),
@@ -52,7 +57,7 @@ describe('TrialStatusPopover component', () => {
 
   describe('interpolated strings', () => {
     it('correctly interpolates them all', () => {
-      wrapper = createComponent(mount);
+      wrapper = createComponent(undefined, mount);
 
       expect(wrapper.text()).not.toMatch(/%{\w+}/);
     });
@@ -75,6 +80,15 @@ describe('TrialStatusPopover component', () => {
   });
 
   describe('startInitiallyShown', () => {
+    const userCalloutProviders = {
+      userCalloutsPath: 'user_callouts/path',
+      userCalloutsFeatureId: 'feature_id',
+    };
+
+    beforeEach(() => {
+      jest.spyOn(axios, 'post').mockResolvedValue('success');
+    });
+
     describe('when set to true', () => {
       beforeEach(() => {
         wrapper = createComponent({ startInitiallyShown: true });
@@ -87,11 +101,32 @@ describe('TrialStatusPopover component', () => {
       it('removes the popover triggers', () => {
         expect(findGlPopover().attributes('triggers')).toBe('');
       });
+
+      describe('and the user callout values are provided', () => {
+        beforeEach(() => {
+          wrapper = createComponent({
+            startInitiallyShown: true,
+            ...userCalloutProviders,
+          });
+        });
+
+        it('sends a request to update the specified UserCallout record', () => {
+          expect(axios.post).toHaveBeenCalledWith(userCalloutProviders.userCalloutsPath, {
+            feature_name: userCalloutProviders.userCalloutsFeatureId,
+          });
+        });
+      });
+
+      describe('but the user callout values are not provided', () => {
+        it('does not send a request to update a UserCallout record', () => {
+          expect(axios.post).not.toHaveBeenCalled();
+        });
+      });
     });
 
     describe('when set to false', () => {
       beforeEach(() => {
-        wrapper = createComponent({ startInitiallyShown: false });
+        wrapper = createComponent({ ...userCalloutProviders });
       });
 
       it('does not cause the popover to be shown by default', () => {
@@ -101,6 +136,10 @@ describe('TrialStatusPopover component', () => {
       it('uses the standard triggers for the popover', () => {
         expect(findGlPopover().attributes('triggers')).toBe('hover focus');
       });
+
+      it('never sends a request to update a UserCallout record', () => {
+        expect(axios.post).not.toHaveBeenCalled();
+      });
     });
   });
 

ee/spec/helpers/trial_status_widget_helper_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe TrialStatusWidgetHelper do
       {
         container_id: 'trial-status-sidebar-widget',
         plan_name: 'Ultimate',
-        plans_href: '/groups/pants-group/-/billings'
+        plans_href: group_billings_path(group)
       }
     end
 
@@ -36,79 +36,95 @@ RSpec.describe TrialStatusWidgetHelper do
       stub_experiments(forcibly_show_trial_status_popover: :candidate)
     end
 
+    after do
+      travel_back
+    end
+
     describe '#trial_status_popover_data_attrs' do
-      let(:popover_shared_expected_attrs) do
-        shared_expected_attrs.merge(
-          group_name: group.name,
-          purchase_href: new_subscriptions_path(namespace_id: group.id, plan_id: described_class::ZUORA_ULTIMATE_PLAN_ID),
-          target_id: shared_expected_attrs[:container_id],
-          start_initially_shown: false,
-          trial_end_date: trial_end_date
-        )
+      using RSpec::Parameterized::TableSyntax
+
+      d14_callout_id = described_class::D14_CALLOUT_ID
+      d3_callout_id = described_class::D3_CALLOUT_ID
+
+      let_it_be(:user) { create(:user) }
+
+      before do
+        allow(helper).to receive(:current_user).and_return(user)
+        allow(user).to receive(:dismissed_callout?).with(feature_name: user_callouts_feature_id).and_return(dismissed_callout)
       end
 
       subject(:data_attrs) { helper.trial_status_popover_data_attrs(group) }
 
-      shared_examples 'returned data attributes' do |shown: false|
-        it 'returns the correct set of data attributes' do
+      shared_examples 'has correct data attributes' do
+        it 'returns the needed data attributes for mounting the popover Vue component' do
           expect(data_attrs).to match(
-            popover_shared_expected_attrs.merge(
-              start_initially_shown: shown
+            shared_expected_attrs.merge(
+              group_name: group.name,
+              purchase_href: new_subscriptions_path(namespace_id: group.id, plan_id: described_class::ZUORA_ULTIMATE_PLAN_ID),
+              target_id: shared_expected_attrs[:container_id],
+              start_initially_shown: start_initially_shown,
+              trial_end_date: trial_end_date,
+              user_callouts_path: user_callouts_path,
+              user_callouts_feature_id: user_callouts_feature_id
             )
           )
         end
       end
 
-      context 'when more than 14 days remain' do
-        where trial_days_remaining: [15, 22, 30]
-
-        with_them do
-          include_examples 'returned data attributes'
-        end
-      end
-
-      context 'when between 7 & 14 days remain' do
-        where trial_days_remaining: [7, 10, 14]
-
-        with_them do
-          include_examples 'returned data attributes', shown: true
-        end
-      end
-
-      context 'when between 4 & 6 days remain' do
-        where trial_days_remaining: [4, 5, 6]
-
-        with_them do
-          include_examples 'returned data attributes'
-        end
+      where(:trial_days_remaining, :user_callouts_feature_id, :dismissed_callout, :start_initially_shown) do
+        # days| callout ID      | dismissed?  | shown?
+        30    | nil             | false       | false
+        20    | nil             | false       | false
+        15    | nil             | false       | false
+        14    | d14_callout_id  | false       | true
+        14    | d14_callout_id  | true        | false
+        10    | d14_callout_id  | false       | true
+        10    | d14_callout_id  | true        | false
+        7     | d14_callout_id  | false       | true
+        7     | d14_callout_id  | true        | false
+        # days| callout ID      | dismissed?  | shown?
+        6     | nil             | false       | false
+        4     | nil             | false       | false
+        3     | d3_callout_id   | false       | true
+        3     | d3_callout_id   | true        | false
+        1     | d3_callout_id   | false       | true
+        1     | d3_callout_id   | true        | false
+        0     | d3_callout_id   | false       | true
+        0     | d3_callout_id   | true        | false
+        -1    | nil             | false       | false
       end
 
-      context 'when between 0 & 3 days remain' do
-        where trial_days_remaining: [0, 1, 3]
-
-        with_them do
-          include_examples 'returned data attributes', shown: true
-        end
-      end
-
-      context 'when fewer than 0 days remain' do
-        where trial_days_remaining: [-1, -5, -12]
-
-        with_them do
-          include_examples 'returned data attributes'
-        end
-      end
+      with_them { include_examples 'has correct data attributes' }
 
       context 'when not part of the experiment' do
         before do
           stub_experiments(forcibly_show_trial_status_popover: :control)
         end
 
-        where trial_days_remaining: [2, 5, 9, 14, 20]
-
-        with_them do
-          include_examples 'returned data attributes', shown: false
+        where(:trial_days_remaining, :user_callouts_feature_id, :dismissed_callout, :start_initially_shown) do
+          # days| callout ID      | dismissed?  | shown?
+          30    | nil             | false       | false
+          20    | nil             | false       | false
+          15    | nil             | false       | false
+          14    | d14_callout_id  | false       | false
+          14    | d14_callout_id  | true        | false
+          10    | d14_callout_id  | false       | false
+          10    | d14_callout_id  | true        | false
+          7     | d14_callout_id  | false       | false
+          7     | d14_callout_id  | true        | false
+          # days| callout ID      | dismissed?  | shown?
+          6     | nil             | false       | false
+          4     | nil             | false       | false
+          3     | d3_callout_id   | false       | false
+          3     | d3_callout_id   | true        | false
+          1     | d3_callout_id   | false       | false
+          1     | d3_callout_id   | true        | false
+          0     | d3_callout_id   | false       | false
+          0     | d3_callout_id   | true        | false
+          -1    | nil             | false       | false
         end
+
+        with_them { include_examples 'has correct data attributes' }
       end
     end
 
@@ -119,7 +135,7 @@ RSpec.describe TrialStatusWidgetHelper do
 
       subject(:data_attrs) { helper.trial_status_widget_data_attrs(group) }
 
-      it 'returns the needed data attributes for mounting the Vue component' do
+      it 'returns the needed data attributes for mounting the widget Vue component' do
         expect(data_attrs).to match(
           shared_expected_attrs.merge(
             days_remaining: trial_days_remaining,