Merge branch 'auto-fsck-skip-wiki' into 'master'

Improve 'auto fsck' feature

- skip wiki repositories when they are disabled (avoids false alarms)
- more context in admin alert emails

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/15332

See merge request !3781

6 files changed, 45 insertions, 5 deletions

diff --git a/app/mailers/repository_check_mailer.rb b/app/mailers/repository_check_mailer.rb
index 2bff5b63cc4..21db2fe04a0 100644
--- a/app/mailers/repository_check_mailer.rb
+++ b/app/mailers/repository_check_mailer.rb
@@ -8,7 +8,7 @@ class RepositoryCheckMailer < BaseMailer
 
     mail(
       to: User.admins.pluck(:email),
-      subject: @message
+      subject: "GitLab Admin | #{@message}"
     )
   end
 end
diff --git a/app/views/repository_check_mailer/notify.html.haml b/app/views/repository_check_mailer/notify.html.haml
index df16f503570..a585147ddd1 100644
--- a/app/views/repository_check_mailer/notify.html.haml
+++ b/app/views/repository_check_mailer/notify.html.haml
@@ -3,3 +3,6 @@
 
 %p
   = link_to "See the affected projects in the GitLab admin panel", admin_namespaces_projects_url(last_repository_check_failed: 1)
+
+%p
+  You are receiving this message because you are a GitLab administrator for #{Gitlab.config.gitlab.url}.
diff --git a/app/views/repository_check_mailer/notify.text.haml b/app/views/repository_check_mailer/notify.text.haml
index 02f3f80288a..93db151329e 100644
--- a/app/views/repository_check_mailer/notify.text.haml
+++ b/app/views/repository_check_mailer/notify.text.haml
@@ -1,3 +1,6 @@
 #{@message}.
 \
 View details: #{admin_namespaces_projects_url(last_repository_check_failed: 1)}
+
+You are receiving this message because you are a GitLab administrator
+for #{Gitlab.config.gitlab.url}.
diff --git a/app/workers/repository_check/single_repository_worker.rb b/app/workers/repository_check/single_repository_worker.rb
index e54ae86d06c..a76729e3c74 100644
--- a/app/workers/repository_check/single_repository_worker.rb
+++ b/app/workers/repository_check/single_repository_worker.rb
@@ -15,10 +15,10 @@ module RepositoryCheck
     private
   
     def check(project)
+      repositories = [project.repository]
+      repositories << project.wiki.repository if project.wiki_enabled?
       # Use 'map do', not 'all? do', to prevent short-circuiting
-      [project.repository, project.wiki.repository].map do |repository|
-        git_fsck(repository.path_to_repo)
-      end.all?
+      repositories.map { |repository| git_fsck(repository.path_to_repo) }.all?
     end
   
     def git_fsck(path)
diff --git a/spec/mailers/repository_check_mailer_spec.rb b/spec/mailers/repository_check_mailer_spec.rb
index 583bf15176f..00613c7b671 100644
--- a/spec/mailers/repository_check_mailer_spec.rb
+++ b/spec/mailers/repository_check_mailer_spec.rb
@@ -15,7 +15,7 @@ describe RepositoryCheckMailer do
     it 'mentions the number of failed checks' do
       mail = described_class.notify(3)
 
-      expect(mail).to have_subject '3 projects failed their last repository check'
+      expect(mail).to have_subject 'GitLab Admin | 3 projects failed their last repository check'
     end
   end
 end
diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb
new file mode 100644
index 00000000000..087e4c667d8
--- /dev/null
+++ b/spec/workers/repository_check/single_repository_worker_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+require 'fileutils'
+
+describe RepositoryCheck::SingleRepositoryWorker do
+  subject { described_class.new }
+
+  it 'fails if the wiki repository is broken' do
+    project = create(:project_empty_repo, wiki_enabled: true)
+    project.create_wiki
+
+    # Test sanity: everything should be fine before the wiki repo is broken
+    subject.perform(project.id)
+    expect(project.reload.last_repository_check_failed).to eq(false)
+
+    destroy_wiki(project)
+    subject.perform(project.id)
+
+    expect(project.reload.last_repository_check_failed).to eq(true)
+  end
+
+  it 'skips wikis when disabled' do
+    project = create(:project_empty_repo, wiki_enabled: false)
+    # Make sure the test would fail if it checked the wiki repo
+    destroy_wiki(project)
+
+    subject.perform(project.id)
+
+    expect(project.reload.last_repository_check_failed).to eq(false)
+  end
+
+  def destroy_wiki(project)
+    FileUtils.rm_rf(project.wiki.repository.path_to_repo)
+  end
+end