Merge branch '290715-has_external_issue_tracker_trigger' into 'master'

Add PG trigger for has_external_issue_tracker

See merge request gitlab-org/gitlab!51852

app/models/project.rb

@@ -1314,21 +1314,11 @@ class Project < ApplicationRecord
   end
 
   def external_issue_tracker
-    if has_external_issue_tracker.nil?
-      cache_has_external_issue_tracker
-    end
+    cache_has_external_issue_tracker if has_external_issue_tracker.nil?
 
-    if has_external_issue_tracker?
-      strong_memoize(:external_issue_tracker) do
-        services.external_issue_trackers.first
-      end
-    else
-      nil
-    end
-  end
+    return unless has_external_issue_tracker?
 
-  def cache_has_external_issue_tracker
-    update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) if Gitlab::Database.read_write?
+    @external_issue_tracker ||= services.external_issue_trackers.first
   end
 
   def external_references_supported?
@@ -2690,6 +2680,10 @@ class Project < ApplicationRecord
   def cache_has_external_wiki
     update_column(:has_external_wiki, services.external_wikis.any?) if Gitlab::Database.read_write?
   end
+
+  def cache_has_external_issue_tracker
+    update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) if Gitlab::Database.read_write?
+  end
 end
 
 Project.prepend_if_ee('EE::Project')

app/models/service.rb

@@ -46,7 +46,6 @@ class Service < ApplicationRecord
   after_initialize :initialize_properties
 
   after_commit :reset_updated_properties
-  after_commit :cache_project_has_external_issue_tracker
 
   belongs_to :project, inverse_of: :services
   belongs_to :group, inverse_of: :services
@@ -438,10 +437,6 @@ class Service < ApplicationRecord
     ProjectServiceWorker.perform_async(id, data)
   end
 
-  def external_issue_tracker?
-    category == :issue_tracker && active?
-  end
-
   def external_wiki?
     type == 'ExternalWikiService' && active?
   end
@@ -461,12 +456,6 @@ class Service < ApplicationRecord
     errors.add(:project_id, 'The service cannot belong to both a project and a group') if project_id && group_id
   end
 
-  def cache_project_has_external_issue_tracker
-    if project && !project.destroyed?
-      project.cache_has_external_issue_tracker
-    end
-  end
-
   def valid_recipients?
     activated? && !importing?
   end

app/services/bulk_create_integration_service.rb

@@ -11,8 +11,6 @@ class BulkCreateIntegrationService
     service_list = ServiceList.new(batch, service_hash, association).to_array
 
     Service.transaction do
-      run_callbacks(batch) if association == 'project'
-
       results = bulk_insert(*service_list)
 
       if integration.data_fields_present?
@@ -33,14 +31,6 @@ class BulkCreateIntegrationService
     klass.insert_all(items_to_insert, returning: [:id])
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
-  def run_callbacks(batch)
-    if integration.external_issue_tracker?
-      Project.where(id: batch.select(:id)).update_all(has_external_issue_tracker: true)
-    end
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-
   def service_hash
     if integration.template?
       integration.to_service_hash

changelogs/unreleased/290715-has_external_issue_tracker_trigger.yml

+---
+title: Add PostgreSQL trigger to maintain projects.has_external_issue_tracker
+merge_request: 51852
+author:
+type: changed

db/migrate/20210117210226_add_has_external_issue_tracker_trigger.rb

