Notify on assignment rejection

Sends a notification email when a placeholder
user reassignment is rejected by the source user.

Changelog: added

app/controllers/import/source_users_controller.rb

@@ -14,7 +14,7 @@ class SourceUsersController < ApplicationController
     def accept
       result = ::Import::SourceUsers::AcceptReassignmentService.new(source_user, current_user: current_user).execute
 
-      if result.status == :success
+      if result.success?
         flash[:raw] = banner('accept_invite')
         redirect_to(dashboard_groups_path)
       else
@@ -23,7 +23,9 @@ def accept
     end
 
     def decline
-      if source_user.reject
+      result = ::Import::SourceUsers::RejectReassignmentService.new(source_user, current_user: current_user).execute
+
+      if result.success?
         flash[:raw] = banner('reject_invite')
         redirect_to(dashboard_groups_path)
       else

app/models/import/source_user.rb

@@ -22,6 +22,9 @@ class SourceUser < ApplicationRecord
     validates :namespace_id, :import_type, :source_hostname, :source_user_identifier, :status, presence: true
     validates :placeholder_user_id, presence: true, unless: :completed?
     validates :reassign_to_user_id, presence: true, if: -> { reassignment_in_progress? || completed? }
+    validates :reassign_to_user_id, absence: true, if: -> {
+                                                         pending_reassignment? || keep_as_placeholder?
+                                                       }
 
     scope :for_namespace, ->(namespace_id) { where(namespace_id: namespace_id) }
     scope :by_statuses, ->(statuses) { where(status: statuses) }
@@ -74,10 +77,6 @@ class SourceUser < ApplicationRecord
       event :fail_reassignment do
         transition reassignment_in_progress: :failed
       end
-
-      after_transition any => [:pending_reassignment, :rejected, :keep_as_placeholder] do |status|
-        status.update!(reassign_to_user: nil)
-      end
     end
 
     class << self

app/services/import/source_users/keep_as_placeholder_service.rb

@@ -22,6 +22,7 @@ def execute
       private
 
       def keep_as_placeholder
+        import_source_user.reassign_to_user = nil
         import_source_user.reassigned_by_user = current_user
         import_source_user.keep_as_placeholder
       end

app/services/import/source_users/reject_reassignment_service.rb

+# frozen_string_literal: true
+
+module Import
+  module SourceUsers
+    class RejectReassignmentService < BaseService
+      def initialize(import_source_user, current_user:)
+        @import_source_user = import_source_user
+        @current_user = current_user
+      end
+
+      def execute
+        return error_invalid_permissions unless current_user_matches_reassign_to_user
+        return error_invalid_status unless import_source_user.awaiting_approval?
+
+        if reject
+          send_user_reassign_rejected_email
+
+          ServiceResponse.success(payload: import_source_user)
+        else
+          ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages)
+        end
+      end
+
+      def send_user_reassign_rejected_email
+        Notify.import_source_user_rejected(import_source_user.id).deliver_now
+      end
+
+      private
+
+      def current_user_matches_reassign_to_user
+        return false if current_user.nil?
+
+        current_user.id == import_source_user.reassign_to_user_id
+      end
+
+      def reject
+        import_source_user.reject
+      end
+    end
+  end
+end

spec/mailers/previews_spec.rb

@@ -16,7 +16,7 @@
   let_it_be(:key) { create(:key, user: user) }
   let_it_be(:bulk_import) { create(:bulk_import, :finished, :with_configuration) }
   let_it_be(:source_user) do
-    create(:import_source_user, :with_reassigned_by_user, namespace: group, reassign_to_user: user)
+    create(:import_source_user, :awaiting_approval, :with_reassigned_by_user, namespace: group, reassign_to_user: user)
   end
 
   Gitlab.ee do

spec/models/import/source_user_spec.rb

@@ -27,6 +27,24 @@
 
       it { is_expected.to validate_presence_of(:reassign_to_user_id) }
     end
+
+    context 'when rejected' do
+      subject { build(:import_source_user, :rejected) }
+
+      it { is_expected.not_to validate_absence_of(:reassign_to_user_id) }
+    end
+
+    context 'when pending_reassignment' do
+      subject { build(:import_source_user, :pending_reassignment) }
+
+      it { is_expected.to validate_absence_of(:reassign_to_user_id) }
+    end
+
+    context 'when keep_as_placeholder' do
+      subject { build(:import_source_user, :keep_as_placeholder) }
+
+      it { is_expected.to validate_absence_of(:reassign_to_user_id) }
+    end
   end
 
   describe 'scopes' do
@@ -78,35 +96,6 @@
     end
   end
 
