From b53d9f8a0ec6c044548fa21a04ba227a0c46d43a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 15 Feb 2016 14:41:40 +0100
Subject: Add scaffold of service that moves issue to another project

---
 spec/services/issues/move_service_spec.rb | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 spec/services/issues/move_service_spec.rb

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
new file mode 100644
index 00000000000..27081e76d48
--- /dev/null
+++ b/spec/services/issues/move_service_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe Issues::MoveService, services: true do
+  let(:user) { create(:user) }
+  let(:issue) { create(:issue, title: 'Some issue', description: 'Some issue description') }
+  let(:current_project) { issue.project }
+  let(:new_project) { create(:project) }
+
+  before do
+    current_project.team << [user, :master]
+  end
+
+  describe '#execute' do
+    let!(:new_issue) do
+      described_class.new(current_project, user).execute(issue, new_project)
+    end
+
+    it 'should create a new issue in a new project' do
+      expect(new_issue.project).to eq new_project
+    end
+  end
+end
-- 
cgit v1.2.3


From 69b89d4ec8f9788dc11f30cdf1f782d316ab252c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 15 Feb 2016 15:14:57 +0100
Subject: Add new system note used when issue has been moved

---
 spec/services/issues/move_service_spec.rb |  4 ++++
 spec/services/system_note_service_spec.rb | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 27081e76d48..25dacb2068b 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -18,5 +18,9 @@ describe Issues::MoveService, services: true do
     it 'should create a new issue in a new project' do
       expect(new_issue.project).to eq new_project
     end
+
+    it 'should add system note to old issue' do
+      expect(issue.notes.last.note).to match /This issue has been moved to/
+    end
   end
 end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 5dcc39f5fdc..9074d3dbe19 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -441,6 +441,26 @@ describe SystemNoteService, services: true do
     end
   end
 
+  describe '.issue_moved_to_another_project' do
+    subject do
+      described_class.issue_moved_to_another_project(noteable, project, new_project, author)
+    end
+
+    let(:new_project) { create(:project) }
+
+    it 'should notify about issue being moved' do
+      expect(subject.note).to match /This issue has been moved to/
+    end
+
+    it 'should mention destination project' do
+      expect(subject.note).to include new_project.to_reference
+    end
+
+    it 'should mention author of that change' do
+      expect(subject.note).to include author.to_reference
+    end
+  end
+
   include JiraServiceHelper
 
   describe 'JIRA integration' do
-- 
cgit v1.2.3


From 9882802a8b70e998ff6850a3d096b3b52730ab85 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 16 Feb 2016 11:47:00 +0100
Subject: Improve system notes that are added when issue is moved

---
 spec/services/issues/move_service_spec.rb |  6 +++-
 spec/services/system_note_service_spec.rb | 51 +++++++++++++++++++++++++------
 2 files changed, 47 insertions(+), 10 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 25dacb2068b..569e155e617 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -20,7 +20,11 @@ describe Issues::MoveService, services: true do
     end
 
     it 'should add system note to old issue' do
-      expect(issue.notes.last.note).to match /This issue has been moved to/
+      expect(issue.notes.last.note).to match /^Moved to/
+    end
+
+    it 'should add system note to new issue' do
+      expect(new_issue.notes.last.note).to match /^Moved from/
     end
   end
 end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 9074d3dbe19..e0ae49ab37c 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -441,23 +441,56 @@ describe SystemNoteService, services: true do
     end
   end
 
-  describe '.issue_moved_to_another_project' do
+  describe '.noteable_moved' do
+    let(:new_project) { create(:project) }
+    let(:new_noteable) { create(:issue, project: new_project) }
+
     subject do
-      described_class.issue_moved_to_another_project(noteable, project, new_project, author)
+      described_class.noteable_moved(direction, noteable, project, new_noteable, author)
     end
 
-    let(:new_project) { create(:project) }
+    shared_examples 'cross project mentionable' do
+      include GitlabMarkdownHelper
+
+      it 'should contain cross reference to new noteable' do
+        expect(subject.note).to include cross_project_reference(new_project, new_noteable)
+      end
+
+      it 'should mention referenced noteable' do
+        expect(subject.note).to include new_noteable.to_reference
+      end
+
+      it 'should mention referenced project' do
+        expect(subject.note).to include new_project.to_reference
+      end
+    end
+
+    context 'moved to' do
+      let(:direction) { :to }
+
+      it_behaves_like 'cross project mentionable'
 
-    it 'should notify about issue being moved' do
-      expect(subject.note).to match /This issue has been moved to/
+      it 'should notify about noteable being moved to' do
+        expect(subject.note).to match /Moved to/
+      end
     end
 
-    it 'should mention destination project' do
-      expect(subject.note).to include new_project.to_reference
+    context 'moved from' do
+      let(:direction) { :from }
+
+      it_behaves_like 'cross project mentionable'
+
+      it 'should notify about noteable being moved from' do
+        expect(subject.note).to match /Moved from/
+      end
     end
 
-    it 'should mention author of that change' do
-      expect(subject.note).to include author.to_reference
+    context 'invalid direction' do
+      let (:direction) { :invalid }
+
+      it 'should raise error' do
+        expect { subject }.to raise_error StandardError, /Invalid direction/
+      end
     end
   end
 
-- 
cgit v1.2.3


From c8e7d1ed8e3eafcc8234a0e6f443bf85369c8de1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 17 Feb 2016 15:59:25 +0100
Subject: Add issue move implementation to controller

---
 spec/services/issues/move_service_spec.rb | 38 +++++++++++++++++++++++--------
 spec/services/system_note_service_spec.rb |  2 +-
 2 files changed, 29 insertions(+), 11 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 569e155e617..412a817208e 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -5,26 +5,44 @@ describe Issues::MoveService, services: true do
   let(:issue) { create(:issue, title: 'Some issue', description: 'Some issue description') }
   let(:current_project) { issue.project }
   let(:new_project) { create(:project) }
+  let(:move_params) { { 'move_to_project_id' => new_project.id } }
+  let(:move_service) { described_class.new(current_project, user, move_params, issue) }
 
   before do
     current_project.team << [user, :master]
   end
 
-  describe '#execute' do
-    let!(:new_issue) do
-      described_class.new(current_project, user).execute(issue, new_project)
+  context 'issue movable' do
+    describe '#move?' do
+      subject { move_service.move? }
+      it { is_expected.to be_truthy }
     end
 
-    it 'should create a new issue in a new project' do
-      expect(new_issue.project).to eq new_project
-    end
+    describe '#execute' do
+      let!(:new_issue) { move_service.execute }
+
+      it 'should create a new issue in a new project' do
+        expect(new_issue.project).to eq new_project
+      end
+
+      it 'should add system note to old issue' do
+        expect(issue.notes.last.note).to match /^Moved to/
+      end
 
-    it 'should add system note to old issue' do
-      expect(issue.notes.last.note).to match /^Moved to/
+      it 'should add system note to new issue' do
+        expect(new_issue.notes.last.note).to match /^Moved from/
+      end
     end
+  end
+
+  context 'issue not movable' do
+    context 'move not requested' do
+      let(:move_params) { {} }
 
-    it 'should add system note to new issue' do
-      expect(new_issue.notes.last.note).to match /^Moved from/
+      describe '#move?' do
+        subject { move_service.move? }
+        it { is_expected.to be_falsey }
+      end
     end
   end
 end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index e0ae49ab37c..7efb9f2ddba 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -486,7 +486,7 @@ describe SystemNoteService, services: true do
     end
 
     context 'invalid direction' do
-      let (:direction) { :invalid }
+      let(:direction) { :invalid }
 
       it 'should raise error' do
         expect { subject }.to raise_error StandardError, /Invalid direction/
-- 
cgit v1.2.3


From 14c983fb696b7b75065df2b5defd458e563444e3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 22 Feb 2016 11:00:40 +0100
Subject: Add implementation that rewrites issue notes when moving

---
 spec/services/issues/move_service_spec.rb | 67 ++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 14 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 412a817208e..6667840ec6e 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -2,15 +2,15 @@ require 'spec_helper'
 
 describe Issues::MoveService, services: true do
   let(:user) { create(:user) }
-  let(:issue) { create(:issue, title: 'Some issue', description: 'Some issue description') }
-  let(:current_project) { issue.project }
+  let(:title) { 'Some issue' }
+  let(:description) { 'Some issue description' }
+  let(:old_issue) { create(:issue, title: title, description: description) }
+  let(:current_project) { old_issue.project }
   let(:new_project) { create(:project) }
   let(:move_params) { { 'move_to_project_id' => new_project.id } }
-  let(:move_service) { described_class.new(current_project, user, move_params, issue) }
+  let(:move_service) { described_class.new(current_project, user, move_params, old_issue) }
 
-  before do
-    current_project.team << [user, :master]
-  end
+  before { current_project.team << [user, :master] }
 
   context 'issue movable' do
     describe '#move?' do
@@ -19,18 +19,57 @@ describe Issues::MoveService, services: true do
     end
 
     describe '#execute' do
-      let!(:new_issue) { move_service.execute }
-
-      it 'should create a new issue in a new project' do
-        expect(new_issue.project).to eq new_project
+      shared_context 'issue move executed' do
+        let!(:new_issue) { move_service.execute }
       end
 
-      it 'should add system note to old issue' do
-        expect(issue.notes.last.note).to match /^Moved to/
+      context 'generic issue' do
+        include_context 'issue move executed'
+
+        it 'creates a new issue in a new project' do
+          expect(new_issue.project).to eq new_project
+        end
+
+        it 'rewrites issue title' do
+          expect(new_issue.title).to eq title
+        end
+
+        it 'rewrites issue description' do
+          expect(new_issue.description).to include description
+        end
+
+        it 'adds system note to old issue at the end' do
+          expect(old_issue.notes.last.note).to match /^Moved to/
+        end
+
+        it 'adds system note to new issue at the end' do
+          expect(new_issue.notes.last.note).to match /^Moved from/
+        end
       end
 
-      it 'should add system note to new issue' do
-        expect(new_issue.notes.last.note).to match /^Moved from/
+      context 'notes exist' do
+        let(:note_contents) do
+          ['Some system note 1', 'Some comment', 'Some system note 2']
+        end
+
+        before do
+          note_params = { noteable: old_issue, project: current_project, author: user}
+          create(:system_note, note_params.merge(note: note_contents.first))
+          create(:note, note_params.merge(note: note_contents.second))
+          create(:system_note, note_params.merge(note: note_contents.third))
+        end
+
+        include_context 'issue move executed'
+
+        let(:new_notes) { new_issue.notes.order('id ASC').pluck(:note) }
+
+        it 'rewrites existing system notes in valid order' do
+          expect(new_notes.first(3)).to eq note_contents
+        end
+
+        it 'adds a system note about move after rewritten notes' do
+          expect(new_notes.last).to match /^Moved from/
+        end
       end
     end
   end
-- 
cgit v1.2.3


From 8d1b7f950708a0b759026816fcadb6324b29a208 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 22 Feb 2016 12:41:33 +0100
Subject: Add issue move ability and use it in move service

---
 spec/services/issues/move_service_spec.rb | 70 ++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 10 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 6667840ec6e..d8357f89172 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -5,14 +5,25 @@ describe Issues::MoveService, services: true do
   let(:title) { 'Some issue' }
   let(:description) { 'Some issue description' }
   let(:old_issue) { create(:issue, title: title, description: description) }
-  let(:current_project) { old_issue.project }
+  let(:old_project) { old_issue.project }
   let(:new_project) { create(:project) }
-  let(:move_params) { { 'move_to_project_id' => new_project.id } }
-  let(:move_service) { described_class.new(current_project, user, move_params, old_issue) }
+  let(:move_service) { described_class.new(old_project, user, move_params, old_issue) }
 
