Merge branch...

Merge branch '11099-removing-the-project-that-holds-the-insights-configuration-raises-an-error' into 'master'

Resolve "Removing the project that holds the Insights configuration raises an error"

Closes #11099

See merge request gitlab-org/gitlab-ee!11030

db/schema.rb

@@ -3605,8 +3605,8 @@ ActiveRecord::Schema.define(version: 20190506135400) do
   add_foreign_key "identities", "saml_providers", name: "fk_aade90f0fc", on_delete: :cascade
   add_foreign_key "import_export_uploads", "projects", on_delete: :cascade
   add_foreign_key "index_statuses", "projects", name: "fk_74b2492545", on_delete: :cascade
-  add_foreign_key "insights", "namespaces"
-  add_foreign_key "insights", "projects"
+  add_foreign_key "insights", "namespaces", on_delete: :cascade
+  add_foreign_key "insights", "projects", on_delete: :cascade
   add_foreign_key "internal_ids", "namespaces", name: "fk_162941d509", on_delete: :cascade
   add_foreign_key "internal_ids", "projects", on_delete: :cascade
   add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade

ee/app/controllers/ee/groups_controller.rb

@@ -25,7 +25,7 @@ module EE
         :membership_lock,
         :repository_size_limit
       ].tap do |params_ee|
-        params_ee << { insight_attributes: :project_id } if current_group&.insights_available?
+        params_ee << { insight_attributes: [:id, :project_id, :_destroy] } if current_group&.insights_available?
         params_ee << :file_template_project_id if current_group&.feature_available?(:custom_file_templates_for_namespace)
         params_ee << :custom_project_templates_group_id if current_group&.group_project_template_available?
       end

ee/app/models/ee/group.rb

@@ -19,7 +19,7 @@ module EE
 
       has_one :saml_provider
       has_one :insight, foreign_key: :namespace_id
-      accepts_nested_attributes_for :insight
+      accepts_nested_attributes_for :insight, allow_destroy: true
       has_one :scim_oauth_access_token
 
       has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent

ee/app/services/ee/groups/update_service.rb

@@ -12,6 +12,8 @@ module EE
           return false if group.errors.present?
         end
 
+        remove_insight_if_insight_project_absent
+
         super.tap { |success| log_audit_event if success }
       end
 
@@ -60,6 +62,13 @@ module EE
         ).execute.exists?
       end
 
+      def remove_insight_if_insight_project_absent
+        if params.dig(:insight_attributes, :project_id) == ''
+          params[:insight_attributes][:_destroy] = true
+          params[:insight_attributes].delete(:project_id)
+        end
+      end
+
       def log_audit_event
         EE::Audit::GroupChangesAuditor.new(current_user, group).execute
       end

ee/app/views/groups/_insights.html.haml

@@ -16,6 +16,7 @@
         .form-group
           - insight = @group.insight || @group.build_insight
           = form.fields_for :insight_attributes, insight do |insight_form|
+            = insight_form.hidden_field :id
             = insight_form.label :project_id, class: 'label-light' do
               .form-text.text-muted
                 = _('Select a repository')

ee/changelogs/unreleased/11099-removing-the-project-that-holds-the-insights-configuration-raises-an-error.yml

+---
+title: Fix removing and updating insights config, and foreign key constraints.
+merge_request: 11030
+author:
+type: fixed

ee/db/migrate/20190415172035_update_insights_foreign_keys.rb

+# frozen_string_literal: true
+
+# rubocop: disable Migration/AddConcurrentForeignKey
+
+class UpdateInsightsForeignKeys < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    remove_foreign_key_if_exists(:insights, column: :project_id)
+    add_foreign_key(:insights, :projects, column: :project_id, on_delete: :cascade)
+
+    remove_foreign_key_if_exists(:insights, column: :namespace_id)
+    add_foreign_key(:insights, :namespaces, column: :namespace_id, on_delete: :cascade)
+  end
+
+  def down
+    remove_foreign_key_if_exists(:insights, column: :namespace_id)
+    add_foreign_key(:insights, :namespaces, column: :namespace_id)
+
+    remove_foreign_key_if_exists(:insights, column: :project_id)
+    add_foreign_key(:insights, :projects, column: :project_id)
+  end
+end

ee/spec/controllers/groups/groups_controller_spec.rb

@@ -45,14 +45,26 @@ describe GroupsController do
             end.to change { group.reload.file_template_project_id }.to(project.id)
           end
 
-          it 'updates insight project_id successfully' do
-            project = create(:project, group: group)
+          context 'with insights feature' do
+            let(:project) { create(:project, group: group) }
+
+            before do
+              stub_licensed_features(insights: true)
+            end
 
-            stub_licensed_features(insights: true)
+            it 'updates insight project_id successfully' do
+              post :update, params: { id: group.to_param, group: { insight_attributes: { project_id: project.id } } }
 
-            post :update, params: { id: group.to_param, group: { insight_attributes: { project_id: project.id } } }
+              expect(group.reload.insight.project).to eq(project)
+            end
+
+            it 'removes insight successfully' do
+              insight = group.create_insight(project: project)
 
-            expect(group.reload.insight.project).to eq(project)
+              post :update, params: { id: group.to_param, group: { insight_attributes: { id: insight.id, project_id: '' } } }
+
+              expect(group.reload.insight).to be_nil
+            end
           end
         end
       end

lib/gitlab/database/migration_helpers.rb

@@ -905,6 +905,12 @@ module Gitlab
         end
       end
 
+      def remove_foreign_key_if_exists(*args)
+        if foreign_key_exists?(*args)
+          remove_foreign_key(*args)
+        end
+      end
+
       def remove_foreign_key_without_error(*args)
         remove_foreign_key(*args)
       rescue ArgumentError