+# frozen_string_literal: true
+
+class AddHasExternalIssueTrackerTrigger < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::SchemaHelpers
+
+  DOWNTIME = false
+  FUNCTION_NAME = 'set_has_external_issue_tracker'
+  TRIGGER_ON_INSERT_NAME = 'trigger_has_external_issue_tracker_on_insert'
+  TRIGGER_ON_UPDATE_NAME = 'trigger_has_external_issue_tracker_on_update'
+  TRIGGER_ON_DELETE_NAME = 'trigger_has_external_issue_tracker_on_delete'
+
+  def up
+    create_trigger_function(FUNCTION_NAME, replace: true) do
+      <<~SQL
+        UPDATE projects SET has_external_issue_tracker = (
+          EXISTS
+          (
+            SELECT 1
+            FROM services
+            WHERE project_id = COALESCE(NEW.project_id, OLD.project_id)
+              AND active = TRUE
+              AND category = 'issue_tracker'
+          )
+        )
+        WHERE projects.id = COALESCE(NEW.project_id, OLD.project_id);
+        RETURN NULL;
+      SQL
+    end
+
+    execute(<<~SQL)
+      CREATE TRIGGER #{TRIGGER_ON_INSERT_NAME}
+      AFTER INSERT ON services
+      FOR EACH ROW
+      WHEN (NEW.category = 'issue_tracker' AND NEW.active = TRUE AND NEW.project_id IS NOT NULL)
+      EXECUTE FUNCTION #{FUNCTION_NAME}();
+    SQL
+
+    execute(<<~SQL)
+      CREATE TRIGGER #{TRIGGER_ON_UPDATE_NAME}
+      AFTER UPDATE ON services
+      FOR EACH ROW
+      WHEN (NEW.category = 'issue_tracker' AND OLD.active != NEW.active AND NEW.project_id IS NOT NULL)
+      EXECUTE FUNCTION #{FUNCTION_NAME}();
+    SQL
+
+    execute(<<~SQL)
+      CREATE TRIGGER #{TRIGGER_ON_DELETE_NAME}
+      AFTER DELETE ON services
+      FOR EACH ROW
+      WHEN (OLD.category = 'issue_tracker' AND OLD.active = TRUE AND OLD.project_id IS NOT NULL)
+      EXECUTE FUNCTION #{FUNCTION_NAME}();
+    SQL
+  end
+
+  def down
+    drop_trigger(:services, TRIGGER_ON_INSERT_NAME)
+    drop_trigger(:services, TRIGGER_ON_UPDATE_NAME)
+    drop_trigger(:services, TRIGGER_ON_DELETE_NAME)
+    drop_function(FUNCTION_NAME)
+  end
+end

db/schema_migrations/20210117210226

+c0d22d00d52a516347930e1a36f350113c0949214925176f08ceed81999746bd
\ No newline at end of file

db/structure.sql

@@ -10,6 +10,26 @@ CREATE EXTENSION IF NOT EXISTS btree_gist;
 
 CREATE EXTENSION IF NOT EXISTS pg_trgm;
 
+CREATE FUNCTION set_has_external_issue_tracker() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+UPDATE projects SET has_external_issue_tracker = (
+  EXISTS
+  (
+    SELECT 1
+    FROM services
+    WHERE project_id = COALESCE(NEW.project_id, OLD.project_id)
+      AND active = TRUE
+      AND category = 'issue_tracker'
+    )
+  )
+WHERE projects.id = COALESCE(NEW.project_id, OLD.project_id);
+RETURN NULL;
+
+END
+$$;
+
 CREATE FUNCTION set_has_external_wiki() RETURNS trigger
     LANGUAGE plpgsql
     AS $$
@@ -23639,6 +23659,12 @@ ALTER INDEX product_analytics_events_experimental_pkey ATTACH PARTITION gitlab_p
 
 CREATE TRIGGER table_sync_trigger_ee39a25f9d AFTER INSERT OR DELETE OR UPDATE ON audit_events FOR EACH ROW EXECUTE PROCEDURE table_sync_function_2be879775d();
 
+CREATE TRIGGER trigger_has_external_issue_tracker_on_delete AFTER DELETE ON services FOR EACH ROW WHEN ((((old.category)::text = 'issue_tracker'::text) AND (old.active = true) AND (old.project_id IS NOT NULL))) EXECUTE PROCEDURE set_has_external_issue_tracker();
+
+CREATE TRIGGER trigger_has_external_issue_tracker_on_insert AFTER INSERT ON services FOR EACH ROW WHEN ((((new.category)::text = 'issue_tracker'::text) AND (new.active = true) AND (new.project_id IS NOT NULL))) EXECUTE PROCEDURE set_has_external_issue_tracker();
+
+CREATE TRIGGER trigger_has_external_issue_tracker_on_update AFTER UPDATE ON services FOR EACH ROW WHEN ((((new.category)::text = 'issue_tracker'::text) AND (old.active <> new.active) AND (new.project_id IS NOT NULL))) EXECUTE PROCEDURE set_has_external_issue_tracker();
+
 CREATE TRIGGER trigger_has_external_wiki_on_delete AFTER DELETE ON services FOR EACH ROW WHEN ((((old.type)::text = 'ExternalWikiService'::text) AND (old.project_id IS NOT NULL))) EXECUTE PROCEDURE set_has_external_wiki();
 
 CREATE TRIGGER trigger_has_external_wiki_on_insert AFTER INSERT ON services FOR EACH ROW WHEN (((new.active = true) AND ((new.type)::text = 'ExternalWikiService'::text) AND (new.project_id IS NOT NULL))) EXECUTE PROCEDURE set_has_external_wiki();

