Merge branch '233092-persist-ff-will-become-read-only-message' into 'master'

Persist when dismissing FF will look different message

See merge request gitlab-org/gitlab!39238

app/models/concerns/enums/user_callout.rb

@@ -23,7 +23,8 @@ module Enums
         web_ide_alert_dismissed: 16,
         personal_access_token_expiry: 21, # EE-only
         suggest_pipeline: 22,
-        customize_homepage: 23
+        customize_homepage: 23,
+        feature_flags_new_version: 24
       }
     end
   end

ee/app/assets/javascripts/feature_flags/components/edit_feature_flag.vue

 <script>
 import { GlAlert, GlLoadingIcon, GlToggle } from '@gitlab/ui';
 import { createNamespacedHelpers } from 'vuex';
+import axios from '~/lib/utils/axios_utils';
 import { sprintf, s__ } from '~/locale';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { LEGACY_FLAG, NEW_FLAG_ALERT } from '../constants';
@@ -39,10 +40,24 @@ export default {
       type: String,
       required: true,
     },
+    showUserCallout: {
+      type: Boolean,
+      required: true,
+    },
+    userCalloutId: {
+      default: '',
+      type: String,
+      required: false,
+    },
+    userCalloutsPath: {
+      default: '',
+      type: String,
+      required: false,
+    },
   },
   data() {
     return {
-      userDidDismissNewFlagAlert: false,
+      userShouldSeeNewFlagAlert: this.showUserCallout,
     };
   },
   translations: {
@@ -91,7 +106,7 @@ export default {
       );
     },
     shouldShowNewFlagAlert() {
-      return !(this.hasNewVersionFlags || this.userDidDismissNewFlagAlert);
+      return !this.hasNewVersionFlags && this.userShouldSeeNewFlagAlert;
     },
   },
   created() {
@@ -106,6 +121,12 @@ export default {
       'fetchFeatureFlag',
       'toggleActive',
     ]),
+    dismissNewVersionFlagAlert() {
+      this.userShouldSeeNewFlagAlert = false;
+      axios.post(this.userCalloutsPath, {
+        feature_name: this.userCalloutId,
+      });
+    },
   },
 };
 </script>
@@ -115,7 +136,7 @@ export default {
       v-if="shouldShowNewFlagAlert"
       variant="warning"
       class="gl-my-5"
-      @dismiss="userDidDismissNewFlagAlert = true"
+      @dismiss="dismissNewVersionFlagAlert"
     >
       {{ $options.translations.newFlagAlert }}
     </gl-alert>

ee/app/assets/javascripts/feature_flags/components/new_feature_flag.vue

 <script>
 import { createNamespacedHelpers } from 'vuex';
 import { GlAlert } from '@gitlab/ui';
+import axios from '~/lib/utils/axios_utils';
 import store from '../store/index';
 import FeatureFlagForm from './form.vue';
 import {
@@ -39,10 +40,24 @@ export default {
       type: String,
       required: true,
     },
+    showUserCallout: {
+      type: Boolean,
+      required: true,
+    },
+    userCalloutId: {
+      default: '',
+      type: String,
+      required: false,
+    },
+    userCalloutsPath: {
+      default: '',
+      type: String,
+      required: false,
+    },
   },
   data() {
     return {
-      userDidDismissNewFlagAlert: false,
+      userShouldSeeNewFlagAlert: this.showUserCallout,
     };
   },
   translations: {
@@ -68,7 +83,7 @@ export default {
       return this.glFeatures.featureFlagsNewVersion;
     },
     shouldShowNewFlagAlert() {
-      return !(this.hasNewVersionFlags || this.userDidDismissNewFlagAlert);
+      return !this.hasNewVersionFlags && this.userShouldSeeNewFlagAlert;
     },
     strategies() {
       return [{ name: ROLLOUT_STRATEGY_ALL_USERS, parameters: {}, scopes: [] }];
@@ -80,6 +95,12 @@ export default {
   },
   methods: {
     ...mapActions(['createFeatureFlag', 'setEndpoint', 'setPath']),
+    dismissNewVersionFlagAlert() {
+      this.userShouldSeeNewFlagAlert = false;
+      axios.post(this.userCalloutsPath, {
+        feature_name: this.userCalloutId,
+      });
+    },
   },
 };
 </script>
@@ -89,7 +110,7 @@ export default {
       v-if="shouldShowNewFlagAlert"
       variant="warning"
       class="gl-my-5"
-      @dismiss="userDidDismissNewFlagAlert = true"
+      @dismiss="dismissNewVersionFlagAlert"
     >
       {{ $options.translations.newFlagAlert }}
     </gl-alert>

ee/app/assets/javascripts/feature_flags/edit.js

 import Vue from 'vue';
 import EditFeatureFlag from 'ee/feature_flags/components/edit_feature_flag.vue';
+import { parseBoolean } from '~/lib/utils/common_utils';
 
 export default () => {
   const el = document.querySelector('#js-edit-feature-flag');
@@ -17,6 +18,9 @@ export default () => {
           environmentsEndpoint: el.dataset.environmentsEndpoint,
           projectId: el.dataset.projectId,
           featureFlagIssuesEndpoint: el.dataset.featureFlagIssuesEndpoint,
+          userCalloutsPath: el.dataset.userCalloutsPath,
+          userCalloutId: el.dataset.userCalloutId,
+          showUserCallout: parseBoolean(el.dataset.showUserCallout),
         },
       });
     },

ee/app/assets/javascripts/feature_flags/new.js

 import Vue from 'vue';
 import NewFeatureFlag from 'ee/feature_flags/components/new_feature_flag.vue';
+import { parseBoolean } from '~/lib/utils/common_utils';
 
 export default () => {
   const el = document.querySelector('#js-new-feature-flag');
@@ -16,6 +17,9 @@ export default () => {
           path: el.dataset.featureFlagsPath,
           environmentsEndpoint: el.dataset.environmentsEndpoint,
           projectId: el.dataset.projectId,
+          userCalloutsPath: el.dataset.userCalloutsPath,
+          userCalloutId: el.dataset.userCalloutId,
+          showUserCallout: parseBoolean(el.dataset.showUserCallout),
         },
       });
     },

