Merge branch 'sh-suppress-duplicate-remote-mirror-notifications' into 'master'

Only send one notification for failed remote mirror

Closes #56222

See merge request gitlab-org/gitlab-ce!24381

(cherry picked from commit 9cd5c5f5359cdebc2ae9ba1d20d2e79bd18edce2)

6fbbd4ab Only send one notification for failed remote mirror

3 files changed, 65 insertions, 0 deletions

diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb
index 224bc9ed935..c06e9a08ab4 100644
--- a/spec/models/remote_mirror_spec.rb
+++ b/spec/models/remote_mirror_spec.rb
@@ -303,6 +303,25 @@ describe RemoteMirror, :mailer do
     end
   end
 
+  context '#url=' do
+    let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first }
+
+    it 'resets all the columns when URL changes' do
+      remote_mirror.update(last_error: Time.now,
+                           last_update_at: Time.now,
+                           last_successful_update_at: Time.now,
+                           update_status: 'started',
+                           error_notification_sent: true)
+
+      expect { remote_mirror.update_attribute(:url, 'http://new.example.com') }
+        .to change { remote_mirror.last_error }.to(nil)
+        .and change { remote_mirror.last_update_at }.to(nil)
+        .and change { remote_mirror.last_successful_update_at }.to(nil)
+        .and change { remote_mirror.update_status }.to('finished')
+        .and change { remote_mirror.error_notification_sent }.to(false)
+    end
+  end
+
   context '#updated_since?' do
     let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first }
     let(:timestamp) { Time.now - 5.minutes }
diff --git a/spec/workers/remote_mirror_notification_worker_spec.rb b/spec/workers/remote_mirror_notification_worker_spec.rb
new file mode 100644
index 00000000000..e3db10ed645
--- /dev/null
+++ b/spec/workers/remote_mirror_notification_worker_spec.rb
@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+describe RemoteMirrorNotificationWorker, :mailer do
+  set(:project) { create(:project, :repository, :remote_mirror) }
+  set(:mirror) { project.remote_mirrors.first }
+
+  describe '#execute' do
+    it 'calls NotificationService#remote_mirror_update_failed when the mirror exists' do
+      mirror.update_column(:last_error, "There was a problem fetching")
+
+      expect(NotificationService).to receive_message_chain(:new, :remote_mirror_update_failed)
+
+      subject.perform(mirror.id)
+
+      expect(mirror.reload.error_notification_sent?).to be_truthy
+    end
+
+    it 'does nothing when the mirror has no errors' do
+      expect(NotificationService).not_to receive(:new)
+
+      subject.perform(mirror.id)
+    end
+
+    it 'does nothing when the mirror does not exist' do
+      expect(NotificationService).not_to receive(:new)
+
+      subject.perform(RemoteMirror.maximum(:id).to_i.succ)
+    end
+
+    it 'does nothing when a notification has already been sent' do
+      mirror.update_columns(last_error: "There was a problem fetching",
+                            error_notification_sent: true)
+
+      expect(NotificationService).not_to receive(:new)
+
+      subject.perform(mirror.id)
+    end
+  end
+end
diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb
index d73b0b53713..b582a3650b6 100644
--- a/spec/workers/repository_update_remote_mirror_worker_spec.rb
+++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb
@@ -22,6 +22,13 @@ describe RepositoryUpdateRemoteMirrorWorker do
         expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.update_status }.to('finished')
       end
 
+      it 'resets the notification flag upon success' do
+        expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success)
+        remote_mirror.update_column(:error_notification_sent, true)
+
+        expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.error_notification_sent }.to(false)
+      end
+
       it 'sets status as failed when update remote mirror service executes with errors' do
         error_message = 'fail!'