Merge branch 'master' into bvl-group-trees

4 files changed, 109 insertions, 65 deletions

diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 40bbb10eaac..129dfa07f15 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -178,57 +178,59 @@ describe CacheMarkdownField do
     end
   end
 
-  describe '#refresh_markdown_cache!' do
+  describe '#refresh_markdown_cache' do
     before do
       thing.foo = updated_markdown
     end
 
-    context 'do_update: false' do
-      it 'fills all html fields' do
-        thing.refresh_markdown_cache!
+    it 'fills all html fields' do
+      thing.refresh_markdown_cache
 
-        expect(thing.foo_html).to eq(updated_html)
-        expect(thing.foo_html_changed?).to be_truthy
-        expect(thing.baz_html_changed?).to be_truthy
-      end
+      expect(thing.foo_html).to eq(updated_html)
+      expect(thing.foo_html_changed?).to be_truthy
+      expect(thing.baz_html_changed?).to be_truthy
+    end
 
-      it 'does not save the result' do
-        expect(thing).not_to receive(:update_columns)
+    it 'does not save the result' do
+      expect(thing).not_to receive(:update_columns)
 
-        thing.refresh_markdown_cache!
-      end
+      thing.refresh_markdown_cache
+    end
 
-      it 'updates the markdown cache version' do
-        thing.cached_markdown_version = nil
-        thing.refresh_markdown_cache!
+    it 'updates the markdown cache version' do
+      thing.cached_markdown_version = nil
+      thing.refresh_markdown_cache
 
-        expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION)
-      end
+      expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION)
     end
+  end
 
-    context 'do_update: true' do
-      it 'fills all html fields' do
-        thing.refresh_markdown_cache!(do_update: true)
+  describe '#refresh_markdown_cache!' do
+    before do
+      thing.foo = updated_markdown
+    end
 
-        expect(thing.foo_html).to eq(updated_html)
-        expect(thing.foo_html_changed?).to be_truthy
-        expect(thing.baz_html_changed?).to be_truthy
-      end
+    it 'fills all html fields' do
+      thing.refresh_markdown_cache!
 
-      it 'skips saving if not persisted' do
-        expect(thing).to receive(:persisted?).and_return(false)
-        expect(thing).not_to receive(:update_columns)
+      expect(thing.foo_html).to eq(updated_html)
+      expect(thing.foo_html_changed?).to be_truthy
+      expect(thing.baz_html_changed?).to be_truthy
+    end
 
-        thing.refresh_markdown_cache!(do_update: true)
-      end
+    it 'skips saving if not persisted' do
+      expect(thing).to receive(:persisted?).and_return(false)
+      expect(thing).not_to receive(:update_columns)
 
-      it 'saves the changes using #update_columns' do
-        expect(thing).to receive(:persisted?).and_return(true)
-        expect(thing).to receive(:update_columns)
-          .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
+      thing.refresh_markdown_cache!
+    end
 
-        thing.refresh_markdown_cache!(do_update: true)
-      end
+    it 'saves the changes using #update_columns' do
+      expect(thing).to receive(:persisted?).and_return(true)
+      expect(thing).to receive(:update_columns)
+        .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
+
+      thing.refresh_markdown_cache!
     end
   end
 
diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb
index a38f2553eb1..6866b43432c 100644
--- a/spec/models/concerns/has_status_spec.rb
+++ b/spec/models/concerns/has_status_spec.rb
@@ -231,6 +231,18 @@ describe HasStatus do
       end
     end
 
+    describe '.alive' do
+      subject { CommitStatus.alive }
+
+      %i[running pending created].each do |status|
+        it_behaves_like 'containing the job', status
+      end
+
+      %i[failed success].each do |status|
+        it_behaves_like 'not containing the job', status
+      end
+    end
+
     describe '.created_or_pending' do
       subject { CommitStatus.created_or_pending }
 
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index fb5fb7daaab..ba57301a3c9 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Issuable do
   let(:issuable_class) { Issue }
-  let(:issue) { create(:issue) }
+  let(:issue) { create(:issue, title: 'An issue', description: 'A description') }
   let(:user) { create(:user) }
 
   describe "Associations" do