-  before { current_project.team << [user, :master] }
+  shared_context 'issue move requested' do
+    let(:move_params) { { 'move_to_project_id' => new_project.id } }
+  end
 
+  shared_context 'user can move issue' do
+    before do
+      old_project.team << [user, :master]
+      new_project.team << [user, :master]
+    end
+  end
+    
   context 'issue movable' do
+    include_context 'issue move requested'
+    include_context 'user can move issue'
+
     describe '#move?' do
       subject { move_service.move? }
       it { is_expected.to be_truthy }
@@ -53,7 +64,7 @@ describe Issues::MoveService, services: true do
         end
 
         before do
-          note_params = { noteable: old_issue, project: current_project, author: user}
+          note_params = { noteable: old_issue, project: old_project, author: user}
           create(:system_note, note_params.merge(note: note_contents.first))
           create(:note, note_params.merge(note: note_contents.second))
           create(:system_note, note_params.merge(note: note_contents.third))
@@ -74,12 +85,51 @@ describe Issues::MoveService, services: true do
     end
   end
 
-  context 'issue not movable' do
-    context 'move not requested' do
-      let(:move_params) { {} }
+  context 'issue move not requested' do
+    let(:move_params) { {} }
+
+    describe '#move?' do
+      subject { move_service.move? }
+
+      context 'user do not have permissions to move issue' do
+        it { is_expected.to be_falsey }
+      end
+
+      context 'user has permissions to move issue' do
+        include_context 'user can move issue'
+        it { is_expected.to be_falsey }
+      end
+    end
+  end
+
+
+  describe 'move permissions' do
+    include_context 'issue move requested'
+
+    describe '#move?' do
+      subject { move_service.move? }
+
+      context 'user is master in both projects' do
+        include_context 'user can move issue'
+        it { is_expected.to be_truthy }
+      end
+
+      context 'user is master only in new project' do
+        before { new_project.team << [user, :master] }
+        it { is_expected.to be_falsey }
+      end
+
+      context 'user is master only in old project' do
+        before { old_project.team << [user, :master] }
+        it { is_expected.to be_falsey }
+      end
+
+      context 'user is master in one project and developer in another' do
+        before do
+          new_project.team << [user, :developer]
+          old_project.team << [user, :master]
+        end
 
-      describe '#move?' do
-        subject { move_service.move? }
         it { is_expected.to be_falsey }
       end
     end
-- 
cgit v1.2.3


From 8a02449440b0491a056106d693d502252121740f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 22 Feb 2016 13:54:21 +0100
Subject: Do not show issue move form unless user can move

---
 spec/services/issues/move_service_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index d8357f89172..6fda8bc6716 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -64,7 +64,7 @@ describe Issues::MoveService, services: true do
         end
 
         before do
-          note_params = { noteable: old_issue, project: old_project, author: user}
+          note_params = { noteable: old_issue, project: old_project, author: user }
           create(:system_note, note_params.merge(note: note_contents.first))
           create(:note, note_params.merge(note: note_contents.second))
           create(:system_note, note_params.merge(note: note_contents.third))
-- 
cgit v1.2.3


From 5ed8a1e107b9b6f91257e5afdd370f0552078786 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 22 Feb 2016 15:18:39 +0100
Subject: Silently close old issue when it has been moved

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

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 6fda8bc6716..3b81cc90c61 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -56,6 +56,15 @@ describe Issues::MoveService, services: true do
         it 'adds system note to new issue at the end' do
           expect(new_issue.notes.last.note).to match /^Moved from/
         end
+
+        it 'closes old issue' do
+          expect(old_issue.closed?).to be true
+        end
+
+        it 'persists changes to old and new issue' do
+          expect(new_issue.changed?).to be false
+          expect(old_issue.changed?).to be false
+        end
       end
 
       context 'notes exist' do
-- 
cgit v1.2.3


From 4cbe87d50ecfad9b97ba76f05935124676c96052 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 23 Feb 2016 14:18:54 +0100
Subject: Rewrite references in issue description when moving it

---
 spec/services/issues/move_service_spec.rb | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 3b81cc90c61..8d9cc09ffc7 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -4,8 +4,8 @@ describe Issues::MoveService, services: true do
   let(:user) { create(:user) }
   let(:title) { 'Some issue' }
   let(:description) { 'Some issue description' }
-  let(:old_issue) { create(:issue, title: title, description: description) }
-  let(:old_project) { old_issue.project }
+  let(:old_project) { create(:project) }
+  let(:old_issue) { create(:issue, title: title, description: description, project: old_project) }
   let(:new_project) { create(:project) }
   let(:move_service) { described_class.new(old_project, user, move_params, old_issue) }
 
@@ -61,13 +61,12 @@ describe Issues::MoveService, services: true do
           expect(old_issue.closed?).to be true
         end
 
-        it 'persists changes to old and new issue' do
-          expect(new_issue.changed?).to be false
-          expect(old_issue.changed?).to be false
+        it 'persists new issue' do
+          expect(new_issue.persisted?).to be true
         end
       end
 
-      context 'notes exist' do
+      context 'issue with notes' do
         let(:note_contents) do
           ['Some system note 1', 'Some comment', 'Some system note 2']
         end
@@ -91,6 +90,20 @@ describe Issues::MoveService, services: true do
           expect(new_notes.last).to match /^Moved from/
         end
       end
+
+      describe 'rewritting references' do
+        include_context 'issue move executed'
+
+        context 'issue reference' do
+          let(:another_issue) { create(:issue, project: old_project) }
+          let(:description) { "Some description #{another_issue.to_reference}" }
+
+          it 'rewrites referenced issues creating cross project reference' do
+            expect(new_issue.description)
+              .to eq "Some description #{old_project.to_reference}#{another_issue.to_reference}"
+          end
+        end
+      end
     end
   end
 
-- 
cgit v1.2.3


From 57eb39548879109dff3813129fca7acbcca23f71 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Feb 2016 11:52:02 +0100
Subject: Do not pass unsanitized params to issue move service

---
 spec/services/issues/move_service_spec.rb | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 8d9cc09ffc7..931ba06f6a1 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -7,10 +7,14 @@ describe Issues::MoveService, services: true do
   let(:old_project) { create(:project) }
   let(:old_issue) { create(:issue, title: title, description: description, project: old_project) }
   let(:new_project) { create(:project) }
-  let(:move_service) { described_class.new(old_project, user, move_params, old_issue) }
+  let(:issue_params) { old_issue.serializable_hash }
+
+  let(:move_service) do
+    described_class.new(old_project, user, issue_params, old_issue, new_project_id)
+  end
 
   shared_context 'issue move requested' do
-    let(:move_params) { { 'move_to_project_id' => new_project.id } }
+    let(:new_project_id) { new_project.id }
   end
 
   shared_context 'user can move issue' do
@@ -108,7 +112,7 @@ describe Issues::MoveService, services: true do
   end
 
   context 'issue move not requested' do
-    let(:move_params) { {} }
+    let(:new_project_id) { nil }
 
     describe '#move?' do
       subject { move_service.move? }
-- 
cgit v1.2.3


From fbb5a8b089d00fb66d8915b0f546dd07e76877e4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Feb 2016 12:18:46 +0100
Subject: Use a new issue create service when moving an issue

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

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 931ba06f6a1..ec913cdde2a 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -68,6 +68,11 @@ describe Issues::MoveService, services: true do
         it 'persists new issue' do
           expect(new_issue.persisted?).to be true
         end
+
+        it 'persist all changes' do
+          expect(old_issue.changed?).to be false
+          expect(new_issue.changed?).to be false
+        end
       end
 
       context 'issue with notes' do
-- 
cgit v1.2.3


From 8d3f072ec4f01017a4ec2e1d2082bafcf2b58188 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Feb 2016 13:20:08 +0100
Subject: Take care about data being rewritten when moving issue

---
 spec/services/issues/move_service_spec.rb | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index ec913cdde2a..282dfd9187d 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -2,13 +2,18 @@ require 'spec_helper'
 
 describe Issues::MoveService, services: true do
   let(:user) { create(:user) }
+  let(:author) { create(:user) }
   let(:title) { 'Some issue' }
   let(:description) { 'Some issue description' }
   let(:old_project) { create(:project) }
-  let(:old_issue) { create(:issue, title: title, description: description, project: old_project) }
   let(:new_project) { create(:project) }
   let(:issue_params) { old_issue.serializable_hash }
 
+  let(:old_issue) do
+    create(:issue, title: title, description: description,
+           project: old_project, author: author)
+  end
+
   let(:move_service) do
     described_class.new(old_project, user, issue_params, old_issue, new_project_id)
   end
@@ -73,6 +78,19 @@ describe Issues::MoveService, services: true do
           expect(old_issue.changed?).to be false
           expect(new_issue.changed?).to be false
         end
+
+        it 'changes author' do
+          expect(new_issue.author).to eq user
+        end
+
+        it 'removes data that is invalid in new context' do
+          expect(new_issue.milestone).to be_nil
+          expect(new_issue.labels).to be_empty
+        end
+
+        it 'creates a new internal id for issue' do
+          expect(new_issue.iid).to be 1
+        end
       end
 
       context 'issue with notes' do
-- 
cgit v1.2.3


From c2d4060f26d06d1ce5450345a8be2bbf9186745f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Feb 2016 15:38:52 +0100
Subject: Rewrite references in notes when moving issue

---
 spec/services/issues/move_service_spec.rb | 57 +++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 15 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 282dfd9187d..596da74b572 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -94,27 +94,54 @@ describe Issues::MoveService, services: true do
       end
 
       context 'issue with notes' do
-        let(:note_contents) do
-          ['Some system note 1', 'Some comment', 'Some system note 2']
-        end
+        context 'notes without references' do
+          let(:notes_params) do
+            [{ system: false, note: 'Some comment 1' },
+             { system: true, note: 'Some system note' },
+             { system: false, note: 'Some comment 2' }]
+          end
 
-        before do
-          note_params = { noteable: old_issue, project: old_project, author: user }
-          create(:system_note, note_params.merge(note: note_contents.first))
-          create(:note, note_params.merge(note: note_contents.second))
-          create(:system_note, note_params.merge(note: note_contents.third))
-        end
+          before do
+            note_params = { noteable: old_issue, project: old_project, author: author }
+            notes_params.each do |note|
+              create(:note, note_params.merge(note))
+            end
+          end
 
-        include_context 'issue move executed'
+          include_context 'issue move executed'
 
-        let(:new_notes) { new_issue.notes.order('id ASC').pluck(:note) }
+          let(:all_notes) { new_issue.notes.order('id ASC') }
+          let(:system_notes) { all_notes.system }
+          let(:user_notes) { all_notes.user }
+
+          it 'rewrites existing notes in valid order' do
+            expect(all_notes.pluck(:note).first(3))
+              .to eq notes_params.map { |n| n[:note] }
+          end
 
-        it 'rewrites existing system notes in valid order' do
-          expect(new_notes.first(3)).to eq note_contents
+          it 'adds a system note about move after rewritten notes' do
+            expect(system_notes.last.note).to match /^Moved from/
+          end
+
+          it 'preserves orignal author of comment' do
+            expect(user_notes.pluck(:author_id)).to all(eq(author.id))
+          end
         end
 
-        it 'adds a system note about move after rewritten notes' do
-          expect(new_notes.last).to match /^Moved from/
+        context 'notes with references' do
+          before do
+            create(:merge_request, source_project: old_project)
+            create(:note, noteable: old_issue, project: old_project, author: author,
+                   note: 'Note with reference to merge request !1')
+          end
+
+          include_context 'issue move executed'
+          let(:new_note) { new_issue.notes.first }
+
+          it 'rewrites references using a cross reference to old project' do
+            expect(new_note.note)
+              .to eq "Note with reference to merge request #{old_project.to_reference}!1"
+          end
         end
       end
 
-- 
cgit v1.2.3


