Merge branch '336634-revert-integrations-from-default-to-custom' into 'master'

Use parent integration settings when reverting from custom settings

See merge request gitlab-org/gitlab!66663

app/controllers/projects/services_controller.rb

@@ -8,6 +8,7 @@ class Projects::ServicesController < Projects::ApplicationController
   before_action :authorize_admin_project!
   before_action :ensure_service_enabled
   before_action :integration
+  before_action :default_integration, only: [:edit, :update]
   before_action :web_hook_logs, only: [:edit, :update]
   before_action :set_deprecation_notice_for_prometheus_integration, only: [:edit, :update]
   before_action :redirect_deprecated_prometheus_integration, only: [:update]
@@ -19,14 +20,22 @@ class Projects::ServicesController < Projects::ApplicationController
   feature_category :integrations
 
   def edit
-    @default_integration = Integration.default_integration(service.type, project)
   end
 
   def update
-    @integration.attributes = integration_params[:integration]
-    @integration.inherit_from_id = nil if integration_params[:integration][:inherit_from_id].blank?
+    attributes = integration_params[:integration]
 
-    saved = @integration.save(context: :manual_change)
+    if use_inherited_settings?(attributes)
+      @integration.inherit_from_id = default_integration.id
+
+      if saved = @integration.save(context: :manual_change)
+        BulkUpdateIntegrationService.new(default_integration, [@integration]).execute
+      end
+    else
+      attributes[:inherit_from_id] = nil
+      @integration.attributes = attributes
+      saved = @integration.save(context: :manual_change)
+    end
 
     respond_to do |format|
       format.html do
@@ -88,6 +97,10 @@ class Projects::ServicesController < Projects::ApplicationController
   end
   alias_method :service, :integration
 
+  def default_integration
+    @default_integration ||= Integration.default_integration(integration.type, project)
+  end
+
   def web_hook_logs
     return unless integration.service_hook.present?
 
@@ -115,4 +128,8 @@ class Projects::ServicesController < Projects::ApplicationController
     message = s_('PrometheusService|You can now manage your Prometheus settings on the %{operations_link_start}Operations%{operations_link_end} page. Fields on this page have been deprecated.') % { operations_link_start: operations_link_start, operations_link_end: "</a>" }
     flash.now[:alert] = message.html_safe
   end
+
+  def use_inherited_settings?(attributes)
+    default_integration && attributes[:inherit_from_id] == default_integration.id.to_s
+  end
 end

app/services/bulk_update_integration_service.rb

@@ -9,10 +9,10 @@ class BulkUpdateIntegrationService
   # rubocop: disable CodeReuse/ActiveRecord
   def execute
     Integration.transaction do
-      Integration.where(id: batch.select(:id)).update_all(integration_hash)
+      Integration.where(id: batch_ids).update_all(integration_hash)
 
       if integration.data_fields_present?
-        integration.data_fields.class.where(service_id: batch.select(:id)).update_all(data_fields_hash)
+        integration.data_fields.class.where(service_id: batch_ids).update_all(data_fields_hash)
       end
     end
   end
@@ -29,4 +29,13 @@ class BulkUpdateIntegrationService
   def data_fields_hash
     integration.to_data_fields_hash
   end
+
+  def batch_ids
+    @batch_ids ||=
+      if batch.is_a?(ActiveRecord::Relation)
+        batch.select(:id)
+      else
+        batch.map(&:id)
+      end
+  end
 end

spec/controllers/projects/services_controller_spec.rb

@@ -174,6 +174,8 @@ RSpec.describe Projects::ServicesController do
       let(:redirect_url) { edit_project_service_path(project, integration) }
 
       before do
+        stub_jira_integration_test
+
         put :update, params: params
       end
 
@@ -222,12 +224,48 @@ RSpec.describe Projects::ServicesController do
         end
       end
 
-      context 'when param `inherit_from_id` is set to some value' do
-        let(:instance_service) { create(:jira_integration, :instance) }
-        let(:integration_params) { { inherit_from_id: instance_service.id } }
+      context 'when param `inherit_from_id` is set to an instance integration' do
+        let(:instance_integration) { create(:jira_integration, :instance, url: 'http://instance.com', password: 'instance') }
+        let(:integration_params) { { inherit_from_id: instance_integration.id, url: 'http://custom.com', password: 'custom' } }
+
+        it 'ignores submitted params and inherits instance settings' do
+          expect(integration.reload).to have_attributes(
+            inherit_from_id: instance_integration.id,
+            url: instance_integration.url,
+            password: instance_integration.password
+          )
+        end
+      end
+
+      context 'when param `inherit_from_id` is set to a group integration' do
+        let_it_be(:group) { create(:group) }
+        let_it_be(:project) { create(:project, group: group) }
+        let_it_be(:jira_integration) { create(:jira_integration, project: project) }
+
+        let(:group_integration) { create(:jira_integration, group: group, project: nil, url: 'http://group.com', password: 'group') }
+        let(:integration_params) { { inherit_from_id: group_integration.id, url: 'http://custom.com', password: 'custom' } }
 
