From cff3497f2c06dd4e1785af3c844040ed7939dcb0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 24 Mar 2016 10:01:30 +0100
Subject: [PATCH 01/12] Add markdown pattern for uploads to file uploader

---
 app/uploaders/file_uploader.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 86d24469e0..ac719f74f6 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -1,6 +1,7 @@
 # encoding: utf-8
 class FileUploader < CarrierWave::Uploader::Base
   include UploaderHelper
+  MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)}
 
   storage :file
 

From 701976e0815c273ff4a4c6e4d3489db0ce2f0860 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 24 Mar 2016 12:28:43 +0100
Subject: [PATCH 02/12] Add uploads rewriter and use it when moving issue

---
 app/services/issues/move_service.rb          | 39 +++++++++-----
 lib/gitlab/gfm/uploads_rewriter.rb           | 56 ++++++++++++++++++++
 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb | 49 +++++++++++++++++
 spec/services/issues/move_service_spec.rb    | 19 +++++++
 4 files changed, 151 insertions(+), 12 deletions(-)
 create mode 100644 lib/gitlab/gfm/uploads_rewriter.rb
 create mode 100644 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb

diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index a5efb21fab..e15e25ee53 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -43,7 +43,7 @@ module Issues
     def create_new_issue
       new_params = { id: nil, iid: nil, label_ids: [], milestone: nil,
                      project: @new_project, author: @old_issue.author,
-                     description: unfold_references(@old_issue.description) }
+                     description: rewrite_content(@old_issue.description) }
 
       new_params = @old_issue.serializable_hash.merge(new_params)
       CreateService.new(@new_project, @current_user, new_params).execute
@@ -53,7 +53,7 @@ module Issues
       @old_issue.notes.find_each do |note|
         new_note = note.dup
         new_params = { project: @new_project, noteable: @new_issue,
-                       note: unfold_references(new_note.note),
+                       note: rewrite_content(new_note.note),
                        created_at: note.created_at,
                        updated_at: note.updated_at }
 
@@ -61,6 +61,29 @@ module Issues
       end
     end
 
+    def rewrite_content(content)
+      rewrite_uploads(
+        unfold_references(content)
+      )
+    end
+
+    def unfold_references(content)
+      return unless content
+
+      rewriter = Gitlab::Gfm::ReferenceRewriter.new(content, @old_project,
+                                                    @current_user)
+      rewriter.rewrite(@new_project)
+    end
+
+    def rewrite_uploads(content)
+      return unless content
+
+      rewriter = Gitlab::Gfm::UploadsRewriter.new(content, @old_project,
+                                                  @current_user)
+      return content unless rewriter.has_uploads?
+      rewriter.rewrite(@new_project)
+    end
+
     def close_issue
       close_service = CloseService.new(@old_project, @current_user)
       close_service.execute(@old_issue, notifications: false, system_note: false)
@@ -78,20 +101,12 @@ module Issues
                                        direction: :to)
     end
 
-    def unfold_references(content)
-      return unless content
-
-      rewriter = Gitlab::Gfm::ReferenceRewriter.new(content, @old_project,
-                                                    @current_user)
-      rewriter.rewrite(@new_project)
+    def mark_as_moved
+      @old_issue.update(moved_to: @new_issue)
     end
 
     def notify_participants
       notification_service.issue_moved(@old_issue, @new_issue, @current_user)
     end
-
-    def mark_as_moved
-      @old_issue.update(moved_to: @new_issue)
-    end
   end
 end
diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb
new file mode 100644
index 0000000000..778b6fe9f9
--- /dev/null
+++ b/lib/gitlab/gfm/uploads_rewriter.rb
@@ -0,0 +1,56 @@
+module Gitlab
+  module Gfm
+    ##
+    # Class that rewrites markdown links for uploads
+    #
+    # Using a pattern defined in `FileUploader` copies files to a new project
+    # and rewrites all links to uploads in ain a given text.
+    #
+    class UploadsRewriter
+      def initialize(text, source_project, _current_user)
+        @text = text
+        @source_project = source_project
+        @pattern = FileUploader::MARKDOWN_PATTERN
+      end
+
+      def rewrite(target_project)
+        return unless @text
+
+        new_uploader = file_uploader(target_project)
+        @text.gsub(@pattern) do |markdown_link|
+          old_file = find_file(@source_project, $~[:secret], $~[:file])
+          return markdown_link unless old_file.exists?
+
+          new_uploader.store!(old_file)
+          new_uploader.to_h[:markdown]
+        end
+      end
+
+      def has_uploads?
+        !(@text =~ @pattern).nil?
+      end
+
+      def files
+        referenced_files = @text.scan(@pattern).map do
+          find_file(@source_project, $~[:secret], $~[:file])
+        end
+
+        referenced_files.compact.select(&:exists?)
+      end
+
+      private
+
+      def find_file(project, secret, file)
+        uploader = file_uploader(project, secret)
+        uploader.retrieve_from_store!(file)
+        uploader.file
+      end
+
+      def file_uploader(*args)
+        uploader = FileUploader.new(*args)
+        uploader.define_singleton_method(:move_to_store) { false }
+        uploader
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
new file mode 100644
index 0000000000..7027954ef2
--- /dev/null
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -0,0 +1,49 @@
+require 'spec_helper'
+
+describe Gitlab::Gfm::UploadsRewriter do
+  let(:user) { create(:user) }
+  let(:old_project) { create(:project) }
+  let(:new_project) { create(:project) }
+  let(:rewriter) { described_class.new(text, old_project, user) }
+
+  context 'text contains links to uploads' do
+    let(:path) { Rails.root + 'spec/fixtures/rails_sample.jpg' }
+    let(:file) { fixture_file_upload(path, 'image/jpg') }
+    let(:uploader) { FileUploader.new(old_project) }
+    let!(:store) { uploader.store!(file) } # TODO, see #xxx (carrierwave issue)
+    let(:markdown) { uploader.to_h[:markdown] }
+    let(:text) { "Text and #{markdown}"}
+
+    describe '#rewrite' do
+      let!(:new_text) { rewriter.rewrite(new_project) }
+      let(:new_rewriter) { described_class.new(new_text, new_project, user) }
+      let(:old_file) { rewriter.files.first }
+      let(:new_file) { new_rewriter.files.first }
+
+      it 'rewrites content' do
+        expect(new_text).to_not eq text
+        expect(new_text.length).to eq text.length
+      end
+
+      it 'copies files' do
+        expect(new_file.exists?).to eq true
+        expect(old_file.path).to_not eq new_file.path
+        expect(new_file.path).to include new_project.path_with_namespace
+      end
+
+      it 'does not remove old files' do
+        expect(old_file.exists?).to be true
+      end
+    end
+
+    describe '#has_uploads?' do
+      subject { rewriter.has_uploads? }
+      it { is_expected.to eq true }
+    end
+
+    describe '#files' do
+      subject { rewriter.files }
+      it { is_expected.to be_an(Array) }
+    end
+  end
+end
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 9b0c73aaf3..1cc2daa908 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -160,6 +160,25 @@ describe Issues::MoveService, services: true do
               .to eq "Note with reference to merge request #{old_project.to_reference}!1"
           end
         end
+
+        context 'issue description with uploads' do
+          let(:path) { Rails.root + 'spec/fixtures/rails_sample.jpg' }
+          let(:file) { fixture_file_upload(path, 'image/jpg') }
+          let(:uploader) { FileUploader.new(old_project) }
+          let!(:store) { uploader.store!(file) }
+          let(:markdown) { uploader.to_h[:markdown] }
+          let(:description) { "Text and #{markdown}"}
+
+          include_context 'issue move executed'
+
+          it 'rewrites uploads in description' do
+            expect(new_issue.description).to_not eq description
+            expect(new_issue.description)
+              .to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/)
+          end
+
+          after { uploader.remove! }
+        end
       end
 
       describe 'rewritting references' do

From 0b8cefd3b2385a21cfed779bd659978c0402766d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 29 Mar 2016 12:12:57 +0200
Subject: [PATCH 03/12] Get FileUploader into test harness using factory

This attempts to get CarrierWave's uploader - `FileUploader` into test
harness using a factory. that makes it easier to build an instance of
it.

