Merge branch 'project-avatar-fork' into 'master'

Copy, don't move uploaded avatar files

Closes #26158

See merge request !8396

app/uploaders/avatar_uploader.rb

@@ -10,4 +10,15 @@ class AvatarUploader < GitlabUploader
   def exists?
     model.avatar.file && model.avatar.file.exists?
   end
+
+  # We set move_to_store and move_to_cache to 'false' to prevent stealing
+  # the avatar file from a project when forking it.
+  # https://gitlab.com/gitlab-org/gitlab-ce/issues/26158
+  def move_to_store
+    false
+  end
+
+  def move_to_cache
+    false
+  end
 end

changelogs/unreleased/project-avatar-fork.yml

+---
+title: Copy, don't move uploaded avatar files
+merge_request: 8396
+author:

spec/services/projects/fork_service_spec.rb

@@ -5,10 +5,12 @@ describe Projects::ForkService, services: true do
     before do
       @from_namespace = create(:namespace)
       @from_user = create(:user, namespace: @from_namespace )
+      avatar = fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")
       @from_project = create(:project,
                              creator_id: @from_user.id,
                              namespace: @from_namespace,
                              star_count: 107,
+                             avatar: avatar,
                              description: 'wow such project')
       @to_namespace = create(:namespace)
       @to_user = create(:user, namespace: @to_namespace)
@@ -36,6 +38,17 @@ describe Projects::ForkService, services: true do
         it { expect(to_project.namespace).to eq(@to_user.namespace) }
         it { expect(to_project.star_count).to be_zero }
         it { expect(to_project.description).to eq(@from_project.description) }
+        it { expect(to_project.avatar.file).to be_exists }
+
+        # This test is here because we had a bug where the from-project lost its
+        # avatar after being forked.
+        # https://gitlab.com/gitlab-org/gitlab-ce/issues/26158
+        it "after forking the from-project still has its avatar" do
+          # If we do not fork the project first we cannot detect the bug.
+          expect(to_project).to be_persisted
+
+          expect(@from_project.avatar.file).to be_exists
+        end
       end
     end
 

spec/uploaders/avatar_uploader_spec.rb

@@ -5,14 +5,14 @@ describe AvatarUploader do
   subject { described_class.new(user) }
 
   describe '#move_to_cache' do
-    it 'is true' do
-      expect(subject.move_to_cache).to eq(true)
+    it 'is false' do
+      expect(subject.move_to_cache).to eq(false)
     end
   end
 
   describe '#move_to_store' do
-    it 'is true' do
-      expect(subject.move_to_store).to eq(true)
+    it 'is false' do
+      expect(subject.move_to_store).to eq(false)
     end
   end
 end

spec/uploaders/file_uploader_spec.rb

@@ -17,7 +17,7 @@ describe FileUploader do
   describe '#image_or_video?' do
     context 'given an image file' do
       before do
-        @uploader.store!(File.new(Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')))
+        @uploader.store!(fixture_file_upload(Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')))
       end
 
       it 'detects an image based on file extension' do
@@ -27,7 +27,7 @@ describe FileUploader do
 
     context 'given an video file' do
       before do
-        video_file = File.new(Rails.root.join('spec', 'fixtures', 'video_sample.mp4'))
+        video_file = fixture_file_upload(Rails.root.join('spec', 'fixtures', 'video_sample.mp4'))
         @uploader.store!(video_file)
       end
 
@@ -37,7 +37,7 @@ describe FileUploader do
     end
 
     it 'does not return image_or_video? for other types' do
-      @uploader.store!(File.new(Rails.root.join('spec', 'fixtures', 'doc_sample.txt')))
+      @uploader.store!(fixture_file_upload(Rails.root.join('spec', 'fixtures', 'doc_sample.txt')))
 
       expect(@uploader.image_or_video?).to be false
     end