From 30522bf491fde73b42fdb1958b80477cef492b05 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@mcgivern.me.uk>
Date: Tue, 3 Jan 2017 15:45:49 +0000
Subject: [PATCH] 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 | 11 +++++++++++
changelogs/unreleased/project-avatar-fork.yml | 4 ++++
spec/services/projects/fork_service_spec.rb | 13 +++++++++++++
spec/uploaders/avatar_uploader_spec.rb | 8 ++++----
spec/uploaders/file_uploader_spec.rb | 6 +++---
5 files changed, 35 insertions(+), 7 deletions(-)
create mode 100644 changelogs/unreleased/project-avatar-fork.yml
diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb
index a1ecb7bc00b..265cea2d2c6 100644
--- a/app/uploaders/avatar_uploader.rb
+++ b/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
diff --git a/changelogs/unreleased/project-avatar-fork.yml b/changelogs/unreleased/project-avatar-fork.yml
new file mode 100644
index 00000000000..6facffe7887
--- /dev/null
+++ b/changelogs/unreleased/project-avatar-fork.yml
@@ -0,0 +1,4 @@
+---
+title: Copy, don't move uploaded avatar files
+merge_request: 8396
+author:
diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb
index 64d15c0523c..8e614211116 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/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
diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb
index 1f0e8732587..76f5a4b42ed 100644
--- a/spec/uploaders/avatar_uploader_spec.rb
+++ b/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
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index bc86adfef68..6a712e33c96 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/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
--
2.30.9