From ec273b8d069bf3f9a12a2ffde545398cd2967eeb Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 2 Jan 2017 12:35:15 +0100 Subject: [PATCH] Copy, don't move uploaded avatar files --- 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 ++++---- 4 files changed, 32 insertions(+), 4 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 -- GitLab