Skip to content

G
gitlab-ce
Commit f6ab8658 authored by Shinya Maeda's avatar Shinya Maeda Committed by Markus Koller
Browse files
Options

Remove feature_flags_legacy_read_only feature flag 

This commit removes the feature flag.
parent 8cf214eb
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 61 additions and 779 deletions
app/assets/javascripts/feature_flags/components/edit_feature_flag.vue
View file @ f6ab8658
......@@ -30,9 +30,6 @@ export default {
};
},
translations: {
legacyFlagAlert: s__(
'FeatureFlags|GitLab is moving to a new way of managing feature flags, and in 13.4, this feature flag will become read-only. Please create a new feature flag.',
),
legacyReadOnlyFlagAlert: s__(
'FeatureFlags|GitLab is moving to a new way of managing feature flags. This feature flag is read-only, and it will be removed in 14.0. Please create a new feature flag.',
),
......@@ -59,18 +56,6 @@ export default {
deprecated() {
return this.version === LEGACY_FLAG;
},
deprecatedAndEditable() {
return this.deprecated && !this.hasLegacyReadOnlyFlags;
},
deprecatedAndReadOnly() {
return this.deprecated && this.hasLegacyReadOnlyFlags;
},
hasLegacyReadOnlyFlags() {
return (
this.glFeatures.featureFlagsLegacyReadOnly &&
!this.glFeatures.featureFlagsLegacyReadOnlyOverride
);
},
},
created() {
return this.fetchFeatureFlag();
......@@ -91,12 +76,9 @@ export default {
<gl-loading-icon v-if="isLoading" size="xl" class="gl-mt-7" />
<template v-else-if="!isLoading && !hasError">
<gl-alert v-if="deprecatedAndEditable" variant="warning" :dismissible="false" class="gl-my-5">
{{ $options.translations.legacyFlagAlert }}
</gl-alert>
<gl-alert v-if="deprecatedAndReadOnly" variant="warning" :dismissible="false" class="gl-my-5">
{{ $options.translations.legacyReadOnlyFlagAlert }}
</gl-alert>
<gl-alert v-if="deprecated" variant="warning" :dismissible="false" class="gl-my-5">{{
$options.translations.legacyReadOnlyFlagAlert
}}</gl-alert>
<div class="gl-display-flex gl-align-items-center gl-mb-4 gl-mt-4">
<gl-toggle
:value="active"
......
app/assets/javascripts/feature_flags/components/feature_flags_table.vue
View file @ f6ab8658
......@@ -31,19 +31,12 @@ export default {
};
},
translations: {
legacyFlagAlert: s__('FeatureFlags|Flag becomes read only soon'),
legacyFlagReadOnlyAlert: s__('FeatureFlags|Flag is read-only'),
},
computed: {
permissions() {
return this.glFeatures.featureFlagPermissions;
},
isLegacyReadOnlyFlagsEnabled() {
return (
this.glFeatures.featureFlagsLegacyReadOnly &&
!this.glFeatures.featureFlagsLegacyReadOnlyOverride
);
},
modalTitle() {
return sprintf(s__('FeatureFlags|Delete %{name}?'), {
name: this.deleteFeatureFlagName,
......@@ -57,18 +50,13 @@ export default {
modalId() {
return 'delete-feature-flag';
},
legacyFlagToolTipText() {
const { legacyFlagReadOnlyAlert, legacyFlagAlert } = this.$options.translations;
return this.isLegacyReadOnlyFlagsEnabled ? legacyFlagReadOnlyAlert : legacyFlagAlert;
},
},
methods: {
isLegacyFlag(flag) {
return flag.version !== NEW_VERSION_FLAG;
},
statusToggleDisabled(flag) {
return this.isLegacyReadOnlyFlagsEnabled && flag.version === LEGACY_FLAG;
return flag.version === LEGACY_FLAG;
},
scopeTooltipText(scope) {
return !scope.active
......@@ -123,9 +111,7 @@ export default {
<template>
<div class="table-holder js-feature-flag-table">
<div class="gl-responsive-table-row table-row-header" role="row">
<div class="table-section section-10">
{{ s__('FeatureFlags|ID') }}
</div>
<div class="table-section section-10">{{ s__('FeatureFlags|ID') }}</div>
<div class="table-section section-10" role="columnheader">
{{ s__('FeatureFlags|Status') }}
</div>
......@@ -161,9 +147,8 @@ export default {
v-else-if="featureFlag.active"
variant="success"
data-testid="feature-flag-status-badge"
>{{ s__('FeatureFlags|Active') }}</gl-badge
>
{{ s__('FeatureFlags|Active') }}
</gl-badge>
<gl-badge v-else variant="danger">{{ s__('FeatureFlags|Inactive') }}</gl-badge>
</div>
</div>
......@@ -179,7 +164,7 @@ export default {
</div>
<gl-icon
v-if="isLegacyFlag(featureFlag)"
v-gl-tooltip.hover="legacyFlagToolTipText"
v-gl-tooltip.hover="$options.translations.legacyFlagReadOnlyAlert"
class="gl-ml-3"
name="information-o"
/>
......@@ -205,9 +190,8 @@ export default {
:variant="badgeVariant(scope)"
:data-qa-selector="`feature-flag-scope-${badgeVariant(scope)}-badge`"
class="gl-mr-3 gl-mt-2"
>{{ badgeText(scope) }}</gl-badge
>
{{ badgeText(scope) }}
</gl-badge>
</template>
<template v-else>
<gl-badge
......@@ -216,9 +200,8 @@ export default {
data-testid="strategy-badge"
variant="info"
class="gl-mr-3 gl-mt-2"
>{{ strategyBadgeText(strategy) }}</gl-badge
>
{{ strategyBadgeText(strategy) }}
</gl-badge>
</template>
</div>
</div>
......
app/assets/javascripts/feature_flags/components/form.vue
View file @ f6ab8658
......@@ -143,11 +143,7 @@ export default {
return this.featureFlagIssuesEndpoint.length > 0;
},
readOnly() {
return (
this.glFeatures.featureFlagsLegacyReadOnly &&
!this.glFeatures.featureFlagsLegacyReadOnlyOverride &&
this.version === LEGACY_FLAG
);
return this.version === LEGACY_FLAG;
},
},
methods: {
......
app/controllers/projects/feature_flags_controller.rb
View file @ f6ab8658
......@@ -10,12 +10,10 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
before_action :feature_flag, only: [:edit, :update, :destroy]
before_action :ensure_legacy_flags_writable!, only: [:update]
before_action :ensure_flag_writable!, only: [:update]
before_action do
push_frontend_feature_flag(:feature_flag_permissions)
push_frontend_feature_flag(:feature_flags_legacy_read_only, project, default_enabled: true)
push_frontend_feature_flag(:feature_flags_legacy_read_only_override, project)
end
feature_category :feature_flags
......@@ -103,10 +101,8 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
@feature_flag ||= @noteable = project.operations_feature_flags.find_by_iid!(params[:iid])
end
def ensure_legacy_flags_writable!
if ::Feature.enabled?(:feature_flags_legacy_read_only, project, default_enabled: true) &&
::Feature.disabled?(:feature_flags_legacy_read_only_override, project) &&
feature_flag.legacy_flag?
def ensure_flag_writable!
if feature_flag.legacy_flag?
render_error_json(['Legacy feature flags are read-only'])
end
end
......
config/feature_flags/development/feature_flags_legacy_read_only.yml deleted 100644 → 0
View file @ 8cf214eb
---
name: feature_flags_legacy_read_only
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38353
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/240985
milestone: '13.3'
type: development
group: group::progressive delivery
default_enabled: true
config/feature_flags/development/feature_flags_legacy_read_only_override.yml deleted 100644 → 0
View file @ 8cf214eb
---
name: feature_flags_legacy_read_only_override
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40431
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/240985
milestone: '13.4'
type: development
group: group::progressive delivery
default_enabled: false
ee/spec/features/projects/feature_flags/user_sees_feature_flag_list_spec.rb
View file @ f6ab8658
......@@ -16,66 +16,6 @@ RSpec.describe 'User sees feature flag list', :js do
sign_in(user)
end
context 'with legacy feature flags' do
before do
create_flag(project, 'ci_live_trace', false).tap do |feature_flag|
create_scope(feature_flag, 'review/*', true)
end
create_flag(project, 'drop_legacy_artifacts', false)
create_flag(project, 'mr_train', true).tap do |feature_flag|
create_scope(feature_flag, 'production', false)
end
stub_feature_flags(feature_flags_legacy_read_only_override: false)
end
context 'when legacy feature flags are not read-only' do
before do
stub_feature_flags(feature_flags_legacy_read_only: false)
end
it 'user updates the status toggle' do
visit(project_feature_flags_path(project))
within_feature_flag_row(1) do
status_toggle_button.click
expect_status_toggle_button_to_be_checked
end
visit(project_audit_events_path(project))
expect(page).to(
have_text('Updated feature flag ci_live_trace. Updated active from "false" to "true".')
)
end
end
context 'when legacy feature flags are read-only but the override is active for a project' do
before do
stub_feature_flags(
feature_flags_legacy_read_only: true,
feature_flags_legacy_read_only_override: project
)
end
it 'user updates the status toggle' do
visit(project_feature_flags_path(project))
within_feature_flag_row(1) do
status_toggle_button.click
expect_status_toggle_button_to_be_checked
end
visit(project_audit_events_path(project))
expect(page).to(
have_text('Updated feature flag ci_live_trace. Updated active from "false" to "true".')
)
end
end
end
context 'with new version flags' do
before do
create(:operations_feature_flag, :new_version_flag, project: project,
......
ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb deleted 100644 → 0
View file @ 8cf214eb
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'User updates feature flag', :js do
include FeatureFlagHelpers
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, namespace: user.namespace) }
before_all do
project.add_developer(user)
end
before do
stub_feature_flags(
feature_flag_permissions: false,
feature_flags_legacy_read_only_override: false
)
sign_in(user)
end
context 'with a legacy feature flag' do
let!(:feature_flag) do
create_flag(project, 'ci_live_trace', true,
description: 'For live trace feature')
end
let!(:scope) { create_scope(feature_flag, 'review/*', true) }
context 'when legacy flags are editable' do
before do
stub_feature_flags(feature_flags_legacy_read_only: false)
visit(edit_project_feature_flag_path(project, feature_flag))
end
context 'when user updates the status of a scope' do
before do
within_scope_row(2) do
within_status { find('.project-feature-toggle').click }
end
click_button 'Save changes'
expect(page).to have_current_path(project_feature_flags_path(project))
end
it 'records audit event' do
visit(project_audit_events_path(project))
expect(page).to(
have_text("Updated feature flag ci_live_trace. Updated rule review/* active state from true to false.")
)
end
end
context 'when user adds a new scope' do
before do
within_scope_row(3) do
within_environment_spec do
find('.js-env-search > input').set('production')
find('.js-create-button').click
end
end
click_button 'Save changes'
expect(page).to have_current_path(project_feature_flags_path(project))
end
it 'records audit event' do
visit(project_audit_events_path(project))
expect(page).to have_text "Updated feature flag ci_live_trace"
end
end
context 'when user deletes a scope' do
before do
within_scope_row(2) do
within_delete { find('.js-delete-scope').click }
end
click_button 'Save changes'
expect(page).to have_current_path(project_feature_flags_path(project))
end
it 'records audit event' do
visit(project_audit_events_path(project))
expect(page).to have_text "Updated feature flag ci_live_trace"
end
end
end
end
end
locale/gitlab.pot
View file @ f6ab8658
......@@ -12113,9 +12113,6 @@ msgstr ""
msgid "FeatureFlags|Feature flags limit reached (%{featureFlagsLimit}). Delete one or more feature flags before adding new ones."
msgstr ""
msgid "FeatureFlags|Flag becomes read only soon"
msgstr ""
msgid "FeatureFlags|Flag is read-only"
msgstr ""
......@@ -12125,9 +12122,6 @@ msgstr ""
msgid "FeatureFlags|Get started with user lists"
msgstr ""
msgid "FeatureFlags|GitLab is moving to a new way of managing feature flags, and in 13.4, this feature flag will become read-only. Please create a new feature flag."
msgstr ""
msgid "FeatureFlags|GitLab is moving to a new way of managing feature flags. This feature flag is read-only, and it will be removed in 14.0. Please create a new feature flag."
msgstr ""
......
spec/features/projects/feature_flags/user_sees_feature_flag_list_spec.rb
View file @ f6ab8658
......@@ -25,7 +25,6 @@ RSpec.describe 'User sees feature flag list', :js do
create_flag(project, 'mr_train', true).tap do |feature_flag|
create_scope(feature_flag, 'production', false)
end
stub_feature_flags(feature_flags_legacy_read_only_override: false)
end
it 'user sees the first flag' do
......@@ -79,41 +78,6 @@ RSpec.describe 'User sees feature flag list', :js do
expect_status_toggle_button_to_be_disabled
end
end
context 'when legacy feature flags are not read-only' do
before do
stub_feature_flags(feature_flags_legacy_read_only: false)
end
it 'user updates the status toggle' do
visit(project_feature_flags_path(project))
within_feature_flag_row(1) do
status_toggle_button.click
expect_status_toggle_button_to_be_checked
end
end
end
context 'when legacy feature flags are read-only but the override is active for a project' do
before do
stub_feature_flags(
feature_flags_legacy_read_only: true,
feature_flags_legacy_read_only_override: project
)
end
it 'user updates the status toggle' do
visit(project_feature_flags_path(project))
within_feature_flag_row(1) do
status_toggle_button.click
expect_status_toggle_button_to_be_checked
end
end
end
end
context 'with new version flags' do
......
spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb
View file @ f6ab8658
......@@ -14,8 +14,7 @@ RSpec.describe 'User updates feature flag', :js do
before do
stub_feature_flags(
feature_flag_permissions: false,
feature_flags_legacy_read_only_override: false
feature_flag_permissions: false
)
sign_in(user)
end
......@@ -79,117 +78,11 @@ RSpec.describe 'User updates feature flag', :js do
let!(:scope) { create_scope(feature_flag, 'review/*', true) }
context 'when legacy flags are editable' do
before do
stub_feature_flags(feature_flags_legacy_read_only: false)
visit(edit_project_feature_flag_path(project, feature_flag))
end
it 'user sees persisted default scope' do
within_scope_row(1) do
within_environment_spec do
expect(page).to have_content('* (All Environments)')
end
within_status do
expect(find('.project-feature-toggle')['aria-label'])
.to eq('Toggle Status: ON')
end
end
end
context 'when user updates the status of a scope' do
before do
within_scope_row(2) do
within_status { find('.project-feature-toggle').click }
end
click_button 'Save changes'
expect(page).to have_current_path(project_feature_flags_path(project))
end
it 'shows the updated feature flag' do
within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect_status_toggle_button_to_be_checked
within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*')
expect(page.find('.badge:nth-child(1)')['class']).to include('badge-info')
expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
expect(page.find('.badge:nth-child(2)')['class']).to include('badge-muted')
end
end
end
end
context 'when user adds a new scope' do
before do
within_scope_row(3) do
within_environment_spec do
find('.js-env-search > input').set('production')
find('.js-create-button').click
end
end
click_button 'Save changes'
expect(page).to have_current_path(project_feature_flags_path(project))
end
it 'shows the newly created scope' do
within_feature_flag_row(1) do
within_feature_flag_scopes do
expect(page.find('.badge:nth-child(3)')).to have_content('production')
expect(page.find('.badge:nth-child(3)')['class']).to include('badge-muted')
end
end
end
end
context 'when user deletes a scope' do
before do
within_scope_row(2) do
within_delete { find('.js-delete-scope').click }
end
click_button 'Save changes'
expect(page).to have_current_path(project_feature_flags_path(project))
end
it 'shows the updated feature flag' do
within_feature_flag_row(1) do
within_feature_flag_scopes do
expect(page).to have_css('.badge:nth-child(1)')
expect(page).not_to have_css('.badge:nth-child(2)')
end
end
end
end
end
context 'when legacy flags are read-only' do
it 'the user cannot edit the flag' do
visit(edit_project_feature_flag_path(project, feature_flag))
expect(page).to have_text 'This feature flag is read-only, and it will be removed in 14.0.'
expect(page).to have_css('button.js-ff-submit.disabled')
end
end
context 'when legacy flags are read-only, but the override is active for one project' do
it 'the user can edit the flag' do
stub_feature_flags(feature_flags_legacy_read_only_override: project)
visit(edit_project_feature_flag_path(project, feature_flag))
status_toggle_button.click
click_button 'Save changes'
it 'the user cannot edit the flag' do
visit(edit_project_feature_flag_path(project, feature_flag))
expect(page).to have_current_path(project_feature_flags_path(project))
within_feature_flag_row(1) do
expect_status_toggle_button_not_to_be_checked
end
end
expect(page).to have_text 'This feature flag is read-only, and it will be removed in 14.0.'
expect(page).to have_css('button.js-ff-submit.disabled')
end
end
end
spec/frontend/feature_flags/components/feature_flags_table_spec.js
View file @ f6ab8658
......@@ -120,13 +120,11 @@ describe('Feature flag table', () => {
describe('when active and with an update toggle', () => {
let toggle;
let spy;
beforeEach(() => {
props.featureFlags[0].update_path = props.featureFlags[0].destroy_path;
createWrapper(props);
toggle = wrapper.find(GlToggle);
spy = mockTracking('_category_', toggle.element, jest.spyOn);
});
it('should have a toggle', () => {
......@@ -142,14 +140,6 @@ describe('Feature flag table', () => {
expect(wrapper.emitted('toggle-flag')).toEqual([[flag]]);
});
});
it('should track a click', () => {
toggle.trigger('click');
expect(spy).toHaveBeenCalledWith('_category_', 'click_button', {
label: 'feature_flag_toggle',
});
});
});
describe('with an active scope and a percentage rollout strategy', () => {
......@@ -180,6 +170,8 @@ describe('Feature flag table', () => {
});
describe('with a new version flag', () => {
let toggle;
let spy;
let badges;
beforeEach(() => {
......@@ -194,6 +186,7 @@ describe('Feature flag table', () => {
description: 'flag description',
destroy_path: 'destroy/path',
edit_path: 'edit/path',
update_path: 'update/path',
version: NEW_VERSION_FLAG,
scopes: [],
strategies: [
......@@ -226,6 +219,8 @@ describe('Feature flag table', () => {
provide: { csrfToken: 'fakeToken', glFeatures: { featureFlagsNewVersion: true } },
});
toggle = wrapper.find(GlToggle);
spy = mockTracking('_category_', toggle.element, jest.spyOn);
badges = wrapper.findAll('[data-testid="strategy-badge"]');
});
......@@ -254,6 +249,14 @@ describe('Feature flag table', () => {
it('shows the name of a user list for user list', () => {
expect(badges.at(3).text()).toContain('User List - test list');
});
it('tracks a click', () => {
toggle.trigger('click');
expect(spy).toHaveBeenCalledWith('_category_', 'click_button', {
label: 'feature_flag_toggle',
});
});
});
it('renders a feature flag without an iid', () => {
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7