Merge branch '328470-make-it-clearer-that-another-issue-tracker-is-enabled-frontend' into 'master'

Add alert and disable active checkbox when another issue tracker is enabled

See merge request gitlab-org/gitlab!81896

app/assets/javascripts/integrations/edit/components/active_checkbox.vue

 <script>
 import { GlFormGroup, GlFormCheckbox } from '@gitlab/ui';
-import { mapGetters } from 'vuex';
+import { mapGetters, mapState } from 'vuex';
 
 export default {
   name: 'ActiveCheckbox',
@@ -15,6 +15,10 @@ export default {
   },
   computed: {
     ...mapGetters(['isInheriting', 'propsSource']),
+    ...mapState(['customState']),
+    disabled() {
+      return this.isInheriting || this.customState.activateDisabled;
+    },
   },
   mounted() {
     this.activated = this.propsSource.initialActivated;
@@ -34,7 +38,7 @@ export default {
     <gl-form-checkbox
       v-model="activated"
       class="gl-display-block"
-      :disabled="isInheriting"
+      :disabled="disabled"
       @change="onChange"
     >
       {{ __('Active') }}

app/assets/javascripts/integrations/edit/index.js

@@ -39,6 +39,7 @@ function parseDatasetToProps(data) {
   const {
     showActive,
     activated,
+    activateDisabled,
     editable,
     canTest,
     commitEvents,
@@ -54,6 +55,7 @@ function parseDatasetToProps(data) {
   return {
     initialActivated: activated,
     showActive,
+    activateDisabled,
     type,
     cancelPath,
     editable,

app/helpers/integrations_helper.rb

@@ -104,7 +104,8 @@ module IntegrationsHelper
     form_data = {
       id: integration.id,
       show_active: integration.show_active_box?.to_s,
-      activated: (integration.active || integration.new_record?).to_s,
+      activated: (integration.active || (integration.new_record? && integration.activate_disabled_reason.nil?)).to_s,
+      activate_disabled: integration.activate_disabled_reason.present?.to_s,
       type: integration.to_param,
       merge_request_events: integration.merge_requests_events.to_s,
       commit_events: integration.commit_events.to_s,

app/models/integration.rb

@@ -360,6 +360,10 @@ class Integration < ApplicationRecord
     true
   end
 
+  def activate_disabled_reason
+    nil
+  end
+
   def category
     read_attribute(:category).to_sym
   end

app/models/integrations/base_issue_tracker.rb

@@ -132,8 +132,18 @@ module Integrations
       # implement inside child
     end
 
+    def activate_disabled_reason
+      { trackers: other_external_issue_trackers } if other_external_issue_trackers.any?
+    end
+
     private
 
+    def other_external_issue_trackers
+      return [] unless project_level?
+
+      @other_external_issue_trackers ||= project.integrations.external_issue_trackers.where.not(id: id)
+    end
+
     def enabled_in_gitlab_config
       Gitlab.config.issues_tracker &&
         Gitlab.config.issues_tracker.values.any? &&
@@ -148,7 +158,7 @@ module Integrations
       return if instance?
       return if project.blank?
 
-      if project.integrations.external_issue_trackers.where.not(id: id).any?
+      if other_external_issue_trackers.any?
         errors.add(:base, _('Another issue tracker is already in use. Only one issue tracker service can be active at a time'))
       end
     end

app/views/projects/services/_form.html.haml

 - if lookup_context.template_exists?('top', "projects/services/#{integration.to_param}", true)
   = render "projects/services/#{integration.to_param}/top", integration: integration
 
+- if integration.activate_disabled_reason.present? && integration.activate_disabled_reason[:trackers].any?
+  -# When using integration.activate_disabled_reason[:trackers], it's potentially insecure to use the raw records
+  -# when passed directly to the frontend. Only use specific fields that are needed for render.
+  -# For example, we can get the link to each tracker with scoped_edit_integration_path(tracker, tracker.project)
+  = render 'shared/global_alert',
+    title: s_('ExternalIssueIntegration|Another issue tracker is already in use'),
+    variant: :warning,
+    dismissible: false do
+    .gl-alert-body
+      = s_('ExternalIssueIntegration|Only one issue tracker integration can be active at a time. Please disable the active tracker first and try again.')
+
 %h3.page-title
   = integration.title
   - if integration.operating?

locale/gitlab.pot

@@ -14955,9 +14955,15 @@ msgstr ""
 msgid "ExternalAuthorization|URL to which the projects make authorization requests. If the URL is blank, cross-project features are available and can still specify classification labels for projects."
 msgstr ""
 
+msgid "ExternalIssueIntegration|Another issue tracker is already in use"
+msgstr ""
+
 msgid "ExternalIssueIntegration|Not all data may be displayed here. To view more details or make changes to this issue, go to %{linkStart}%{trackerName}%{linkEnd}."
 msgstr ""
 
+msgid "ExternalIssueIntegration|Only one issue tracker integration can be active at a time. Please disable the active tracker first and try again."
+msgstr ""
+
 msgid "ExternalIssueIntegration|This issue is synchronized with %{trackerName}"
 msgstr ""
 

spec/factories/integrations.rb

@@ -224,6 +224,12 @@ FactoryBot.define do
     recipients { 'test@example.com' }
   end
 
+  factory :pivotaltracker_integration, class: 'Integrations::Pivotaltracker' do
+    project
+    active { true }
+    token { 'test' }
+  end
+
   # this is for testing storing values inside properties, which is deprecated and will be removed in
   # https://gitlab.com/gitlab-org/gitlab/issues/29404
   trait :without_properties_callback do

spec/frontend/integrations/edit/components/active_checkbox_spec.js

@@ -35,6 +35,15 @@ describe('ActiveCheckbox', () => {
       });
     });
 
+    describe('when activateDisabled is true', () => {
+      it('renders GlFormCheckbox as disabled', () => {
+        createComponent({ activateDisabled: true });
+
+        expect(findGlFormCheckbox().exists()).toBe(true);
+        expect(findInputInCheckbox().attributes('disabled')).toBe('disabled');
+      });
+    });
+
     describe('initialActivated is `false`', () => {
       beforeEach(() => {
         createComponent({

spec/helpers/integrations_helper_spec.rb

@@ -55,6 +55,7 @@ RSpec.describe IntegrationsHelper do
         :id,
         :show_active,
         :activated,
+        :activate_disabled,
         :type,
         :merge_request_events,
         :commit_events,

spec/models/integrations/base_issue_tracker_spec.rb

@@ -3,12 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Integrations::BaseIssueTracker do
-  describe 'Validations' do
-    let(:project) { create :project }
+  let(:integration) { Integrations::Redmine.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) }
 
-    describe 'only one issue tracker per project' do
-      let(:integration) { Integrations::Redmine.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) }
+  let_it_be_with_refind(:project) { create :project }
 
+  describe 'Validations' do
+    describe 'only one issue tracker per project' do
       before do
         create(:custom_issue_tracker_integration, project: project)
       end
@@ -31,4 +31,18 @@ RSpec.describe Integrations::BaseIssueTracker do
       end
     end
   end
+
+  describe '#activate_disabled_reason' do
+    subject { integration.activate_disabled_reason }
+
+    context 'when there is an existing issue tracker integration' do
+      let_it_be(:custom_tracker) { create(:custom_issue_tracker_integration, project: project) }
+
+      it { is_expected.to eq(trackers: [custom_tracker]) }
+    end
+
+    context 'when there is no existing issue tracker integration' do
+      it { is_expected.to be(nil) }
+    end
+  end
 end