spec/migrations/add_has_external_issue_tracker_trigger_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe AddHasExternalIssueTrackerTrigger do
+  let(:migration) { described_class.new }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:services) { table(:services) }
+
+  before do
+    @namespace = namespaces.create!(name: 'foo', path: 'foo')
+    @project = projects.create!(namespace_id: @namespace.id)
+  end
+
+  describe '#up' do
+    before do
+      migrate!
+    end
+
+    describe 'INSERT trigger' do
+      it 'sets `has_external_issue_tracker` to true when active `issue_tracker` is inserted' do
+        expect do
+          services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+        end.to change { @project.reload.has_external_issue_tracker }.to(true)
+      end
+
+      it 'does not set `has_external_issue_tracker` to true when service is for a different project' do
+        different_project = projects.create!(namespace_id: @namespace.id)
+
+        expect do
+          services.create!(category: 'issue_tracker', active: true, project_id: different_project.id)
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+
+      it 'does not set `has_external_issue_tracker` to true when inactive `issue_tracker` is inserted' do
+        expect do
+          services.create!(category: 'issue_tracker', active: false, project_id: @project.id)
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+
+      it 'does not set `has_external_issue_tracker` to true when a non-`issue tracker` active service is inserted' do
+        expect do
+          services.create!(category: 'my_type', active: true, project_id: @project.id)
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+    end
+
+    describe 'UPDATE trigger' do
+      it 'sets `has_external_issue_tracker` to true when `issue_tracker` is made active' do
+        service = services.create!(category: 'issue_tracker', active: false, project_id: @project.id)
+
+        expect do
+          service.update!(active: true)
+        end.to change { @project.reload.has_external_issue_tracker }.to(true)
+      end
+
+      it 'sets `has_external_issue_tracker` to false when `issue_tracker` is made inactive' do
+        service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+
+        expect do
+          service.update!(active: false)
+        end.to change { @project.reload.has_external_issue_tracker }.to(false)
+      end
+
+      it 'sets `has_external_issue_tracker` to false when `issue_tracker` is made inactive, and an inactive `issue_tracker` exists' do
+        services.create!(category: 'issue_tracker', active: false, project_id: @project.id)
+        service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+
+        expect do
+          service.update!(active: false)
+        end.to change { @project.reload.has_external_issue_tracker }.to(false)
+      end
+
+      it 'does not change `has_external_issue_tracker` when `issue_tracker` is made inactive, if an active `issue_tracker` exists' do
+        services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+        service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+
+        expect do
+          service.update!(active: false)
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+
+      it 'does not change `has_external_issue_tracker` when service is for a different project' do
+        different_project = projects.create!(namespace_id: @namespace.id)
+        service = services.create!(category: 'issue_tracker', active: false, project_id: different_project.id)
+
+        expect do
+          service.update!(active: true)
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+    end
+
+    describe 'DELETE trigger' do
+      it 'sets `has_external_issue_tracker` to false when `issue_tracker` is deleted' do
+        service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+
+        expect do
+          service.delete
+        end.to change { @project.reload.has_external_issue_tracker }.to(false)
+      end
+
+      it 'sets `has_external_issue_tracker` to false when `issue_tracker` is deleted, if an inactive `issue_tracker` still exists' do
+        services.create!(category: 'issue_tracker', active: false, project_id: @project.id)
+        service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+
+        expect do
+          service.delete
+        end.to change { @project.reload.has_external_issue_tracker }.to(false)
+      end
+
+      it 'does not change `has_external_issue_tracker` when `issue_tracker` is deleted, if an active `issue_tracker` still exists' do
+        services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+        service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+
+        expect do
+          service.delete
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+
+      it 'does not change `has_external_issue_tracker` when service is for a different project' do
+        different_project = projects.create!(namespace_id: @namespace.id)
+        service = services.create!(category: 'issue_tracker', active: true, project_id: different_project.id)
+
+        expect do
+          service.delete
+        end.not_to change { @project.reload.has_external_issue_tracker }
+      end
+    end
+  end
+
+  describe '#down' do
+    before do
+      migration.up
+      migration.down
+    end
+
+    it 'drops the INSERT trigger' do
+      expect do
+        services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+      end.not_to change { @project.reload.has_external_issue_tracker }
+    end
+
+    it 'drops the UPDATE trigger' do
+      service = services.create!(category: 'issue_tracker', active: false, project_id: @project.id)
+      @project.update!(has_external_issue_tracker: false)
+
+      expect do
+        service.update!(active: true)
+      end.not_to change { @project.reload.has_external_issue_tracker }
+    end
+
+    it 'drops the DELETE trigger' do
+      service = services.create!(category: 'issue_tracker', active: true, project_id: @project.id)
+      @project.update!(has_external_issue_tracker: true)
+
+      expect do
+        service.delete
+      end.not_to change { @project.reload.has_external_issue_tracker }
+    end
+  end
+end