ee/app/helpers/ee/user_callouts_helper.rb

@@ -13,6 +13,7 @@ module EE
     ACCOUNT_RECOVERY_REGULAR_CHECK = 'account_recovery_regular_check'
     ACTIVE_USER_COUNT_THRESHOLD = 'active_user_count_threshold'
     PERSONAL_ACCESS_TOKEN_EXPIRY = 'personal_access_token_expiry'
+    FEATURE_FLAGS_NEW_VERISION = 'feature_flags_new_version'
 
     def show_canary_deployment_callout?(project)
       !user_dismissed?(CANARY_DEPLOYMENT) &&
@@ -90,6 +91,10 @@ module EE
         !user_dismissed?(PERSONAL_ACCESS_TOKEN_EXPIRY, 1.week.ago)
     end
 
+    def show_feature_flags_new_version?
+      !user_dismissed?(FEATURE_FLAGS_NEW_VERISION)
+    end
+
     private
 
     def hashed_storage_enabled?

ee/app/helpers/feature_flags_helper.rb

@@ -10,4 +10,10 @@ module FeatureFlagsHelper
   def unleash_api_instance_id(project)
     project.feature_flags_client_token
   end
+
+  def feature_flag_issues_links_endpoint(project, feature_flag, user)
+    return '' unless Feature.enabled?(:feature_flags_issue_links, project, default_enabled: true) && can?(user, :read_issue_link, project)
+
+    project_feature_flag_issues_path(project, feature_flag)
+  end
 end

ee/app/views/projects/feature_flags/edit.html.haml

@@ -8,4 +8,7 @@
                       project_id: @project.id,
                       feature_flags_path: project_feature_flags_path(@project),
                       environments_endpoint: search_project_environments_path(@project, format: :json),
-                      feature_flag_issues_endpoint: Feature.enabled?(:feature_flags_issue_links, @project, default_enabled: true) && can?(current_user, :read_issue_link, @project) ? project_feature_flag_issues_path(@project, @feature_flag) : ''} }
+                      user_callouts_path: user_callouts_path,
+                      user_callout_id: UserCalloutsHelper::FEATURE_FLAGS_NEW_VERISION,
+                      show_user_callout: show_feature_flags_new_version?.to_s,
+                      feature_flag_issues_endpoint: feature_flag_issues_links_endpoint(@project, @feature_flag, current_user) } }

ee/app/views/projects/feature_flags/new.html.haml

@@ -6,4 +6,7 @@
 #js-new-feature-flag{ data: { endpoint: project_feature_flags_path(@project, format: :json),
                      feature_flags_path: project_feature_flags_path(@project),
                      environments_endpoint: search_project_environments_path(@project, format: :json),