From efd8251751c85285e6519fd3dd756f74579da15b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Feb 2016 16:27:06 +0100
Subject: Minor refactoring of issue move service and specs

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

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 596da74b572..cd051175612 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -101,6 +101,8 @@ describe Issues::MoveService, services: true do
              { system: false, note: 'Some comment 2' }]
           end
 
+          let(:notes_contents) { notes_params.map { |n| n[:note] } }
+
           before do
             note_params = { noteable: old_issue, project: old_project, author: author }
             notes_params.each do |note|
@@ -115,8 +117,7 @@ describe Issues::MoveService, services: true do
           let(:user_notes) { all_notes.user }
 
           it 'rewrites existing notes in valid order' do
-            expect(all_notes.pluck(:note).first(3))
-              .to eq notes_params.map { |n| n[:note] }
+            expect(all_notes.pluck(:note).first(3)).to eq notes_contents
           end
 
           it 'adds a system note about move after rewritten notes' do
-- 
cgit v1.2.3


From 3c493c24c70f7c8dc8e1f3bcf29e18d1ef0944a7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 26 Feb 2016 12:04:29 +0100
Subject: Add reference unfold pipeline used when moving issue

---
 .../pipeline/reference_unfold_pipeline_spec.rb     | 59 ++++++++++++++++++++++
 spec/services/issues/move_service_spec.rb          |  4 +-
 2 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb

(limited to 'spec')

diff --git a/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb b/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
new file mode 100644
index 00000000000..af9d6f4ad0d
--- /dev/null
+++ b/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
@@ -0,0 +1,59 @@
+require 'spec_helper'
+
+describe Banzai::Pipeline::ReferenceUnfoldPipeline do
+  let(:text) { 'some text' }
+  let(:project) { create(:project) }
+  let(:objects) { [] }
+
+  let(:result) do
+    described_class.to_html(text, project: project, objects: objects)
+  end
+
+  context 'invalid initializers' do
+    subject { -> { result } }
+
+    context 'project context is invalid' do
+      let(:project) { nil }
+      it { is_expected.to raise_error StandardError, /No valid project/ }
+    end
+
+    context 'objects context is invalid' do
+      let(:objects) { ['issue'] }
+      it { is_expected.to raise_error StandardError, /No `to_reference` method/ }
+    end
+  end
+
+  context 'multiple issues and merge requests referenced' do
+    subject { result }
+
+    let(:main_project) { create(:project) }
+
+    let(:issue_first) { create(:issue, project: main_project) }
+    let(:issue_second) { create(:issue, project: main_project) }
+    let(:merge_request) { create(:merge_request, source_project: main_project) }
+
+    let(:objects) { [issue_first, issue_second, merge_request] }
+
+    context 'plain text description' do
+      let(:text) { 'Description that references #1, #2 and !1' }
+
+      it { is_expected.to include issue_first.to_reference(project) }
+      it { is_expected.to include issue_second.to_reference(project) }
+      it { is_expected.to include merge_request.to_reference(project) }
+    end
+
+    context 'description with ignored elements' do
+      let(:text) do
+        <<-EOF
+          Hi. This references #1, but not `#2`
+          <pre>and not !1</pre>
+        EOF
+      end
+
+
+      it { is_expected.to include issue_first.to_reference(project) }
+      it { is_expected.to_not include issue_second.to_reference(project) }
+      it { is_expected.to_not include merge_request.to_reference(project) }
+    end
+  end
+end
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index cd051175612..d516bd4830a 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -11,7 +11,7 @@ describe Issues::MoveService, services: true do
 
   let(:old_issue) do
     create(:issue, title: title, description: description,
-           project: old_project, author: author)
+                   project: old_project, author: author)
   end
 
   let(:move_service) do
@@ -133,7 +133,7 @@ describe Issues::MoveService, services: true do
           before do
             create(:merge_request, source_project: old_project)
             create(:note, noteable: old_issue, project: old_project, author: author,
-                   note: 'Note with reference to merge request !1')
+                          note: 'Note with reference to merge request !1')
           end
 
           include_context 'issue move executed'
-- 
cgit v1.2.3


From 9124ebce982981b74ae644793a680b2b6060ce21 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 29 Feb 2016 09:49:22 +0100
Subject: Use internal reference extractor in banzai unfold pipeline

---
 .../pipeline/reference_unfold_pipeline_spec.rb     | 57 ++++++++++++----------
 1 file changed, 30 insertions(+), 27 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb b/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
index af9d6f4ad0d..c02dc5a2c2c 100644
--- a/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
@@ -2,58 +2,61 @@ require 'spec_helper'
 
 describe Banzai::Pipeline::ReferenceUnfoldPipeline do
   let(:text) { 'some text' }
-  let(:project) { create(:project) }
-  let(:objects) { [] }
+  let(:old_project) { create(:project) }
+  let(:new_project) { create(:project) }
+  let(:pipeline_context) do
+    { project: old_project, new_project: new_project }
+  end
 
   let(:result) do
-    described_class.to_html(text, project: project, objects: objects)
+    described_class.to_document(text, pipeline_context)
   end
 
   context 'invalid initializers' do
     subject { -> { result } }
 
-    context 'project context is invalid' do
-      let(:project) { nil }
-      it { is_expected.to raise_error StandardError, /No valid project/ }
+    context 'project context is missing' do
+      let(:pipeline_context) { { new_project: new_project } }
+      it { is_expected.to raise_error ArgumentError, /Missing context keys/ }
     end
 
-    context 'objects context is invalid' do
-      let(:objects) { ['issue'] }
-      it { is_expected.to raise_error StandardError, /No `to_reference` method/ }
+    context 'new project context is missing' do
+      let(:pipeline_context) { { project: old_project } }
+      it { is_expected.to raise_error ArgumentError, /Missing context keys/ }
     end
   end
 
   context 'multiple issues and merge requests referenced' do
-    subject { result }
-
-    let(:main_project) { create(:project) }
-
-    let(:issue_first) { create(:issue, project: main_project) }
-    let(:issue_second) { create(:issue, project: main_project) }
-    let(:merge_request) { create(:merge_request, source_project: main_project) }
+    subject { result[:output] }
 
-    let(:objects) { [issue_first, issue_second, merge_request] }
+    let!(:issue_first) { create(:issue, project: old_project) }
+    let!(:issue_second) { create(:issue, project: old_project) }
+    let!(:merge_request) { create(:merge_request, source_project: old_project) }
 
     context 'plain text description' do
       let(:text) { 'Description that references #1, #2 and !1' }
 
-      it { is_expected.to include issue_first.to_reference(project) }
-      it { is_expected.to include issue_second.to_reference(project) }
-      it { is_expected.to include merge_request.to_reference(project) }
+      it { is_expected.to include issue_first.to_reference(new_project) }
+      it { is_expected.to include issue_second.to_reference(new_project) }
+      it { is_expected.to include merge_request.to_reference(new_project) }
     end
 
     context 'description with ignored elements' do
       let(:text) do
-        <<-EOF
-          Hi. This references #1, but not `#2`
-          <pre>and not !1</pre>
-        EOF
+        "Hi. This references #1, but not `#2`\n" +
+        '<pre>and not !1</pre>'
       end
 
+      it { is_expected.to include issue_first.to_reference(new_project) }
+      it { is_expected.to_not include issue_second.to_reference(new_project) }
+      it { is_expected.to_not include merge_request.to_reference(new_project) }
+    end
+
+    context 'description ambigous elements' do
+      let(:url) { 'http://gitlab.com/#1' }
+      let(:text) { "This references #1, but not #{url}" }
 
-      it { is_expected.to include issue_first.to_reference(project) }
-      it { is_expected.to_not include issue_second.to_reference(project) }
-      it { is_expected.to_not include merge_request.to_reference(project) }
+      it { is_expected.to include url }
     end
   end
 end
-- 
cgit v1.2.3


From fd8394faae25b54c4d9ac485a0ce746cffec3a0f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 7 Mar 2016 13:09:53 +0100
Subject: Move reference unfolder for GFM to separate class

---
 .../pipeline/reference_unfold_pipeline_spec.rb     | 62 ----------------------
 spec/lib/gitlab/gfm/reference_unfolder_spec.rb     | 43 +++++++++++++++
 spec/lib/gitlab/reference_extractor_spec.rb        | 12 +++++
 3 files changed, 55 insertions(+), 62 deletions(-)
 delete mode 100644 spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
 create mode 100644 spec/lib/gitlab/gfm/reference_unfolder_spec.rb

(limited to 'spec')

diff --git a/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb b/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
deleted file mode 100644
index c02dc5a2c2c..00000000000
--- a/spec/lib/banzai/pipeline/reference_unfold_pipeline_spec.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-require 'spec_helper'
-
-describe Banzai::Pipeline::ReferenceUnfoldPipeline do
-  let(:text) { 'some text' }
-  let(:old_project) { create(:project) }
-  let(:new_project) { create(:project) }
-  let(:pipeline_context) do
-    { project: old_project, new_project: new_project }
-  end
-
-  let(:result) do
-    described_class.to_document(text, pipeline_context)
-  end
-
-  context 'invalid initializers' do
-    subject { -> { result } }
-
-    context 'project context is missing' do
-      let(:pipeline_context) { { new_project: new_project } }
-      it { is_expected.to raise_error ArgumentError, /Missing context keys/ }
-    end
-
-    context 'new project context is missing' do
-      let(:pipeline_context) { { project: old_project } }
-      it { is_expected.to raise_error ArgumentError, /Missing context keys/ }
-    end
-  end
-
-  context 'multiple issues and merge requests referenced' do
-    subject { result[:output] }
-
-    let!(:issue_first) { create(:issue, project: old_project) }
-    let!(:issue_second) { create(:issue, project: old_project) }
-    let!(:merge_request) { create(:merge_request, source_project: old_project) }
-
-    context 'plain text description' do
-      let(:text) { 'Description that references #1, #2 and !1' }
-
-      it { is_expected.to include issue_first.to_reference(new_project) }
-      it { is_expected.to include issue_second.to_reference(new_project) }
-      it { is_expected.to include merge_request.to_reference(new_project) }
-    end
-
-    context 'description with ignored elements' do
-      let(:text) do
-        "Hi. This references #1, but not `#2`\n" +
-        '<pre>and not !1</pre>'
-      end
-
-      it { is_expected.to include issue_first.to_reference(new_project) }
-      it { is_expected.to_not include issue_second.to_reference(new_project) }
-      it { is_expected.to_not include merge_request.to_reference(new_project) }
-    end
-
-    context 'description ambigous elements' do
-      let(:url) { 'http://gitlab.com/#1' }
-      let(:text) { "This references #1, but not #{url}" }
-
-      it { is_expected.to include url }
-    end
-  end
-end
diff --git a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
new file mode 100644
index 00000000000..4f8b960350c
--- /dev/null
+++ b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Gitlab::Gfm::ReferenceUnfolder do
+  let(:text) { 'some text' }
+  let(:old_project) { create(:project) }
+  let(:new_project) { create(:project) }
+
+  describe '#unfold' do
+    subject { described_class.new(text, old_project).unfold(new_project) }
+
+    context 'multiple issues and merge requests referenced' do
+      let!(:issue_first) { create(:issue, project: old_project) }
+      let!(:issue_second) { create(:issue, project: old_project) }
+      let!(:merge_request) { create(:merge_request, source_project: old_project) }
+
+      context 'plain text description' do
+        let(:text) { 'Description that references #1, #2 and !1' }
+
+        it { is_expected.to include issue_first.to_reference(new_project) }
+        it { is_expected.to include issue_second.to_reference(new_project) }
+        it { is_expected.to include merge_request.to_reference(new_project) }
+      end
+
+      context 'description with ignored elements' do
+        let(:text) do
+          "Hi. This references #1, but not `#2`\n" +
+          '<pre>and not !1</pre>'
+        end
+
+        it { is_expected.to include issue_first.to_reference(new_project) }
+        it { is_expected.to_not include issue_second.to_reference(new_project) }
+        it { is_expected.to_not include merge_request.to_reference(new_project) }
+      end
+
+      context 'description ambigous elements' do
+        let(:url) { 'http://gitlab.com/#1' }
+        let(:text) { "This references #1, but not #{url}" }
+
+        it { is_expected.to include url }
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 7d963795e17..78ab162e5d6 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -122,4 +122,16 @@ describe Gitlab::ReferenceExtractor, lib: true do
       expect(extracted).to match_array([issue])
     end
   end
