From 5126d06627fa81860c623692a62051147831cae6 Mon Sep 17 00:00:00 2001
From: Mark Florian <mflorian@gitlab.com>
Date: Wed, 5 Feb 2020 17:41:19 +0000
Subject: [PATCH 01/15] Use unique feature flag name for ISD

This should allow _only_ the Instance Security Dashboard to be disabled,
without affecting the other Security Dashboards.
---
 .../assets/javascripts/pages/security/dashboard/show/index.js | 2 +-
 ee/app/controllers/security/application_controller.rb         | 4 ++--
 ee/app/helpers/ee/dashboard_helper.rb                         | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ee/app/assets/javascripts/pages/security/dashboard/show/index.js b/ee/app/assets/javascripts/pages/security/dashboard/show/index.js
index 4d1aba7ae1b39f..c08aba49040792 100644
--- a/ee/app/assets/javascripts/pages/security/dashboard/show/index.js
+++ b/ee/app/assets/javascripts/pages/security/dashboard/show/index.js
@@ -5,7 +5,7 @@ import syncWithRouter from 'ee/security_dashboard/store/plugins/sync_with_router
 import createStore from 'ee/security_dashboard/store';
 import InstanceSecurityDashboard from 'ee/security_dashboard/components/instance_security_dashboard.vue';
 