spec/models/merge_request_spec.rb

@@ -754,9 +754,8 @@ RSpec.describe MergeRequest, factory_default: :keep do
 
     context 'when both internal and external issue trackers are enabled' do
       before do
-        subject.project.has_external_issue_tracker = true
-        subject.project.save!
         create(:jira_service, project: subject.project)
+        subject.project.reload
       end
 
       it 'does not cache issues from external trackers' do
@@ -1263,9 +1262,10 @@ RSpec.describe MergeRequest, factory_default: :keep do
   describe '#issues_mentioned_but_not_closing' do
     let(:closing_issue) { create :issue, project: subject.project }
     let(:mentioned_issue) { create :issue, project: subject.project }
-
     let(:commit) { double('commit', safe_message: "Fixes #{closing_issue.to_reference}") }
 
+    subject { create(:merge_request, source_project: create(:project)) }
+
     it 'detects issues mentioned in description but not closed' do
       subject.project.add_developer(subject.author)
       subject.description = "Is related to #{mentioned_issue.to_reference} and #{closing_issue.to_reference}"
@@ -1279,13 +1279,12 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
 
     context 'when the project has an external issue tracker' do
-      subject { create(:merge_request, source_project: create(:project, :repository)) }
-
       before do
         subject.project.add_developer(subject.author)
         commit = double(:commit, safe_message: 'Fixes TEST-3')
 
         create(:jira_service, project: subject.project)
+        subject.project.reload
 
         allow(subject).to receive(:commits).and_return([commit])
         allow(subject).to receive(:description).and_return('Is related to TEST-2 and TEST-3')
@@ -1469,6 +1468,8 @@ RSpec.describe MergeRequest, factory_default: :keep do
   end
 
   describe '#default_merge_commit_message' do
+    subject { create(:merge_request, source_project: create(:project)) }
+
     it 'includes merge information as the title' do
       request = build(:merge_request, source_branch: 'source', target_branch: 'target')
 
@@ -1645,7 +1646,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
   end
 
   it_behaves_like 'an editable mentionable' do
-    subject { create(:merge_request, :simple) }
+    subject { create(:merge_request, :simple, source_project: create(:project, :repository)) }
 
     let(:backref_text) { "merge request #{subject.to_reference}" }
     let(:set_mentionable_text) { ->(txt) { subject.description = txt } }

spec/models/project_spec.rb

@@ -1002,103 +1002,125 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
-  describe '#external_issue_tracker' do
-    let(:project) { create(:project) }
-    let(:ext_project) { create(:redmine_project) }
+  describe '#has_wiki?' do
+    let(:no_wiki_project)       { create(:project, :wiki_disabled, has_external_wiki: false) }
+    let(:wiki_enabled_project)  { create(:project) }
+    let(:external_wiki_project) { create(:project, has_external_wiki: true) }
 