Along with !3435 it may be easier to use uploaders in tests
---
 app/uploaders/file_uploader.rb               |  4 ++--
 spec/factories/file_uploader.rb              | 19 +++++++++++++++++++
 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb |  5 +----
 3 files changed, 22 insertions(+), 6 deletions(-)
 create mode 100644 spec/factories/file_uploader.rb

diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index ac719f74f6..25d879ff44 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -7,9 +7,9 @@ class FileUploader < CarrierWave::Uploader::Base
 
   attr_accessor :project, :secret
 
-  def initialize(project, secret = self.class.generate_secret)
+  def initialize(project, secret = nil)
     @project = project
-    @secret = secret
+    @secret = secret || self.class.generate_secret
   end
 
   def base_dir
diff --git a/spec/factories/file_uploader.rb b/spec/factories/file_uploader.rb
new file mode 100644
index 0000000000..69a4e6d28f
--- /dev/null
+++ b/spec/factories/file_uploader.rb
@@ -0,0 +1,19 @@
+FactoryGirl.define do
+  factory :file_uploader, class: FileUploader do
+    project
+    secret nil
+
+    transient do
+      path { File.join(Rails.root, 'spec/fixtures/rails_sample.jpg') }
+      file { Rack::Test::UploadedFile.new(path) }
+    end
+
+    after(:build) do |uploader, evaluator|
+      uploader.store!(evaluator.file)
+    end
+
+    initialize_with do
+      new(project, secret)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index 7027954ef2..770c08148c 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -7,10 +7,7 @@ describe Gitlab::Gfm::UploadsRewriter do
   let(:rewriter) { described_class.new(text, old_project, user) }
 
   context 'text contains links to uploads' do
-    let(:path) { Rails.root + 'spec/fixtures/rails_sample.jpg' }
-    let(:file) { fixture_file_upload(path, 'image/jpg') }
-    let(:uploader) { FileUploader.new(old_project) }
-    let!(:store) { uploader.store!(file) } # TODO, see #xxx (carrierwave issue)
+    let(:uploader) { build(:file_uploader, project: old_project) }
     let(:markdown) { uploader.to_h[:markdown] }
     let(:text) { "Text and #{markdown}"}
 

From f2674c7b98c69668093583e4590223b7040b5b33 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 29 Mar 2016 13:21:57 +0200
Subject: [PATCH 04/12] Refactor uploads rewriter used when moving issue

---
 app/services/issues/move_service.rb          | 23 +++++---------------
 lib/gitlab/gfm/reference_rewriter.rb         |  9 +++++---
 lib/gitlab/gfm/uploads_rewriter.rb           | 21 +++++++++---------
 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb |  4 ++--
 spec/services/issues/move_service_spec.rb    |  9 ++------
 5 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index e15e25ee53..096a364396 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -62,26 +62,15 @@ module Issues
     end
 
     def rewrite_content(content)
-      rewrite_uploads(
-        unfold_references(content)
-      )
-    end
-
-    def unfold_references(content)
       return unless content
 
-      rewriter = Gitlab::Gfm::ReferenceRewriter.new(content, @old_project,
-                                                    @current_user)
-      rewriter.rewrite(@new_project)
-    end
+      rewriters = [Gitlab::Gfm::ReferenceRewriter,
+                   Gitlab::Gfm::UploadsRewriter]
 
-    def rewrite_uploads(content)
-      return unless content
-
-      rewriter = Gitlab::Gfm::UploadsRewriter.new(content, @old_project,
-                                                  @current_user)
-      return content unless rewriter.has_uploads?
-      rewriter.rewrite(@new_project)
+      rewriters.inject(content) do |text, klass|
+        rewriter = klass.new(text, @old_project, @current_user)
+        rewriter.needs_rewrite? ? rewriter.rewrite(@new_project) : text
+      end
     end
 
     def close_issue
diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb
index a1c6ee7bd6..5f906d0717 100644
--- a/lib/gitlab/gfm/reference_rewriter.rb
+++ b/lib/gitlab/gfm/reference_rewriter.rb
@@ -34,16 +34,19 @@ module Gitlab
         @source_project = source_project
         @current_user = current_user
         @original_html = markdown(text)