-  describe 'after_transition callbacks' do
-    subject(:source_user) { create(:import_source_user, :awaiting_approval, :with_reassign_to_user) }
-
-    it 'does not unset reassign_to_user on other transitions' do
-      expect { source_user.accept! }
-        .not_to change { source_user.reload.reassign_to_user }
-    end
-
-    it 'unsets reassign_to_user when rejected' do
-      expect { source_user.reject! }
-        .to change { source_user.reload.reassign_to_user }
-        .from(an_instance_of(User)).to(nil)
-    end
-
-    it 'unsets reassign_to_user when assignment is cancelled' do
-      expect { source_user.cancel_reassignment! }
-        .to change { source_user.reload.reassign_to_user }
-              .from(an_instance_of(User)).to(nil)
-    end
-
-    it 'unsets reassign_to_user when kept as placeholder' do
-      source_user = create(:import_source_user, :with_reassign_to_user)
-
-      expect { source_user.keep_as_placeholder! }
-        .to change { source_user.reload.reassign_to_user }
-        .from(an_instance_of(User)).to(nil)
-    end
-  end
-
   describe '.find_source_user' do
     let_it_be(:namespace_1) { create(:namespace) }
     let_it_be(:namespace_2) { create(:namespace) }

spec/requests/import/source_users_controller_spec.rb

@@ -93,16 +93,18 @@
 
   describe 'POST /decline' do
     let(:path) { decline_import_source_user_path(source_user) }
+    let(:message_delivery) { instance_double(ActionMailer::MessageDelivery) }
 
     subject(:reject_invite) { post path }
 
     context 'when signed in' do
       before do
         sign_in(source_user.reassign_to_user)
+        allow(message_delivery).to receive(:deliver_now)
+        allow(Notify).to receive(:import_source_user_rejected).and_return(message_delivery)
       end
 
       it { expect { reject_invite }.to change { source_user.reload.rejected? }.from(false).to(true) }
-      it { expect { reject_invite }.to change { source_user.reload.reassign_to_user }.from(instance_of(User)).to(nil) }
 
       it 'redirects with a notice' do
         reject_invite

spec/services/import/reassign_placeholder_user_records_service_spec.rb

@@ -149,7 +149,7 @@
 
     context 'when the source user is not in reassignment_in_progress status' do
       before do
-        source_user.update!(status: 0)
+        source_user.update!(status: 1)
       end
 
       it 'does not reassign any contributions' do

spec/services/import/source_users/keep_as_placeholder_service_spec.rb

@@ -15,6 +15,7 @@
 
         expect(result).to be_success
         expect(result.payload.reload).to eq(import_source_user)
+        expect(result.payload.reassign_to_user).to eq(nil)
         expect(result.payload.reassigned_by_user).to eq(current_user)
         expect(result.payload.keep_as_placeholder?).to eq(true)
       end

spec/services/import/source_users/reject_reassignment_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Import::SourceUsers::RejectReassignmentService, feature_category: :importers do
+  let(:import_source_user) { create(:import_source_user, :awaiting_approval, :with_reassign_to_user) }
+  let(:current_user) { import_source_user.reassign_to_user }
+  let(:service) { described_class.new(import_source_user, current_user: current_user) }
+
+  describe '#execute' do
+    let(:message_delivery) { instance_double(ActionMailer::MessageDelivery) }
+
+    before do
+      allow(message_delivery).to receive(:deliver_now)
+      allow(Notify).to receive(:import_source_user_rejected).and_return(message_delivery)
+    end
+
+    it 'returns success' do
+      expect(Notify).to receive_message_chain(:import_source_user_rejected, :deliver_now)
+      expect(service.execute).to be_success
+    end
+
+    it 'sets the source user to rejected' do
+      service.execute
+      expect(import_source_user.reload).to be_rejected
+    end
+
+    context 'when current user does not have permission to reject' do
+      let(:current_user) { create(:user) }
+
+      it 'returns error no permissions' do
+        result = service.execute
+
+        expect(Notify).not_to receive(:import_source_user_rejected)
+
+        expect(result).to be_error
+        expect(result.message).to eq('You have insufficient permissions to update the import source user')
+      end
+    end
+
+    context 'when import source user does not have a rejectable status' do
+      let(:import_source_user) { create(:import_source_user, :reassignment_in_progress) }
+
+      it 'returns error invalid status' do
+        result = service.execute
+
+        expect(result).to be_error
+        expect(result.message).to eq("Import source user has an invalid status for this operation")
+      end
+    end
+
+    context 'when an error occurs' do
+      before do
+        allow(import_source_user).to receive(:reject).and_return(false)
+        allow(import_source_user).to receive(:errors).and_return(instance_double(ActiveModel::Errors,
+          full_messages: ['Error']))
+      end
+
+      it 'returns an error' do
+        result = service.execute
+
+        expect(result).to be_error
+        expect(result.message).to eq(['Error'])
+      end
+    end
+  end
+end