Use parent integration settings when reverting from custom settings

When switching from custom settings to inherited settings, we rely on
the frontend to fill in the inherited values. For encrypted fields we
can't do that, so if one of them is required it's not possible to revert
an integration from custom settings back to default settings.

This is fixed by ignoring the settings sent by the frontend and instead
propagating the values from the parent integration.

Changelog: fixed

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]
 
+    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 '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) }
 
-        it 'sets inherit_from_id to value' do
-          expect(integration.reload.inherit_from_id).to eq(instance_service.id)
+        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)