Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into awardables

# Conflicts:
#	app/controllers/projects/merge_requests_controller.rb
#	app/models/note.rb
#	db/schema.rb
#	spec/models/note_spec.rb

13 files changed, 163 insertions, 41 deletions

diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 1ce22feed5c..d84f3e998f5 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -20,6 +20,15 @@ describe ApplicationSetting, models: true do
     it { is_expected.to allow_value(https).for(:after_sign_out_path) }
     it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) }
 
+    describe 'disabled_oauth_sign_in_sources validations' do
+      before do
+        allow(Devise).to receive(:omniauth_providers).and_return([:github])
+      end
+
+      it { is_expected.to allow_value(['github']).for(:disabled_oauth_sign_in_sources) }
+      it { is_expected.not_to allow_value(['test']).for(:disabled_oauth_sign_in_sources) }
+    end
+
     it { is_expected.to validate_presence_of(:max_attachment_size) }
 
     it do
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index ad47e338a33..ccb100cd96f 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -56,7 +56,7 @@ describe Commit, models: true do
     end
 
     it "does not truncates a message with a newline after 80 but less 100 characters" do
-      message =<<eos
+      message = <<eos
 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit.
 Vivamus egestas lacinia lacus, sed rutrum mauris.
 eos
diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb
index e31fdb0bffb..b7fc5a92497 100644
--- a/spec/models/concerns/subscribable_spec.rb
+++ b/spec/models/concerns/subscribable_spec.rb
@@ -44,6 +44,16 @@ describe Subscribable, 'Subscribable' do
     end
   end
 
+  describe '#subscribe' do
+    it 'subscribes the given user' do
+      expect(resource.subscribed?(user)).to be_falsey
+
+      resource.subscribe(user)
+
+      expect(resource.subscribed?(user)).to be_truthy
+    end
+  end
+
   describe '#unsubscribe' do
     it 'unsubscribes the given current user' do
       resource.subscriptions.create(user: user, subscribed: true)
diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb
index f800f415bd2..534e1b4f128 100644
--- a/spec/models/hooks/service_hook_spec.rb
+++ b/spec/models/hooks/service_hook_spec.rb
@@ -34,14 +34,14 @@ describe ServiceHook, models: true do
     it "POSTs to the webhook URL" do
       @service_hook.execute(@data)
       expect(WebMock).to have_requested(:post, @service_hook.url).with(
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' }
       ).once
     end
 
     it "POSTs the data as JSON" do
       @service_hook.execute(@data)
       expect(WebMock).to have_requested(:post, @service_hook.url).with(
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' }
       ).once
     end
 
diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb
index 56a9fbe9720..4078b9e4ff5 100644
--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -33,7 +33,7 @@ describe SystemHook, models: true do
       Projects::CreateService.new(user, name: 'empty').execute
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /project_create/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -42,7 +42,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /project_destroy/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -51,7 +51,7 @@ describe SystemHook, models: true do
       
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_create/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -60,7 +60,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_destroy/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -69,7 +69,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_add_to_team/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -79,7 +79,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_remove_from_team/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -88,7 +88,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /group_create/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -97,7 +97,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /group_destroy/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -106,7 +106,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_add_to_group/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -116,7 +116,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_remove_from_group/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
   end
diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb
new file mode 100644
index 00000000000..7c29bef54e4
--- /dev/null
+++ b/spec/models/legacy_diff_note_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+describe LegacyDiffNote, models: true do
+  describe "Commit diff line notes" do
+    let!(:note) { create(:note_on_commit_diff, note: "+1 from me") }
+    let!(:commit) { note.noteable }
+
+    it "should save a valid note" do
+      expect(note.commit_id).to eq(commit.id)
+      expect(note.noteable.id).to eq(commit.id)
+    end
+
+    it "should be recognized by #legacy_diff_note?" do
+      expect(note).to be_legacy_diff_note
+    end
+  end
+
+  describe '#active?' do
+    it 'is always true when the note has no associated diff' do
+      note = build(:note_on_merge_request_diff)
+
+      expect(note).to receive(:diff).and_return(nil)
+
+      expect(note).to be_active
+    end
+
+    it 'is never true when the note has no noteable associated' do
+      note = build(:note_on_merge_request_diff)
+
+      expect(note).to receive(:diff).and_return(double)
+      expect(note).to receive(:noteable).and_return(nil)
+
+      expect(note).not_to be_active
+    end
+
+    it 'returns the memoized value if defined' do
+      note = build(:note_on_merge_request_diff)
+
+      note.instance_variable_set(:@active, 'foo')
+      expect(note).not_to receive(:find_noteable_diff)
+
+      expect(note.active?).to eq 'foo'
+    end
+
+    context 'for a merge request noteable' do
+      it 'is false when noteable has no matching diff' do
+        merge = build_stubbed(:merge_request, :simple)
+        note = build(:note_on_merge_request_diff, noteable: merge)
+
+        allow(note).to receive(:diff).and_return(double)
+        expect(note).to receive(:find_noteable_diff).and_return(nil)
+
+        expect(note).not_to be_active
+      end
+
+      it 'is true when noteable has a matching diff' do
+        merge = create(:merge_request, :simple)
+
+        # Generate a real line_code value so we know it will match. We use a
+        # random line from a random diff just for funsies.
+        diff = merge.diffs.to_a.sample
+        line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample
+        code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
+
+        # We're persisting in order to trigger the set_diff callback
+        note = create(:note_on_merge_request_diff, noteable: merge, line_code: code)
+
+        # Make sure we don't get a false positive from a guard clause
+        expect(note).to receive(:find_noteable_diff).and_call_original
+        expect(note).to be_active
+      end
+    end
+  end
+end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index c8578749b21..9eef08c6d00 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -64,7 +64,13 @@ describe MergeRequest, models: true do
 
   describe '#target_sha' do
     context 'when the target branch does not exist anymore' do
-      subject { create(:merge_request).tap { |mr| mr.update_attribute(:target_branch, 'deleted') } }
+      let(:project) { create(:project) }
+
+      subject { create(:merge_request, source_project: project, target_project: project) }
+
+      before do
+        project.repository.raw_repository.delete_branch(subject.target_branch)
+      end
 
       it 'returns nil' do
         expect(subject.target_sha).to be_nil
@@ -289,7 +295,12 @@ describe MergeRequest, models: true do
     let(:fork_project) { create(:project, forked_from_project: project) }
 
     context 'when the target branch does not exist anymore' do
-      subject { create(:merge_request).tap { |mr| mr.update_attribute(:target_branch, 'deleted') } }
+      subject { create(:merge_request, source_project: project, target_project: project) }
+
+      before do
+        project.repository.raw_repository.delete_branch(subject.target_branch)
+        subject.reload
+      end
 
       it 'does not crash' do
         expect{ subject.diverged_commits_count }.not_to raise_error
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 247a9fa9910..1e18c788b50 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -204,4 +204,37 @@ describe Milestone, models: true do
         to eq([milestone])
     end
   end
+
+  describe '.upcoming_ids_by_projects' do
+    let(:project_1) { create(:empty_project) }
+    let(:project_2) { create(:empty_project) }
+    let(:project_3) { create(:empty_project) }
+    let(:projects) { [project_1, project_2, project_3] }
+
+    let!(:past_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now - 1.day) }
+    let!(:current_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 1.day) }
+    let!(:future_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 2.days) }
+
+    let!(:past_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now - 1.day) }
+    let!(:closed_milestone_project_2) { create(:milestone, :closed, project: project_2, due_date: Time.now + 1.day) }
+    let!(:current_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now + 2.days) }
+
+    let!(:past_milestone_project_3) { create(:milestone, project: project_3, due_date: Time.now - 1.day) }
+
+    # The call to `#try` is because this returns a relation with a Postgres DB,
+    # and an array of IDs with a MySQL DB.
+    let(:milestone_ids) { Milestone.upcoming_ids_by_projects(projects).map { |id| id.try(:id) || id } }
+
+    it 'returns the next upcoming open milestone ID for each project' do
+      expect(milestone_ids).to contain_exactly(current_milestone_project_1.id, current_milestone_project_2.id)
+    end
+
+    context 'when the projects have no open upcoming milestones' do
+      let(:projects) { [project_3] }
+
+      it 'returns no results' do
+        expect(milestone_ids).to be_empty
+      end
+    end
+  end
 end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index b479f1c2f1a..4448eefad00 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -34,24 +34,6 @@ describe Note, models: true do
     end
   end
 