-    context 'on existing projects with no value for has_external_issue_tracker' do
-      before do
-        project.update_column(:has_external_issue_tracker, nil)
-        ext_project.update_column(:has_external_issue_tracker, nil)
+    it 'returns true if project is wiki enabled or has external wiki' do
+      expect(wiki_enabled_project).to have_wiki
+      expect(external_wiki_project).to have_wiki
+      expect(no_wiki_project).not_to have_wiki
+    end
   end
 
-      it 'updates the has_external_issue_tracker boolean' do
-        expect do
-          project.external_issue_tracker
-        end.to change { project.reload.has_external_issue_tracker }.to(false)
+  describe '#default_owner' do
+    let_it_be(:owner)     { create(:user) }
+    let_it_be(:namespace) { create(:namespace, owner: owner) }
 
-        expect do
-          ext_project.external_issue_tracker
-        end.to change { ext_project.reload.has_external_issue_tracker }.to(true)
+    context 'the project does not have a group' do
+      let(:project) { build(:project, namespace: namespace) }
+
+      it 'is the namespace owner' do
+        expect(project.default_owner).to eq(owner)
       end
     end
 
-    it 'returns nil and does not query services when there is no external issue tracker' do
-      expect(project).not_to receive(:services)
-
-      expect(project.external_issue_tracker).to eq(nil)
-    end
+    context 'the project is in a group' do
+      let(:group)   { build(:group) }
+      let(:project) { build(:project, group: group, namespace: namespace) }
 
-    it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do
-      ext_project.reload # Factory returns a project with changed attributes
-      expect(ext_project).to receive(:services).once.and_call_original
+      it 'is the group owner' do
+        allow(group).to receive(:default_owner).and_return(Object.new)
 
-      2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) }
+        expect(project.default_owner).to eq(group.default_owner)
+      end
     end
   end
 
-  describe '#cache_has_external_issue_tracker' do
-    let_it_be(:project) { create(:project, has_external_issue_tracker: nil) }
+  describe '#external_issue_tracker' do
+    it 'sets Project#has_external_issue_tracker when it is nil' do
+      project_with_no_tracker = create(:project, has_external_issue_tracker: nil)
+      project_with_tracker = create(:redmine_project, has_external_issue_tracker: nil)
 
-    it 'stores true if there is any external_issue_tracker' do
-      services = double(:service, external_issue_trackers: [RedmineService.new])
-      expect(project).to receive(:services).and_return(services)
+      expect do
+        project_with_no_tracker.external_issue_tracker
+      end.to change { project_with_no_tracker.reload.has_external_issue_tracker }.from(nil).to(false)
 
       expect do
-        project.cache_has_external_issue_tracker
-      end.to change { project.has_external_issue_tracker}.to(true)
+        project_with_tracker.external_issue_tracker
+      end.to change { project_with_tracker.reload.has_external_issue_tracker }.from(nil).to(true)
     end
 
-    it 'stores false if there is no external_issue_tracker' do
-      services = double(:service, external_issue_trackers: [])
-      expect(project).to receive(:services).and_return(services)
+    it 'returns nil and does not query services when there is no external issue tracker' do
+      project = create(:project)
 
-      expect do
-        project.cache_has_external_issue_tracker
-      end.to change { project.has_external_issue_tracker}.to(false)
+      expect(project).not_to receive(:services)
+      expect(project.external_issue_tracker).to eq(nil)
     end
 
-    it 'does not cache data when in a read-only GitLab instance' do
-      allow(Gitlab::Database).to receive(:read_only?) { true }
+    it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do
+      project = create(:redmine_project)
 
-      expect do
-        project.cache_has_external_issue_tracker
-      end.not_to change { project.has_external_issue_tracker }
+      expect(project).to receive(:services).once.and_call_original
+      2.times { expect(project.external_issue_tracker).to be_a_kind_of(RedmineService) }
     end
   end
 
-  describe '#has_wiki?' do
-    let(:no_wiki_project)       { create(:project, :wiki_disabled, has_external_wiki: false) }
-    let(:wiki_enabled_project)  { create(:project) }
-    let(:external_wiki_project) { create(:project, has_external_wiki: true) }
+  describe '#has_external_issue_tracker' do
+    let_it_be(:project) { create(:project) }
 
-    it 'returns true if project is wiki enabled or has external wiki' do
-      expect(wiki_enabled_project).to have_wiki
-      expect(external_wiki_project).to have_wiki
-      expect(no_wiki_project).not_to have_wiki
+    def subject
+      project.reload.has_external_issue_tracker
     end
