Commit bef0f644 authored by Angelo Gulina's avatar Angelo Gulina

Persist FF will change alert on user dismissal

- apply User Callouts solution to persist user alert dismissal
- add/update relevant test cases
parent 128b7689
......@@ -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
......
<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>
......
<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>
......
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),
},
});
},
......
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),
},
});
},
......
......@@ -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?
......
......@@ -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
......@@ -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) } }
......@@ -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 } }
---
title: Persist when dismissing FF will look different message
merge_request: 39238
author:
type: changed
......@@ -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);
});
});
});
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);
});
});
});
......@@ -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? }
......
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment