Move controllers/services service desk code to core

Part of the plan to move service desk feature
into core tier.

More info: https://gitlab.com/gitlab-org/gitlab/-/issues/215364

app/controllers/projects/issues_controller.rb

@@ -11,11 +11,11 @@ class Projects::IssuesController < Projects::ApplicationController
   include RecordUserLastActivity
   def issue_except_actions
-    %i[index calendar new create bulk_update import_csv export_csv]
+    %i[index calendar new create bulk_update import_csv export_csv service_desk]
   end
   def set_issuables_index_only_actions
-    %i[index calendar]
+    %i[index calendar service_desk]
   end
   prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
@@ -223,6 +223,11 @@ class Projects::IssuesController < Projects::ApplicationController
     redirect_to project_issues_path(project)
   end
+  def service_desk
+    @issues = @issuables # rubocop:disable Gitlab/ModuleWithInstanceVariables
+    @users.push(::User.support_bot) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  end
+
   protected
   def sorting_field
@@ -320,6 +325,17 @@ class Projects::IssuesController < Projects::ApplicationController
   private
+  def finder_options
+    options = super
+
+    return options unless service_desk?
+
+    options.reject! { |key| key == 'author_username' || key == 'author_id' }
+    options[:author_id] = ::User.support_bot
+
+    options
+  end
+
   def branch_link(branch)
     project_compare_path(project, from: project.default_branch, to: branch[:name])
   end
@@ -337,6 +353,10 @@ class Projects::IssuesController < Projects::ApplicationController
   def rate_limiter
     ::Gitlab::ApplicationRateLimiter
   end
+
+  def service_desk?
+    action_name == 'service_desk'
+  end
 end
 Projects::IssuesController.prepend_if_ee('EE::Projects::IssuesController')

app/controllers/projects_controller.rb

@@ -38,6 +38,7 @@ class ProjectsController < Projects::ApplicationController
   before_action only: [:new, :create] do
     frontend_experimentation_tracking_data(:new_create_project_ui, 'click_tab')
     push_frontend_feature_flag(:new_create_project_ui) if experiment_enabled?(:new_create_project_ui)
+    push_frontend_feature_flag(:service_desk_custom_address, @project)
   end
   layout :determine_layout
@@ -391,6 +392,7 @@ class ProjectsController < Projects::ApplicationController
       :initialize_with_readme,
       :autoclose_referenced_issues,
       :suggestion_commit_message,
