Change notification recipient from mirror user to project owners and masters

app/mailers/emails/projects.rb

@@ -40,11 +40,11 @@ module Emails
     end
 
     def mirror_was_hard_failed_email(project_id, user_id)
-      @user = User.find(user_id)
       @project = Project.find(project_id)
+      user = User.find(user_id)
 
-      mail(to: @user.notification_email,
-           subject: subject('Pull mirroring paused'))
+      mail(to: user.notification_email,
+           subject: subject('Repository mirroring paused'))
     end
   end
 end

app/views/notify/mirror_was_hard_failed_email.html.haml

-%p
-  Mirroring from #{@project.import_url} to #{@project.full_path} failed with the following error:
-
-%p
-  = @project.import_error.try(:strip)
-
-Repository mirroring has been paused due to too many failed attempts, and can be resumed by a project admin.

app/views/notify/mirror_was_hard_failed_email.text.erb

-Mirroring from <%= @project.import_url %> to <%= @project.full_path %> failed with the following error:
-
-<%= @project.import_error.try(:strip) %>
-
-Repository mirroring has been paused due to too many failed attempts, and can be resumed by a project admin.
-
-

ee/app/models/ee/project/import_status_state_machine.rb

@@ -21,7 +21,9 @@ module EE
           end
 
           after_transition started: :failed do |project, _|
-            ::NotificationService.new.mirror_was_hard_failed(project) if project.mirror?
+            if project.mirror? && project.mirror_hard_failed?
+              ::NotificationService.new.mirror_was_hard_failed(project)
+            end
           end
 
           after_transition [:scheduled, :started] => [:finished, :failed] do |project, _|

ee/app/services/ee/notification_service.rb

@@ -24,11 +24,15 @@ module EE
     end
 
     def mirror_was_hard_failed(project)
-      return unless project.mirror_hard_failed?
+      recipients = project.members.owners_and_masters
 
-      recepient = project.mirror_user
+      unless recipients.present?
+        recipients = project.group.members.owners_and_masters
+      end
 
-      mailer.mirror_was_hard_failed_email(project.id, recepient.id).deliver_later
+      recipients.each do |recipient|
+        mailer.mirror_was_hard_failed_email(project.id, recipient.user.id).deliver_later
+      end
     end
   end
 end

ee/app/views/notify/mirror_was_hard_failed_email.html.haml

+%p
+  Repository mirroring on #{@project.full_path} has been paused due to too many failures. The last failure was:
+
+%pre
+  = @project.import_error
+
+%p
+  To resume mirroring update your #{link_to("repository mirroring settings", project_settings_repository_path(@project))}.

ee/app/views/notify/mirror_was_hard_failed_email.text.erb

+Repository mirroring on <%= @project.full_path %> has been paused due to too many failures. The last failure was:
+
+<%= @project.import_error %>
+
+To resume mirroring update your repository settings at <%= project_settings_repository_url(@project) %>.

ee/app/workers/repository_update_mirror_worker.rb

@@ -62,10 +62,9 @@ class RepositoryUpdateMirrorWorker
   end
 
   def fail_mirror(project, message)
-    error_message = "Mirror update for #{project.full_path} failed with the following message: #{message}"
-    project.mark_import_as_failed(error_message)
+    project.mark_import_as_failed(message)
 
-    Rails.logger.error(error_message)
+    Rails.logger.error("Mirror update for #{project.full_path} failed with the following message: #{message}")
     Gitlab::Metrics.add_event(:mirrors_failed, path: project.full_path)
   end
 

ee/spec/models/project_spec.rb

@@ -102,6 +102,17 @@ describe Project do
     end
   end
 
+  describe 'hard failing a mirror' do
+    it 'sends a notification' do
+      project = create(:project, :mirror, :import_started)
+      project.mirror_data.update_attributes(retry_count: Gitlab::Mirror::MAX_RETRY)
+
+      expect_any_instance_of(EE::NotificationService).to receive(:mirror_was_hard_failed).with(project)
+
+      project.import_fail
+    end
+  end
+
   describe '#push_rule' do
     let(:project) { create(:project, push_rule: create(:push_rule)) }
 

ee/spec/services/ee/notification_service_spec.rb

@@ -123,20 +123,66 @@ describe EE::NotificationService, :mailer do
   describe 'mirror hard failed' do
     let(:user) { create(:user) }
 
-    it 'does not send email when mirror is not hard failed' do
-      project = create(:project, :mirror)
+    context 'when user is owner' do
+      it 'sends email' do
+        project = create(:project, :mirror, :import_hard_failed)
 
-      expect(Notify).not_to receive(:mirror_was_hard_failed_email)
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, project.owner.id).and_call_original
 
-      subject.mirror_was_hard_failed(project)
+        subject.mirror_was_hard_failed(project)
+      end
+    end
+
+    context 'when user is master' do
+      it 'sends email' do
+        project = create(:project, :mirror, :import_hard_failed)
+        project.add_master(user)
+
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, project.owner.id).and_call_original
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
+
+        subject.mirror_was_hard_failed(project)
+      end
     end
 
-    it 'sends email to mirror user when mirror hard failed' do
-      project = create(:project, :mirror, :import_hard_failed, mirror_user: user)
+    context 'when user is not owner nor master' do
+      it 'does not send email' do
+        project = create(:project, :mirror, :import_hard_failed)
+        project.add_developer(user)
+
+        expect(Notify).not_to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
+        expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, project.creator.id).and_call_original
+
+        subject.mirror_was_hard_failed(project)
+      end
+
+      context 'when user is group owner' do
+        it 'sends email' do
+          group = create(:group, :public) do |group|
+            group.add_owner(user)
+          end
 
-      expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
+          project = create(:project, :mirror, :import_hard_failed, namespace: group)
 
-      subject.mirror_was_hard_failed(project)
+          expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
+
+          subject.mirror_was_hard_failed(project)
+        end
+      end
+
+      context 'when user is group master' do
+        it 'sends email' do
+          group = create(:group, :public) do |group|
+            group.add_master(user)
+          end
+
+          project = create(:project, :mirror, :import_hard_failed, namespace: group)
+
+          expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original
+
+          subject.mirror_was_hard_failed(project)
+        end
+      end
     end
   end
 end

spec/mailers/notify_spec.rb

@@ -1429,8 +1429,7 @@ describe Notify do
     it_behaves_like "a user cannot unsubscribe through footer link"
 
     it 'has the correct subject and body' do
-      is_expected.to have_subject("#{project.name} | Pull mirroring paused")
-      is_expected.to have_html_escaped_body_text(project.import_url)
+      is_expected.to have_subject("#{project.name} | Repository mirroring paused")
       is_expected.to have_html_escaped_body_text(project.full_path)
     end
   end