+
+  describe '#all' do
+    let(:issue) { create(:issue, project: project) }
+    let(:label) { create(:label, project: project) }
+    let(:text) { "Ref. #{issue.to_reference} and #{label.to_reference}" }
+
+    before { subject.analyze(text) }
+
+    it 'returns all referables' do
+      expect(subject.all).to match_array([issue, label])
+    end
+  end
 end
-- 
cgit v1.2.3


From 4354bfaba55c9238d5245fe2f5823665790c9817 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 11 Mar 2016 11:49:04 +0100
Subject: Add implementation of reference unfolder using banzai

---
 spec/lib/gitlab/gfm/reference_unfolder_spec.rb | 32 +++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
index 4f8b960350c..40cdb7e1452 100644
--- a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
+++ b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
@@ -33,10 +33,36 @@ describe Gitlab::Gfm::ReferenceUnfolder do
       end
 
       context 'description ambigous elements' do
-        let(:url) { 'http://gitlab.com/#1' }
-        let(:text) { "This references #1, but not #{url}" }
+        context 'url' do
+          let(:url) { 'http://gitlab.com/#1' }
+          let(:text) { "This references #1, but not #{url}" }
 
-        it { is_expected.to include url }
+          it { is_expected.to include url }
+        end
+
+        context 'code' do
+          let(:text) { "#1, but not `[#1]`" }
+          it { is_expected.to eq "#{issue_first.to_reference(new_project)}, but not `[#1]`" }
+        end
+
+        context 'code reverse' do
+          let(:text) { "not `#1`, but #1" }
+          it { is_expected.to eq "not `#1`, but #{issue_first.to_reference(new_project)}" }
+        end
+
+        context 'code in random order' do
+          let(:text) { "#1, `#1`, #1, `#1`" }
+          let(:ref) { issue_first.to_reference(new_project) }
+
+          it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" }
+        end
+      end
+
+      context 'reference contains milestone' do
+        let(:milestone) { create(:milestone) }
+        let(:text) { "milestone ref: #{milestone.to_reference}" }
+
+        it { is_expected.to eq text }
       end
     end
   end
-- 
cgit v1.2.3


From 310b417b0f2336e5e7a9b44da2fabf5d4abb0cb4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 14 Mar 2016 08:25:37 +0100
Subject: Preserve original author when moving issue

This also wrapps entire process into transation, as rewriting references
may have large memory footprint.
---
 spec/services/issues/move_service_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index d516bd4830a..4fcfceaddd4 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -74,13 +74,13 @@ describe Issues::MoveService, services: true do
           expect(new_issue.persisted?).to be true
         end
 
-        it 'persist all changes' do
+        it 'persists all changes' do
           expect(old_issue.changed?).to be false
-          expect(new_issue.changed?).to be false
+          expect(new_issue.persisted?).to be true
         end
 
-        it 'changes author' do
-          expect(new_issue.author).to eq user
+        it 'preserves author' do
+          expect(new_issue.author).to eq author
         end
 
         it 'removes data that is invalid in new context' do
-- 
cgit v1.2.3


From a91101b19a44d98005dd21b06d8509abcd82ddfc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 14 Mar 2016 08:30:37 +0100
Subject: Make it possible to move issue if user is a reporter

Discussed it here:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2831#note_4190228
---
 spec/services/issues/move_service_spec.rb | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 4fcfceaddd4..38fa5707cf7 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -24,8 +24,8 @@ describe Issues::MoveService, services: true do
 
   shared_context 'user can move issue' do
     before do
-      old_project.team << [user, :master]
-      new_project.team << [user, :master]
+      old_project.team << [user, :reporter]
+      new_project.team << [user, :reporter]
     end
   end
     
@@ -186,25 +186,25 @@ describe Issues::MoveService, services: true do
     describe '#move?' do
       subject { move_service.move? }
 
-      context 'user is master in both projects' do
+      context 'user is reporter in both projects' do
         include_context 'user can move issue'
         it { is_expected.to be_truthy }
       end
 
-      context 'user is master only in new project' do
-        before { new_project.team << [user, :master] }
+      context 'user is reporter only in new project' do
+        before { new_project.team << [user, :reporter] }
         it { is_expected.to be_falsey }
       end
 
-      context 'user is master only in old project' do
-        before { old_project.team << [user, :master] }
+      context 'user is reporter only in old project' do
+        before { old_project.team << [user, :reporter] }
         it { is_expected.to be_falsey }
       end
 
-      context 'user is master in one project and developer in another' do
+      context 'user is reporter in one project and guest in another' do
         before do
-          new_project.team << [user, :developer]
-          old_project.team << [user, :master]
+          new_project.team << [user, :guest]
+          old_project.team << [user, :reporter]
         end
 
         it { is_expected.to be_falsey }
-- 
cgit v1.2.3


From a23f0e8c9ebd3a7922786d2fc4f3450c1fdecad6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 15 Mar 2016 10:14:39 +0100
Subject: Reuse existing issue services when moving issue

---
 spec/services/issues/move_service_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 38fa5707cf7..69f4748ae67 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -76,7 +76,7 @@ describe Issues::MoveService, services: true do
 
         it 'persists all changes' do
           expect(old_issue.changed?).to be false
-          expect(new_issue.persisted?).to be true
+          expect(new_issue.changed?).to be false
         end
 
         it 'preserves author' do
-- 
cgit v1.2.3


From 15d32b6a11380c257180dbc8e2691bc12e2792bc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 15 Mar 2016 11:35:40 +0100
Subject: Add new notifications for issue move action