@@ -264,55 +264,75 @@ describe Issuable do
     end
   end
 
-  describe "#to_hook_data" do
-    let(:data) { issue.to_hook_data(user) }
-    let(:project) { issue.project }
-
-    it "returns correct hook data" do
-      expect(data[:object_kind]).to eq("issue")
-      expect(data[:user]).to eq(user.hook_attrs)
-      expect(data[:object_attributes]).to eq(issue.hook_attrs)
-      expect(data).not_to have_key(:assignee)
-    end
+  describe '#to_hook_data' do
+    context 'labels are updated' do
+      let(:labels) { create_list(:label, 2) }
 
-    context "issue is assigned" do
       before do
-        issue.assignees << user
+        issue.update(labels: [labels[1]])
       end
 
-      it "returns correct hook data" do
-        expect(data[:assignees].first).to eq(user.hook_attrs)
+      it 'delegates to Gitlab::HookData::IssuableBuilder#build' do
+        builder = double
+
+        expect(Gitlab::HookData::IssuableBuilder)
+          .to receive(:new).with(issue).and_return(builder)
+        expect(builder).to receive(:build).with(
+          user: user,
+          changes: hash_including(
+            'labels' => [[labels[0].hook_attrs], [labels[1].hook_attrs]]
+          ))
+
+        issue.to_hook_data(user, old_labels: [labels[0]])
       end
     end
 
-    context "merge_request is assigned" do
-      let(:merge_request) { create(:merge_request) }
-      let(:data) { merge_request.to_hook_data(user) }
+    context 'issue is assigned' do
+      let(:user2) { create(:user) }
 
       before do
-        merge_request.update_attribute(:assignee, user)
+        issue.assignees << user << user2
       end
 
-      it "returns correct hook data" do
-        expect(data[:object_attributes]['assignee_id']).to eq(user.id)
-        expect(data[:assignee]).to eq(user.hook_attrs)
+      it 'delegates to Gitlab::HookData::IssuableBuilder#build' do
+        builder = double
+
+        expect(Gitlab::HookData::IssuableBuilder)
+          .to receive(:new).with(issue).and_return(builder)
+        expect(builder).to receive(:build).with(
+          user: user,
+          changes: hash_including(
+            'assignees' => [[user.hook_attrs], [user.hook_attrs, user2.hook_attrs]]
+          ))
+
+        issue.to_hook_data(user, old_assignees: [user])
       end
     end
 
-    context 'issue has labels' do
-      let(:labels) { [create(:label), create(:label)] }
+    context 'merge_request is assigned' do
+      let(:merge_request) { create(:merge_request) }
+      let(:user2) { create(:user) }
 
       before do
-        issue.update_attribute(:labels, labels)
+        merge_request.update(assignee: user)
+        merge_request.update(assignee: user2)
       end
 
-      it 'includes labels in the hook data' do
-        expect(data[:labels]).to eq(labels.map(&:hook_attrs))
+      it 'delegates to Gitlab::HookData::IssuableBuilder#build' do
+        builder = double
+
+        expect(Gitlab::HookData::IssuableBuilder)
+          .to receive(:new).with(merge_request).and_return(builder)
+        expect(builder).to receive(:build).with(
+          user: user,
+          changes: hash_including(
+            'assignee_id' => [user.id, user2.id],
+            'assignee' => [user.hook_attrs, user2.hook_attrs]
+          ))
+
+        merge_request.to_hook_data(user, old_assignees: [user])
       end
     end
-
-    include_examples 'project hook data'
-    include_examples 'deprecated repository hook data'
   end
 
   describe '#labels_array' do
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index b463d12e448..ab8773b7ede 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -12,6 +12,16 @@ describe Group, 'Routable' do
     it { is_expected.to have_many(:redirect_routes).dependent(:destroy) }
   end
 
+  describe 'GitLab read-only instance' do
+    it 'does not save route if route is not present' do
+      group.route.path = ''
+      allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+      expect(group).to receive(:update_route).and_call_original
+
+      expect { group.full_path }.to change { Route.count }.by(0)
+    end
+  end
+
   describe 'Callbacks' do
     it 'creates route record on create' do
       expect(group.route.path).to eq(group.path)