+
+    it 'is false when external issue tracker service is not active' do
+      create(:service, project: project, category: 'issue_tracker', active: false)
+
+      is_expected.to eq(false)
     end
 
-  describe '#default_owner' do
-    let_it_be(:owner)     { create(:user) }
-    let_it_be(:namespace) { create(:namespace, owner: owner) }
+    it 'is false when other service is active' do
+      create(:service, project: project, category: 'not_issue_tracker', active: true)
 
-    context 'the project does not have a group' do
-      let(:project) { build(:project, namespace: namespace) }
+      is_expected.to eq(false)
+    end
 
-      it 'is the namespace owner' do
-        expect(project.default_owner).to eq(owner)
+    context 'when there is an active external issue tracker service' do
+      let!(:service) do
+        create(:service, project: project, type: 'JiraService', category: 'issue_tracker', active: true)
+      end
+
+      specify { is_expected.to eq(true) }
+
+      it 'becomes false when external issue tracker service is destroyed' do
+        expect do
+          Service.find(service.id).delete
+        end.to change { subject }.to(false)
       end
+
+      it 'becomes false when external issue tracker service becomes inactive' do
+        expect do
+          service.update_column(:active, false)
+        end.to change { subject }.to(false)
       end
 
-    context 'the project is in a group' do
-      let(:group)   { build(:group) }
-      let(:project) { build(:project, group: group, namespace: namespace) }
+      context 'when there are two active external issue tracker services' do
+        let_it_be(:second_service) do
+          create(:service, project: project, type: 'CustomIssueTracker', category: 'issue_tracker', active: true)
+        end
 
-      it 'is the group owner' do
-        allow(group).to receive(:default_owner).and_return(Object.new)
+        it 'does not become false when external issue tracker service is destroyed' do
+          expect do
+            Service.find(service.id).delete
+          end.not_to change { subject }
+        end
 
-        expect(project.default_owner).to eq(group.default_owner)
+        it 'does not become false when external issue tracker service becomes inactive' do
+          expect do
+            service.update_column(:active, false)
+          end.not_to change { subject }
+        end
       end
     end
   end

spec/models/service_spec.rb

@@ -753,38 +753,6 @@ RSpec.describe Service do
     end
   end
 
-  describe "callbacks" do
-    let!(:service) do
-      RedmineService.new(
-        project: project,
-        active: true,
-        properties: {
-          project_url: 'http://redmine/projects/project_name_in_redmine',
-          issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id",
-          new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new'
-        }
-      )
-    end
-
-    describe "on create" do
-      it "updates the has_external_issue_tracker boolean" do
-        expect do
-          service.save!
-        end.to change { service.project.has_external_issue_tracker }.from(false).to(true)
-      end
-    end
-
-    describe "on update" do
-      it "updates the has_external_issue_tracker boolean" do
-        service.save!
-
-        expect do
-          service.update(active: false)
-        end.to change { service.project.has_external_issue_tracker }.from(true).to(false)
-      end
-    end
-  end
-
   describe '#api_field_names' do
     let(:fake_service) do
       Class.new(Service) do
@@ -864,20 +832,6 @@ RSpec.describe Service do
     end
   end
 
-  describe '#external_issue_tracker?' do
-    where(:category, :active, :result) do
-      :issue_tracker | true  | true
-      :issue_tracker | false | false
-      :common        | true  | false
-    end
-
-    with_them do
-      it 'returns the right result' do
-        expect(build(:service, category: category, active: active).external_issue_tracker?).to eq(result)
-      end
-    end
-  end
-
   describe '#external_wiki?' do
     where(:type, :active, :result) do
       'ExternalWikiService' | true  | true

spec/services/bulk_create_integration_service_spec.rb

@@ -43,46 +43,6 @@ RSpec.describe BulkCreateIntegrationService do
     end
   end
 