[ci skip]
---
 spec/mailers/notify_spec.rb               | 27 +++++++++++++++++++++++++++
 spec/services/issues/move_service_spec.rb |  2 +-
 2 files changed, 28 insertions(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index f910424d85b..9b47acfe0cd 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -158,6 +158,33 @@ describe Notify do
             is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
           end
         end
+
+        describe 'moved to another project' do
+          let(:new_issue) { create(:issue) }
+          subject { Notify.issue_moved_email(recipient, issue, new_issue, current_user) }
+
+          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like 'an unsubscribeable thread'
+
+          it 'contains description about action taken' do
+            is_expected.to have_body_text 'Issue was moved to another project'
+          end
+
+          it 'has the correct subject' do
+            is_expected.to have_subject /#{issue.title} \(##{issue.iid}\)/i
+          end
+
+          it 'contains link to new issue' do
+            new_issue_url = namespace_project_issue_path(new_issue.project.namespace,
+                                                         new_issue.project, new_issue)
+            is_expected.to have_body_text new_issue_url
+          end
+
+          it 'contains a link to the original issue' do
+            is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
+          end
+        end
       end
 
       context 'for merge requests' do
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 69f4748ae67..b71ce311afd 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -55,7 +55,7 @@ describe Issues::MoveService, services: true do
         end
 
         it 'rewrites issue description' do
-          expect(new_issue.description).to include description
+          expect(new_issue.description).to eq description
         end
 
         it 'adds system note to old issue at the end' do
-- 
cgit v1.2.3


From 1dd279d83335de71c69d0acfdcdd7eb0ebe7f3dd Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 15 Mar 2016 15:01:26 +0100
Subject: Use helper to create list of projects issue can be moved to

This also adds confirmation message if issue move has been requested.
---
 spec/services/issues/move_service_spec.rb | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index b71ce311afd..cb02b8721ac 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -28,7 +28,7 @@ describe Issues::MoveService, services: true do
       new_project.team << [user, :reporter]
     end
   end
-    
+
   context 'issue movable' do
     include_context 'issue move requested'
     include_context 'user can move issue'
@@ -162,6 +162,18 @@ describe Issues::MoveService, services: true do
     end
   end
 
+  context 'moving to same project' do
+    let(:new_project) { old_project }
+
+    include_context 'issue move requested'
+    include_context 'user can move issue'
+
+    it 'raises error' do
+      expect { move_service }
+        .to raise_error(StandardError, /Cannot move issue/)
+    end
+  end
+
   context 'issue move not requested' do
     let(:new_project_id) { nil }
 
@@ -179,7 +191,6 @@ describe Issues::MoveService, services: true do
     end
   end
 
-
   describe 'move permissions' do
     include_context 'issue move requested'
 
-- 
cgit v1.2.3


From 5e3c9475a9332d7104a791f7bdfef30e069dd848 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 16 Mar 2016 10:24:44 +0100
Subject: Add minor improvements in code related to issue move

---
 spec/services/system_note_service_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 7efb9f2ddba..7c93ce304f9 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -446,7 +446,7 @@ describe SystemNoteService, services: true do
     let(:new_noteable) { create(:issue, project: new_project) }
 
     subject do
-      described_class.noteable_moved(direction, noteable, project, new_noteable, author)
+      described_class.noteable_moved(noteable, project, new_noteable, author, direction: direction)
     end
 
     shared_examples 'cross project mentionable' do
-- 
cgit v1.2.3


From e3d32ff036ab442b9613d4dbceb473f30da048af Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 16 Mar 2016 15:11:07 +0100
Subject: Add feature specs for issue move

---
 spec/features/issues/move_spec.rb | 77 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 spec/features/issues/move_spec.rb

(limited to 'spec')

diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb
new file mode 100644
index 00000000000..6303b117c09
--- /dev/null
+++ b/spec/features/issues/move_spec.rb
@@ -0,0 +1,77 @@
+require 'rails_helper'
+
+feature 'issue move to another project' do
+  let(:user) { create(:user) }
+  let(:old_project) { create(:project) }
+  let(:text) { 'Some issue description' }
+
+  let(:issue) do
+    create(:issue, description: text, project: old_project, author: user)
+  end
+
+  background { login_as(user) }
+
+  context 'user does not have permission to move issue' do
+    background do
+      old_project.team << [user, :guest]
+
+      edit_issue(issue)
+    end
+
+    scenario 'moving issue to another project not allowed' do
+      expect(page).to have_no_select('move_to_project_id')
+    end
+  end
+
+  context 'user has permission to move issue' do
+    let!(:mr) { create(:merge_request, source_project: old_project) }
+    let(:new_project) { create(:project) }
+    let(:text) { 'Text with !1' }
+    let(:cross_reference) { old_project.to_reference }
+
+    background do
+      old_project.team << [user, :reporter]
+      new_project.team << [user, :reporter]
+
+      edit_issue(issue)
+    end
+
+    scenario 'moving issue to another project', js: true do
+      find('#s2id_move_to_project_id').click
+      find('.select2-drop li', text: new_project.name_with_namespace).click
+      click_button('Save changes')
+
+      expect(current_url).to include project_path(new_project)
+
+      page.within('.issue') do
+        expect(page).to have_content("Text with #{cross_reference}!1")
+        expect(page).to have_content("Moved from #{cross_reference}#1")
+        expect(page).to have_content(issue.title)
+      end
+    end
+
+    context 'projects user does not have permission to move issue to exist' do
+      let!(:private_project) { create(:project, :private) }
+      let(:another_project) { create(:project) }
+      background { another_project.team << [user, :guest] }
+
+      scenario 'browsing projects in projects select' do
+        options = [ '', 'No project', new_project.name_with_namespace ]
+        expect(page).to have_select('move_to_project_id', options: options)
+      end
+    end
+  end
+
+  def edit_issue(issue)
+    visit issue_path(issue)
+    page.within('.issuable-header') { click_link 'Edit' }
+  end
+
+  def issue_path(issue)
+    namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  def project_path(project)
+    namespace_project_path(new_project.namespace, new_project)
+  end
+end
-- 
cgit v1.2.3


From b9036ba61012e6723426f98171a2c111abb0bae5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 17 Mar 2016 11:11:22 +0100
Subject: Prevent issue move if issue has been already moved

---
 spec/features/issues/move_spec.rb         | 11 +++++++
 spec/models/issue_spec.rb                 | 50 +++++++++++++++++++++++++++++++
 spec/services/issues/move_service_spec.rb | 18 +++++++++++
 3 files changed, 79 insertions(+)

(limited to 'spec')

diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb
index 6303b117c09..db26b3a4671 100644
--- a/spec/features/issues/move_spec.rb
+++ b/spec/features/issues/move_spec.rb
@@ -60,6 +60,17 @@ feature 'issue move to another project' do
         expect(page).to have_select('move_to_project_id', options: options)
       end
     end
+
+    context 'issue has been already moved' do
+      let(:new_issue) { create(:issue, project: new_project) }
+      let(:issue) do
+        create(:issue, project: old_project, author: user, moved_to: new_issue)
+      end
+
+      scenario 'user wants to move issue that has already been moved' do
+        expect(page).to have_no_select('move_to_project_id')
+      end
+    end
   end
 
   def edit_issue(issue)
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 7f44ca2f7db..0d6e9cb3a4c 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -130,6 +130,56 @@ describe Issue, models: true do
     end
   end
 
+  describe '#can_move?' do
+    let(:user) { create(:user) }
+    let(:issue) { create(:issue) }
+    subject { issue.can_move?(user) }
+
+    context 'user is not a member of project issue belongs to' do
+      it { is_expected.to eq false}
+    end
+
+    context 'user is reporter in project issue belongs to' do
+      let(:project) { create(:project) }
+      let(:issue) { create(:issue, project: project) }
+
+      before { project.team << [user, :reporter] }
+
+      it { is_expected.to eq true }
+
+      context 'checking destination project also' do
+        subject { issue.can_move?(user, to_project) }
+        let(:to_project) { create(:project) }
+
+        context 'destination project allowed' do
+          before { to_project.team << [user, :reporter] }
+          it { is_expected.to eq true }
+        end
+
+        context 'destination project not allowed' do
+          before { to_project.team << [user, :guest] }
+          it { is_expected.to eq false }
+        end
+      end
+    end
+  end
+
+  describe '#moved?' do
+    let(:issue) { create(:issue) }
+    subject { issue.moved? }
+
+    context 'issue not moved' do
+      it { is_expected.to eq false }
+    end
+
+    context 'issue already moved' do
+      let(:moved_to_issue) { create(:issue) }
+      let(:issue) { create(:issue, moved_to: moved_to_issue) }
+
+      it { is_expected.to eq true }
+    end
+  end
+
   it_behaves_like 'an editable mentionable' do
     subject { create(:issue) }
 
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index cb02b8721ac..02ade91ac03 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -91,6 +91,11 @@ describe Issues::MoveService, services: true do
         it 'creates a new internal id for issue' do
           expect(new_issue.iid).to be 1
         end
+
+        it 'marks issue as moved' do
+          expect(old_issue.moved?).to eq true
+          expect(old_issue.moved_to).to eq new_issue
+        end
       end
 
       context 'issue with notes' do
@@ -220,6 +225,19 @@ describe Issues::MoveService, services: true do
 
         it { is_expected.to be_falsey }
       end
+
+      context 'issue has already been moved' do
+        include_context 'user can move issue'
+
+        let(:moved_to_issue) { create(:issue) }
+
+        let(:old_issue) do
+          create(:issue, project: old_project, author: author,
+                         moved_to: moved_to_issue)
+        end
+
+        it { is_expected.to be_falsey }
+      end
     end
   end
 end
-- 
cgit v1.2.3


From fcf106897e2ff38e16e785ad9bcb18d117490fbf Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 17 Mar 2016 12:25:17 +0100
Subject: Do not use javascript in specs for issue move

Feature specs that were using javascript took a long time to finish
(about 10x longer) and where hitting Poltergeist timeouts.

This modification skips using javascript with select2 selecbox, thus it
is faster, but this also does not check some JavaScript related
features.
---
 spec/features/issues/move_spec.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

(limited to 'spec')

diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb
index db26b3a4671..6fda0c31866 100644
--- a/spec/features/issues/move_spec.rb
+++ b/spec/features/issues/move_spec.rb
@@ -36,9 +36,8 @@ feature 'issue move to another project' do
       edit_issue(issue)
     end
 
-    scenario 'moving issue to another project', js: true do
-      find('#s2id_move_to_project_id').click
-      find('.select2-drop li', text: new_project.name_with_namespace).click
+    scenario 'moving issue to another project' do
+      select(new_project.name_with_namespace, from: 'move_to_project_id')
       click_button('Save changes')
 
       expect(current_url).to include project_path(new_project)
-- 
cgit v1.2.3


From 9b13ce0b7a50e65dfba31d4865a728c725daa3fe Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 18 Mar 2016 14:48:55 +0100
Subject: Improvements in issue move feaure (refactoring)

According to endbosses' suggestions.
---
 spec/lib/gitlab/reference_extractor_spec.rb |  5 ++
 spec/services/issues/move_service_spec.rb   | 72 ++++++++---------------------
 2 files changed, 24 insertions(+), 53 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 78ab162e5d6..ba47a3540ff 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -134,4 +134,9 @@ describe Gitlab::ReferenceExtractor, lib: true do
       expect(subject.all).to match_array([issue, label])
     end
   end
+
+  describe '.references_pattern' do
+    subject { described_class.references_pattern }
+    it { is_expected.to be_kind_of Regexp }
+  end
 end
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 02ade91ac03..fd958a44e3e 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -15,11 +15,7 @@ describe Issues::MoveService, services: true do
   end
 
   let(:move_service) do
-    described_class.new(old_project, user, issue_params, old_issue, new_project_id)
-  end
-
-  shared_context 'issue move requested' do
-    let(:new_project_id) { new_project.id }
+    described_class.new(old_project, user, issue_params)
   end
 
   shared_context 'user can move issue' do
@@ -29,19 +25,13 @@ describe Issues::MoveService, services: true do
     end
   end
 
-  context 'issue movable' do
-    include_context 'issue move requested'
-    include_context 'user can move issue'
-
-    describe '#move?' do
-      subject { move_service.move? }
-      it { is_expected.to be_truthy }
+  describe '#execute' do
+    shared_context 'issue move executed' do
+      let!(:new_issue) { move_service.execute(old_issue, new_project) }
     end
 
-    describe '#execute' do
-      shared_context 'issue move executed' do
-        let!(:new_issue) { move_service.execute }
-      end
+    context 'issue movable' do
+      include_context 'user can move issue'
 
       context 'generic issue' do
         include_context 'issue move executed'
@@ -164,57 +154,33 @@ describe Issues::MoveService, services: true do
           end
         end
       end
-    end
-  end
-
-  context 'moving to same project' do
-    let(:new_project) { old_project }
-
-    include_context 'issue move requested'
-    include_context 'user can move issue'
-
-    it 'raises error' do
-      expect { move_service }
-        .to raise_error(StandardError, /Cannot move issue/)
-    end
-  end
-
-  context 'issue move not requested' do
-    let(:new_project_id) { nil }
 
-    describe '#move?' do
-      subject { move_service.move? }
-
-      context 'user do not have permissions to move issue' do
-        it { is_expected.to be_falsey }
-      end
+      context 'moving to same project' do
+        let(:new_project) { old_project }
 
-      context 'user has permissions to move issue' do
-        include_context 'user can move issue'
-        it { is_expected.to be_falsey }
+        it 'raises error' do
+          expect { move_service.execute(old_issue, new_project) }
+            .to raise_error(StandardError, /Cannot move issue/)
+        end
       end
     end
-  end
-
-  describe 'move permissions' do
-    include_context 'issue move requested'
 
-    describe '#move?' do
-      subject { move_service.move? }
+    describe 'move permissions' do
+      let(:move) { move_service.execute(old_issue, new_project) }
 
       context 'user is reporter in both projects' do
         include_context 'user can move issue'
-        it { is_expected.to be_truthy }
+        it { expect { move }.to_not raise_error }
       end
 
       context 'user is reporter only in new project' do
         before { new_project.team << [user, :reporter] }
-        it { is_expected.to be_falsey }
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
       end
 
       context 'user is reporter only in old project' do
         before { old_project.team << [user, :reporter] }
-        it { is_expected.to be_falsey }
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
       end
 
       context 'user is reporter in one project and guest in another' do
@@ -223,7 +189,7 @@ describe Issues::MoveService, services: true do
           old_project.team << [user, :reporter]
         end
 
-        it { is_expected.to be_falsey }
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
       end
 
       context 'issue has already been moved' do
@@ -236,7 +202,7 @@ describe Issues::MoveService, services: true do
                          moved_to: moved_to_issue)
         end
 
-        it { is_expected.to be_falsey }
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
       end
     end
   end
-- 
cgit v1.2.3


From 18f25bc94282a29029721ceeb9b9c6db354ce45f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 19 Mar 2016 18:58:52 +0100
Subject: Update reference unfolder according to recent ability changes

Commit 43d8bdb4f048cbeb5675ed9120cb1aeb415b9586 introduced additional
checks for permissions to read issue in references extractor.
---
 spec/lib/gitlab/gfm/reference_unfolder_spec.rb | 7 ++++++-
 spec/lib/gitlab/reference_extractor_spec.rb    | 5 ++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
index 40cdb7e1452..2e3b77d9180 100644
--- a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
+++ b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
@@ -4,9 +4,14 @@ describe Gitlab::Gfm::ReferenceUnfolder do
   let(:text) { 'some text' }
   let(:old_project) { create(:project) }
   let(:new_project) { create(:project) }
+  let(:user) { create(:user) }
+
+  before { old_project.team << [user, :guest] }
 
   describe '#unfold' do
-    subject { described_class.new(text, old_project).unfold(new_project) }
+    subject do
+      described_class.new(text, old_project, user).unfold(new_project)
+    end
 
     context 'multiple issues and merge requests referenced' do
       let!(:issue_first) { create(:issue, project: old_project) }
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index f2922160331..7c617723e6d 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -130,7 +130,10 @@ describe Gitlab::ReferenceExtractor, lib: true do
     let(:label) { create(:label, project: project) }
     let(:text) { "Ref. #{issue.to_reference} and #{label.to_reference}" }
 
-    before { subject.analyze(text) }
+    before do
+      project.team << [project.creator, :developer]
+      subject.analyze(text)
+    end
 
     it 'returns all referables' do
       expect(subject.all).to match_array([issue, label])
-- 
cgit v1.2.3


From 212e83bab3f1f9055f1321c3e934b4f4659250bf Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <zegerjan@gitlab.com>
Date: Fri, 26 Feb 2016 09:55:43 +0100
Subject: Soft delete issuables

---
 .../controllers/projects/issues_controller_spec.rb | 28 ++++++++++++++++++----
 .../projects/merge_requests_controller_spec.rb     | 21 ++++++++++++++++
 spec/models/issue_spec.rb                          |  5 ++++
 spec/models/merge_request_spec.rb                  |  5 ++++
 spec/requests/api/issues_spec.rb                   | 13 ++++++++--
 spec/requests/api/merge_requests_spec.rb           | 18 ++++++++++++++
 6 files changed, 84 insertions(+), 6 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 2cd81231144..62643a755b7 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1,11 +1,11 @@
 require('spec_helper')
 
 describe Projects::IssuesController do
-  describe "GET #index" do
-    let(:project) { create(:project) }
-    let(:user) { create(:user) }
-    let(:issue) { create(:issue, project: project) }
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+  let(:issue)   { create(:issue, project: project) }
 
