Commit 1e61436a authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch 'ff-toggle-backend' into 'master'

Add Feature Flag Override Toggle

See merge request gitlab-org/gitlab!21598
parents e77a7025 10ef2cf5
---
title: Add feature flag override toggle
merge_request: 21598
author:
type: added
# frozen_string_literal: true
class BackfillOperationsFeatureFlagsActive < ActiveRecord::Migration[5.2]
DOWNTIME = false
disable_ddl_transaction!
class OperationsFeatureFlag < ActiveRecord::Base
self.table_name = 'operations_feature_flags'
self.inheritance_column = :_type_disabled
end
def up
OperationsFeatureFlag.where(active: false).update_all(active: true)
end
def down
# no-op
end
end
...@@ -51,7 +51,10 @@ with ability to edit or remove them. ...@@ -51,7 +51,10 @@ with ability to edit or remove them.
To make a feature flag active or inactive, click the pencil icon to edit it, To make a feature flag active or inactive, click the pencil icon to edit it,
and toggle the status for each [spec](#define-environment-specs). and toggle the status for each [spec](#define-environment-specs).
![Feature flags list](img/feature_flags_list.png) The toggles next to each feature flag on the list page function as global shutoff switches.
If a toggle is off, that feature flag is disabled for every environment.
![Feature flags list](img/feature_flags_list_v12_7.png)
## Define environment specs ## Define environment specs
......
...@@ -49,9 +49,6 @@ export default { ...@@ -49,9 +49,6 @@ export default {
hasFeatureFlagsIID() { hasFeatureFlagsIID() {
return this.glFeatures.featureFlagIID && this.iid; return this.glFeatures.featureFlagIID && this.iid;
}, },
hasFeatureFlagToggle() {
return this.glFeatures.featureFlagToggle;
},
}, },
created() { created() {
this.setPath(this.path); this.setPath(this.path);
...@@ -74,12 +71,7 @@ export default { ...@@ -74,12 +71,7 @@ export default {
<template v-else-if="!isLoading && !hasError"> <template v-else-if="!isLoading && !hasError">
<div class="d-flex align-items-center mb-3 mt-3"> <div class="d-flex align-items-center mb-3 mt-3">
<gl-toggle <gl-toggle :value="active" class="m-0 mr-3" @change="toggleActive" />
v-if="hasFeatureFlagToggle"
:value="active"
class="m-0 mr-3"
@change="toggleActive"
/>
<h3 class="page-title m-0">{{ title }}</h3> <h3 class="page-title m-0">{{ title }}</h3>
</div> </div>
...@@ -94,7 +86,7 @@ export default { ...@@ -94,7 +86,7 @@ export default {
:cancel-path="path" :cancel-path="path"
:submit-text="__('Save changes')" :submit-text="__('Save changes')"
:environments-endpoint="environmentsEndpoint" :environments-endpoint="environmentsEndpoint"
:active="active || !hasFeatureFlagToggle" :active="active"
@handleSubmit="data => updateFeatureFlag(data)" @handleSubmit="data => updateFeatureFlag(data)"
/> />
</template> </template>
......
...@@ -62,9 +62,6 @@ export default { ...@@ -62,9 +62,6 @@ export default {
modalId() { modalId() {
return 'delete-feature-flag'; return 'delete-feature-flag';
}, },
hasFeatureFlagToggle() {
return this.glFeatures.featureFlagToggle;
},
}, },
methods: { methods: {
scopeTooltipText(scope) { scopeTooltipText(scope) {
...@@ -133,7 +130,7 @@ export default { ...@@ -133,7 +130,7 @@ export default {
<div class="table-mobile-header" role="rowheader">{{ s__('FeatureFlags|Status') }}</div> <div class="table-mobile-header" role="rowheader">{{ s__('FeatureFlags|Status') }}</div>
<div class="table-mobile-content js-feature-flag-status"> <div class="table-mobile-content js-feature-flag-status">
<gl-toggle <gl-toggle
v-if="hasFeatureFlagToggle && featureFlag.update_path" v-if="featureFlag.update_path"
:value="featureFlag.active" :value="featureFlag.active"
@change="toggleFeatureFlag(featureFlag)" @change="toggleFeatureFlag(featureFlag)"
/> />
......
...@@ -7,10 +7,14 @@ const mapFlag = flag => ({ ...flag, scopes: mapToScopesViewModel(flag.scopes || ...@@ -7,10 +7,14 @@ const mapFlag = flag => ({ ...flag, scopes: mapToScopesViewModel(flag.scopes ||
const updateFlag = (state, flag) => { const updateFlag = (state, flag) => {
const i = state.featureFlags.findIndex(({ id }) => id === flag.id); const i = state.featureFlags.findIndex(({ id }) => id === flag.id);
const staleFlag = state.featureFlags.find(({ id }) => id === flag.id);
Vue.set(state.featureFlags, i, flag); Vue.set(state.featureFlags, i, flag);
Vue.set(state.count, 'enabled', state.featureFlags.filter(({ active }) => active).length); if (staleFlag.active !== flag.active) {
Vue.set(state.count, 'disabled', state.featureFlags.filter(({ active }) => !active).length); const change = flag.active ? 1 : -1;
Vue.set(state.count, 'enabled', state.count.enabled + change);
Vue.set(state.count, 'disabled', state.count.disabled - change);
}
}; };
export default { export default {
...@@ -67,7 +71,7 @@ export default { ...@@ -67,7 +71,7 @@ export default {
state.hasRotateError = true; state.hasRotateError = true;
}, },
[types.UPDATE_FEATURE_FLAG](state, flag) { [types.UPDATE_FEATURE_FLAG](state, flag) {
updateFlag(state, mapFlag(flag)); updateFlag(state, flag);
}, },
[types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](state, data) { [types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](state, data) {
updateFlag(state, mapFlag(data)); updateFlag(state, mapFlag(data));
......
...@@ -94,7 +94,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController ...@@ -94,7 +94,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
protected protected
def feature_flag def feature_flag
@feature_flag ||= project.operations_feature_flags.for_list.find(params[:id]) @feature_flag ||= project.operations_feature_flags.find(params[:id])
end end
def create_params def create_params
......
...@@ -17,7 +17,6 @@ class FeatureFlagsFinder ...@@ -17,7 +17,6 @@ class FeatureFlagsFinder
items = feature_flags items = feature_flags
items = by_scope(items) items = by_scope(items)
items = for_list(items)
items = items.preload_relations if preload items = items.preload_relations if preload
items.ordered items.ordered
...@@ -35,8 +34,4 @@ class FeatureFlagsFinder ...@@ -35,8 +34,4 @@ class FeatureFlagsFinder
items items
end end
end end
def for_list(items)
items.for_list
end
end end
# frozen_string_literal: true # frozen_string_literal: true
module Operations module Operations
##
# NOTE:
# "operations_feature_flags.active" column is not used in favor of
# operations_feature_flag_scopes's override policy.
# You can calculate actual `active` values with `for_environment` method.
class FeatureFlag < ApplicationRecord class FeatureFlag < ApplicationRecord
self.table_name = 'operations_feature_flags' self.table_name = 'operations_feature_flags'
...@@ -29,32 +24,15 @@ module Operations ...@@ -29,32 +24,15 @@ module Operations
validate :first_default_scope, on: :create, if: :has_scopes? validate :first_default_scope, on: :create, if: :has_scopes?
before_create :build_default_scope, unless: :has_scopes? before_create :build_default_scope, unless: :has_scopes?
after_update :update_default_scope
accepts_nested_attributes_for :scopes, allow_destroy: true accepts_nested_attributes_for :scopes, allow_destroy: true
scope :ordered, -> { order(:name) } scope :ordered, -> { order(:name) }
scope :enabled, -> do scope :enabled, -> { where(active: true) }
where('EXISTS (?)', join_enabled_scopes) scope :disabled, -> { where(active: false) }
end
scope :disabled, -> do
where('NOT EXISTS (?)', join_enabled_scopes)
end
scope :for_list, -> do
select("operations_feature_flags.*" \
", COALESCE((#{join_enabled_scopes.to_sql}), FALSE) AS active")
end
class << self class << self
def join_enabled_scopes
Operations::FeatureFlagScope
.where('operations_feature_flags.id = feature_flag_id')
.enabled.limit(1).select('TRUE')
end
def preload_relations def preload_relations
preload(:scopes) preload(:scopes)
end end
...@@ -75,9 +53,5 @@ module Operations ...@@ -75,9 +53,5 @@ module Operations
def has_scopes? def has_scopes?
scopes.any? scopes.any?
end end
def update_default_scope
default_scope.update(active: self.active) if saved_change_to_active?
end
end end
end end
...@@ -32,7 +32,16 @@ module Operations ...@@ -32,7 +32,16 @@ module Operations
end end
def self.for_unleash_client(project, environment) def self.for_unleash_client(project, environment)
select('DISTINCT ON (operations_feature_flag_scopes.feature_flag_id) operations_feature_flag_scopes.*') select_columns = [
'DISTINCT ON (operations_feature_flag_scopes.feature_flag_id) operations_feature_flag_scopes.id',
'(operations_feature_flags.active AND operations_feature_flag_scopes.active) AS active',
'operations_feature_flag_scopes.strategies',
'operations_feature_flag_scopes.environment_scope',
'operations_feature_flag_scopes.created_at',
'operations_feature_flag_scopes.updated_at'
]
select(select_columns)
.with_name_and_description .with_name_and_description
.where(feature_flag_id: project.operations_feature_flags.select(:id)) .where(feature_flag_id: project.operations_feature_flags.select(:id))
.order(:feature_flag_id) .order(:feature_flag_id)
......
...@@ -14,6 +14,10 @@ class FeatureFlagEntity < Grape::Entity ...@@ -14,6 +14,10 @@ class FeatureFlagEntity < Grape::Entity
edit_project_feature_flag_path(feature_flag.project, feature_flag) edit_project_feature_flag_path(feature_flag.project, feature_flag)
end end
expose :update_path, if: -> (feature_flag, _) { can_update?(feature_flag) } do |feature_flag|
project_feature_flag_path(feature_flag.project, feature_flag)
end
expose :destroy_path, if: -> (feature_flag, _) { can_destroy?(feature_flag) } do |feature_flag| expose :destroy_path, if: -> (feature_flag, _) { can_destroy?(feature_flag) } do |feature_flag|
project_feature_flag_path(feature_flag.project, feature_flag) project_feature_flag_path(feature_flag.project, feature_flag)
end end
......
...@@ -4,7 +4,7 @@ module FeatureFlags ...@@ -4,7 +4,7 @@ module FeatureFlags
class BaseService < ::BaseService class BaseService < ::BaseService
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
AUDITABLE_ATTRIBUTES = %w(name description).freeze AUDITABLE_ATTRIBUTES = %w(name description active).freeze
protected protected
......
...@@ -79,11 +79,18 @@ describe Projects::FeatureFlagsController do ...@@ -79,11 +79,18 @@ describe Projects::FeatureFlagsController do
expect(json_response['feature_flags'].second['name']).to eq(feature_flag_inactive.name) expect(json_response['feature_flags'].second['name']).to eq(feature_flag_inactive.name)
end end
it 'returns edit path and destroy path' do it 'returns CRUD paths' do
subject subject
expect(json_response['feature_flags'].first['edit_path']).not_to be_nil expected_edit_path = edit_project_feature_flag_path(project, feature_flag_active)
expect(json_response['feature_flags'].first['destroy_path']).not_to be_nil expected_update_path = project_feature_flag_path(project, feature_flag_active)
expected_destroy_path = project_feature_flag_path(project, feature_flag_active)
feature_flag_json = json_response['feature_flags'].first
expect(feature_flag_json['edit_path']).to eq(expected_edit_path)
expect(feature_flag_json['update_path']).to eq(expected_update_path)
expect(feature_flag_json['destroy_path']).to eq(expected_destroy_path)
end end
it 'returns the summary of feature flags' do it 'returns the summary of feature flags' do
...@@ -100,12 +107,26 @@ describe Projects::FeatureFlagsController do ...@@ -100,12 +107,26 @@ describe Projects::FeatureFlagsController do
expect(response).to match_response_schema('feature_flags', dir: 'ee') expect(response).to match_response_schema('feature_flags', dir: 'ee')
end end
it 'returns false for active when the feature flag is inactive even if it has an active scope' do
create(:operations_feature_flag_scope,
feature_flag: feature_flag_inactive,
environment_scope: 'production',
active: true)
subject
expect(response).to have_gitlab_http_status(:ok)
feature_flag_json = json_response['feature_flags'].second
expect(feature_flag_json['active']).to eq(false)
end
context 'when scope is specified' do context 'when scope is specified' do
let(:view_params) do let(:view_params) do
{ namespace_id: project.namespace, project_id: project, scope: scope } { namespace_id: project.namespace, project_id: project, scope: scope }
end end
context 'when scope is all' do context 'when all feature flags are requested' do
let(:scope) { 'all' } let(:scope) { 'all' }
it 'returns all feature flags' do it 'returns all feature flags' do
...@@ -115,7 +136,7 @@ describe Projects::FeatureFlagsController do ...@@ -115,7 +136,7 @@ describe Projects::FeatureFlagsController do
end end
end end
context 'when scope is enabled' do context 'when enabled feature flags are requested' do
let(:scope) { 'enabled' } let(:scope) { 'enabled' }
it 'returns enabled feature flags' do it 'returns enabled feature flags' do
...@@ -126,7 +147,7 @@ describe Projects::FeatureFlagsController do ...@@ -126,7 +147,7 @@ describe Projects::FeatureFlagsController do
end end
end end
context 'when scope is disabled' do context 'when disabled feature flags are requested' do
let(:scope) { 'disabled' } let(:scope) { 'disabled' }
it 'returns disabled feature flags' do it 'returns disabled feature flags' do
...@@ -161,13 +182,13 @@ describe Projects::FeatureFlagsController do ...@@ -161,13 +182,13 @@ describe Projects::FeatureFlagsController do
expect(json_response['count']['disabled']).to eq(1) expect(json_response['count']['disabled']).to eq(1)
end end
it 'recongnizes feature flag 1 as active' do it 'recognizes feature flag 1 as active' do
subject subject
expect(json_response['feature_flags'].first['active']).to be_truthy expect(json_response['feature_flags'].first['active']).to be_truthy
end end
it 'recongnizes feature flag 2 as inactive' do it 'recognizes feature flag 2 as inactive' do
subject subject
expect(json_response['feature_flags'].second['active']).to be_falsy expect(json_response['feature_flags'].second['active']).to be_falsy
...@@ -274,10 +295,10 @@ describe Projects::FeatureFlagsController do ...@@ -274,10 +295,10 @@ describe Projects::FeatureFlagsController do
active: true) active: true)
end end
it 'recongnizes the feature flag as active' do it 'returns false for active' do
subject subject
expect(json_response['active']).to be_truthy expect(json_response['active']).to eq(false)
end end
end end
...@@ -293,7 +314,7 @@ describe Projects::FeatureFlagsController do ...@@ -293,7 +314,7 @@ describe Projects::FeatureFlagsController do
active: false) active: false)
end end
it 'recongnizes the feature flag as inactive' do it 'recognizes the feature flag as inactive' do
subject subject
expect(json_response['active']).to be_falsy expect(json_response['active']).to be_falsy
...@@ -361,6 +382,27 @@ describe Projects::FeatureFlagsController do ...@@ -361,6 +382,27 @@ describe Projects::FeatureFlagsController do
end end
end end
context 'without the active parameter' do
let(:params) do
{
namespace_id: project.namespace,
project_id: project,
operations_feature_flag: {
name: 'my_feature_flag'
}
}
end
it 'creates a flag with active set to true' do
expect { subject }.to change { Operations::FeatureFlag.count }.by(1)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('feature_flag', dir: 'ee')
expect(json_response['active']).to eq(true)
expect(Operations::FeatureFlag.last.active).to eq(true)
end
end
context 'when user is reporter' do context 'when user is reporter' do
let(:user) { reporter } let(:user) { reporter }
...@@ -593,9 +635,28 @@ describe Projects::FeatureFlagsController do ...@@ -593,9 +635,28 @@ describe Projects::FeatureFlagsController do
.to change { feature_flag.reload.active }.from(true).to(false) .to change { feature_flag.reload.active }.from(true).to(false)
end end
it "updates default scope's active too" do it "does not change default scope's active" do
expect { subject } expect { subject }
.to change { feature_flag.default_scope.reload.active }.from(true).to(false) .not_to change { feature_flag.default_scope.reload.active }.from(true)
end
it 'updates active from false to true when an inactive feature flag has an active scope' do
feature_flag = create(:operations_feature_flag, project: project, name: 'my_flag', active: false)
create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: true)
params = {
namespace_id: project.namespace,
project_id: project,
id: feature_flag.id,
operations_feature_flag: { active: true }
}
put(:update, params: params, format: :json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('feature_flag', dir: 'ee')
expect(json_response['active']).to eq(true)
expect(feature_flag.reload.active).to eq(true)
expect(feature_flag.default_scope.reload.active).to eq(false)
end end
end end
......
...@@ -26,7 +26,7 @@ describe 'User creates feature flag', :js do ...@@ -26,7 +26,7 @@ describe 'User creates feature flag', :js do
it 'shows the created feature flag' do it 'shows the created feature flag' do
within_feature_flag_row(1) do within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect(page).to have_css('.js-feature-flag-status .badge-success') expect(page).to have_css('.js-feature-flag-status button.is-checked')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
...@@ -57,7 +57,7 @@ describe 'User creates feature flag', :js do ...@@ -57,7 +57,7 @@ describe 'User creates feature flag', :js do
it 'shows the created feature flag' do it 'shows the created feature flag' do
within_feature_flag_row(1) do within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect(page).to have_css('.js-feature-flag-status .badge-danger') expect(page).to have_css('.js-feature-flag-status button.is-checked')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
...@@ -89,7 +89,7 @@ describe 'User creates feature flag', :js do ...@@ -89,7 +89,7 @@ describe 'User creates feature flag', :js do
it 'shows the created feature flag' do it 'shows the created feature flag' do
within_feature_flag_row(1) do within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('mr_train') expect(page.find('.feature-flag-name')).to have_content('mr_train')
expect(page).to have_css('.js-feature-flag-status .badge-success') expect(page).to have_css('.js-feature-flag-status button.is-checked')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
...@@ -121,7 +121,7 @@ describe 'User creates feature flag', :js do ...@@ -121,7 +121,7 @@ describe 'User creates feature flag', :js do
it 'shows the created feature flag' do it 'shows the created feature flag' do
within_feature_flag_row(1) do within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('mr_train') expect(page.find('.feature-flag-name')).to have_content('mr_train')
expect(page).to have_css('.js-feature-flag-status .badge-success') expect(page).to have_css('.js-feature-flag-status button.is-checked')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
......
...@@ -30,7 +30,7 @@ describe 'User sees feature flag list', :js do ...@@ -30,7 +30,7 @@ describe 'User sees feature flag list', :js do
it 'user sees the first flag' do it 'user sees the first flag' do
within_feature_flag_row(1) do within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect(page).to have_css('.js-feature-flag-status .badge-success') expect(page).to have_css('.js-feature-flag-status button:not(.is-checked)')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
...@@ -44,7 +44,7 @@ describe 'User sees feature flag list', :js do ...@@ -44,7 +44,7 @@ describe 'User sees feature flag list', :js do
it 'user sees the second flag' do it 'user sees the second flag' do
within_feature_flag_row(2) do within_feature_flag_row(2) do
expect(page.find('.feature-flag-name')).to have_content('drop_legacy_artifacts') expect(page.find('.feature-flag-name')).to have_content('drop_legacy_artifacts')
expect(page).to have_css('.js-feature-flag-status .badge-danger') expect(page).to have_css('.js-feature-flag-status button:not(.is-checked)')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
...@@ -56,7 +56,7 @@ describe 'User sees feature flag list', :js do ...@@ -56,7 +56,7 @@ describe 'User sees feature flag list', :js do
it 'user sees the third flag' do it 'user sees the third flag' do
within_feature_flag_row(3) do within_feature_flag_row(3) do
expect(page.find('.feature-flag-name')).to have_content('mr_train') expect(page.find('.feature-flag-name')).to have_content('mr_train')
expect(page).to have_css('.js-feature-flag-status .badge-success') expect(page).to have_css('.js-feature-flag-status button.is-checked')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
...@@ -66,6 +66,20 @@ describe 'User sees feature flag list', :js do ...@@ -66,6 +66,20 @@ describe 'User sees feature flag list', :js do
end end
end end
end end
it 'user updates the status toggle' do
within_feature_flag_row(1) do
page.find('.js-feature-flag-status button').click
expect(page).to have_css('.js-feature-flag-status button.is-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 'when there are no feature flags' do context 'when there are no feature flags' do
......
...@@ -9,7 +9,7 @@ describe 'User updates feature flag', :js do ...@@ -9,7 +9,7 @@ describe 'User updates feature flag', :js do
let(:project) { create(:project, namespace: user.namespace) } let(:project) { create(:project, namespace: user.namespace) }
let!(:feature_flag) do let!(:feature_flag) do
create_flag(project, 'ci_live_trace', false, create_flag(project, 'ci_live_trace', true,
description: 'For live trace feature') description: 'For live trace feature')
end end
...@@ -32,7 +32,7 @@ describe 'User updates feature flag', :js do ...@@ -32,7 +32,7 @@ describe 'User updates feature flag', :js do
within_status do within_status do
expect(find('.project-feature-toggle')['aria-label']) expect(find('.project-feature-toggle')['aria-label'])
.to eq('Toggle Status: OFF') .to eq('Toggle Status: ON')
end end
end end
end end
...@@ -50,11 +50,11 @@ describe 'User updates feature flag', :js do ...@@ -50,11 +50,11 @@ describe 'User updates feature flag', :js do
it 'shows the updated feature flag' do it 'shows the updated feature flag' do
within_feature_flag_row(1) do within_feature_flag_row(1) do
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect(page).to have_css('.js-feature-flag-status .badge-danger') expect(page).to have_css('.js-feature-flag-status button.is-checked')
within_feature_flag_scopes do within_feature_flag_scopes do
expect(page.find('.badge:nth-child(1)')).to have_content('*') expect(page.find('.badge:nth-child(1)')).to have_content('*')
expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive') expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
expect(page.find('.badge:nth-child(2)')).to have_content('review/*') expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive') expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
end end
......
...@@ -74,26 +74,5 @@ describe FeatureFlagsFinder do ...@@ -74,26 +74,5 @@ describe FeatureFlagsFinder do
subject subject
end end
end end
context 'when it is presented for list' do
let!(:feature_flag_1) { create(:operations_feature_flag, project: project, active: false) }
let!(:feature_flag_2) { create(:operations_feature_flag, project: project, active: false) }
context 'when there is an active scope' do
before do
create_scope(feature_flag_1, 'review/*', true)
end
it 'presents a virtual active value' do
expect(subject.map(&:active)).to eq([true, false])
end
end
context 'when there are no active scopes' do
it 'presents a virtual active value' do
expect(subject.map(&:active)).to eq([false, false])
end
end
end
end end
end end
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
"active": { "type": "boolean" }, "active": { "type": "boolean" },
"description": { "type": ["string", "null"] }, "description": { "type": ["string", "null"] },
"edit_path": { "type": ["string", "null"] }, "edit_path": { "type": ["string", "null"] },
"update_path": { "type": ["string", "null"] },
"destroy_path": { "type": ["string", "null"] }, "destroy_path": { "type": ["string", "null"] },
"scopes": { "type": "array", "items": { "$ref": "feature_flag_scope.json" } } "scopes": { "type": "array", "items": { "$ref": "feature_flag_scope.json" } }
}, },
......
...@@ -21,7 +21,7 @@ describe('Edit feature flag form', () => { ...@@ -21,7 +21,7 @@ describe('Edit feature flag form', () => {
}, },
}); });
const factory = (featureFlagToggle = true) => { const factory = () => {
wrapper = shallowMount(localVue.extend(EditFeatureFlag), { wrapper = shallowMount(localVue.extend(EditFeatureFlag), {
localVue, localVue,
propsData: { propsData: {
...@@ -31,7 +31,6 @@ describe('Edit feature flag form', () => { ...@@ -31,7 +31,6 @@ describe('Edit feature flag form', () => {
}, },
provide: { provide: {
glFeatures: { glFeatures: {
featureFlagToggle,
featureFlagIID: true, featureFlagIID: true,
}, },
}, },
...@@ -86,18 +85,6 @@ describe('Edit feature flag form', () => { ...@@ -86,18 +85,6 @@ describe('Edit feature flag form', () => {
expect(wrapper.find(GlToggle).props('value')).toBe(true); expect(wrapper.find(GlToggle).props('value')).toBe(true);
}); });
describe('without featureFlagToggle', () => {
beforeEach(done => {
wrapper.destroy();
factory(false);
setImmediate(() => done());
});
it('should not render the toggle', () => {
expect(wrapper.find(GlToggle).exists()).toBe(false);
});
});
describe('with error', () => { describe('with error', () => {
it('should render the error', () => { it('should render the error', () => {
store.dispatch('edit/receiveUpdateFeatureFlagError', { message: ['The name is required'] }); store.dispatch('edit/receiveUpdateFeatureFlagError', { message: ['The name is required'] });
......
...@@ -116,7 +116,7 @@ describe('Feature flag table', () => { ...@@ -116,7 +116,7 @@ describe('Feature flag table', () => {
beforeEach(() => { beforeEach(() => {
props.featureFlags[0].update_path = props.featureFlags[0].destroy_path; props.featureFlags[0].update_path = props.featureFlags[0].destroy_path;
createWrapper(props, { provide: { glFeatures: { featureFlagToggle: true } } }); createWrapper(props);
toggle = wrapper.find(GlToggle); toggle = wrapper.find(GlToggle);
}); });
......
...@@ -37,7 +37,6 @@ describe('feature flag form', () => { ...@@ -37,7 +37,6 @@ describe('feature flag form', () => {
glFeatures: { glFeatures: {
featureFlagPermissions: true, featureFlagPermissions: true,
featureFlagsUsersPerEnvironment: true, featureFlagsUsersPerEnvironment: true,
featureFlagToggle: true,
}, },
}, },
sync: false, sync: false,
......
...@@ -162,6 +162,7 @@ describe('Feature flags store Mutations', () => { ...@@ -162,6 +162,7 @@ describe('Feature flags store Mutations', () => {
mutations[types.UPDATE_FEATURE_FLAG](stateCopy, { mutations[types.UPDATE_FEATURE_FLAG](stateCopy, {
...featureFlag, ...featureFlag,
scopes: mapToScopesViewModel(featureFlag.scopes || []),
active: false, active: false,
}); });
}); });
...@@ -182,21 +183,25 @@ describe('Feature flags store Mutations', () => { ...@@ -182,21 +183,25 @@ describe('Feature flags store Mutations', () => {
expect(stateCopy.count.disabled).toBe(1); expect(stateCopy.count.disabled).toBe(1);
}); });
}); });
describe('RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS', () => { describe('RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS', () => {
beforeEach(() => { const runUpdate = (stateCount, flagState, featureFlagUpdateParams) => {
stateCopy.featureFlags = getRequestData.feature_flags.map(flag => ({ stateCopy.featureFlags = getRequestData.feature_flags.map(flag => ({
...flag, ...flag,
...flagState,
scopes: mapToScopesViewModel(flag.scopes || []), scopes: mapToScopesViewModel(flag.scopes || []),
})); }));
stateCopy.count = { enabled: 1, disabled: 0 }; stateCopy.count = stateCount;
mutations[types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](stateCopy, { mutations[types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](stateCopy, {
...featureFlag, ...featureFlag,
active: false, ...featureFlagUpdateParams,
}); });
}); };
it('updates the flag with the matching ID', () => {
runUpdate({ all: 1, enabled: 1, disabled: 0 }, { active: true }, { active: false });
it('should update the flag with the matching ID', () => {
expect(stateCopy.featureFlags).toEqual([ expect(stateCopy.featureFlags).toEqual([
{ {
...featureFlag, ...featureFlag,
...@@ -205,13 +210,34 @@ describe('Feature flags store Mutations', () => { ...@@ -205,13 +210,34 @@ describe('Feature flags store Mutations', () => {
}, },
]); ]);
}); });
it('should update the enabled count', () => {
expect(stateCopy.count.enabled).toBe(0); it('updates the count data', () => {
runUpdate({ all: 1, enabled: 1, disabled: 0 }, { active: true }, { active: false });
expect(stateCopy.count).toEqual({ all: 1, enabled: 0, disabled: 1 });
}); });
it('should update the disabled count', () => {
expect(stateCopy.count.disabled).toBe(1); describe('when count data does not match up with the number of flags in state', () => {
it('updates the count data when the flag changes to inactive', () => {
runUpdate({ all: 4, enabled: 1, disabled: 3 }, { active: true }, { active: false });
expect(stateCopy.count).toEqual({ all: 4, enabled: 0, disabled: 4 });
});
it('updates the count data when the flag changes to active', () => {
runUpdate({ all: 4, enabled: 1, disabled: 3 }, { active: false }, { active: true });
expect(stateCopy.count).toEqual({ all: 4, enabled: 2, disabled: 2 });
});
it('retains the count data when flag.active does not change', () => {
runUpdate({ all: 4, enabled: 1, disabled: 3 }, { active: true }, { active: true });
expect(stateCopy.count).toEqual({ all: 4, enabled: 1, disabled: 3 });
});
}); });
}); });
describe('RECEIVE_UPDATE_FEATURE_FLAG_ERROR', () => { describe('RECEIVE_UPDATE_FEATURE_FLAG_ERROR', () => {
beforeEach(() => { beforeEach(() => {
stateCopy.featureFlags = getRequestData.feature_flags.map(flag => ({ stateCopy.featureFlags = getRequestData.feature_flags.map(flag => ({
......
...@@ -57,7 +57,7 @@ describe Operations::FeatureFlag do ...@@ -57,7 +57,7 @@ describe Operations::FeatureFlag do
describe '.enabled' do describe '.enabled' do
subject { described_class.enabled } subject { described_class.enabled }
context 'when the feature flag has an active scope' do context 'when the feature flag is active' do
let!(:feature_flag) { create(:operations_feature_flag, active: true) } let!(:feature_flag) { create(:operations_feature_flag, active: true) }
it 'returns the flag' do it 'returns the flag' do
...@@ -65,19 +65,39 @@ describe Operations::FeatureFlag do ...@@ -65,19 +65,39 @@ describe Operations::FeatureFlag do
end end
end end
context 'when the feature flag does not have an active scope' do context 'when the feature flag is active and all scopes are inactive' do
let!(:feature_flag) { create(:operations_feature_flag, active: true) }
it 'returns the flag' do
feature_flag.default_scope.update!(active: false)
is_expected.to eq([feature_flag])
end
end
context 'when the feature flag is inactive' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) } let!(:feature_flag) { create(:operations_feature_flag, active: false) }
it 'does not return the flag' do it 'does not return the flag' do
is_expected.to be_empty is_expected.to be_empty
end end
end end
context 'when the feature flag is inactive and all scopes are active' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) }
it 'does not return the flag' do
feature_flag.default_scope.update!(active: true)
is_expected.to be_empty
end
end
end end
describe '.disabled' do describe '.disabled' do
subject { described_class.disabled } subject { described_class.disabled }
context 'when the feature flag has an active scope' do context 'when the feature flag is active' do
let!(:feature_flag) { create(:operations_feature_flag, active: true) } let!(:feature_flag) { create(:operations_feature_flag, active: true) }
it 'does not return the flag' do it 'does not return the flag' do
...@@ -85,42 +105,31 @@ describe Operations::FeatureFlag do ...@@ -85,42 +105,31 @@ describe Operations::FeatureFlag do
end end
end end
context 'when the feature flag does not have an active scope' do context 'when the feature flag is active and all scopes are inactive' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) }
it 'returns the flag' do
is_expected.to eq([feature_flag])
end
end
end
describe '.for_list' do
subject { described_class.for_list }
context 'when all scopes are active' do
let!(:feature_flag) { create(:operations_feature_flag, active: true) } let!(:feature_flag) { create(:operations_feature_flag, active: true) }
let!(:scope) { create_scope(feature_flag, 'production', true) }
it 'returns virtual active value' do it 'does not return the flag' do
expect(subject.first.active).to be_truthy feature_flag.default_scope.update!(active: false)
is_expected.to be_empty
end end
end end
context 'when all scopes are inactive' do context 'when the feature flag is inactive' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) } let!(:feature_flag) { create(:operations_feature_flag, active: false) }
let!(:scope) { create_scope(feature_flag, 'production', false) }
it 'returns virtual active value' do it 'returns the flag' do
expect(subject.first.active).to be_falsy is_expected.to eq([feature_flag])
end end
end end
context 'when one scopes is active' do context 'when the feature flag is inactive and all scopes are active' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) } let!(:feature_flag) { create(:operations_feature_flag, active: false) }
let!(:scope) { create_scope(feature_flag, 'production', true) }
it 'returns virtual active value' do it 'returns the flag' do
expect(subject.first.active).to be_truthy feature_flag.default_scope.update!(active: true)
is_expected.to eq([feature_flag])
end end
end end
end end
......
...@@ -54,13 +54,13 @@ describe API::FeatureFlagScopes do ...@@ -54,13 +54,13 @@ describe API::FeatureFlagScopes do
params: params params: params
end end
let(:feature_flag_1) { create_flag(project, 'flag_1', false) } let(:feature_flag_1) { create_flag(project, 'flag_1', true) }
let(:feature_flag_2) { create_flag(project, 'flag_2', false) } let(:feature_flag_2) { create_flag(project, 'flag_2', true) }
before do before do
create_scope(feature_flag_1, 'staging', true) create_scope(feature_flag_1, 'staging', false)
create_scope(feature_flag_1, 'production', false) create_scope(feature_flag_1, 'production', true)
create_scope(feature_flag_2, 'review/*', true) create_scope(feature_flag_2, 'review/*', false)
end end
context 'when environment is production' do context 'when environment is production' do
...@@ -73,8 +73,8 @@ describe API::FeatureFlagScopes do ...@@ -73,8 +73,8 @@ describe API::FeatureFlagScopes do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag_detailed_scopes', dir: 'ee') expect(response).to match_response_schema('public_api/v4/feature_flag_detailed_scopes', dir: 'ee')
expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false }) expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true })
expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false }) expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true })
end end
end end
...@@ -85,8 +85,8 @@ describe API::FeatureFlagScopes do ...@@ -85,8 +85,8 @@ describe API::FeatureFlagScopes do
subject subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true }) expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false })
expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false }) expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true })
end end
end end
...@@ -97,8 +97,8 @@ describe API::FeatureFlagScopes do ...@@ -97,8 +97,8 @@ describe API::FeatureFlagScopes do
subject subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false }) expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true })
expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true }) expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false })
end end
end end
end end
......
...@@ -134,7 +134,7 @@ describe API::Unleash do ...@@ -134,7 +134,7 @@ describe API::Unleash do
feature_flag_2 = json_response['features'].select { |f| f['name'] == 'feature_flag_2' }.first feature_flag_2 = json_response['features'].select { |f| f['name'] == 'feature_flag_2' }.first
expect(feature_flag_1['enabled']).to eq(true) expect(feature_flag_1['enabled']).to eq(true)
expect(feature_flag_2['enabled']).to eq(true) expect(feature_flag_2['enabled']).to eq(false)
end end
end end
...@@ -160,7 +160,7 @@ describe API::Unleash do ...@@ -160,7 +160,7 @@ describe API::Unleash do
it_behaves_like 'authenticated request' it_behaves_like 'authenticated request'
it_behaves_like 'support multiple environments' it_behaves_like 'support multiple environments'
context 'with a list of feature flag' do context 'with a list of feature flags' do
let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" }} let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" }}
let!(:enable_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true) } let!(:enable_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true) }
let!(:disabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature2', active: false) } let!(:disabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature2', active: false) }
...@@ -247,6 +247,17 @@ describe API::Unleash do ...@@ -247,6 +247,17 @@ describe API::Unleash do
}]) }])
end end
it 'returns a disabled feature when the flag is disabled' do
flag = create(:operations_feature_flag, project: project, name: 'test_feature', active: false)
create(:operations_feature_flag_scope, feature_flag: flag, environment_scope: 'production', active: true)
headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" }
get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['features'].first['enabled']).to eq(false)
end
context "with an inactive scope" do context "with an inactive scope" do
let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: [{ name: "default", parameters: {} }]) } let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: [{ name: "default", parameters: {} }]) }
let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } } let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } }
......
...@@ -7,7 +7,7 @@ describe FeatureFlags::UpdateService do ...@@ -7,7 +7,7 @@ describe FeatureFlags::UpdateService do
let(:developer) { create(:user) } let(:developer) { create(:user) }
let(:reporter) { create(:user) } let(:reporter) { create(:user) }
let(:user) { developer } let(:user) { developer }
let(:feature_flag) { create(:operations_feature_flag, project: project) } let(:feature_flag) { create(:operations_feature_flag, project: project, active: true) }
before do before do
stub_licensed_features(feature_flags: true) stub_licensed_features(feature_flags: true)
...@@ -88,14 +88,29 @@ describe FeatureFlags::UpdateService do ...@@ -88,14 +88,29 @@ describe FeatureFlags::UpdateService do
end end
end end
context 'when active state is changed' do context 'when flag active state is changed' do
let(:params) do
{
active: false
}
end
it 'creates audit event about changing active state' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include('Updated active from <strong>"true"</strong> to <strong>"false"</strong>.')
)
end
end
context 'when scope active state is changed' do
let(:params) do let(:params) do
{ {
scopes_attributes: [{ id: feature_flag.scopes.first.id, active: false }] scopes_attributes: [{ id: feature_flag.scopes.first.id, active: false }]
} }
end end
it 'creates audit event about changing active stae' do it 'creates audit event about changing active state' do
expect { subject }.to change { AuditEvent.count }.by(1) expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to( expect(audit_event_message).to(
include("Updated rule <strong>*</strong> active state "\ include("Updated rule <strong>*</strong> active state "\
......
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'migrate', '20191213184609_backfill_operations_feature_flags_active.rb')
describe BackfillOperationsFeatureFlagsActive, :migration do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:flags) { table(:operations_feature_flags) }
def setup
namespace = namespaces.create!(name: 'foo', path: 'foo')
project = projects.create!(namespace_id: namespace.id)
project
end
it 'executes successfully when there are no flags in the table' do
setup
disable_migrations_output { migrate! }
expect(flags.count).to eq(0)
end
it 'updates active to true' do
project = setup
flag = flags.create!(project_id: project.id, name: 'test_flag', active: false)
disable_migrations_output { migrate! }
expect(flag.reload.active).to eq(true)
end
it 'updates active to true for multiple flags' do
project = setup
flag_a = flags.create!(project_id: project.id, name: 'test_flag', active: false)
flag_b = flags.create!(project_id: project.id, name: 'other_flag', active: false)
disable_migrations_output { migrate! }
expect(flag_a.reload.active).to eq(true)
expect(flag_b.reload.active).to eq(true)
end
it 'leaves active true if it is already true' do
project = setup
flag = flags.create!(project_id: project.id, name: 'test_flag', active: true)
disable_migrations_output { migrate! }
expect(flag.reload.active).to eq(true)
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