From 78cd19631a5e02d30c22944a201d7074a3653ccf Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 10 Sep 2018 17:41:51 +0200 Subject: [PATCH 1/4] fix avatar uploader error --- app/uploaders/avatar_uploader.rb | 2 +- spec/uploaders/avatar_uploader_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index 8526bc16390..c0165759203 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -19,7 +19,7 @@ class AvatarUploader < GitlabUploader end def absolute_path - self.class.absolute_path(model.avatar) + self.class.absolute_path(model.avatar.upload) end private diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb index b0468bc35ff..6aaec7a4fef 100644 --- a/spec/uploaders/avatar_uploader_spec.rb +++ b/spec/uploaders/avatar_uploader_spec.rb @@ -35,5 +35,13 @@ describe AvatarUploader do it_behaves_like "migrates", to_store: described_class::Store::REMOTE it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL + + it 'sets the right absolute path' do + storage_path = Gitlab.config.uploads.storage_path + absolute_path = File.join(storage_path, upload.path) + + expect(uploader.absolute_path.scan(storage_path).size).to eq(1) + expect(uploader.absolute_path).to eq(absolute_path) + end end end From 6e394c44d53fd990422195ffb9f6e70d3d164d48 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 10 Sep 2018 17:42:13 +0200 Subject: [PATCH 2/4] add changelog --- .../51318-project-export-broken-when-avatar-is-set.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml diff --git a/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml b/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml new file mode 100644 index 00000000000..c0f7e88f2b7 --- /dev/null +++ b/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken exports when they include a projet avatar +merge_request: 21649 +author: +type: fixed From 994d91883b0c68524668eb2c475fd5317fca5f9e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 11 Sep 2018 08:59:14 +0200 Subject: [PATCH 3/4] fix avatar restorer --- lib/gitlab/import_export/avatar_restorer.rb | 2 +- .../import_export/avatar_restorer_spec.rb | 33 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/gitlab/import_export/avatar_restorer.rb b/lib/gitlab/import_export/avatar_restorer.rb index ded05f73cf8..264a9a41ff0 100644 --- a/lib/gitlab/import_export/avatar_restorer.rb +++ b/lib/gitlab/import_export/avatar_restorer.rb @@ -19,7 +19,7 @@ module Gitlab private def avatar_export_file - @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].first + @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].reject { |f| File.directory?(f) }.first end def avatar_export_path diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index 4897d604bc1..e44ff6bbcbd 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -6,22 +6,35 @@ describe Gitlab::ImportExport::AvatarRestorer do let(:shared) { project.import_export_shared } let(:project) { create(:project) } - before do - allow_any_instance_of(described_class).to receive(:avatar_export_file) - .and_return(uploaded_image_temp_path) - end - after do project.remove_avatar! end - it 'restores a project avatar' do - expect(described_class.new(project: project, shared: shared).restore).to be true + context 'with avatar' do + before do + allow_any_instance_of(described_class).to receive(:avatar_export_file) + .and_return(uploaded_image_temp_path) + end + + it 'restores a project avatar' do + expect(described_class.new(project: project, shared: shared).restore).to be true + end + + it 'saves the avatar into the project' do + described_class.new(project: project, shared: shared).restore + + expect(project.reload.avatar.file.exists?).to be true + end end - it 'saves the avatar into the project' do - described_class.new(project: project, shared: shared).restore + it 'does not break if there is just a directory' do + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p("#{tmpdir}/a/b") - expect(project.reload.avatar.file.exists?).to be true + allow_any_instance_of(described_class).to receive(:avatar_export_path) + .and_return("#{tmpdir}/a") + + expect(described_class.new(project: project, shared: shared).restore).to be true + end end end From 678ceb257ef829377c197ef368ea8e1b7fce9c4e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 11 Sep 2018 10:22:34 +0200 Subject: [PATCH 4/4] use find instead of reject in avatar export file --- lib/gitlab/import_export/avatar_restorer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/import_export/avatar_restorer.rb b/lib/gitlab/import_export/avatar_restorer.rb index 264a9a41ff0..17796430811 100644 --- a/lib/gitlab/import_export/avatar_restorer.rb +++ b/lib/gitlab/import_export/avatar_restorer.rb @@ -19,7 +19,7 @@ module Gitlab private def avatar_export_file - @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].reject { |f| File.directory?(f) }.first + @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].find { |f| File.file?(f) } end def avatar_export_path