+        @pattern = Gitlab::ReferenceExtractor.references_pattern
       end
 
       def rewrite(target_project)
-        pattern = Gitlab::ReferenceExtractor.references_pattern
-
-        @text.gsub(pattern) do |reference|
+        @text.gsub(@pattern) do |reference|
           unfold_reference(reference, Regexp.last_match, target_project)
         end
       end
 
+      def needs_rewrite?
+        !(@text =~ @pattern).nil?
+      end
+
       private
 
       def unfold_reference(reference, match, target_project)
diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb
index 778b6fe9f9..5818766c97 100644
--- a/lib/gitlab/gfm/uploads_rewriter.rb
+++ b/lib/gitlab/gfm/uploads_rewriter.rb
@@ -3,8 +3,9 @@ module Gitlab
     ##
     # Class that rewrites markdown links for uploads
     #
-    # Using a pattern defined in `FileUploader` copies files to a new project
-    # and rewrites all links to uploads in ain a given text.
+    # Using a pattern defined in `FileUploader` it copies files to a new
+    # project and rewrites all links to uploads in in a given text.
+    #
     #
     class UploadsRewriter
       def initialize(text, source_project, _current_user)
@@ -17,17 +18,17 @@ module Gitlab
         return unless @text
 
         new_uploader = file_uploader(target_project)
-        @text.gsub(@pattern) do |markdown_link|
-          old_file = find_file(@source_project, $~[:secret], $~[:file])
-          return markdown_link unless old_file.exists?
+        @text.gsub(@pattern) do |markdown|
+          file = find_file(@source_project, $~[:secret], $~[:file])
+          return markdown unless file.try(:exists?)
 
-          new_uploader.store!(old_file)
+          new_uploader.store!(file)
           new_uploader.to_h[:markdown]
         end
       end
 
-      def has_uploads?
-        !(@text =~ @pattern).nil?
+      def needs_rewrite?
+        files.any?
       end
 
       def files
@@ -46,8 +47,8 @@ module Gitlab
         uploader.file
       end
 
-      def file_uploader(*args)
-        uploader = FileUploader.new(*args)
+      def file_uploader(project, secret = nil)
+        uploader = FileUploader.new(project, secret)
         uploader.define_singleton_method(:move_to_store) { false }
         uploader
       end
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index 770c08148c..ec6c7d6bee 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -33,8 +33,8 @@ describe Gitlab::Gfm::UploadsRewriter do
       end
     end
 
-    describe '#has_uploads?' do
-      subject { rewriter.has_uploads? }
+    describe '#needs_rewrite?' do
+      subject { rewriter.needs_rewrite? }
       it { is_expected.to eq true }
     end
 
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 1cc2daa908..d4e7294968 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -162,12 +162,9 @@ describe Issues::MoveService, services: true do
         end
 
         context 'issue description with uploads' do
-          let(:path) { Rails.root + 'spec/fixtures/rails_sample.jpg' }
-          let(:file) { fixture_file_upload(path, 'image/jpg') }
-          let(:uploader) { FileUploader.new(old_project) }
-          let!(:store) { uploader.store!(file) }
+          let(:uploader) { build(:file_uploader, project: old_project) }
           let(:markdown) { uploader.to_h[:markdown] }
-          let(:description) { "Text and #{markdown}"}
+          let(:description) { "Text and #{markdown}" }
 
           include_context 'issue move executed'
 
@@ -176,8 +173,6 @@ describe Issues::MoveService, services: true do
             expect(new_issue.description)
               .to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/)
           end
-
-          after { uploader.remove! }
         end
       end
 

From d08de5ed0e894b4d201a7737db630667b9760a35 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 29 Mar 2016 13:39:27 +0200
Subject: [PATCH 05/12] Add support for not Active Record based factories

---
 spec/factories_spec.rb | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb
index 457859deda..62de081661 100644
--- a/spec/factories_spec.rb
+++ b/spec/factories_spec.rb
@@ -1,9 +1,17 @@
 require 'spec_helper'
 