+  describe "GET #index" do
     before do
       sign_in(user)
       project.team << [user, :developer]
@@ -186,4 +186,24 @@ describe Projects::IssuesController do
       end
     end
   end
+
+  describe "DELETE #destroy" do
+    it "rejects a developer to destory an issue" do
+      delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
+      expect(response.status).to eq 404
+    end
+
+    context "user is an admin" do
+      before do
+        user.admin = true
+        user.save
+      end
+
+      it "lets an admin delete an issue" do
+        delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
+
+        expect(response.status).to eq 302
+      end
+    end
+  end
 end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index e82fe26c7a6..6c1aa8368ea 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -157,6 +157,27 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe "DELETE #destroy" do
+    it "lets mere mortals not acces this endpoint" do
+      delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
+
+      expect(response.status).to eq 404
+    end
+
+    context "user is an admin" do
+      before do
+        user.admin = true
+        user.save
+      end
+
+      it "lets an admin delete an issue" do
+        delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
+
+        expect(response.status).to be 302
+      end
+    end
+  end
+
   describe 'GET diffs' do
     def go(format: 'html')
       get :diffs,
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 540a62eb1f8..5107e9a5030 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -37,6 +37,11 @@ describe Issue, models: true do
 
   subject { create(:issue) }
 
+  describe "act_as_paranoid" do
+    it { is_expected.to have_db_column(:deleted_at) }
+    it { is_expected.to have_db_index(:deleted_at) }
+  end
+
   describe '#to_reference' do
     it 'returns a String reference to the object' do
       expect(subject.to_reference).to eq "##{subject.iid}"
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index f2f07e4ee17..bd0a4ebe337 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -49,6 +49,11 @@ describe MergeRequest, models: true do
     it { is_expected.to include_module(Taskable) }
   end
 
+  describe "act_as_paranoid" do
+    it { is_expected.to have_db_column(:deleted_at) }
+    it { is_expected.to have_db_index(:deleted_at) }
+  end
+
   describe 'validation' do
     it { is_expected.to validate_presence_of(:target_branch) }
     it { is_expected.to validate_presence_of(:source_branch) }
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index bb2ab058003..7d6ebc9b76e 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -469,9 +469,18 @@ describe API::API, api: true  do
   end
 
   describe "DELETE /projects/:id/issues/:issue_id" do
-    it "should delete a project issue" do
+    it "should reject non admins form deleting an issue" do
       delete api("/projects/#{project.id}/issues/#{issue.id}", user)
-      expect(response.status).to eq(405)
+      expect(response.status).to eq(403)
+    end
+
+    it "deletes the issue if an admin requests it" do
+      user.admin = true
+      user.save
+
+      delete api("/projects/#{project.id}/issues/#{issue.id}", user)
+      expect(response.status).to eq(200)
+      expect(json_response['state']).to eq 'opened'
     end
   end
 end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 4fd1df25568..5a0bc4ff076 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -315,6 +315,24 @@ describe API::API, api: true  do
     end
   end
 
+  describe "DELETE /projects/:id/merge_request/:merge_request_id" do
+    it "rejects non admin users from deletions" do
+      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+      expect(response.status).to eq(403)
+    end
+
+    it "let's Admins delete a merge request" do
+      user.admin = true
+      user.save
+
+      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+      expect(response.status).to eq(200)
+      expect(json_response['id']).to eq merge_request.id
+    end
+  end
+
   describe "PUT /projects/:id/merge_requests/:merge_request_id to close MR" do
     it "should return merge_request" do
       put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close"
-- 
cgit v1.2.3


From 7342a4566cc2eef0e434f3aea0eac48674baaaf1 Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <zegerjan@gitlab.com>
Date: Mon, 14 Mar 2016 21:46:44 +0100
Subject: Dry destroy action on issuables

---
 spec/controllers/projects/merge_requests_controller_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 6c1aa8368ea..99d1e1c595e 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -123,7 +123,7 @@ describe Projects::MergeRequestsController do
     end
   end
 
-  describe 'GET #index' do
+  describe '#index' do
     def get_merge_requests
       get :index,
           namespace_id: project.namespace.to_param,
@@ -157,8 +157,8 @@ describe Projects::MergeRequestsController do
     end
   end
 
-  describe "DELETE #destroy" do
-    it "lets mere mortals not acces this endpoint" do
+  describe "#destroy" do
+    it "lets mere mortals not access this endpoint" do
       delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
 
       expect(response.status).to eq 404
@@ -170,7 +170,7 @@ describe Projects::MergeRequestsController do
         user.save
       end
 
-      it "lets an admin delete an issue" do
+      it "lets an admin or owner delete an issue" do
         delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
 
         expect(response.status).to be 302
-- 
cgit v1.2.3


From 1d7782281693a94b2d8efcdd9d05f81fefae75f9 Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <zegerjan@gitlab.com>
Date: Fri, 18 Mar 2016 19:11:25 +0100
Subject: minor improvements and fixed specs

---
 .../projects/merge_requests_controller_spec.rb     |  2 +-
 spec/requests/api/issues_spec.rb                   | 28 ++++++++++++----------
 spec/requests/api/merge_requests_spec.rb           | 22 ++++++++++-------
 3 files changed, 29 insertions(+), 23 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 99d1e1c595e..af154fadb6f 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -164,7 +164,7 @@ describe Projects::MergeRequestsController do
       expect(response.status).to eq 404
     end
 
-    context "user is an admin" do
+    context "user is an admin or owner" do
       before do
         user.admin = true
         user.save
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 7d6ebc9b76e..6298771925e 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -2,12 +2,12 @@ require 'spec_helper'
 
 describe API::API, api: true  do
   include ApiHelpers
-  let(:user) { create(:user) }
-  let(:non_member) { create(:user) }
-  let(:author) { create(:author) }
-  let(:assignee) { create(:assignee) }
-  let(:admin) { create(:admin) }
-  let!(:project) { create(:project, :public, namespace: user.namespace ) }
+  let(:user)        { create(:user) }
+  let(:non_member)  { create(:user) }
+  let(:author)      { create(:author) }
+  let(:assignee)    { create(:assignee) }
+  let(:admin)       { create(:user, :admin) }
+  let!(:project)    { create(:project, :public, namespace: user.namespace ) }
   let!(:closed_issue) do
     create :closed_issue,
            author: user,
@@ -469,16 +469,18 @@ describe API::API, api: true  do
   end
 
   describe "DELETE /projects/:id/issues/:issue_id" do
-    it "should reject non admins form deleting an issue" do
-      delete api("/projects/#{project.id}/issues/#{issue.id}", user)
-      expect(response.status).to eq(403)
+    it "should reject a non member from deleting an issue" do
+      delete api("/projects/#{project.id}/issues/#{issue.id}", non_member)
+      expect(response.status).to be(403)
     end
 
-    it "deletes the issue if an admin requests it" do
-      user.admin = true
-      user.save
+    it "should reject a developer from deleting an issue" do
+      delete api("/projects/#{project.id}/issues/#{issue.id}", author)
+      expect(response.status).to be(403)
+    end
 
-      delete api("/projects/#{project.id}/issues/#{issue.id}", user)
+    it "deletes the issue if an admin requests it" do
+      delete api("/projects/#{project.id}/issues/#{issue.id}", admin)
       expect(response.status).to eq(200)
       expect(json_response['state']).to eq 'opened'
     end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 5a0bc4ff076..43e6a59d1ee 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -3,8 +3,10 @@ require "spec_helper"
 describe API::API, api: true  do
   include ApiHelpers
   let(:base_time) { Time.now }
-  let(:user) { create(:user) }
-  let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) }
+  let(:user)      { create(:user) }
+  let(:admin)     { create(:user, :admin) }
+  let(:non_member)  { create(:user) }
+  let!(:project)  { create(:project, creator_id: user.id, namespace: user.namespace) }
   let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
   let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) }
   let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds) }
@@ -316,21 +318,23 @@ describe API::API, api: true  do
   end
 
   describe "DELETE /projects/:id/merge_request/:merge_request_id" do
-    it "rejects non admin users from deletions" do
+    it "owners can destroy" do
       delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
 
-      expect(response.status).to eq(403)
+      expect(response.status).to eq(200)
     end
 
-    it "let's Admins delete a merge request" do
-      user.admin = true
-      user.save
-
-      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+    it "let's Admins and owners delete a merge request" do
+      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", admin)
 
       expect(response.status).to eq(200)
       expect(json_response['id']).to eq merge_request.id
     end
+
+    it "rejects removal from other users" do
+      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", non_member)
+      expect(response.status).to eq(404)
+    end
   end
 
   describe "PUT /projects/:id/merge_requests/:merge_request_id to close MR" do
-- 
cgit v1.2.3


From 41b8d22631053e66043d05695d65f4961b91efd8 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Fri, 18 Mar 2016 12:47:36 +0100
Subject: Tweaked performance of Issue#related_branches

Requesting the branch names of a repository works even when it's empty,
thus there's no need to explicitly check for an empty repository.
Removing this check cuts down the amount of Git operations which in turn
cuts down request timings a bit. The regular expression used to compare
branches was also moved out of the loop so it's created only once.
---
 spec/models/issue_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 540a62eb1f8..77d046fb0eb 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -133,9 +133,9 @@ describe Issue, models: true do
   describe '#related_branches' do
     it "selects the right branches" do
       allow(subject.project.repository).to receive(:branch_names).
-                                    and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name])
+        and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name])
 
-      expect(subject.related_branches).to eq [subject.to_branch_name]
+      expect(subject.related_branches).to eq([subject.to_branch_name])
     end
   end
 
-- 
cgit v1.2.3


From 68a4c98f5074bd34f0178f2f967153c8d5c71237 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Fri, 18 Mar 2016 15:31:19 +0100
Subject: Cache output of Repository#exists?

This caches the output of Repository#exists? in Redis while making sure
it's flushed properly when creating new repositories, deleting them,
etc.

For the ProjectWiki tests to work I had to make ProjectWiki#create_repo!
public as testing private methods in RSpec is a bit of a pain.
---
 spec/models/project_spec.rb                 | 41 +++++++++++++++++++++++++++++
 spec/models/project_wiki_spec.rb            | 12 +++++++++
 spec/models/repository_spec.rb              | 30 +++++++++++++++++++++
 spec/workers/repository_fork_worker_spec.rb | 19 ++++++++++---
 4 files changed, 98 insertions(+), 4 deletions(-)

(limited to 'spec')

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b8b9a455b83..624022c1dda 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -720,4 +720,45 @@ describe Project, models: true do
       expect(described_class.search_by_title('KITTENS')).to eq([project])
     end
   end
+
+  describe '#create_repository' do
+    let(:project) { create(:project) }
+    let(:shell) { Gitlab::Shell.new }
+
+    before do
+      allow(project).to receive(:gitlab_shell).and_return(shell)
+    end
+
+    context 'using a regular repository' do
+      it 'creates the repository' do
+        expect(shell).to receive(:add_repository).
+          with(project.path_with_namespace).
+          and_return(true)
+
+        expect(project.repository).to receive(:after_create)
+
+        expect(project.create_repository).to eq(true)
+      end
+
+      it 'adds an error if the repository could not be created' do
+        expect(shell).to receive(:add_repository).
+          with(project.path_with_namespace).
+          and_return(false)
+
+        expect(project.repository).not_to receive(:after_create)
+
+        expect(project.create_repository).to eq(false)
+        expect(project.errors).not_to be_empty
+      end
+    end
+
+    context 'using a forked repository' do
+      it 'does nothing' do
+        expect(project).to receive(:forked?).and_return(true)
+        expect(shell).not_to receive(:add_repository)
+
+        project.create_repository
+      end
+    end
+  end
 end
diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb
index a2085df5bcd..532e3f013fd 100644
--- a/spec/models/project_wiki_spec.rb
+++ b/spec/models/project_wiki_spec.rb
@@ -244,6 +244,18 @@ describe ProjectWiki, models: true do
     end
   end
 
+  describe '#create_repo!' do
+    it 'creates a repository' do
+      expect(subject).to receive(:init_repo).
+        with(subject.path_with_namespace).
+        and_return(true)
+
+      expect(subject.repository).to receive(:after_create)
+
+      expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki)
+    end
+  end
+
   private
 
   def create_temp_repo(path)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index a57229a4fdf..7eac70ae948 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -537,6 +537,12 @@ describe Repository, models: true do
 
         repository.before_delete
       end
+
+      it 'flushes the exists cache' do
+        expect(repository).to receive(:expire_exists_cache)
+
+        repository.before_delete
+      end
     end
 
     describe 'when a repository exists' do
@@ -593,6 +599,12 @@ describe Repository, models: true do
 
       repository.after_import
     end
+
+    it 'flushes the exists cache' do
+      expect(repository).to receive(:expire_exists_cache)
+
+      repository.after_import
+    end
   end
 
   describe '#after_push_commit' do
@@ -619,6 +631,14 @@ describe Repository, models: true do
     end
   end
 
+  describe '#after_create' do
+    it 'flushes the exists cache' do
+      expect(repository).to receive(:expire_exists_cache)
+
+      repository.after_create
+    end
+  end
+
   describe "#main_language" do
     it 'shows the main language of the project' do
       expect(repository.main_language).to eq("Ruby")
@@ -781,6 +801,16 @@ describe Repository, models: true do
     end
   end
 
+  describe '#expire_exists_cache' do
+    let(:cache) { repository.send(:cache) }
+
+    it 'expires the cache' do
+      expect(cache).to receive(:expire).with(:exists?)
+
+      repository.expire_exists_cache
+    end
+  end
+
   describe '#build_cache' do
     let(:cache) { repository.send(:cache) }
 
diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb
index 172537474ee..4ef05eb29d2 100644
--- a/spec/workers/repository_fork_worker_spec.rb
+++ b/spec/workers/repository_fork_worker_spec.rb
@@ -3,12 +3,17 @@ require 'spec_helper'
 describe RepositoryForkWorker do
   let(:project) { create(:project) }
   let(:fork_project) { create(:project, forked_from_project: project) }
+  let(:shell) { Gitlab::Shell.new }
 
   subject { RepositoryForkWorker.new }
 
+  before do
+    allow(subject).to receive(:gitlab_shell).and_return(shell)
+  end
+
   describe "#perform" do
     it "creates a new repository from a fork" do
-      expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).with(
+      expect(shell).to receive(:fork_repository).with(
         project.path_with_namespace,
         fork_project.namespace.path
       ).and_return(true)
@@ -19,20 +24,26 @@ describe RepositoryForkWorker do
         fork_project.namespace.path)
     end
 
-    it 'flushes the empty caches' do
-      expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).
+    it 'flushes various caches' do
+      expect(shell).to receive(:fork_repository).
         with(project.path_with_namespace, fork_project.namespace.path).
         and_return(true)
 
       expect_any_instance_of(Repository).to receive(:expire_emptiness_caches).
         and_call_original
 
+      expect_any_instance_of(Repository).to receive(:expire_exists_cache).
+        and_call_original
+
       subject.perform(project.id, project.path_with_namespace,
                       fork_project.namespace.path)
     end
 
     it "handles bad fork" do
-      expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(false)
+      expect(shell).to receive(:fork_repository).and_return(false)
+
+      expect(subject.logger).to receive(:error)
+
       subject.perform(
         project.id,
         project.path_with_namespace,
-- 
cgit v1.2.3


From 295fdf720ae664200fce8c316fbda1931f4ae61d Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Sat, 19 Mar 2016 15:35:24 +0100
Subject: Create repositories in IssuesController specs

In the real world a project always has a repository. This fact allows
code such as Issue#related_branches to work without explicitly checking
if a repository exists.
---
 spec/controllers/projects/issues_controller_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 2cd81231144..a49bd8960ee 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -2,7 +2,7 @@ require('spec_helper')
 
 describe Projects::IssuesController do
   describe "GET #index" do
-    let(:project) { create(:project) }
+    let(:project) { create(:project_empty_repo) }
     let(:user) { create(:user) }
     let(:issue) { create(:issue, project: project) }
 
@@ -41,7 +41,7 @@ describe Projects::IssuesController do
   end
 
   describe 'Confidential Issues' do
-    let(:project) { create(:empty_project, :public) }
+    let(:project) { create(:project_empty_repo, :public) }
     let(:assignee) { create(:assignee) }
     let(:author) { create(:user) }
     let(:non_member) { create(:user) }
-- 
cgit v1.2.3


From 68fa4de6e39dfa036bb74eeb01fa569c716aacdf Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 19 Mar 2016 12:30:00 -0700
Subject: Make HTTP(s) label consistent on clone bar

Sites that use http:// for the external_url should always display HTTP on
the clone bar. Similarly, sites that use https:// should show HTTPS.
---
 spec/helpers/projects_helper_spec.rb | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

(limited to 'spec')

diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 53207767581..86cbd29830c 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -94,4 +94,23 @@ describe ProjectsHelper do
       end
     end
   end
+
+  describe 'default_clone_protocol' do
+    describe 'using HTTP' do
+      it 'returns HTTP' do
+        expect(helper).to receive(:current_user).and_return(nil)
+
+        expect(helper.send(:default_clone_protocol)).to eq('http')
+      end
+    end
+
+    describe 'using HTTPS' do
+      it 'returns HTTPS' do
+        allow(Gitlab.config.gitlab).to receive(:protocol).and_return('https')
+        expect(helper).to receive(:current_user).and_return(nil)
+
+        expect(helper.send(:default_clone_protocol)).to eq('https')
+      end
+    end
+  end
 end
-- 
cgit v1.2.3


From f0211a4ea9e14293e2aea6f93798f23a01287bed Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 20 Mar 2016 09:28:06 +0100
Subject: Do not pass params that are not used in issue move service

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

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index fd958a44e3e..cd24af88d5f 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -7,7 +7,6 @@ describe Issues::MoveService, services: true do
   let(:description) { 'Some issue description' }
   let(:old_project) { create(:project) }
   let(:new_project) { create(:project) }
-  let(:issue_params) { old_issue.serializable_hash }
 
   let(:old_issue) do
     create(:issue, title: title, description: description,
@@ -15,7 +14,7 @@ describe Issues::MoveService, services: true do
   end
 
   let(:move_service) do
-    described_class.new(old_project, user, issue_params)
+    described_class.new(old_project, user)
   end
 
   shared_context 'user can move issue' do
-- 
cgit v1.2.3


From 323d328c8644e3ff01b806f7754d33c0c7dedd7b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 20 Mar 2016 10:11:26 +0100
Subject: Rename reference unfolder to rewriter, minor refactorings

---
 spec/lib/gitlab/gfm/reference_rewriter_spec.rb | 74 ++++++++++++++++++++++++++
 spec/lib/gitlab/gfm/reference_unfolder_spec.rb | 74 --------------------------
 2 files changed, 74 insertions(+), 74 deletions(-)
 create mode 100644 spec/lib/gitlab/gfm/reference_rewriter_spec.rb
 delete mode 100644 spec/lib/gitlab/gfm/reference_unfolder_spec.rb

(limited to 'spec')

diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
new file mode 100644
index 00000000000..db7bee110af
--- /dev/null
+++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+describe Gitlab::Gfm::ReferenceRewriter do
+  let(:text) { 'some text' }
+  let(:old_project) { create(:project) }
+  let(:new_project) { create(:project) }
+  let(:user) { create(:user) }
+
+  before { old_project.team << [user, :guest] }
+
+  describe '#rewrite' do
+    subject do
+      described_class.new(text, old_project, user).rewrite(new_project)
+    end
+
+    context 'multiple issues and merge requests referenced' do
+      let!(:issue_first) { create(:issue, project: old_project) }
+      let!(:issue_second) { create(:issue, project: old_project) }
+      let!(:merge_request) { create(:merge_request, source_project: old_project) }
+
+      context 'plain text description' do
+        let(:text) { 'Description that references #1, #2 and !1' }
+
+        it { is_expected.to include issue_first.to_reference(new_project) }
+        it { is_expected.to include issue_second.to_reference(new_project) }
+        it { is_expected.to include merge_request.to_reference(new_project) }
+      end
+
+      context 'description with ignored elements' do
+        let(:text) do
+          "Hi. This references #1, but not `#2`\n" +
+          '<pre>and not !1</pre>'
+        end
+
+        it { is_expected.to include issue_first.to_reference(new_project) }
+        it { is_expected.to_not include issue_second.to_reference(new_project) }
+        it { is_expected.to_not include merge_request.to_reference(new_project) }
+      end
+
+      context 'description ambigous elements' do
+        context 'url' do
+          let(:url) { 'http://gitlab.com/#1' }
+          let(:text) { "This references #1, but not #{url}" }
+
+          it { is_expected.to include url }
+        end
+
+        context 'code' do
+          let(:text) { "#1, but not `[#1]`" }
+          it { is_expected.to eq "#{issue_first.to_reference(new_project)}, but not `[#1]`" }
+        end
+
+        context 'code reverse' do
+          let(:text) { "not `#1`, but #1" }
+          it { is_expected.to eq "not `#1`, but #{issue_first.to_reference(new_project)}" }
+        end
+
+        context 'code in random order' do
+          let(:text) { "#1, `#1`, #1, `#1`" }
+          let(:ref) { issue_first.to_reference(new_project) }
+
+          it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" }
+        end
+      end
+
+      context 'reference contains milestone' do
+        let(:milestone) { create(:milestone) }
+        let(:text) { "milestone ref: #{milestone.to_reference}" }
+
+        it { is_expected.to eq text }
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb b/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
deleted file mode 100644
index 2e3b77d9180..00000000000
--- a/spec/lib/gitlab/gfm/reference_unfolder_spec.rb
+++ /dev/null
@@ -1,74 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Gfm::ReferenceUnfolder do
-  let(:text) { 'some text' }
-  let(:old_project) { create(:project) }
-  let(:new_project) { create(:project) }
-  let(:user) { create(:user) }
-
-  before { old_project.team << [user, :guest] }
-
-  describe '#unfold' do
-    subject do
-      described_class.new(text, old_project, user).unfold(new_project)
-    end
-
-    context 'multiple issues and merge requests referenced' do
-      let!(:issue_first) { create(:issue, project: old_project) }
-      let!(:issue_second) { create(:issue, project: old_project) }
-      let!(:merge_request) { create(:merge_request, source_project: old_project) }
-
-      context 'plain text description' do
-        let(:text) { 'Description that references #1, #2 and !1' }
-
-        it { is_expected.to include issue_first.to_reference(new_project) }
-        it { is_expected.to include issue_second.to_reference(new_project) }
-        it { is_expected.to include merge_request.to_reference(new_project) }
-      end
-
-      context 'description with ignored elements' do
-        let(:text) do
-          "Hi. This references #1, but not `#2`\n" +
-          '<pre>and not !1</pre>'
-        end
-
-        it { is_expected.to include issue_first.to_reference(new_project) }
-        it { is_expected.to_not include issue_second.to_reference(new_project) }
-        it { is_expected.to_not include merge_request.to_reference(new_project) }
-      end
-
-      context 'description ambigous elements' do
-        context 'url' do
-          let(:url) { 'http://gitlab.com/#1' }
-          let(:text) { "This references #1, but not #{url}" }
-
-          it { is_expected.to include url }
-        end
-
-        context 'code' do
-          let(:text) { "#1, but not `[#1]`" }
-          it { is_expected.to eq "#{issue_first.to_reference(new_project)}, but not `[#1]`" }
-        end
-
-        context 'code reverse' do
-          let(:text) { "not `#1`, but #1" }
-          it { is_expected.to eq "not `#1`, but #{issue_first.to_reference(new_project)}" }
-        end
-
-        context 'code in random order' do
-          let(:text) { "#1, `#1`, #1, `#1`" }
-          let(:ref) { issue_first.to_reference(new_project) }
-
-          it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" }
-        end
-      end
-
-      context 'reference contains milestone' do
-        let(:milestone) { create(:milestone) }
-        let(:text) { "milestone ref: #{milestone.to_reference}" }
-
-        it { is_expected.to eq text }
-      end
-    end
-  end
-end
-- 
cgit v1.2.3


From 6eb31056346ed07db4f66e4ba2369ff9779230c5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 20 Mar 2016 10:52:01 +0100
Subject: Find referable for each ref found in references rewriter

---
 spec/lib/gitlab/gfm/reference_rewriter_spec.rb | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

(limited to 'spec')

diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
index db7bee110af..0a7ca3ec848 100644
--- a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
@@ -61,6 +61,21 @@ describe Gitlab::Gfm::ReferenceRewriter do
 
           it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" }
         end
+
+        context 'description with labels' do
+          let!(:label) { create(:label, id: 123, name: 'test', project: old_project) }
+          let(:project_ref) { old_project.to_reference }
+
+          context 'label referenced by id' do
+            let(:text) { '#1 and ~123' }
+            it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} }
+          end
+
+          context 'label referenced by text' do
+            let(:text) { '#1 and ~"test"' }
+            it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} }
+          end
+        end
       end
 
       context 'reference contains milestone' do