-        it 'sets inherit_from_id to value' do
-          expect(integration.reload.inherit_from_id).to eq(instance_service.id)
+        it 'ignores submitted params and inherits group settings' do
+          expect(integration.reload).to have_attributes(
+            inherit_from_id: group_integration.id,
+            url: group_integration.url,
+            password: group_integration.password
+          )
+        end
+      end
+
+      context 'when param `inherit_from_id` is set to an unrelated group' do
+        let_it_be(:group) { create(:group) }
+
+        let(:group_integration) { create(:jira_integration, group: group, project: nil, url: 'http://group.com', password: 'group') }
+        let(:integration_params) { { inherit_from_id: group_integration.id, url: 'http://custom.com', password: 'custom' } }
+
+        it 'ignores the param and saves the submitted settings' do
+          expect(integration.reload).to have_attributes(
+            inherit_from_id: nil,
+            url: 'http://custom.com',
+            password: 'custom'
+          )
         end
       end
     end

spec/features/projects/integrations/user_uses_inherited_settings_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User uses inherited settings', :js do
+  include JiraServiceHelper
+
+  include_context 'project service activation'
+
+  before do
+    stub_jira_integration_test
+  end
+
+  shared_examples 'inherited settings' do
+    let_it_be(:project_settings) { { url: 'http://project.com', password: 'project' } }
+
+    describe 'switching from inherited to custom settings' do
+      let_it_be(:integration) { create(:jira_integration, project: project, inherit_from_id: parent_integration.id, **project_settings) }
+
+      it 'clears the form fields and saves the entered values' do
+        visit_project_integration('Jira')
+
+        expect(page).not_to have_button('Use custom settings')
+        expect(page).to have_field('Web URL', with: parent_settings[:url], readonly: true)
+        expect(page).to have_field('Enter new password or API token', with: '', readonly: true)
+
+        click_on 'Use default settings'
+        click_on 'Use custom settings'
+
+        expect(page).not_to have_button('Use default settings')
+        expect(page).to have_field('Web URL', with: project_settings[:url], readonly: false)
+        expect(page).to have_field('Enter new password or API token', with: '', readonly: false)
+
+        fill_in 'Web URL', with: 'http://custom.com'
+        fill_in 'Enter new password or API token', with: 'custom'
+
+        click_save_integration
+
+        expect(page).to have_text('Jira settings saved and active.')
+        expect(integration.reload).to have_attributes(
+          inherit_from_id: nil,
+          url: 'http://custom.com',
+          password: 'custom'
+        )
+      end
+    end
+
+    describe 'switching from custom to inherited settings' do
+      let_it_be(:integration) { create(:jira_integration, project: project, **project_settings) }
+
+      it 'resets the form fields, makes them read-only, and saves the inherited values' do
+        visit_project_integration('Jira')
+
+        expect(page).not_to have_button('Use default settings')
+        expect(page).to have_field('URL', with: project_settings[:url], readonly: false)
+        expect(page).to have_field('Enter new password or API token', with: '', readonly: false)
+
+        click_on 'Use custom settings'
+        click_on 'Use default settings'
+
+        expect(page).not_to have_button('Use custom settings')
+        expect(page).to have_field('URL', with: parent_settings[:url], readonly: true)
+        expect(page).to have_field('Enter new password or API token', with: '', readonly: true)
+
+        click_save_integration
+
+        expect(page).to have_text('Jira settings saved and active.')
+        expect(integration.reload).to have_attributes(
+          inherit_from_id: parent_integration.id,
+          **parent_settings
+        )
+      end
+    end
+  end
+
+  context 'with instance settings' do
+    let_it_be(:parent_settings) { { url: 'http://instance.com', password: 'instance' } }
+    let_it_be(:parent_integration) { create(:jira_integration, :instance, **parent_settings) }
+
+    it_behaves_like 'inherited settings'
+  end
+
+  context 'with group settings' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
+    let_it_be(:parent_settings) { { url: 'http://group.com', password: 'group' } }
+    let_it_be(:parent_integration) { create(:jira_integration, group: group, project: nil, **parent_settings) }
+
+    it_behaves_like 'inherited settings'
+  end
+end

spec/services/bulk_update_integration_service_spec.rb

@@ -76,4 +76,16 @@ RSpec.describe BulkUpdateIntegrationService do
       end
     end
   end
+
+  it 'works with batch as an ActiveRecord::Relation' do
+    expect do
+      described_class.new(group_integration, Integration.where(id: integration.id)).execute
+    end.to change { integration.reload.url }.to(group_integration.url)
+  end
+
+  it 'works with batch as an array of ActiveRecord objects' do
+    expect do
+      described_class.new(group_integration, [integration]).execute
+    end.to change { integration.reload.url }.to(group_integration.url)
+  end
 end

spec/support/shared_contexts/features/integrations/project_integrations_shared_context.rb

@@ -3,8 +3,8 @@
 RSpec.shared_context 'project service activation' do
   include_context 'integration activation'
 
-  let(:project) { create(:project) }
-  let(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
 
   before do
     project.add_maintainer(user)