-FactoryGirl.factories.map(&:name).each do |factory_name|
-  describe "#{factory_name} factory" do
-    it 'should be valid' do
-      expect(build(factory_name)).to be_valid
+describe 'factories' do
+  FactoryGirl.factories.each do |factory|
+    describe "#{factory.name} factory" do
+      let(:entity) { build(factory.name) }
+
+      it 'does not raise error when created 'do
+        expect { entity }.to_not raise_error
+      end
+
+      it 'should be valid', if: factory.build_class < ActiveRecord::Base do
+        expect(entity).to be_valid
+      end
     end
   end
 end

From e64b1e52a23016e51d581b87c08beaa4b18da689 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 30 Mar 2016 10:42:39 +0200
Subject: [PATCH 06/12] Check if GFM rewriters need rewrite internally

---
 app/services/issues/move_service.rb  | 2 +-
 lib/gitlab/gfm/reference_rewriter.rb | 2 ++
 lib/gitlab/gfm/uploads_rewriter.rb   | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index 096a364396..82e7090f1e 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -69,7 +69,7 @@ module Issues
 
       rewriters.inject(content) do |text, klass|
         rewriter = klass.new(text, @old_project, @current_user)
-        rewriter.needs_rewrite? ? rewriter.rewrite(@new_project) : text
+        rewriter.rewrite(@new_project)
       end
     end
 
diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb
index 5f906d0717..47e1aa6797 100644
--- a/lib/gitlab/gfm/reference_rewriter.rb
+++ b/lib/gitlab/gfm/reference_rewriter.rb
@@ -38,6 +38,8 @@ module Gitlab
       end
 
       def rewrite(target_project)
+        return @text unless needs_rewrite?
+
         @text.gsub(@pattern) do |reference|
           unfold_reference(reference, Regexp.last_match, target_project)
         end
diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb
index 5818766c97..bdf054a619 100644
--- a/lib/gitlab/gfm/uploads_rewriter.rb
+++ b/lib/gitlab/gfm/uploads_rewriter.rb
@@ -15,7 +15,7 @@ module Gitlab
       end
 
       def rewrite(target_project)
-        return unless @text
+        return @text unless needs_rewrite?
 
         new_uploader = file_uploader(target_project)
         @text.gsub(@pattern) do |markdown|

From 99ee822857cf3fdf0a2ac91c0d13ea68c79e8ba8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 30 Mar 2016 10:56:25 +0200
Subject: [PATCH 07/12] Add method that returns markdown in file uploader

---
 app/uploaders/file_uploader.rb       | 4 ++++
 lib/gitlab/gfm/reference_rewriter.rb | 2 +-
 lib/gitlab/gfm/uploads_rewriter.rb   | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 25d879ff44..730153a474 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -32,6 +32,10 @@ class FileUploader < CarrierWave::Uploader::Base
     File.join("/uploads", @secret, file.filename)
   end
 
+  def to_markdown
+    to_h[:markdown]
+  end
+
   def to_h
     filename = image? ? self.file.basename : self.file.filename
     escaped_filename = filename.gsub("]", "\\]")
diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb
index 47e1aa6797..78d7a4f27c 100644
--- a/lib/gitlab/gfm/reference_rewriter.rb
+++ b/lib/gitlab/gfm/reference_rewriter.rb
@@ -46,7 +46,7 @@ module Gitlab
       end
 
       def needs_rewrite?
-        !(@text =~ @pattern).nil?
+        @text =~ @pattern
       end
 
       private
diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb
index bdf054a619..2e61f799a0 100644
--- a/lib/gitlab/gfm/uploads_rewriter.rb
+++ b/lib/gitlab/gfm/uploads_rewriter.rb
@@ -23,7 +23,7 @@ module Gitlab
           return markdown unless file.try(:exists?)
 
           new_uploader.store!(file)
-          new_uploader.to_h[:markdown]
+          new_uploader.to_markdown
         end
       end
 

From b9f57192853d100c90b1d46491838a98d5ae4bae Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 30 Mar 2016 12:11:27 +0200
Subject: [PATCH 08/12] Remove reduntant `move_to_store` override