-- 
cgit v1.2.3


From d6474f22d263e5c04318c64979dfec3f7f45b7bc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 20 Mar 2016 17:05:21 +0100
Subject: Preserve created at time of notes when moving issue

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

(limited to 'spec')

diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index cd24af88d5f..14cc20e529a 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -121,6 +121,11 @@ describe Issues::MoveService, services: true do
           it 'preserves orignal author of comment' do
             expect(user_notes.pluck(:author_id)).to all(eq(author.id))
           end
+
+          it 'preserves time when note has been created at' do
+            expect(old_issue.notes.first.created_at)
+              .to eq new_issue.notes.first.created_at
+          end
         end
 
         context 'notes with references' do
-- 
cgit v1.2.3


From 367818d2934af8f3eb7313147198801d021445e5 Mon Sep 17 00:00:00 2001
From: Arinde Eniola <eniolaarinde1@gmail.com>
Date: Sun, 20 Mar 2016 20:01:46 +0100
Subject: change the css class has_tooltip to has-tooltip universally

---
 spec/helpers/labels_helper_spec.rb                    | 6 +++---
 spec/lib/banzai/filter/label_reference_filter_spec.rb | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

(limited to 'spec')

diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb
index eca8bc8ab2d..39042ff7e91 100644
--- a/spec/helpers/labels_helper_spec.rb
+++ b/spec/helpers/labels_helper_spec.rb
@@ -11,7 +11,7 @@ describe LabelsHelper do
       end
 
       it 'uses the instance variable' do
-        expect(link_to_label(label)).to match %r{<a href="/#{@project.to_reference}/issues\?label_name=#{label.name}"><span class="[\w\s\-]*has_tooltip".*</span></a>}
+        expect(link_to_label(label)).to match %r{<a href="/#{@project.to_reference}/issues\?label_name=#{label.name}"><span class="[\w\s\-]*has-tooltip".*</span></a>}
       end
     end
 
@@ -41,8 +41,8 @@ describe LabelsHelper do
 
     context 'with a tooltip argument' do
       context 'set to false' do
-        it 'does not include the has_tooltip class' do
-          expect(link_to_label(label, tooltip: false)).not_to match %r{has_tooltip}
+        it 'does not include the has-tooltip class' do
+          expect(link_to_label(label, tooltip: false)).not_to match %r{has-tooltip}
         end
       end
     end
diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb
index 4c1d4a2d24c..94468abcbb3 100644
--- a/spec/lib/banzai/filter/label_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb
@@ -56,7 +56,7 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
   describe 'label span element' do
     it 'includes default classes' do
       doc = reference_filter("Label #{reference}")
-      expect(doc.css('a span').first.attr('class')).to eq 'label color-label has_tooltip'
+      expect(doc.css('a span').first.attr('class')).to eq 'label color-label has-tooltip'
     end
 
     it 'includes a style attribute' do
-- 
cgit v1.2.3


From 3b088fc5b53f03605484ebef1945b8839abe19de Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <zegerjan@gitlab.com>
Date: Mon, 21 Mar 2016 14:12:52 +0100
Subject: Minor improvements on IssuableActions

---
 .../controllers/projects/issues_controller_spec.rb | 27 ++++++++++++--------
 .../projects/merge_requests_controller_spec.rb     | 24 ++++++++++--------
 spec/requests/api/issues_spec.rb                   | 17 ++++++++-----
 spec/requests/api/merge_requests_spec.rb           | 29 ++++++++++++----------
 4 files changed, 56 insertions(+), 41 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 62643a755b7..86930e317ce 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1,7 +1,7 @@
 require('spec_helper')
 
 describe Projects::IssuesController do
-  let(:project) { create(:project) }
+  let(:project)   { create(:project) }
   let(:user)    { create(:user) }
   let(:issue)   { create(:issue, project: project) }
 
@@ -188,21 +188,26 @@ describe Projects::IssuesController do
   end
 
   describe "DELETE #destroy" do
-    it "rejects a developer to destory an issue" do
-      delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
-      expect(response.status).to eq 404
+    context "when the user is a developer" do
+      before { sign_in(user) }
+      it "rejects a developer to destroy an issue" do
+        delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
+        expect(response.status).to eq(404)
+      end
     end
 
-    context "user is an admin" do
-      before do
-        user.admin = true
-        user.save
-      end
+    context "when the user is owner" do
+      let(:owner)     { create(:user) }
+      let(:namespace) { create(:namespace, owner: owner) }
+      let(:project)   { create(:project, namespace: namespace) }
+
+      before { sign_in owner }
 
-      it "lets an admin delete an issue" do
+      it "deletes the issue" do
         delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
 
-        expect(response.status).to eq 302
+        expect(response.status).to eq(302)
+        expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./).now
       end
     end
   end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index af154fadb6f..c5b034dc064 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -123,7 +123,7 @@ describe Projects::MergeRequestsController do
     end
   end
 
-  describe '#index' do
+  describe 'GET #index' do
     def get_merge_requests
       get :index,
           namespace_id: project.namespace.to_param,
@@ -157,23 +157,25 @@ describe Projects::MergeRequestsController do
     end
   end
 
-  describe "#destroy" do
-    it "lets mere mortals not access this endpoint" do
+  describe "DELETE #destroy" do
+    it "denies access to users unless they're admin or project owner" do
       delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
 
-      expect(response.status).to eq 404
+      expect(response.status).to eq(404)
     end
 
-    context "user is an admin or owner" do
-      before do
-        user.admin = true
-        user.save
-      end
+    context "when the user is owner" do
+      let(:owner)     { create(:user) }
+      let(:namespace) { create(:namespace, owner: owner) }
+      let(:project)   { create(:project, namespace: namespace) }
+
+      before { sign_in owner }
 
-      it "lets an admin or owner delete an issue" do
+      it "deletes the merge request" do
         delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
 
-        expect(response.status).to be 302
+        expect(response.status).to eq(302)
+        expect(controller).to set_flash[:notice].to(/The merge request was successfully deleted\./).now
       end
     end
   end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 6298771925e..ce55cb7b0ae 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -469,20 +469,25 @@ describe API::API, api: true  do
   end
 
   describe "DELETE /projects/:id/issues/:issue_id" do
-    it "should reject a non member from deleting an issue" do
+    it "rejects a non member from deleting an issue" do
       delete api("/projects/#{project.id}/issues/#{issue.id}", non_member)
       expect(response.status).to be(403)
     end
 
-    it "should reject a developer from deleting an issue" do
+    it "rejects a developer from deleting an issue" do
       delete api("/projects/#{project.id}/issues/#{issue.id}", author)
       expect(response.status).to be(403)
     end
 
-    it "deletes the issue if an admin requests it" do
-      delete api("/projects/#{project.id}/issues/#{issue.id}", admin)
-      expect(response.status).to eq(200)
-      expect(json_response['state']).to eq 'opened'
+    context "when the user is project owner" do
+      let(:owner)     { create(:user) }
+      let(:project)   { create(:project, namespace: owner.namespace) }
+
+      it "deletes the issue if an admin requests it" do
+        delete api("/projects/#{project.id}/issues/#{issue.id}", owner)
+        expect(response.status).to eq(200)
+        expect(json_response['state']).to eq 'opened'
+      end
     end
   end
 end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 43e6a59d1ee..ae34684b932 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -317,23 +317,26 @@ describe API::API, api: true  do
     end
   end
 
-  describe "DELETE /projects/:id/merge_request/:merge_request_id" do
-    it "owners can destroy" do
-      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+  describe "DELETE /projects/:id/merge_requests/:merge_request_id" do
+    context "when the user is developer" do
+      let(:developer) { create(:user) }
 
-      expect(response.status).to eq(200)
-    end
-
-    it "let's Admins and owners delete a merge request" do
-      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", admin)
+      before do
+        project.team << [developer, :developer]
+      end
 
-      expect(response.status).to eq(200)
-      expect(json_response['id']).to eq merge_request.id
+      it "denies the deletion of the merge request" do
+        delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", developer)
+        expect(response.status).to be(403)
+      end
     end
 
-    it "rejects removal from other users" do
-      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", non_member)
-      expect(response.status).to eq(404)
+    context "when the user is project owner" do
+      it "destroys the merge request owners can destroy" do
+        delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+        expect(response.status).to eq(200)
+      end
     end
   end
 
-- 
cgit v1.2.3


From d28a587e82836d28524339586e1b6c1546a4bff5 Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <zegerjan@gitlab.com>
Date: Mon, 21 Mar 2016 17:12:03 +0100
Subject: Fix typos and denting

---
 spec/controllers/projects/issues_controller_spec.rb |  4 ++--
 spec/requests/api/merge_requests_spec.rb            | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 86930e317ce..53d27bb636d 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1,7 +1,7 @@
 require('spec_helper')
 
 describe Projects::IssuesController do
-  let(:project)   { create(:project) }
+  let(:project) { create(:project) }
   let(:user)    { create(:user) }
   let(:issue)   { create(:issue, project: project) }
 
@@ -201,7 +201,7 @@ describe Projects::IssuesController do
       let(:namespace) { create(:namespace, owner: owner) }
       let(:project)   { create(:project, namespace: namespace) }
 
-      before { sign_in owner }
+      before { sign_in(owner) }
 
       it "deletes the issue" do
         delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index ae34684b932..c9175a4d6eb 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -2,17 +2,17 @@ require "spec_helper"
 
 describe API::API, api: true  do
   include ApiHelpers
-  let(:base_time) { Time.now }
-  let(:user)      { create(:user) }
-  let(:admin)     { create(:user, :admin) }
+  let(:base_time)   { Time.now }
+  let(:user)        { create(:user) }
+  let(:admin)       { create(:user, :admin) }
   let(:non_member)  { create(:user) }
-  let!(:project)  { create(:project, creator_id: user.id, namespace: user.namespace) }
+  let!(:project)    { create(:project, creator_id: user.id, namespace: user.namespace) }
   let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
   let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) }
   let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds) }
-  let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
-  let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
-  let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
+  let!(:note)       { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
+  let!(:note2)      { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
+  let(:milestone)   { create(:milestone, title: '1.0.0', project: project) }
 
   before do
     project.team << [user, :reporters]
-- 
cgit v1.2.3