-  describe "Commit diff line notes" do
-    let!(:note) { create(:note_on_commit_diff, note: "+1 from me") }
-    let!(:commit) { note.noteable }
-
-    it "should save a valid note" do
-      expect(note.commit_id).to eq(commit.id)
-      expect(note.noteable.id).to eq(commit.id)
-    end
-
-    it "should be recognized by #for_diff_line?" do
-      expect(note).to be_for_diff_line
-    end
-
-    it "should be recognized by #for_commit_diff_line?" do
-      expect(note).to be_for_commit_diff_line
-    end
-  end
-
   describe 'authorization' do
     before do
       @p1 = create(:project)
diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb
index f648cbe2dee..0f8889bdf3c 100644
--- a/spec/models/project_services/slack_service/issue_message_spec.rb
+++ b/spec/models/project_services/slack_service/issue_message_spec.rb
@@ -25,7 +25,7 @@ describe SlackService::IssueMessage, models: true do
     }
   end
 
-  let(:color) { '#345' }
+  let(:color) { '#C95823' }
 
   context '#initialize' do
     before do
@@ -40,10 +40,11 @@ describe SlackService::IssueMessage, models: true do
   context 'open' do
     it 'returns a message regarding opening of issues' do
       expect(subject.pretext).to eq(
-        'Test User opened <url|issue #100> in <somewhere.com|project_name>: '\
-        '*Issue title*')
+        '<somewhere.com|[project_name>] Issue opened by Test User')
       expect(subject.attachments).to eq([
         {
+          title: "#100 Issue title",
+          title_link: "url",
           text: "issue description",
           color: color,
         }
@@ -56,10 +57,10 @@ describe SlackService::IssueMessage, models: true do
       args[:object_attributes][:action] = 'close'
       args[:object_attributes][:state] = 'closed'
     end
+
     it 'returns a message regarding closing of issues' do
       expect(subject.pretext). to eq(
-        'Test User closed <url|issue #100> in <somewhere.com|project_name>: '\
-        '*Issue title*')
+        '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by Test User')
       expect(subject.attachments).to be_empty
     end
   end
diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb
index d37590cab75..379c3e1219c 100644
--- a/spec/models/project_services/slack_service/note_message_spec.rb
+++ b/spec/models/project_services/slack_service/note_message_spec.rb
@@ -65,7 +65,7 @@ describe SlackService::NoteMessage, models: true do
       expect(message.pretext).to eq("Test User commented on " \
       "<url|merge request !30> in <somewhere.com|project_name>: " \
       "*merge request title*")
-      expected_attachments =  [
+      expected_attachments = [
           {
               text: "comment on a merge request",
               color: color,
@@ -117,7 +117,7 @@ describe SlackService::NoteMessage, models: true do
       expect(message.pretext).to eq("Test User commented on " \
       "<url|snippet #5> in <somewhere.com|project_name>: " \
       "*snippet title*")
-      expected_attachments =  [
+      expected_attachments = [
           {
               text: "comment on a snippet",
               color: color,
diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb
index 532e3f013fd..91ebb612baa 100644
--- a/spec/models/project_wiki_spec.rb
+++ b/spec/models/project_wiki_spec.rb
@@ -38,7 +38,8 @@ describe ProjectWiki, models: true do
 
   describe "#wiki_base_path" do
     it "returns the wiki base path" do
-      wiki_base_path = "/#{project.path_with_namespace}/wikis"
+      wiki_base_path = "#{Gitlab.config.gitlab.relative_url_root}/#{project.path_with_namespace}/wikis"
+
       expect(subject.wiki_base_path).to eq(wiki_base_path)
     end
   end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index e170cc85a1e..77f745b2660 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -142,6 +142,7 @@ describe User, models: true do
   end
 
   describe '#confirm' do
+    before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) }
     let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') }
 
     it 'returns unconfirmed' do