---
 app/uploaders/file_uploader.rb               |  8 ++++----
 lib/gitlab/gfm/uploads_rewriter.rb           | 10 ++--------
 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb |  8 ++++----
 3 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 730153a474..1af9e9b0ed 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -24,10 +24,6 @@ class FileUploader < CarrierWave::Uploader::Base
     File.join(base_dir, 'tmp', @project.path_with_namespace, @secret)
   end
 
-  def self.generate_secret
-    SecureRandom.hex
-  end
-
   def secure_url
     File.join("/uploads", @secret, file.filename)
   end
@@ -50,4 +46,8 @@ class FileUploader < CarrierWave::Uploader::Base
       markdown: markdown
     }
   end
+
+  def self.generate_secret
+    SecureRandom.hex
+  end
 end
diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb
index 2e61f799a0..abc8c8c55e 100644
--- a/lib/gitlab/gfm/uploads_rewriter.rb
+++ b/lib/gitlab/gfm/uploads_rewriter.rb
@@ -17,11 +17,11 @@ module Gitlab
       def rewrite(target_project)
         return @text unless needs_rewrite?
 
-        new_uploader = file_uploader(target_project)
         @text.gsub(@pattern) do |markdown|
           file = find_file(@source_project, $~[:secret], $~[:file])
           return markdown unless file.try(:exists?)
 
+          new_uploader = FileUploader.new(target_project)
           new_uploader.store!(file)
           new_uploader.to_markdown
         end
@@ -42,16 +42,10 @@ module Gitlab
       private
 
       def find_file(project, secret, file)
-        uploader = file_uploader(project, secret)
+        uploader = FileUploader.new(project, secret)
         uploader.retrieve_from_store!(file)
         uploader.file
       end
-
-      def file_uploader(project, secret = nil)
-        uploader = FileUploader.new(project, secret)
-        uploader.define_singleton_method(:move_to_store) { false }
-        uploader
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index ec6c7d6bee..0a3856b6de 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -8,13 +8,13 @@ describe Gitlab::Gfm::UploadsRewriter do
 
   context 'text contains links to uploads' do
     let(:uploader) { build(:file_uploader, project: old_project) }
-    let(:markdown) { uploader.to_h[:markdown] }
-    let(:text) { "Text and #{markdown}"}
+    let(:text) { "Text and #{uploader.to_markdown}"}
 
     describe '#rewrite' do
       let!(:new_text) { rewriter.rewrite(new_project) }
+
       let(:new_rewriter) { described_class.new(new_text, new_project, user) }
-      let(:old_file) { rewriter.files.first }
+      let(:old_file) { uploader.file }
       let(:new_file) { new_rewriter.files.first }
 
       it 'rewrites content' do
@@ -29,7 +29,7 @@ describe Gitlab::Gfm::UploadsRewriter do
       end
 
       it 'does not remove old files' do
-        expect(old_file.exists?).to be true
+        expect(old_file).to exist
       end
     end
 

From 57ea33bfd0b8d7591f6617ebb19e1b35498437ab Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 31 Mar 2016 09:43:47 +0200
Subject: [PATCH 09/12] Extend specs for GFM uploads rewriter

---
 spec/factories/file_uploader.rb              |  5 +--
 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb | 38 +++++++++++++++-----
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/spec/factories/file_uploader.rb b/spec/factories/file_uploader.rb
index 69a4e6d28f..1b36e21f2b 100644
--- a/spec/factories/file_uploader.rb
+++ b/spec/factories/file_uploader.rb
@@ -1,10 +1,11 @@
 FactoryGirl.define do
-  factory :file_uploader, class: FileUploader do
+  factory :file_uploader do
     project
     secret nil
 
     transient do
-      path { File.join(Rails.root, 'spec/fixtures/rails_sample.jpg') }
+      fixture { 'rails_sample.jpg' }
+      path { File.join(Rails.root, 'spec/fixtures', fixture) }
       file { Rack::Test::UploadedFile.new(path) }
     end
 
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index 0a3856b6de..f076e7b71f 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -7,15 +7,29 @@ describe Gitlab::Gfm::UploadsRewriter do
   let(:rewriter) { described_class.new(text, old_project, user) }
 
   context 'text contains links to uploads' do