-if (gon.features && gon.features.securityDashboard) {
+if (gon.features && gon.features.instanceSecurityDashboard) {
   document.addEventListener('DOMContentLoaded', () => {
     const el = document.querySelector('#js-security');
     const {
diff --git a/ee/app/controllers/security/application_controller.rb b/ee/app/controllers/security/application_controller.rb
index 4a037051849460..634d1adbfe374d 100644
--- a/ee/app/controllers/security/application_controller.rb
+++ b/ee/app/controllers/security/application_controller.rb
@@ -6,13 +6,13 @@ class ApplicationController < ::ApplicationController
 
     before_action :check_feature_enabled!
     before_action do
-      push_frontend_feature_flag(:security_dashboard, default_enabled: true)
+      push_frontend_feature_flag(:instance_security_dashboard, default_enabled: true)
     end
 
     protected
 
     def check_feature_enabled!
-      render_404 unless Feature.enabled?(:security_dashboard, default_enabled: true)
+      render_404 unless Feature.enabled?(:instance_security_dashboard, default_enabled: true)
     end
 
     def vulnerable
diff --git a/ee/app/helpers/ee/dashboard_helper.rb b/ee/app/helpers/ee/dashboard_helper.rb
index 5fd3a0fa71adfb..d865190014172a 100644
--- a/ee/app/helpers/ee/dashboard_helper.rb
+++ b/ee/app/helpers/ee/dashboard_helper.rb
@@ -64,7 +64,7 @@ def get_dashboard_nav_links
     def security_dashboard_available?
       security_dashboard = InstanceSecurityDashboard.new(current_user)
 
-      ::Feature.enabled?(:security_dashboard, default_enabled: true) &&
+      ::Feature.enabled?(:instance_security_dashboard, default_enabled: true) &&
         security_dashboard.feature_available?(:security_dashboard) &&
         can?(current_user, :read_instance_security_dashboard, security_dashboard)
     end
-- 
GitLab


From fee9762f9a10f33a9d149a65d9303cfaa731882f Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Wed, 5 Feb 2020 19:09:15 -0500
Subject: [PATCH 02/15] Update feature flag in specs

These specs should be passing now!
---
 ee/spec/controllers/security/projects_controller_spec.rb        | 2 +-
 .../security/application_controller_shared_examples.rb          | 2 +-
 .../security_dashboard_json_endpoint_shared_examples.rb         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ee/spec/controllers/security/projects_controller_spec.rb b/ee/spec/controllers/security/projects_controller_spec.rb
index 11fb4718f42c0a..af84e4dfd98f6c 100644
--- a/ee/spec/controllers/security/projects_controller_spec.rb
+++ b/ee/spec/controllers/security/projects_controller_spec.rb
@@ -155,7 +155,7 @@
 
       context 'and the security dashboard feature is disabled' do
         it '404s' do
-          stub_feature_flags(security_dashboard: false)
+          stub_feature_flags(instance_security_dashboard: false)
 
           subject
 
diff --git a/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb b/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb
index 990c81cf25d739..6c2dfe8b9b3f31 100644
--- a/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb
+++ b/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb
@@ -27,7 +27,7 @@
 
     context 'and the security dashboard feature is disabled' do
       it '404s' do
-        stub_feature_flags(security_dashboard: false)
+        stub_feature_flags(instance_security_dashboard: false)
 
         security_application_controller_child_action
 
diff --git a/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb b/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb
index 5471df8cf76908..8c41492810a14c 100644
--- a/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb
+++ b/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb
@@ -28,7 +28,7 @@
 
     context 'and the security dashboard feature is disabled' do
       it '404s' do
-        stub_feature_flags(security_dashboard: false)
+        stub_feature_flags(instance_security_dashboard: false)
 
         security_dashboard_request
 
-- 
GitLab


From 8086e90d83557f05203f5c745532e45f935f2135 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Wed, 5 Feb 2020 19:46:26 -0500
Subject: [PATCH 03/15] Refactor DashboardHelper specs

We needed to update the security nav link portion, and the
coupling of the operations, environments, and security dashboard
links was getting hard to handle. So I've split them all out into
their own `describe`s and updated the security link specs.
---
 ee/spec/helpers/ee/dashboard_helper_spec.rb | 165 ++++++++++++--------
 1 file changed, 100 insertions(+), 65 deletions(-)

diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb
index bcff607bc2dc63..73d1a7088ebf46 100644
--- a/ee/spec/helpers/ee/dashboard_helper_spec.rb
+++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb
@@ -10,13 +10,10 @@
   describe '#dashboard_nav_links' do
     before do
       allow(helper).to receive(:current_user).and_return(user)
+      allow(helper).to receive(:can?).and_return(true)
     end
 
     describe 'analytics' do
-      before do
-        allow(helper).to receive(:can?) { true }
-      end
-
       context 'when at least one analytics feature is enabled' do
         before do
           enable_only_one_analytics_feature_flag
@@ -50,94 +47,132 @@
       end
     end
 
-    describe 'operations, environments and security' do
-      using RSpec::Parameterized::TableSyntax
+    describe 'operations dashboard link' do
+      context 'when the feature is available on the license' do
+        context 'and the user is authenticated' do
+          it 'is included in the nav' do
+            allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(true)
 
-      before do
-        allow(helper).to receive(:can?).and_return(false)
+            expect(helper.dashboard_nav_links).to include(:operations)
+          end
+        end
+
+        context 'and the user is not authenticated' do
+          it 'is not included in the nav' do
+            allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+
+            expect(helper.dashboard_nav_links).not_to include(:operations)
+          end
+        end
       end
 
-      where(:ability, :feature_flag, :nav_link) do
-        :read_operations_dashboard        | nil                     | :operations
-        :read_operations_dashboard        | :environments_dashboard | :environments
-        :read_instance_security_dashboard | :security_dashboard     | :security
+      context 'when the feature is not available on the license' do
+        it 'is not included in the nav' do
+          allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+
+          expect(helper.dashboard_nav_links).not_to include(:operations)
+        end
       end
+    end
 
-      with_them do
-        describe 'when the feature is enabled' do
-          before do
-            stub_feature_flags(feature_flag => true) unless feature_flag.nil?
+    describe 'environments dashboard link' do
+      context 'when the feature is enabled' do
+        before do
+          stub_feature_flags(environments_dashboard: true)
+        end
+
+        context 'and the feature is available on the license' do
+          context 'and the user is authenticated' do
+            it 'is included in the nav' do
+              allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(true)
+
+              expect(helper.dashboard_nav_links).to include(:environments)
+            end
           end
 
-          context 'and the feature is available on the license' do
-            context 'and the user is authenticated' do
-              before do
-                stub_resource_visibility(
-                  feature_flag,
-                  read_other_resources: true,
-                  read_security_dashboard: true,
-                  security_dashboard_available: true
-                )
-              end
+          context 'and the user is not authenticated' do
+            it 'is not included in the nav' do
+              allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
 
-              it 'includes the nav link' do
-                expect(helper.dashboard_nav_links).to include(nav_link)
-              end
+              expect(helper.dashboard_nav_links).not_to include(:environments)
             end
+          end
+        end
 
-            context 'and the user is not authenticated' do
-              let(:user) { nil }
+        context 'and the feature is not available on the license' do
+          it 'is not included in the nav' do
+            allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
 
-              before do
-                stub_resource_visibility(
-                  feature_flag,
-                  read_other_resources: false,
-                  read_security_dashboard: false,
-                  security_dashboard_available: true
-                )
-              end
+            expect(helper.dashboard_nav_links).not_to include(:environments)
+          end
+        end
+      end
+
+      context 'when the feature is not enabled' do
+        it 'is not included in the nav' do
+          stub_feature_flags(environments_dashboard: false)
+          allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(true)
+
+          expect(helper.dashboard_nav_links).not_to include(:environments)
+        end
+      end
+    end
+
+    describe 'security dashboard link' do
+      context 'when the feature is enabled' do
+        before do
+          stub_feature_flags(instance_security_dashboard: true)
+        end
 
-              it 'does not include the nav link' do
-                expect(helper.dashboard_nav_links).not_to include(nav_link)
+        context 'and the feature is available on the license' do
+          before do
+            stub_licensed_features(security_dashboard: true)
+          end
+
+          context 'and the user is authenticated' do
+            it 'is included in the nav' do
+              allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+                allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(true)
               end
+
+              expect(helper.dashboard_nav_links).to include(:security)
             end
           end
 
-          context 'and the feature is not available on the license' do
-            before do
-              stub_resource_visibility(
-                feature_flag,
-                read_other_resources: false,
-                read_security_dashboard: true,
-                security_dashboard_available: false
-              )
-            end
+          context 'and the user is not authenticated' do
+            it 'is not included in the nav' do
+              allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+                allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(false)
+              end
 
-            it 'does not include the nav link' do
-              expect(helper.dashboard_nav_links).not_to include(nav_link)
+              expect(helper.dashboard_nav_links).not_to include(:security)
             end
           end
+        end
 
-          def stub_resource_visibility(feature_flag, read_other_resources:, read_security_dashboard:, security_dashboard_available:)
-            if feature_flag == :security_dashboard
-              stub_licensed_features(feature_flag => security_dashboard_available)
+        context 'when the feature is not available on the license' do
+          it 'is not included in the nav' do
+            stub_licensed_features(security_dashboard: false)
 
-              allow(helper).to receive(:can?).and_return(read_security_dashboard)
-            else
-              allow(helper).to receive(:can?).with(user, ability).and_return(read_other_resources)
+            allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+              allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(true)
             end
+
+            expect(helper.dashboard_nav_links).not_to include(:security)
           end
         end
+      end
 
-        describe 'when the feature is disabled' do
-          before do
-            stub_feature_flags(feature_flag => false) unless feature_flag.nil?
-            allow(helper).to receive(:can?).and_return(false)
-          end
+      context 'when the feature is not enabled' do
+        it 'is not included in the nav' do
+          stub_feature_flags(instance_security_dashboard: false)
+          stub_licensed_features(security_dashboard: true)
 
-          it 'does not include the nav link' do
-            expect(helper.dashboard_nav_links).not_to include(nav_link)
+          allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+            allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(true)
           end
+
+          expect(helper.dashboard_nav_links).not_to include(:security)
         end
       end
     end
-- 
GitLab


From 5db8284ec434cd8bcce75223e7267676eff932e9 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Tue, 11 Feb 2020 20:44:46 -0500
Subject: [PATCH 04/15] Improve DashboardHelper specs

Always specifying arguments when stubbing `can?` reduces the risk of
false positives.
---
 ee/spec/helpers/ee/dashboard_helper_spec.rb | 115 ++++++++++++++------
 1 file changed, 81 insertions(+), 34 deletions(-)

diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb
index 73d1a7088ebf46..cc0271dd0649db 100644
--- a/ee/spec/helpers/ee/dashboard_helper_spec.rb
+++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb
@@ -10,13 +10,14 @@
   describe '#dashboard_nav_links' do
     before do
       allow(helper).to receive(:current_user).and_return(user)
-      allow(helper).to receive(:can?).and_return(true)
     end
 
     describe 'analytics' do
       context 'when at least one analytics feature is enabled' do
         before do
           enable_only_one_analytics_feature_flag
+
+          stub_user_permissions_for(:analytics, false)
         end
 
         it 'includes analytics' do
@@ -31,7 +32,7 @@
 
         context 'and the user has no access to instance statistics features' do
           before do
-            allow(helper).to receive(:can?) { false }
+            stub_user_permissions_for(:analytics, false)
           end
 
           it 'does not include analytics' do
@@ -40,6 +41,10 @@
         end
 
         context 'and the user has access to instance statistics features' do
+          before do
+            stub_user_permissions_for(:analytics, true)
+          end
+
           it 'does include analytics' do
             expect(helper.dashboard_nav_links).to include(:analytics)
           end
@@ -50,26 +55,32 @@
     describe 'operations dashboard link' do
       context 'when the feature is available on the license' do
         context 'and the user is authenticated' do
-          it 'is included in the nav' do
-            allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(true)
+          before do
+            stub_user_permissions_for(:operations, true)
+          end
 
+          it 'is included in the nav' do
             expect(helper.dashboard_nav_links).to include(:operations)
           end
         end
 
         context 'and the user is not authenticated' do
-          it 'is not included in the nav' do
-            allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+          before do
+            stub_user_permissions_for(:operations, false)
+          end
 
+          it 'is not included in the nav' do
             expect(helper.dashboard_nav_links).not_to include(:operations)
           end
         end
       end
 
       context 'when the feature is not available on the license' do
-        it 'is not included in the nav' do
-          allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+        before do
+          stub_user_permissions_for(:operations, false)
+        end
 
+        it 'is not included in the nav' do
           expect(helper.dashboard_nav_links).not_to include(:operations)
         end
       end
@@ -83,36 +94,44 @@
 
         context 'and the feature is available on the license' do
           context 'and the user is authenticated' do
-            it 'is included in the nav' do
-              allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(true)
+            before do
+              stub_user_permissions_for(:operations, true)
+            end
 
+            it 'is included in the nav' do
               expect(helper.dashboard_nav_links).to include(:environments)
             end
           end
 
           context 'and the user is not authenticated' do
-            it 'is not included in the nav' do
-              allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+            before do
+              stub_user_permissions_for(:operations, false)
+            end
 
+            it 'is not included in the nav' do
               expect(helper.dashboard_nav_links).not_to include(:environments)
             end
           end
         end
 
         context 'and the feature is not available on the license' do
-          it 'is not included in the nav' do
-            allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+          before do
+            stub_user_permissions_for(:operations, false)
+          end
 
+          it 'is not included in the nav' do
             expect(helper.dashboard_nav_links).not_to include(:environments)
           end
         end
       end
 
       context 'when the feature is not enabled' do
-        it 'is not included in the nav' do
+        before do
           stub_feature_flags(environments_dashboard: false)
-          allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(true)
+          stub_user_permissions_for(:operations, false)
+        end
 
+        it 'is not included in the nav' do
           expect(helper.dashboard_nav_links).not_to include(:environments)
         end
       end
@@ -130,52 +149,80 @@
           end
 
           context 'and the user is authenticated' do
-            it 'is included in the nav' do
-              allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-                allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(true)
-              end
+            before do
+              stub_user_permissions_for(:security, true)
+            end
 
+            it 'is included in the nav' do
               expect(helper.dashboard_nav_links).to include(:security)
             end
           end
 
           context 'and the user is not authenticated' do
-            it 'is not included in the nav' do
-              allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-                allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(false)
-              end
+            before do
+              stub_user_permissions_for(:security, false)
+            end
 
+            it 'is not included in the nav' do
               expect(helper.dashboard_nav_links).not_to include(:security)
             end
           end
         end
 
         context 'when the feature is not available on the license' do
-          it 'is not included in the nav' do
+          before do
             stub_licensed_features(security_dashboard: false)
+            stub_user_permissions_for(:security, true)
+          end
 
-            allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-              allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(true)
-            end
-
+          it 'is not included in the nav' do
             expect(helper.dashboard_nav_links).not_to include(:security)
           end
         end
       end
 
       context 'when the feature is not enabled' do
-        it 'is not included in the nav' do
+        before do
           stub_feature_flags(instance_security_dashboard: false)
           stub_licensed_features(security_dashboard: true)
+          stub_user_permissions_for(:security, true)
+        end
 
-          allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-            allow(helper).to receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(true)
-          end
-
+        it 'is not included in the nav' do
           expect(helper.dashboard_nav_links).not_to include(:security)
         end
       end
     end
+
+    def stub_user_permissions_for(feature, enabled)
+      allow(helper).to receive(:can?).with(user, :read_cross_project).and_return(false)
+
+      if feature == :analytics
+        allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(enabled)
+      else
+        allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(false)
+      end
+
+      if feature == :security
+        allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+          allow(helper).to(
+            receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(enabled)
+          )
+        end
+      else
+        allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+          allow(helper).to(
+            receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(false)
+          )
+        end
+      end
+
+      if feature == :operations
+        allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(enabled)
+      else
+        allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+      end
+    end
   end
 
   describe '.has_start_trial?' do
-- 
GitLab


From 63ae1ab488048f953ebedb5d6223d948de81c949 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Thu, 13 Feb 2020 22:45:51 -0500
Subject: [PATCH 05/15] Add migration to create instance sec dash flag

The migration looks for an existing security_dashboard feature flag and
copies it into a new row with the feature key
instance_security_dashboard.
---
 ...ure_flag_to_instance_security_dashboard.rb | 49 +++++++++++++++++++
 ...lag_to_instance_security_dashboard_spec.rb | 42 ++++++++++++++++
 2 files changed, 91 insertions(+)
 create mode 100644 db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
 create mode 100644 spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb

diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
new file mode 100644
index 00000000000000..82efa7b34a6862
--- /dev/null
+++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  class FeatureGate < ActiveRecord::Base
+    self.table_name = 'feature_gates'
+  end
+
+  def up
+    security_dashboard_feature = FeatureGate.where(feature_key: :security_dashboard).first
+
+    if security_dashboard_feature.present?
+      FeatureGate.create!(
+        feature_key: :instance_security_dashboard,
+        key: security_dashboard_feature.key,
+        value: security_dashboard_feature.value
+      )
+    end
+  end
+
+  def down
+    FeatureGate.where(feature_key: :instance_security_dashboard).delete_all
+  end
+end
diff --git a/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
new file mode 100644
index 00000000000000..131af97160aa2c
--- /dev/null
+++ b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require Rails.root.join('db', 'migrate', '20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb')
+
+describe RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard, :migration do
+  let(:feature_gates) { table(:feature_gates) }
+
+  subject(:migration) { described_class.new }
+
+  describe '#up' do
+    it 'copies the security_dashboard feature gate to a new instance_security_dashboard gate' do
+      feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false')
+
+      migration.up
+
+      instance_security_dashboard_feature = feature_gates.where(feature_key: :instance_security_dashboard).first
+      expect(instance_security_dashboard_feature.key).to eq('boolean')
+      expect(instance_security_dashboard_feature.value).to eq('false')
+    end
+
+    context 'when there is no security_dashboard gate' do
+      it 'does nothing' do
+        migration.up
+
+        instance_security_dashboard_feature = feature_gates.where(feature_key: :instance_security_dashboard)
+        expect(instance_security_dashboard_feature).to be_empty
+      end
+    end
+  end
+
+  describe '#down' do
+    it 'removes the instance_security_dashboard gate' do
+      instance_security_dashboard_feature = feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false')
+
+      migration.down
+
+      expect { instance_security_dashboard_feature.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+end
-- 
GitLab


From 2ed5ae96b6a49f71f43290668f97a5fc89ab83a0 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Thu, 13 Feb 2020 23:10:38 -0500
Subject: [PATCH 06/15] Add post migration to remove sec dashboard flag

This migration removes the security_dashboard feature flag once we can
be sure that the instance_security_dashboard flag has been created.
---
 ..._remove_security_dashboard_feature_flag.rb | 43 +++++++++++++++++++
 ...ve_security_dashboard_feature_flag_spec.rb | 42 ++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
 create mode 100644 spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb

diff --git a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
new file mode 100644
index 00000000000000..bc8f4df629abcf
--- /dev/null
+++ b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveSecurityDashboardFeatureFlag < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  class FeatureGate < ActiveRecord::Base
+    self.table_name = 'feature_gates'
+  end
+
+  def up
+    FeatureGate.where(feature_key: :security_dashboard).delete_all
+  end
+
+  def down
+    instance_security_dashboard_feature = FeatureGate.where(feature_key: :instance_security_dashboard).first
+
+    if instance_security_dashboard_feature.present?
+      FeatureGate.create!(
+        feature_key: :security_dashboard,
+        key: instance_security_dashboard_feature.key,
+        value: instance_security_dashboard_feature.value
+      )
+    end
+  end
+end
diff --git a/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb b/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb
new file mode 100644
index 00000000000000..e255022c1dab1e
--- /dev/null
+++ b/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require Rails.root.join('db', 'post_migrate', '20200214034836_remove_security_dashboard_feature_flag.rb')
+
+describe RemoveSecurityDashboardFeatureFlag, :migration do
+  let(:feature_gates) { table(:feature_gates) }
+
+  subject(:migration) { described_class.new }
+
+  describe '#up' do
+    it 'deletes the security_dashboard feature gate' do
+      security_dashboard_feature = feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false')
+
+      migration.up
+
+      expect { security_dashboard_feature.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+
+  describe '#down' do
+    it 'copies the instance_security_dashboard feature gate to a security_dashboard gate' do
+      feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false')
+
+      migration.down
+
+      security_dashboard_feature = feature_gates.where(feature_key: :security_dashboard).first
+      expect(security_dashboard_feature.key).to eq('boolean')
+      expect(security_dashboard_feature.value).to eq('false')
+    end
+
+    context 'when there is no instance_security_dashboard gate' do
+      it 'does nothing' do
+        migration.down
+
+        security_dashboard_feature = feature_gates.where(feature_key: :security_dashboard)
+        expect(security_dashboard_feature).to be_empty
+      end
+    end
+  end
+end
-- 
GitLab


From f002881074f0830a3e5e4b61af8b5b2a4748ee7f Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Thu, 13 Feb 2020 23:27:20 -0500
Subject: [PATCH 07/15] Update schema version

I hope that it should be updated even though this is only a data
migration...
---
 db/schema.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/schema.rb b/db/schema.rb
index fec0ba6e7530ff..962f9eb6012671 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_02_13_220211) do
+ActiveRecord::Schema.define(version: 2020_02_14_034836) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
-- 
GitLab


From 96652abf0fe5f0d0c60fbd86be0623f7105b680c Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Fri, 14 Feb 2020 19:36:17 -0500
Subject: [PATCH 08/15] Remove unneeded comments and includes

Yay cleaner migrations!
---
 ...ure_flag_to_instance_security_dashboard.rb | 21 -------------------
 ..._remove_security_dashboard_feature_flag.rb | 18 ----------------
 2 files changed, 39 deletions(-)

diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
index 82efa7b34a6862..b5140b689c6f48 100644
--- a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
+++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
@@ -4,29 +4,8 @@
 # for more information on how to write migrations for GitLab.
 
 class RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard < ActiveRecord::Migration[6.0]
-  include Gitlab::Database::MigrationHelpers
-
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
-  # "add_column_with_default" you must disable the use of transactions
-  # as these methods can not run in an existing transaction.
-  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
-  # that either of them is the _only_ method called in the migration,
-  # any other changes should go in a separate migration.
-  # This ensures that upon failure _only_ the index creation or removing fails
-  # and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
   class FeatureGate < ActiveRecord::Base
     self.table_name = 'feature_gates'
   end
diff --git a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
index bc8f4df629abcf..305f27b30ecf21 100644
--- a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
+++ b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
@@ -1,26 +1,8 @@
 # frozen_string_literal: true
 
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class RemoveSecurityDashboardFeatureFlag < ActiveRecord::Migration[6.0]
-  include Gitlab::Database::MigrationHelpers
-
   DOWNTIME = false
 
-  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
-  # "add_column_with_default" you must disable the use of transactions
-  # as these methods can not run in an existing transaction.
-  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
-  # that either of them is the _only_ method called in the migration,
-  # any other changes should go in a separate migration.
-  # This ensures that upon failure _only_ the index creation or removing fails
-  # and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
   class FeatureGate < ActiveRecord::Base
     self.table_name = 'feature_gates'
   end
-- 
GitLab


From 3484d8b5700e7ac1664ff578983dc0462e1ee6ea Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Fri, 14 Feb 2020 20:17:32 -0500
Subject: [PATCH 09/15] Update migrations for multiple gates

When using actors, there may be multiple records named
:security_dashboard. We want to make sure we only copy and delete the
boolean record.
---
 ..._feature_flag_to_instance_security_dashboard.rb |  6 +++---
 ...34836_remove_security_dashboard_feature_flag.rb |  4 ++--
 ..._remove_security_dashboard_feature_flag_spec.rb | 12 +++++++-----
 ...ure_flag_to_instance_security_dashboard_spec.rb | 14 ++++++++------
 4 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
index b5140b689c6f48..59cf3ff2d9a266 100644
--- a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
+++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
@@ -11,18 +11,18 @@ class FeatureGate < ActiveRecord::Base
   end
 
   def up
-    security_dashboard_feature = FeatureGate.where(feature_key: :security_dashboard).first
+    security_dashboard_feature = FeatureGate.find_by(feature_key: :security_dashboard, key: :boolean)
 
     if security_dashboard_feature.present?
       FeatureGate.create!(
         feature_key: :instance_security_dashboard,
-        key: security_dashboard_feature.key,
+        key: :boolean,
         value: security_dashboard_feature.value
       )
     end
   end
 
   def down
-    FeatureGate.where(feature_key: :instance_security_dashboard).delete_all
+    FeatureGate.find_by(feature_key: :instance_security_dashboard, key: :boolean)&.delete
   end
 end
diff --git a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
index 305f27b30ecf21..abbb1caf42b346 100644
--- a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
+++ b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
@@ -8,11 +8,11 @@ class FeatureGate < ActiveRecord::Base
   end
 
   def up
-    FeatureGate.where(feature_key: :security_dashboard).delete_all
+    FeatureGate.find_by(feature_key: :security_dashboard, key: :boolean)&.delete
   end
 
   def down
-    instance_security_dashboard_feature = FeatureGate.where(feature_key: :instance_security_dashboard).first
+    instance_security_dashboard_feature = FeatureGate.find_by(feature_key: :instance_security_dashboard, key: :boolean)
 
     if instance_security_dashboard_feature.present?
       FeatureGate.create!(
diff --git a/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb b/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb
index e255022c1dab1e..6eb128704d6f49 100644
--- a/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb
+++ b/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb
@@ -11,22 +11,24 @@
 
   describe '#up' do
     it 'deletes the security_dashboard feature gate' do
-      security_dashboard_feature = feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false')
+      security_dashboard_feature = feature_gates.create!(feature_key: :security_dashboard, key: :boolean, value: 'false')
+      actors_security_dashboard_feature = feature_gates.create!(feature_key: :security_dashboard, key: :actors, value: 'Project:1')
 
       migration.up
 
       expect { security_dashboard_feature.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(actors_security_dashboard_feature.reload).to be_present
     end
   end
 
   describe '#down' do
     it 'copies the instance_security_dashboard feature gate to a security_dashboard gate' do
+      feature_gates.create!(feature_key: :instance_security_dashboard, key: :actors, value: 'Project:1')
       feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false')
 
       migration.down
 
-      security_dashboard_feature = feature_gates.where(feature_key: :security_dashboard).first
-      expect(security_dashboard_feature.key).to eq('boolean')
+      security_dashboard_feature = feature_gates.find_by(feature_key: :security_dashboard, key: :boolean)
       expect(security_dashboard_feature.value).to eq('false')
     end
 
@@ -34,8 +36,8 @@
       it 'does nothing' do
         migration.down
 
-        security_dashboard_feature = feature_gates.where(feature_key: :security_dashboard)
-        expect(security_dashboard_feature).to be_empty
+        security_dashboard_feature = feature_gates.find_by(feature_key: :security_dashboard, key: :boolean)
+        expect(security_dashboard_feature).to be_nil
       end
     end
   end
diff --git a/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
index 131af97160aa2c..d1819293367a27 100644
--- a/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
+++ b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
@@ -11,12 +11,12 @@
 
   describe '#up' do
     it 'copies the security_dashboard feature gate to a new instance_security_dashboard gate' do
-      feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false')
+      feature_gates.create!(feature_key: :security_dashboard, key: :actors, value: 'Project:1')
+      feature_gates.create!(feature_key: :security_dashboard, key: :boolean, value: 'false')
 
       migration.up
 
-      instance_security_dashboard_feature = feature_gates.where(feature_key: :instance_security_dashboard).first
-      expect(instance_security_dashboard_feature.key).to eq('boolean')
+      instance_security_dashboard_feature = feature_gates.find_by(feature_key: :instance_security_dashboard, key: :boolean)
       expect(instance_security_dashboard_feature.value).to eq('false')
     end
 
@@ -24,19 +24,21 @@
       it 'does nothing' do
         migration.up
 
-        instance_security_dashboard_feature = feature_gates.where(feature_key: :instance_security_dashboard)
-        expect(instance_security_dashboard_feature).to be_empty
+        instance_security_dashboard_feature = feature_gates.find_by(feature_key: :instance_security_dashboard, key: :boolean)
+        expect(instance_security_dashboard_feature).to be_nil
       end
     end
   end
 
   describe '#down' do
     it 'removes the instance_security_dashboard gate' do
-      instance_security_dashboard_feature = feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false')
+      actors_instance_security_dashboard_feature = feature_gates.create!(feature_key: :instance_security_dashboard, key: :actors, value: 'Project:1')
+      instance_security_dashboard_feature = feature_gates.create!(feature_key: :instance_security_dashboard, key: :boolean, value: 'false')
 
       migration.down
 
       expect { instance_security_dashboard_feature.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(actors_instance_security_dashboard_feature.reload).to be_present
     end
   end
 end
-- 
GitLab


From a24fa32ef788aa2de4d5344502dfe797362d2f60 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Tue, 18 Feb 2020 18:23:53 -0500
Subject: [PATCH 10/15] Remove the last comment

I missed that one. Oops.
---
 ...ty_dashboard_feature_flag_to_instance_security_dashboard.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
index 59cf3ff2d9a266..a6feb00836c5e1 100644
--- a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
+++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
@@ -1,8 +1,5 @@
 # frozen_string_literal: true
 
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard < ActiveRecord::Migration[6.0]
   DOWNTIME = false
 
-- 
GitLab


From 72c09fb0f9de54c65743eb98bbc1b4e0ed91db90 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Tue, 18 Feb 2020 18:27:17 -0500
Subject: [PATCH 11/15] Rename spec file to match example in docs

The example spec in the migration docs doesn't include a timestamp
in its filename, so I'm going to follow that example.
---
 ...lag_spec.rb => remove_security_dashboard_feature_flag_spec.rb} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename spec/migrations/{20200214034836_remove_security_dashboard_feature_flag_spec.rb => remove_security_dashboard_feature_flag_spec.rb} (100%)

diff --git a/spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb b/spec/migrations/remove_security_dashboard_feature_flag_spec.rb
similarity index 100%
rename from spec/migrations/20200214034836_remove_security_dashboard_feature_flag_spec.rb
rename to spec/migrations/remove_security_dashboard_feature_flag_spec.rb
-- 
GitLab


From 7c7c809aa23b4fc8a3b6ceb2abb86c16121209b6 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Tue, 18 Feb 2020 18:54:44 -0500
Subject: [PATCH 12/15] Avoid exceptions from duplicate gates

In an edge where the gate we're trying to create already exists in the
database, we want to fail silently rather than raise an error. This is
only likely to be a problem on our dev machines, where we may migrate
and rollback several times as we update the GDK and switch branches.
---
 ...hboard_feature_flag_to_instance_security_dashboard.rb | 2 +-
 ...00214034836_remove_security_dashboard_feature_flag.rb | 2 +-
 .../remove_security_dashboard_feature_flag_spec.rb       | 9 +++++++++
 ...d_feature_flag_to_instance_security_dashboard_spec.rb | 9 +++++++++
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
index a6feb00836c5e1..042fc9670e6531 100644
--- a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
+++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
@@ -11,7 +11,7 @@ def up
     security_dashboard_feature = FeatureGate.find_by(feature_key: :security_dashboard, key: :boolean)
 
     if security_dashboard_feature.present?
-      FeatureGate.create!(
+      FeatureGate.find_or_create_by!(
         feature_key: :instance_security_dashboard,
         key: :boolean,
         value: security_dashboard_feature.value
diff --git a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
index abbb1caf42b346..20bc5ceb820f2d 100644
--- a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
+++ b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
@@ -15,7 +15,7 @@ def down
     instance_security_dashboard_feature = FeatureGate.find_by(feature_key: :instance_security_dashboard, key: :boolean)
 
     if instance_security_dashboard_feature.present?
-      FeatureGate.create!(
+      FeatureGate.find_or_create_by!(
         feature_key: :security_dashboard,
         key: instance_security_dashboard_feature.key,
         value: instance_security_dashboard_feature.value
diff --git a/spec/migrations/remove_security_dashboard_feature_flag_spec.rb b/spec/migrations/remove_security_dashboard_feature_flag_spec.rb
index 6eb128704d6f49..7ef43134d24d85 100644
--- a/spec/migrations/remove_security_dashboard_feature_flag_spec.rb
+++ b/spec/migrations/remove_security_dashboard_feature_flag_spec.rb
@@ -40,5 +40,14 @@
         expect(security_dashboard_feature).to be_nil
       end
     end
+
+    context 'when there already is a security_dashboard gate' do
+      it 'does nothing' do
+        feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false')
+        feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false')
+
+        expect { migration.down }.not_to raise_error
+      end
+    end
   end
 end
diff --git a/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
index d1819293367a27..bc982e8952e2f0 100644
--- a/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
+++ b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb
@@ -28,6 +28,15 @@
         expect(instance_security_dashboard_feature).to be_nil
       end
     end
+
+    context 'when there is already an instance_security_dashboard gate' do
+      it 'does nothing' do
+        feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false')
+        feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false')
+
+        expect { migration.up }.not_to raise_error
+      end
+    end
   end
 
   describe '#down' do
-- 
GitLab


From 2c81a552888c92c8a9f7bf36e4c2e276c2945e40 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Wed, 19 Feb 2020 00:03:05 +0000
Subject: [PATCH 13/15] Apply suggestion to
 ee/spec/helpers/ee/dashboard_helper_spec.rb

---
 ee/spec/helpers/ee/dashboard_helper_spec.rb | 34 ++++++---------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb
index cc0271dd0649db..75d7cf27fc30e6 100644
--- a/ee/spec/helpers/ee/dashboard_helper_spec.rb
+++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb
@@ -197,30 +197,16 @@
     def stub_user_permissions_for(feature, enabled)
       allow(helper).to receive(:can?).with(user, :read_cross_project).and_return(false)
 
-      if feature == :analytics
-        allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(enabled)
-      else
-        allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(false)
-      end
-
-      if feature == :security
-        allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-          allow(helper).to(
-            receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(enabled)
-          )
-        end
-      else
-        allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-          allow(helper).to(
-            receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(false)
-          )
-        end
-      end
-
-      if feature == :operations
-        allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(enabled)
-      else
-        allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(false)
+      can_read_instance_statistics = enabled && feature == :analytics
+      can_read_operations_dashboard = enabled && feature == :operations
+      can_read_instance_security_dashboard = enabled && feature == :security
+      
+      allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(can_read_instance_statistics)
+      allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(can_read_operations_dashboard)
+      allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
+        allow(helper).to(
+          receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(can_read_instance_security_dashboard)
+        )
       end
     end
   end
-- 
GitLab


From 06c3d164d2ddcfaf9eb6079686f1c3654b56f034 Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Tue, 18 Feb 2020 19:49:29 -0500
Subject: [PATCH 14/15] Remove trailing whitespace

Thanks, rubocop
---
 ee/spec/helpers/ee/dashboard_helper_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb
index 75d7cf27fc30e6..ed5828ac4a2646 100644
--- a/ee/spec/helpers/ee/dashboard_helper_spec.rb
+++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb
@@ -200,7 +200,7 @@ def stub_user_permissions_for(feature, enabled)
       can_read_instance_statistics = enabled && feature == :analytics
       can_read_operations_dashboard = enabled && feature == :operations
       can_read_instance_security_dashboard = enabled && feature == :security
-      
+
       allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(can_read_instance_statistics)
       allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(can_read_operations_dashboard)
       allow_next_instance_of(InstanceSecurityDashboard) do |dashboard|
-- 
GitLab


From 35b80f4319ea5745db614fbbc15609a3eb13aa5a Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Wed, 19 Feb 2020 14:25:41 -0500
Subject: [PATCH 15/15] Use the safe find or create method

It's really neat that GitLab has this.
---
 ...y_dashboard_feature_flag_to_instance_security_dashboard.rb | 4 ++--
 .../20200214034836_remove_security_dashboard_feature_flag.rb  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
index 042fc9670e6531..8d37f6c1dd4a10 100644
--- a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
+++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb
@@ -3,7 +3,7 @@
 class RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard < ActiveRecord::Migration[6.0]
   DOWNTIME = false
 
-  class FeatureGate < ActiveRecord::Base
+  class FeatureGate < ApplicationRecord
     self.table_name = 'feature_gates'
   end
 
@@ -11,7 +11,7 @@ def up
     security_dashboard_feature = FeatureGate.find_by(feature_key: :security_dashboard, key: :boolean)
 
     if security_dashboard_feature.present?
-      FeatureGate.find_or_create_by!(
+      FeatureGate.safe_find_or_create_by!(
         feature_key: :instance_security_dashboard,
         key: :boolean,
         value: security_dashboard_feature.value
diff --git a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
index 20bc5ceb820f2d..7972361953375a 100644
--- a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
+++ b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb
@@ -3,7 +3,7 @@
 class RemoveSecurityDashboardFeatureFlag < ActiveRecord::Migration[6.0]
   DOWNTIME = false
 
-  class FeatureGate < ActiveRecord::Base
+  class FeatureGate < ApplicationRecord
     self.table_name = 'feature_gates'
   end
 
@@ -15,7 +15,7 @@ def down
     instance_security_dashboard_feature = FeatureGate.find_by(feature_key: :instance_security_dashboard, key: :boolean)
 
     if instance_security_dashboard_feature.present?
-      FeatureGate.find_or_create_by!(
+      FeatureGate.safe_find_or_create_by!(
         feature_key: :security_dashboard,
         key: instance_security_dashboard_feature.key,
         value: instance_security_dashboard_feature.value
-- 
GitLab