-  shared_examples 'updates project callbacks' do
-    it 'updates projects#has_external_issue_tracker for issue tracker services' do
-      described_class.new(integration, batch, association).execute
-
-      expect(project.reload.has_external_issue_tracker).to eq(true)
-      expect(excluded_project.reload.has_external_issue_tracker).to eq(false)
-    end
-
-    context 'with an external wiki integration' do
-      before do
-        integration.update!(category: 'common', type: 'ExternalWikiService')
-      end
-
-      it 'updates projects#has_external_wiki for external wiki services' do
-        described_class.new(integration, batch, association).execute
-
-        expect(project.reload.has_external_wiki).to eq(true)
-        expect(excluded_project.reload.has_external_wiki).to eq(false)
-      end
-    end
-  end
-
-  shared_examples 'does not update project callbacks' do
-    it 'does not update projects#has_external_issue_tracker for issue tracker services' do
-      described_class.new(integration, batch, association).execute
-
-      expect(project.reload.has_external_issue_tracker).to eq(false)
-    end
-
-    context 'with an inactive external wiki integration' do
-      let(:integration) { create(:external_wiki_service, :instance, active: false) }
-
-      it 'does not update projects#has_external_wiki for external wiki services' do
-        described_class.new(integration, batch, association).execute
-
-        expect(project.reload.has_external_wiki).to eq(false)
-      end
-    end
-  end
-
   context 'passing an instance-level integration' do
     let(:integration) { instance_integration }
     let(:inherit_from_id) { integration.id }
@@ -95,15 +55,6 @@ RSpec.describe BulkCreateIntegrationService do
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates inherit_from_id'
-      it_behaves_like 'updates project callbacks'
-
-      context 'when integration is not active' do
-        before do
-          integration.update!(active: false)
-        end
-
-        it_behaves_like 'does not update project callbacks'
-      end
     end
 
     context 'with a group association' do
@@ -130,7 +81,6 @@ RSpec.describe BulkCreateIntegrationService do
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates inherit_from_id'
-      it_behaves_like 'updates project callbacks'
     end
 
     context 'with a group association' do
@@ -157,7 +107,6 @@ RSpec.describe BulkCreateIntegrationService do
       let(:inherit_from_id) { integration.id }
 
       it_behaves_like 'creates integration from batch ids'
-      it_behaves_like 'updates project callbacks'
     end
   end
 end

spec/services/issues/close_service_spec.rb

@@ -84,6 +84,7 @@ RSpec.describe Issues::CloseService do
         let!(:external_issue_tracker) { create(:jira_service, project: project) }
 
         it 'closes the issue on the external issue tracker' do
+          project.reload
           expect(project.external_issue_tracker).to receive(:close_issue)
 
           described_class.new(project, user).close_issue(external_issue)
@@ -94,6 +95,7 @@ RSpec.describe Issues::CloseService do
         let!(:external_issue_tracker) { create(:jira_service, project: project, active: false) }
 
         it 'does not close the issue on the external issue tracker' do
+          project.reload
           expect(project.external_issue_tracker).not_to receive(:close_issue)
 
           described_class.new(project, user).close_issue(external_issue)
@@ -104,6 +106,7 @@ RSpec.describe Issues::CloseService do
         let!(:external_issue_tracker) { create(:bugzilla_service, project: project) }
 
         it 'does not close the issue on the external issue tracker' do
+          project.reload
           expect(project.external_issue_tracker).not_to receive(:close_issue)
 
           described_class.new(project, user).close_issue(external_issue)

spec/services/merge_requests/build_service_spec.rb

@@ -252,6 +252,7 @@ RSpec.describe MergeRequests::BuildService do
               issue.update!(iid: 123)
             else
               create(:"#{issue_tracker}_service", project: project)
+              project.reload
             end
           end
 
@@ -351,6 +352,7 @@ RSpec.describe MergeRequests::BuildService do
               issue.update!(iid: 123)
             else
               create(:"#{issue_tracker}_service", project: project)
+              project.reload
             end
           end
 

spec/services/system_notes/issuables_service_spec.rb

@@ -729,12 +729,14 @@ RSpec.describe ::SystemNotes::IssuablesService do
 
       it 'is false with issue tracker supporting referencing' do
         create(:jira_service, project: project)
+        project.reload
 
         expect(service.cross_reference_disallowed?(noteable)).to be_falsey
       end
 
       it 'is true with issue tracker not supporting referencing' do
         create(:bugzilla_service, project: project)
+        project.reload
 
         expect(service.cross_reference_disallowed?(noteable)).to be_truthy
       end