+                     user_callouts_path: user_callouts_path,
+                     user_callout_id: UserCalloutsHelper::FEATURE_FLAGS_NEW_VERISION,
+                     show_user_callout: show_feature_flags_new_version?.to_s,
                      project_id: @project.id } }

ee/changelogs/unreleased/233092-persist-when-dismissing-ff-will-look-different-message.yml

+---
+title: Persist when dismissing FF will look different message
+merge_request: 39238
+author:
+type: changed

ee/spec/frontend/feature_flags/components/edit_feature_flag_spec.js

@@ -13,6 +13,9 @@ import axios from '~/lib/utils/axios_utils';
 const localVue = createLocalVue();
 localVue.use(Vuex);
 
+const userCalloutId = 'feature_flags_new_version';
+const userCalloutsPath = `${TEST_HOST}/user_callouts`;
+
 describe('Edit feature flag form', () => {
   let wrapper;
   let mock;
@@ -36,6 +39,9 @@ describe('Edit feature flag form', () => {
         environmentsEndpoint: 'environments.json',
         projectId: '8',
         featureFlagIssuesEndpoint: `${TEST_HOST}/feature_flags/5/issues`,
+        showUserCallout: true,
+        userCalloutId,
+        userCalloutsPath,
       },
       store,
       provide: {
@@ -49,7 +55,6 @@ describe('Edit feature flag form', () => {
 
   beforeEach(done => {
     mock = new MockAdapter(axios);
-
     mock.onGet(`${TEST_HOST}/feature_flags.json`).replyOnce(200, {
       id: 21,
       iid: 5,
@@ -71,9 +76,7 @@ describe('Edit feature flag form', () => {
         },
       ],
     });
-
     factory();
-
     setImmediate(() => done());
   });
 
@@ -82,6 +85,8 @@ describe('Edit feature flag form', () => {
     mock.restore();
   });
 
+  const findAlert = () => wrapper.find(GlAlert);
+
   it('should display the iid', () => {
     expect(wrapper.find('h3').text()).toContain('^5');
   });
@@ -95,13 +100,12 @@ describe('Edit feature flag form', () => {
   });
 
   it('should not alert users that feature flags are changing soon', () => {
-    expect(wrapper.find(GlAlert).text()).not.toBe(NEW_FLAG_ALERT);
+    expect(findAlert().text()).toContain('GitLab is moving to a new way of managing feature flags');
   });
 
   describe('with error', () => {
     it('should render the error', () => {
       store.dispatch('edit/receiveUpdateFeatureFlagError', { message: ['The name is required'] });
-
       return wrapper.vm.$nextTick(() => {
         expect(wrapper.find('.alert-danger').exists()).toEqual(true);
         expect(wrapper.find('.alert-danger').text()).toContain('The name is required');
@@ -167,11 +171,28 @@ describe('Edit feature flag form', () => {
     beforeEach(() => factory({ provide: { glFeatures: { featureFlagsNewVersion: false } } }));
 
     it('should alert users that feature flags are changing soon', () => {
-      expect(wrapper.find(GlAlert).text()).toBe(NEW_FLAG_ALERT);
+      expect(findAlert().text()).toBe(NEW_FLAG_ALERT);
+    });
+  });
+
+  describe('dismissing new version alert', () => {
+    beforeEach(() => {
+      factory({ provide: { glFeatures: { featureFlagsNewVersion: false } } });
+      mock.onPost(userCalloutsPath, { feature_name: userCalloutId }).reply(200);
+      findAlert().vm.$emit('dismiss');
+      return wrapper.vm.$nextTick();
+    });
+
+    afterEach(() => {
+      mock.restore();
+    });
+
+    it('should hide the alert', () => {
+      expect(findAlert().exists()).toBe(false);
     });
 
-    it('the new feature flags alert should be dismissable', () => {
-      expect(wrapper.find(GlAlert).props('dismissible')).toBe(true);
+    it('should send the dismissal event', () => {
+      expect(mock.history.post.length).toBe(1);
     });
   });
 });

ee/spec/frontend/feature_flags/components/new_feature_flag_spec.js

 import Vuex from 'vuex';
 import { shallowMount } from '@vue/test-utils';
+import MockAdapter from 'axios-mock-adapter';
 import { GlAlert } from '@gitlab/ui';
 import Form from 'ee/feature_flags/components/form.vue';
 import newModule from 'ee/feature_flags/store/modules/new';
@@ -10,8 +11,12 @@ import {
   NEW_FLAG_ALERT,
 } from 'ee/feature_flags/constants';
 import { TEST_HOST } from 'spec/test_constants';
+import axios from '~/lib/utils/axios_utils';
 import { allUsersStrategy } from '../mock_data';
 
+const userCalloutId = 'feature_flags_new_version';
+const userCalloutsPath = `${TEST_HOST}/user_callouts`;
+
 describe('New feature flag form', () => {
   let wrapper;
 
@@ -32,6 +37,9 @@ describe('New feature flag form', () => {
         path: '/feature_flags',
         environmentsEndpoint: 'environments.json',
         projectId: '8',
+        showUserCallout: true,
+        userCalloutId,
+        userCalloutsPath,
       },
       store,
       provide: {
@@ -51,6 +59,8 @@ describe('New feature flag form', () => {
     wrapper.destroy();
   });
 
+  const findAlert = () => wrapper.find(GlAlert);
+
   describe('with error', () => {
     it('should render the error', () => {
       store.dispatch('new/receiveCreateFeatureFlagError', { message: ['The name is required'] });
@@ -105,11 +115,31 @@ describe('New feature flag form', () => {
     beforeEach(() => factory({ provide: { glFeatures: { featureFlagsNewVersion: false } } }));
 
     it('should alert users that feature flags are changing soon', () => {
-      expect(wrapper.find(GlAlert).text()).toBe(NEW_FLAG_ALERT);
+      expect(findAlert().text()).toBe(NEW_FLAG_ALERT);
+    });
+  });
+
+  describe('dismissing new version alert', () => {
+    let mock;
+
+    beforeEach(() => {
+      mock = new MockAdapter(axios);
+      mock.onPost(userCalloutsPath, { feature_name: userCalloutId }).reply(200);
+      factory({ provide: { glFeatures: { featureFlagsNewVersion: false } } });
+      findAlert().vm.$emit('dismiss');
+      return wrapper.vm.$nextTick();
+    });
+
+    afterEach(() => {
+      mock.restore();
+    });
+
+    it('should hide the alert', () => {
+      expect(findAlert().exists()).toBe(false);
     });
 
-    it('the new feature flags alert should be dismissable', () => {
-      expect(wrapper.find(GlAlert).props('dismissible')).toBe(true);
+    it('should send the dismissal event', () => {
+      expect(mock.history.post.length).toBe(1);
     });
   });
 });

ee/spec/helpers/ee/user_callouts_helper_spec.rb

@@ -344,6 +344,28 @@ RSpec.describe EE::UserCalloutsHelper do
     end
   end
 
+  describe '.show_feature_flags_new_version?' do
+    subject { helper.show_feature_flags_new_version? }
+
+    let(:user) { create(:user) }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    context 'when the feature flags new version info has not been dismissed' do
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when the feature flags new version has been dismissed' do
+      before do
+        create(:user_callout, user: user, feature_name: described_class::FEATURE_FLAGS_NEW_VERISION)
+      end
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
   describe '.show_token_expiry_notification?' do
     subject { helper.show_token_expiry_notification? }
 

ee/spec/helpers/feature_flags_helper_spec.rb

@@ -4,6 +4,8 @@ require 'spec_helper'
 
 RSpec.describe FeatureFlagsHelper do
   let_it_be(:project) { create(:project) }
+  let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) }
+  let_it_be(:user) { create(:user) }
 
   describe '#unleash_api_url' do
     subject { helper.unleash_api_url(project) }
@@ -16,4 +18,26 @@ RSpec.describe FeatureFlagsHelper do
 
     it { is_expected.not_to be_empty }
   end
+
+  describe '#feature_flag_issues_links_endpoint' do
+    subject { helper.feature_flag_issues_links_endpoint(project, feature_flag, user) }
+
+    it 'returns an empty string when the feature flag is disabled' do
+      stub_feature_flags(feature_flags_issue_links: false)
+
+      is_expected.to be_empty
+    end
+
+    it 'returns an empty string when the user is not allowed' do
+      allow(helper).to receive(:can?).with(user, :read_issue_link, project).and_return(false)
+
+      is_expected.to be_empty
+    end
+
+    it 'returns the issue endpoint when the user is allowed' do
+      allow(helper).to receive(:can?).with(user, :read_issue_link, project).and_return(true)
+
+      is_expected.to eq("/#{project.full_path}/-/feature_flags/#{feature_flag.iid}/issues")
+    end
+  end
 end