Add override toggle to Operations Feature Flags

Add backend support
Remove frontend feature flag

changelogs/unreleased/ff-toggle-backend.yml

+---
+title: Add feature flag override toggle
+merge_request: 21598
+author:
+type: added

db/migrate/20191213184609_backfill_operations_feature_flags_active.rb

+# 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

doc/user/project/operations/feature_flags.md

@@ -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,
 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
 

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

@@ -49,9 +49,6 @@ export default {
     hasFeatureFlagsIID() {
       return this.glFeatures.featureFlagIID && this.iid;
     },
-    hasFeatureFlagToggle() {
-      return this.glFeatures.featureFlagToggle;
-    },
   },
   created() {
     this.setPath(this.path);
@@ -74,12 +71,7 @@ export default {
 
     <template v-else-if="!isLoading && !hasError">
       <div class="d-flex align-items-center mb-3 mt-3">
-        <gl-toggle
-          v-if="hasFeatureFlagToggle"
-          :value="active"
-          class="m-0 mr-3"
-          @change="toggleActive"
-        />
+        <gl-toggle :value="active" class="m-0 mr-3" @change="toggleActive" />
         <h3 class="page-title m-0">{{ title }}</h3>
       </div>
 
@@ -94,7 +86,7 @@ export default {
         :cancel-path="path"
         :submit-text="__('Save changes')"
         :environments-endpoint="environmentsEndpoint"
-        :active="active || !hasFeatureFlagToggle"
+        :active="active"
         @handleSubmit="data => updateFeatureFlag(data)"
       />
     </template>

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

@@ -62,9 +62,6 @@ export default {
     modalId() {
       return 'delete-feature-flag';
     },
-    hasFeatureFlagToggle() {
-      return this.glFeatures.featureFlagToggle;
-    },
   },
   methods: {
     scopeTooltipText(scope) {
@@ -133,7 +130,7 @@ export default {
           <div class="table-mobile-header" role="rowheader">{{ s__('FeatureFlags|Status') }}</div>
           <div class="table-mobile-content js-feature-flag-status">
             <gl-toggle
-              v-if="hasFeatureFlagToggle && featureFlag.update_path"
+              v-if="featureFlag.update_path"
               :value="featureFlag.active"
               @change="toggleFeatureFlag(featureFlag)"
             />

ee/app/assets/javascripts/feature_flags/store/modules/index/mutations.js

@@ -7,10 +7,14 @@ const mapFlag = flag => ({ ...flag, scopes: mapToScopesViewModel(flag.scopes ||
 
 const updateFlag = (state, flag) => {
   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.count, 'enabled', state.featureFlags.filter(({ active }) => active).length);
-  Vue.set(state.count, 'disabled', state.featureFlags.filter(({ active }) => !active).length);
+  if (staleFlag.active !== flag.active) {
+    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 {
@@ -67,7 +71,7 @@ export default {
     state.hasRotateError = true;
   },
   [types.UPDATE_FEATURE_FLAG](state, flag) {
-    updateFlag(state, mapFlag(flag));
+    updateFlag(state, flag);
   },
   [types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](state, data) {
     updateFlag(state, mapFlag(data));

ee/app/controllers/projects/feature_flags_controller.rb

@@ -94,7 +94,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
   protected
 
   def feature_flag
-    @feature_flag ||= project.operations_feature_flags.for_list.find(params[:id])
+    @feature_flag ||= project.operations_feature_flags.find(params[:id])
   end
 
   def create_params

ee/app/finders/feature_flags_finder.rb

@@ -17,7 +17,6 @@ class FeatureFlagsFinder
 
     items = feature_flags
     items = by_scope(items)
-    items = for_list(items)
 
     items = items.preload_relations if preload
     items.ordered
@@ -35,8 +34,4 @@ class FeatureFlagsFinder
       items
     end
   end
-
-  def for_list(items)
-    items.for_list
-  end
 end

ee/app/models/operations/feature_flag.rb

 # frozen_string_literal: true
 
 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
     self.table_name = 'operations_feature_flags'
 
@@ -29,32 +24,15 @@ module Operations
     validate :first_default_scope, on: :create, if: :has_scopes?
 
     before_create :build_default_scope, unless: :has_scopes?
-    after_update :update_default_scope
 
     accepts_nested_attributes_for :scopes, allow_destroy: true
 
     scope :ordered, -> { order(:name) }
 
-    scope :enabled, -> do
-      where('EXISTS (?)', join_enabled_scopes)
-    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
+    scope :enabled, -> { where(active: true) }
+    scope :disabled, -> { where(active: false) }
 
     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
         preload(:scopes)
       end
@@ -75,9 +53,5 @@ module Operations
     def has_scopes?
       scopes.any?
     end
-
-    def update_default_scope
-      default_scope.update(active: self.active) if saved_change_to_active?
-    end
   end
 end

ee/app/models/operations/feature_flag_scope.rb

@@ -32,7 +32,16 @@ module Operations
     end
 
     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
         .where(feature_flag_id: project.operations_feature_flags.select(:id))
         .order(:feature_flag_id)

ee/app/serializers/feature_flag_entity.rb

@@ -14,6 +14,10 @@ class FeatureFlagEntity < Grape::Entity
     edit_project_feature_flag_path(feature_flag.project, feature_flag)
   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|
     project_feature_flag_path(feature_flag.project, feature_flag)
   end

ee/app/services/feature_flags/base_service.rb

@@ -4,7 +4,7 @@ module FeatureFlags
   class BaseService < ::BaseService
     include Gitlab::Utils::StrongMemoize
 
-    AUDITABLE_ATTRIBUTES = %w(name description).freeze
+    AUDITABLE_ATTRIBUTES = %w(name description active).freeze
 
     protected
 

ee/spec/controllers/projects/feature_flags_controller_spec.rb

@@ -79,11 +79,18 @@ describe Projects::FeatureFlagsController do
       expect(json_response['feature_flags'].second['name']).to eq(feature_flag_inactive.name)
     end
 
-    it 'returns edit path and destroy path' do
+    it 'returns CRUD paths' do
       subject
 
-      expect(json_response['feature_flags'].first['edit_path']).not_to be_nil
-      expect(json_response['feature_flags'].first['destroy_path']).not_to be_nil
+      expected_edit_path = edit_project_feature_flag_path(project, feature_flag_active)
+      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
 
     it 'returns the summary of feature flags' do
@@ -100,12 +107,26 @@ describe Projects::FeatureFlagsController do
       expect(response).to match_response_schema('feature_flags', dir: 'ee')
     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
       let(:view_params) do
         { namespace_id: project.namespace, project_id: project, scope: scope }
       end
 
-      context 'when scope is all' do
+      context 'when all feature flags are requested' do
         let(:scope) { 'all' }
 
         it 'returns all feature flags' do
@@ -115,7 +136,7 @@ describe Projects::FeatureFlagsController do
         end
       end
 
-      context 'when scope is enabled' do
+      context 'when enabled feature flags are requested' do
         let(:scope) { 'enabled' }
 
         it 'returns enabled feature flags' do
@@ -126,7 +147,7 @@ describe Projects::FeatureFlagsController do
         end
       end
 
-      context 'when scope is disabled' do
+      context 'when disabled feature flags are requested' do
         let(:scope) { 'disabled' }
 
         it 'returns disabled feature flags' do
@@ -161,13 +182,13 @@ describe Projects::FeatureFlagsController do
         expect(json_response['count']['disabled']).to eq(1)
       end
 
-      it 'recongnizes feature flag 1 as active' do
+      it 'recognizes feature flag 1 as active' do
         subject
 
         expect(json_response['feature_flags'].first['active']).to be_truthy
       end
 
-      it 'recongnizes feature flag 2 as inactive' do
+      it 'recognizes feature flag 2 as inactive' do
         subject
 
         expect(json_response['feature_flags'].second['active']).to be_falsy
@@ -274,10 +295,10 @@ describe Projects::FeatureFlagsController do
                 active: true)
         end
 
-        it 'recongnizes the feature flag as active' do
+        it 'returns false for active' do
           subject
 
-          expect(json_response['active']).to be_truthy
+          expect(json_response['active']).to eq(false)
         end
       end
 
@@ -293,7 +314,7 @@ describe Projects::FeatureFlagsController do
                 active: false)
         end
 
-        it 'recongnizes the feature flag as inactive' do
+        it 'recognizes the feature flag as inactive' do
           subject
 
           expect(json_response['active']).to be_falsy
@@ -361,6 +382,27 @@ describe Projects::FeatureFlagsController do
       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
       let(:user) { reporter }
 
@@ -593,9 +635,28 @@ describe Projects::FeatureFlagsController do
           .to change { feature_flag.reload.active }.from(true).to(false)
       end
 
-      it "updates default scope's active too" do
+      it "does not change default scope's active" do
         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
 

ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb

@@ -26,7 +26,7 @@ describe 'User creates feature flag', :js do
     it 'shows the created feature flag' do
       within_feature_flag_row(1) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -57,7 +57,7 @@ describe 'User creates feature flag', :js do
     it 'shows the created feature flag' do
       within_feature_flag_row(1) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -89,7 +89,7 @@ describe 'User creates feature flag', :js do
     it 'shows the created feature flag' do
       within_feature_flag_row(1) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -121,7 +121,7 @@ describe 'User creates feature flag', :js do
     it 'shows the created feature flag' do
       within_feature_flag_row(1) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')

ee/spec/features/projects/feature_flags/user_sees_feature_flag_list_spec.rb

@@ -30,7 +30,7 @@ describe 'User sees feature flag list', :js do
     it 'user sees the first flag' do
       within_feature_flag_row(1) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -44,7 +44,7 @@ describe 'User sees feature flag list', :js do
     it 'user sees the second flag' do
       within_feature_flag_row(2) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -56,7 +56,7 @@ describe 'User sees feature flag list', :js do
     it 'user sees the third flag' do
       within_feature_flag_row(3) do
         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
           expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -66,6 +66,20 @@ describe 'User sees feature flag list', :js do
         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
 
   context 'when there are no feature flags' do

ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb

@@ -9,7 +9,7 @@ describe 'User updates feature flag', :js do
   let(:project) { create(:project, namespace: user.namespace) }
 
   let!(:feature_flag) do
-    create_flag(project, 'ci_live_trace', false,
+    create_flag(project, 'ci_live_trace', true,
                 description: 'For live trace feature')
   end
 
@@ -32,7 +32,7 @@ describe 'User updates feature flag', :js do
 
       within_status do
         expect(find('.project-feature-toggle')['aria-label'])
-          .to eq('Toggle Status: OFF')
+          .to eq('Toggle Status: ON')
       end
     end
   end
@@ -50,11 +50,11 @@ describe 'User updates feature flag', :js do
     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(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
           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)')['class']).to include('badge-inactive')
         end

ee/spec/finders/feature_flags_finder_spec.rb

@@ -74,26 +74,5 @@ describe FeatureFlagsFinder do
         subject
       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

ee/spec/fixtures/api/schemas/feature_flag.json

@@ -12,6 +12,7 @@
     "active": { "type": "boolean" },
     "description": { "type": ["string", "null"] },
     "edit_path": { "type": ["string", "null"] },
+    "update_path": { "type": ["string", "null"] },
     "destroy_path": { "type": ["string", "null"] },
     "scopes": { "type": "array", "items": { "$ref": "feature_flag_scope.json" } }
   },

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

@@ -21,7 +21,7 @@ describe('Edit feature flag form', () => {
     },
   });
 
-  const factory = (featureFlagToggle = true) => {
+  const factory = () => {
     wrapper = shallowMount(localVue.extend(EditFeatureFlag), {
       localVue,
       propsData: {
@@ -31,7 +31,6 @@ describe('Edit feature flag form', () => {
       },
       provide: {
         glFeatures: {
-          featureFlagToggle,
           featureFlagIID: true,
         },
       },
@@ -86,18 +85,6 @@ describe('Edit feature flag form', () => {
     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', () => {
     it('should render the error', () => {
       store.dispatch('edit/receiveUpdateFeatureFlagError', { message: ['The name is required'] });

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

@@ -116,7 +116,7 @@ describe('Feature flag table', () => {
 
     beforeEach(() => {
       props.featureFlags[0].update_path = props.featureFlags[0].destroy_path;
-      createWrapper(props, { provide: { glFeatures: { featureFlagToggle: true } } });
+      createWrapper(props);
       toggle = wrapper.find(GlToggle);
     });
 

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

@@ -37,7 +37,6 @@ describe('feature flag form', () => {
         glFeatures: {
           featureFlagPermissions: true,
           featureFlagsUsersPerEnvironment: true,
-          featureFlagToggle: true,
         },
       },
       sync: false,

ee/spec/frontend/feature_flags/store/index/mutations_spec.js

@@ -162,6 +162,7 @@ describe('Feature flags store Mutations', () => {
 
       mutations[types.UPDATE_FEATURE_FLAG](stateCopy, {
         ...featureFlag,
+        scopes: mapToScopesViewModel(featureFlag.scopes || []),
         active: false,
       });
     });
@@ -182,21 +183,25 @@ describe('Feature flags store Mutations', () => {
       expect(stateCopy.count.disabled).toBe(1);
     });
   });
+
   describe('RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS', () => {
-    beforeEach(() => {
+    const runUpdate = (stateCount, flagState, featureFlagUpdateParams) => {
       stateCopy.featureFlags = getRequestData.feature_flags.map(flag => ({
         ...flag,
+        ...flagState,
         scopes: mapToScopesViewModel(flag.scopes || []),
       }));
-      stateCopy.count = { enabled: 1, disabled: 0 };
+      stateCopy.count = stateCount;
 
       mutations[types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](stateCopy, {
         ...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([
         {
           ...featureFlag,
@@ -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', () => {
     beforeEach(() => {
       stateCopy.featureFlags = getRequestData.feature_flags.map(flag => ({

ee/spec/models/operations/feature_flag_spec.rb

@@ -57,7 +57,7 @@ describe Operations::FeatureFlag do
   describe '.enabled' do
     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) }
 
       it 'returns the flag' do
@@ -65,19 +65,39 @@ describe Operations::FeatureFlag do
       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) }
 
       it 'does not return the flag' do
         is_expected.to be_empty
       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
 
   describe '.disabled' do
     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) }
 
       it 'does not return the flag' do
@@ -85,42 +105,31 @@ describe Operations::FeatureFlag do
       end
     end
 
-    context 'when the feature flag does not have an active scope' 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
+    context 'when the feature flag is active and all scopes are inactive' do
       let!(:feature_flag) { create(:operations_feature_flag, active: true) }
-      let!(:scope) { create_scope(feature_flag, 'production', true) }
 
-      it 'returns virtual active value' do
-        expect(subject.first.active).to be_truthy
+      it 'does not return the flag' do
+        feature_flag.default_scope.update!(active: false)
+
+        is_expected.to be_empty
       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!(:scope) { create_scope(feature_flag, 'production', false) }
 
-      it 'returns virtual active value' do
-        expect(subject.first.active).to be_falsy
+      it 'returns the flag' do
+        is_expected.to eq([feature_flag])
       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!(:scope) { create_scope(feature_flag, 'production', true) }
 
-      it 'returns virtual active value' do
-        expect(subject.first.active).to be_truthy
+      it 'returns the flag' do
+        feature_flag.default_scope.update!(active: true)
+
+        is_expected.to eq([feature_flag])
       end
     end
   end

ee/spec/requests/api/feature_flag_scopes_spec.rb

@@ -54,13 +54,13 @@ describe API::FeatureFlagScopes do
           params: params
     end
 
-    let(:feature_flag_1) { create_flag(project, 'flag_1', false) }
-    let(:feature_flag_2) { create_flag(project, 'flag_2', false) }
+    let(:feature_flag_1) { create_flag(project, 'flag_1', true) }
+    let(:feature_flag_2) { create_flag(project, 'flag_2', true) }
 
     before do
-      create_scope(feature_flag_1, 'staging', true)
-      create_scope(feature_flag_1, 'production', false)
-      create_scope(feature_flag_2, 'review/*', true)
+      create_scope(feature_flag_1, 'staging', false)
+      create_scope(feature_flag_1, 'production', true)
+      create_scope(feature_flag_2, 'review/*', false)
     end
 
     context 'when environment is production' do
@@ -73,8 +73,8 @@ describe API::FeatureFlagScopes do
 
         expect(response).to have_gitlab_http_status(:ok)
         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.first).to include({ 'name' => 'flag_2', 'active' => false })
+        expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true })
+        expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true })
       end
     end
 
@@ -85,8 +85,8 @@ describe API::FeatureFlagScopes do
         subject
 
         expect(response).to have_gitlab_http_status(:ok)
-        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.second).to include({ 'name' => 'flag_1', 'active' => false })
+        expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true })
       end
     end
 
@@ -97,8 +97,8 @@ describe API::FeatureFlagScopes do
         subject
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false })
-        expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true })
+        expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true })
+        expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false })
       end
     end
   end

ee/spec/requests/api/unleash_spec.rb

@@ -134,7 +134,7 @@ describe API::Unleash do
         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_2['enabled']).to eq(true)
+        expect(feature_flag_2['enabled']).to eq(false)
       end
     end
 
@@ -160,7 +160,7 @@ describe API::Unleash do
       it_behaves_like 'authenticated request'
       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!(: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) }
@@ -247,6 +247,17 @@ describe API::Unleash do
         }])
       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
         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" } }

ee/spec/services/feature_flags/update_service_spec.rb

@@ -7,7 +7,7 @@ describe FeatureFlags::UpdateService do
   let(:developer) { create(:user) }
   let(:reporter) { create(:user) }
   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
     stub_licensed_features(feature_flags: true)
@@ -88,14 +88,29 @@ describe FeatureFlags::UpdateService do
       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
         {
           scopes_attributes: [{ id: feature_flag.scopes.first.id, active: false }]
         }
       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(audit_event_message).to(
           include("Updated rule <strong>*</strong> active state "\

spec/migrations/backfill_operations_feature_flags_active_spec.rb

+# 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