+      :service_desk_enabled,
       project_feature_attributes: %i[
         builds_access_level

app/services/issues/move_service.rb

@@ -19,11 +19,22 @@ module Issues
       notify_participants
+      # Updates old issue sent notifications allowing
+      # to receive service desk emails on the new moved issue.
+      update_service_desk_sent_notifications
+
       new_entity
     end
     private
+    def update_service_desk_sent_notifications
+      return unless original_entity.from_service_desk?
+
+      original_entity
+        .sent_notifications.update_all(project_id: new_entity.project_id, noteable_id: new_entity.id)
+    end
+
     def update_old_entity
       super

app/services/notification_service.rb

@@ -15,6 +15,9 @@
 #
 #   NotificationService.new.async.new_issue(issue, current_user)
 #
+
+require 'gitlab/service_desk'
+
 class NotificationService
   class Async
     attr_reader :parent
@@ -294,6 +297,7 @@ class NotificationService
     return true if note.system_note_with_references?
     send_new_note_notifications(note)
+    send_service_desk_notification(note)
   end
   def send_new_note_notifications(note)
@@ -305,6 +309,21 @@ class NotificationService
     end
   end
+  def send_service_desk_notification(note)
+    return unless Gitlab::ServiceDesk.supported?
+    return unless note.noteable_type == 'Issue'
+
+    issue = note.noteable
+    support_bot = User.support_bot
+
+    return unless issue.service_desk_reply_to.present?
+    return unless issue.project.service_desk_enabled?
+    return if note.author == support_bot
+    return unless issue.subscribed?(support_bot, issue.project)
+
+    mailer.service_desk_new_note_email(issue.id, note.id).deliver_later
+  end
+
   # Notify users when a new release is created
   def send_new_release_notifications(release)
     recipients = NotificationRecipients::BuildService.build_new_release_recipients(release)

config/routes/issues.rb

@@ -17,6 +17,7 @@ resources :issues, concerns: :awardable, constraints: { id: /\d+/ } do
   end
   collection do
+    get :service_desk
     post :bulk_update
     post :import_csv
     post :export_csv

config/routes/project.rb

@@ -340,6 +340,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       # All new routes should go under /-/ scope.
       # Look for scope '-' at the top of the file.
+      #
+      # Service Desk
+      #
+      get '/service_desk' => 'service_desk#show', as: :service_desk
+      put '/service_desk' => 'service_desk#update', as: :service_desk_refresh
+
       #
       # Templates
       #

ee/app/controllers/ee/projects/issues_controller.rb

@@ -16,21 +16,6 @@ module EE
         end
       end
-      override :issue_except_actions
-      def issue_except_actions
-        super + %i[service_desk]
-      end
-
-      override :set_issuables_index_only_actions
-      def set_issuables_index_only_actions
-        super + %i[service_desk]
-      end
-
-      def service_desk
-        @issues = @issuables # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        @users.push(::User.support_bot) # rubocop:disable Gitlab/ModuleWithInstanceVariables
-      end
-
       private
       def issue_params_attributes
@@ -41,20 +26,13 @@ module EE
         attrs
       end
+      override :finder_options
       def finder_options
         options = super
-        options.reject! { |key| key == 'weight' } unless project.feature_available?(:issue_weights)
-
-        if service_desk?
-          options.reject! { |key| key == 'author_username' || key == 'author_id' }
-          options[:author_id] = ::User.support_bot
-        end
-        options
-      end
-      def service_desk?
-        action_name == 'service_desk'
+        return super if project.feature_available?(:issue_weights)
+        options.reject { |key| key == 'weight' }
       end
       def whitelist_query_limiting_ee

ee/app/controllers/ee/projects_controller.rb

@@ -9,10 +9,6 @@ module EE
       before_action :log_download_export_audit_event, only: [:download_export]
       before_action :log_archive_audit_event, only: [:archive]
       before_action :log_unarchive_audit_event, only: [:unarchive]
-
-      before_action do
-        push_frontend_feature_flag(:service_desk_custom_address, @project)
-      end
     end
     def restore
@@ -79,7 +75,6 @@ module EE
         merge_requests_template
         repository_size_limit
         reset_approvals_on_push
-        service_desk_enabled
         ci_cd_only
         use_custom_template
         packages_enabled

ee/app/services/ee/issues/move_service.rb

@@ -5,17 +5,6 @@ module EE
     module MoveService
       extend ::Gitlab::Utils::Override
-      override :execute
-      def execute(issue, target_project)
-        super
-
-        # Updates old issue sent notifications allowing
-        # to receive service desk emails on the new moved issue.
-        update_service_desk_sent_notifications
-
-        new_entity
-      end
-
       override :update_old_entity
       def update_old_entity
         rewrite_epic_issue
@@ -25,13 +14,6 @@ module EE
       private
-      def update_service_desk_sent_notifications
-        return unless original_entity.from_service_desk?
-
-        original_entity
-          .sent_notifications.update_all(project_id: new_entity.project_id, noteable_id: new_entity.id)
-      end
-
       def rewrite_epic_issue
         return unless epic_issue = original_entity.epic_issue
         return unless can?(current_user, :update_epic, epic_issue.epic.group)

ee/app/services/ee/notification_service.rb

 # frozen_string_literal: true
-require 'gitlab/service_desk'
-
 module EE
   module NotificationService
-    extend ::Gitlab::Utils::Override
-
     # When we add approvers to a merge request we should send an email to:
     #
     #  * the new approvers
@@ -24,12 +20,6 @@ module EE
       unapprove_mr_email(merge_request, merge_request.target_project, current_user)
     end
-    override :send_new_note_notifications
-    def send_new_note_notifications(note)
-      super
-      send_service_desk_notification(note)
-    end
-
     def mirror_was_hard_failed(project)
       return if project.emails_disabled?
@@ -96,21 +86,6 @@ module EE
       end
     end
-    def send_service_desk_notification(note)
-      return unless Gitlab::ServiceDesk.supported?
-      return unless note.noteable_type == 'Issue'
-
-      issue = note.noteable
-      support_bot = ::User.support_bot
-
-      return unless issue.service_desk_reply_to.present?
-      return unless issue.project.service_desk_enabled?
-      return if note.author == support_bot
-      return unless issue.subscribed?(support_bot, issue.project)
-
-      mailer.service_desk_new_note_email(issue.id, note.id).deliver_later
-    end
-
     def removed_iteration_resource_email(target, current_user)
       recipients = ::NotificationRecipients::BuildService.build_recipients(
         target,

ee/config/routes/issues.rb

@@ -6,9 +6,5 @@ resources :issues, only: [], constraints: { id: /\d+/ } do
     delete '/descriptions/:version_id', action: :delete_description_version, as: :delete_description_version
   end
-  collection do
-    get :service_desk
-  end
-
   resources :issue_links, only: [:index, :create, :destroy], as: 'links', path: 'links'
 end

ee/config/routes/project.rb

@@ -122,9 +122,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       resource :tracing, only: [:show]
-      get '/service_desk' => 'service_desk#show', as: :service_desk
-      put '/service_desk' => 'service_desk#update', as: :service_desk_refresh
-
       post '/restore' => '/projects#restore', as: :restore
       resource :insights, only: [:show], trailing_slash: true do

ee/spec/controllers/projects/issues_controller_spec.rb

@@ -115,42 +115,6 @@ RSpec.describe Projects::IssuesController do
     end
   end
-  describe 'GET service_desk' do
-    let(:support_bot) { User.support_bot }
-    let(:other_user) { create(:user) }
-    let!(:service_desk_issue_1) { create(:issue, project: project, author: support_bot) }
-    let!(:service_desk_issue_2) { create(:issue, project: project, author: support_bot, assignees: [other_user]) }
-    let!(:other_user_issue) { create(:issue, project: project, author: other_user) }
-
-    def get_service_desk(extra_params = {})
-      get :service_desk, params: extra_params.merge(namespace_id: project.namespace, project_id: project)
-    end
-
-    it 'adds an author filter for the support bot user' do
-      get_service_desk
-
-      expect(assigns(:issues)).to contain_exactly(service_desk_issue_1, service_desk_issue_2)
-    end
-
-    it 'does not allow any other author to be set' do
-      get_service_desk(author_username: other_user.username)
-
-      expect(assigns(:issues)).to contain_exactly(service_desk_issue_1, service_desk_issue_2)
-    end
-
-    it 'supports other filters' do
-      get_service_desk(assignee_username: other_user.username)
-
-      expect(assigns(:issues)).to contain_exactly(service_desk_issue_2)
-    end
-
-    it 'allows an assignee to be specified by id' do
-      get_service_desk(assignee_id: other_user.id)
-
-      expect(assigns(:users)).to contain_exactly(other_user, support_bot)
-    end
-  end
-
   describe 'GET #discussions' do
     let(:issue) { create(:issue, project: project) }
     let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }

ee/spec/controllers/projects_controller_spec.rb

@@ -216,25 +216,6 @@ RSpec.describe ProjectsController do
       end
     end
-    it 'updates Service Desk attributes' do
-      allow(Gitlab::IncomingEmail).to receive(:enabled?) { true }
-      allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
-      params = {
-        service_desk_enabled: true
-      }
-
-      put :update,
-          params: {
-            namespace_id: project.namespace,
-            id: project,
-            project: params
-          }
-      project.reload
-
-      expect(response).to have_gitlab_http_status(:found)
-      expect(project.service_desk_enabled).to eq(true)
-    end
-
     context 'when merge_pipelines_enabled param is specified' do
       let(:params) { { merge_pipelines_enabled: true } }

ee/spec/services/ee/issues/move_service_spec.rb

@@ -109,47 +109,4 @@ RSpec.describe Issues::MoveService do
       end
     end
   end
-
-  context 'updating sent notifications' do
-    let!(:old_issue_notification_1) { create(:sent_notification, project: old_issue.project, noteable: old_issue) }
-    let!(:old_issue_notification_2) { create(:sent_notification, project: old_issue.project, noteable: old_issue) }
-    let!(:other_issue_notification) { create(:sent_notification, project: old_issue.project) }
-
-    context 'when issue is from service desk' do
-      before do
-        allow(old_issue).to receive(:from_service_desk?).and_return(true)
-      end
-
-      it 'updates moved issue sent notifications' do
-        new_issue = move_service.execute(old_issue, new_project)
-
-        old_issue_notification_1.reload
-        old_issue_notification_2.reload
-        expect(old_issue_notification_1.project_id).to eq(new_issue.project_id)
-        expect(old_issue_notification_1.noteable_id).to eq(new_issue.id)
-        expect(old_issue_notification_2.project_id).to eq(new_issue.project_id)
-        expect(old_issue_notification_2.noteable_id).to eq(new_issue.id)
-      end
-
-      it 'does not update other issues sent notifications' do
-        expect do
-          move_service.execute(old_issue, new_project)
-          other_issue_notification.reload
-        end.not_to change { other_issue_notification.noteable_id }
-      end
-    end
-
-    context 'when issue is not from service desk' do
-      it 'does not update sent notifications' do
-        move_service.execute(old_issue, new_project)
-
-        old_issue_notification_1.reload
-        old_issue_notification_2.reload
-        expect(old_issue_notification_1.project_id).to eq(old_issue.project_id)
-        expect(old_issue_notification_1.noteable_id).to eq(old_issue.id)
-        expect(old_issue_notification_2.project_id).to eq(old_issue.project_id)
-        expect(old_issue_notification_2.noteable_id).to eq(old_issue.id)
-      end
-    end
-  end
 end

ee/spec/services/ee/notification_service_spec.rb

@@ -10,76 +10,6 @@ RSpec.describe EE::NotificationService, :mailer do
   let(:mailer) { double(deliver_later: true) }
-  context 'service desk issues' do
-    before do
-      allow(Notify).to receive(:service_desk_new_note_email)
-                         .with(Integer, Integer).and_return(mailer)
-
-      allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
-      allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
-    end
-
-    def should_email!
-      expect(Notify).to receive(:service_desk_new_note_email)
-        .with(issue.id, note.id)
-    end
-
-    def should_not_email!
-      expect(Notify).not_to receive(:service_desk_new_note_email)
-    end
-
-    def execute!
-      subject.new_note(note)
-    end
-
-    def self.it_should_email!
-      it 'sends the email' do
-        should_email!
-        execute!
-      end
-    end
-
-    def self.it_should_not_email!
-      it 'doesn\'t send the email' do
-        should_not_email!
-        execute!
-      end
-    end
-
-    let(:issue) { create(:issue, author: User.support_bot) }
-    let(:project) { issue.project }
-    let(:note) { create(:note, noteable: issue, project: project) }
-
-    context 'a non-service-desk issue' do
-      it_should_not_email!
-    end
-
-    context 'a service-desk issue' do
-      before do
-        issue.update!(service_desk_reply_to: 'service.desk@example.com')
-        project.update!(service_desk_enabled: true)
-      end
-
-      it_should_email!
-
-      context 'where the project has disabled the feature' do
-        before do
-          project.update(service_desk_enabled: false)
-        end
-
-        it_should_not_email!
-      end
-
-      context 'when the support bot has unsubscribed' do
-        before do
-          issue.unsubscribe(User.support_bot, project)
-        end
-
-        it_should_not_email!
-      end
-    end
-  end
-
   context 'new review' do
     let(:project) { create(:project, :repository) }
     let(:user) { create(:user) }

spec/controllers/projects/issues_controller_spec.rb

@@ -1564,6 +1564,43 @@ RSpec.describe Projects::IssuesController do
     end
   end
+  describe 'GET service_desk' do
+    let_it_be(:project) { create(:project_empty_repo, :public) }
+    let_it_be(:support_bot) { User.support_bot }
+    let_it_be(:other_user) { create(:user) }
+    let_it_be(:service_desk_issue_1) { create(:issue, project: project, author: support_bot) }
+    let_it_be(:service_desk_issue_2) { create(:issue, project: project, author: support_bot, assignees: [other_user]) }
+    let_it_be(:other_user_issue) { create(:issue, project: project, author: other_user) }
+
+    def get_service_desk(extra_params = {})
+      get :service_desk, params: extra_params.merge(namespace_id: project.namespace, project_id: project)
+    end
+
+    it 'adds an author filter for the support bot user' do
+      get_service_desk
+
+      expect(assigns(:issues)).to contain_exactly(service_desk_issue_1, service_desk_issue_2)
+    end
+
+    it 'does not allow any other author to be set' do
+      get_service_desk(author_username: other_user.username)
+
+      expect(assigns(:issues)).to contain_exactly(service_desk_issue_1, service_desk_issue_2)
+    end
+
+    it 'supports other filters' do
+      get_service_desk(assignee_username: other_user.username)
+
+      expect(assigns(:issues)).to contain_exactly(service_desk_issue_2)
+    end
+
+    it 'allows an assignee to be specified by id' do
+      get_service_desk(assignee_id: other_user.id)
+
+      expect(assigns(:users)).to contain_exactly(other_user, support_bot)
+    end
+  end
+
   describe 'GET #discussions' do
     let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }

ee/spec/controllers/projects/service_desk_controller_spec.rb → spec/controllers/projects/service_desk_controller_spec.rb

@@ -56,7 +56,7 @@ RSpec.describe Projects::ServiceDeskController do
     context 'when issue template file becomes outdated' do
       it 'returns template_file_missing as true' do
         service = ServiceDeskSetting.new(project_id: project.id, issue_template_key: 'deleted')
-        service.save(validate: false)
+        service.save!(validate: false)
         get :show, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json