From ba861a22198255bb461596e7fe6ec88248d1a2fe Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 3 Jan 2019 22:59:23 -0800
Subject: [PATCH] Only validate project visibility when it has changed

On GitLab.com, there are hundreds of projects that have visibility
levels that are inconsistent with the fork or group settings. Most
likely, this happened during a GitLab project import because the
validation was bypassed.  Attempting to migrate these projects to hashed
storage fails even though the migration doesn't touch the visibility
settings. Skipping the visibility validation allows the migration to go
through.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/55881
---
 app/models/project.rb                         |  4 +--
 .../sh-skip-validation-visibility-changed.yml |  5 ++++
 spec/models/project_spec.rb                   | 25 +++++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/sh-skip-validation-visibility-changed.yml

diff --git a/app/models/project.rb b/app/models/project.rb
index cd558752080..e13627ceb87 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -330,8 +330,8 @@ class Project < ActiveRecord::Base
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
-  validate :visibility_level_allowed_by_group
-  validate :visibility_level_allowed_as_fork
+  validate :visibility_level_allowed_by_group, if: -> { changes.has_key?(:visibility_level) }
+  validate :visibility_level_allowed_as_fork, if: -> { changes.has_key?(:visibility_level) }
   validate :check_wiki_path_conflict
   validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) }
   validates :repository_storage,
diff --git a/changelogs/unreleased/sh-skip-validation-visibility-changed.yml b/changelogs/unreleased/sh-skip-validation-visibility-changed.yml
new file mode 100644
index 00000000000..405be698b2b
--- /dev/null
+++ b/changelogs/unreleased/sh-skip-validation-visibility-changed.yml
@@ -0,0 +1,5 @@
+---
+title: Only validate project visibility when it has changed
+merge_request: 24142
+author:
+type: fixed
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 4b6592020c1..36be76aad8c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2959,6 +2959,24 @@ describe Project do
     end
   end
 
+  describe '#update' do
+    let(:project) { create(:project) }
+
+    it 'validates the visibility' do
+      expect(project).to receive(:visibility_level_allowed_as_fork).and_call_original
+      expect(project).to receive(:visibility_level_allowed_by_group).and_call_original
+
+      project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+    end
+
+    it 'does not validate the visibility' do
+      expect(project).not_to receive(:visibility_level_allowed_as_fork).and_call_original
+      expect(project).not_to receive(:visibility_level_allowed_by_group).and_call_original
+
+      project.update(updated_at: Time.now)
+    end
+  end
+
   describe '#last_repository_updated_at' do
     it 'sets to created_at upon creation' do
       project = create(:project, created_at: 2.hours.ago)
@@ -3185,6 +3203,13 @@ describe Project do
         expect { project.migrate_to_hashed_storage! }.to change { project.repository_read_only }.to(true)
       end
 
+      it 'does not validate project visibility' do
+        expect(project).not_to receive(:visibility_level_allowed_as_fork)
+        expect(project).not_to receive(:visibility_level_allowed_by_group)
+
+        project.migrate_to_hashed_storage!
+      end
+
       it 'schedules ProjectMigrateHashedStorageWorker with delayed start when the project repo is in use' do
         Gitlab::ReferenceCounter.new(project.gl_repository(is_wiki: false)).increase