-    let(:uploader) { build(:file_uploader, project: old_project) }
-    let(:text) { "Text and #{uploader.to_markdown}"}
+    let(:image_uploader) do
+      build(:file_uploader, project: old_project)
+    end
+
+    let(:zip_uploader) do
+      build(:file_uploader, project: old_project,
+            fixture: 'ci_build_artifacts.zip')
+    end
+
+    let(:text) do
+      "Text and #{image_uploader.to_markdown} and #{zip_uploader.to_markdown}"
+    end
 
     describe '#rewrite' do
       let!(:new_text) { rewriter.rewrite(new_project) }
 
-      let(:new_rewriter) { described_class.new(new_text, new_project, user) }
-      let(:old_file) { uploader.file }
-      let(:new_file) { new_rewriter.files.first }
+      let(:old_files) { [image_uploader, zip_uploader].map(&:file) }
+      let(:new_files) do
+        described_class.new(new_text, new_project, user).files
+      end
+
+      let(:old_paths) { old_files.map(&:path) }
+      let(:new_paths) { new_files.map(&:path) }
 
       it 'rewrites content' do
         expect(new_text).to_not eq text
@@ -23,13 +37,19 @@ describe Gitlab::Gfm::UploadsRewriter do
       end
 
       it 'copies files' do
-        expect(new_file.exists?).to eq true
-        expect(old_file.path).to_not eq new_file.path
-        expect(new_file.path).to include new_project.path_with_namespace
+        expect(new_files).to all(exist)
+        expect(old_paths).to_not match_array new_paths
+        expect(old_paths).to all(include(old_project.path_with_namespace))
+        expect(new_paths).to all(include(new_project.path_with_namespace))
       end
 
       it 'does not remove old files' do
-        expect(old_file).to exist
+        expect(old_files).to all(exist)
+      end
+
+      it 'generates a new secret for each file' do
+        expect(new_paths).to_not include image_uploader.secret
+        expect(new_paths).to_not include zip_uploader.secret
       end
     end
 

From 5ac61d7b241b2332513650e2287cfde09d9c1fb7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 31 Mar 2016 09:47:05 +0200
Subject: [PATCH 10/12] Improve specs for issue move service

---
 spec/services/issues/move_service_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index d4e7294968..2a5e4ac3ec 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -163,8 +163,7 @@ describe Issues::MoveService, services: true do
 
         context 'issue description with uploads' do
           let(:uploader) { build(:file_uploader, project: old_project) }
-          let(:markdown) { uploader.to_h[:markdown] }
-          let(:description) { "Text and #{markdown}" }
+          let(:description) { "Text and #{uploader.to_markdown}" }
 
           include_context 'issue move executed'
 
@@ -172,6 +171,7 @@ describe Issues::MoveService, services: true do
             expect(new_issue.description).to_not eq description
             expect(new_issue.description)
               .to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/)
+            expect(new_issue.description).to_not include uploader.secret
           end
         end
       end

From c3ba64921e819c980dce453c4cafd7c4a4bd466e Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 31 Mar 2016 10:07:54 +0200
Subject: [PATCH 11/12] Add Changelog entry for uploads fix when moving issue

---
 CHANGELOG | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGELOG b/CHANGELOG
index 25eeb24b49..972242c19f 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,9 @@ v 8.7.0 (unreleased)
   - Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.)
   - Gracefully handle notes on deleted commits in merge requests (Stan Hu)
 
+v 8.6.3
+  - Fix copying uploads when moving issue to another project
+
 v 8.6.2
   - Fix dropdown alignment. !3298
   - Fix issuable sidebar overlaps on tablet. !3299

From cf21fd7a95b9962f16367ad2bbb965112e397929 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 31 Mar 2016 10:41:57 +0200
Subject: [PATCH 12/12] Fix rubocop offenses in upload rewriter specs

---
 spec/lib/gitlab/gfm/uploads_rewriter_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index f076e7b71f..eda956e6f0 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -13,7 +13,7 @@ describe Gitlab::Gfm::UploadsRewriter do
 
     let(:zip_uploader) do
       build(:file_uploader, project: old_project,
-            fixture: 'ci_build_artifacts.zip')
+                            fixture: 'ci_build_artifacts.zip')
     end
 
     let(:text) do