Merge branch 'master-ce' into fix-environment-status-in-merge-request-widget

12 files changed, 154 insertions, 45 deletions

diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index 2adfc04deb8..3ce2398f1de 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -91,7 +91,14 @@ module EventsHelper
       words << "##{event.target_iid}" if event.target_iid
       words << "in"
     elsif event.target
-      words << "##{event.target_iid}:"
+      prefix =
+        if event.merge_request?
+          MergeRequest.reference_prefix
+        else
+          Issue.reference_prefix
+        end
+
+      words << "#{prefix}#{event.target_iid}:" if event.target_iid
       words << event.target.title if event.target.respond_to?(:title)
       words << "at"
     end
diff --git a/app/models/concerns/each_batch.rb b/app/models/concerns/each_batch.rb
index 8cf0b8b154d..6314b46a7e3 100644
--- a/app/models/concerns/each_batch.rb
+++ b/app/models/concerns/each_batch.rb
@@ -39,7 +39,15 @@ module EachBatch
     #
     # of - The number of rows to retrieve per batch.
     # column - The column to use for ordering the batches.
-    def each_batch(of: 1000, column: primary_key)
+    # order_hint - An optional column to append to the `ORDER BY id`
+    #   clause to help the query planner. PostgreSQL might perform badly
+    #   with a LIMIT 1 because the planner is guessing that scanning the
+    #   index in ID order will come across the desired row in less time
+    #   it will take the planner than using another index. The
+    #   order_hint does not affect the search results. For example,
+    #   `ORDER BY id ASC, updated_at ASC` means the same thing as `ORDER
+    #   BY id ASC`.
+    def each_batch(of: 1000, column: primary_key, order_hint: nil)
       unless column
         raise ArgumentError,
           'the column: argument must be set to a column name to use for ordering rows'
@@ -48,7 +56,9 @@ module EachBatch
       start = except(:select)
         .select(column)
         .reorder(column => :asc)
-        .take
+
+      start = start.order(order_hint) if order_hint
+      start = start.take
 
       return unless start
 
@@ -60,6 +70,9 @@ module EachBatch
           .select(column)
           .where(arel_table[column].gteq(start_id))
           .reorder(column => :asc)
+
+        stop = stop.order(order_hint) if order_hint
+        stop = stop
           .offset(of)
           .limit(1)
           .take
diff --git a/changelogs/unreleased/gt-use-merge-request-prefix-in-event-feed-title.yml b/changelogs/unreleased/gt-use-merge-request-prefix-in-event-feed-title.yml
new file mode 100644
index 00000000000..51af2807a03
--- /dev/null
+++ b/changelogs/unreleased/gt-use-merge-request-prefix-in-event-feed-title.yml
@@ -0,0 +1,5 @@
+---
+title: Use merge request prefix symbol in event feed title
+merge_request: 22449
+author: George Tsiolis
+type: changed
diff --git a/changelogs/unreleased/sh-fix-issue-52649.yml b/changelogs/unreleased/sh-fix-issue-52649.yml
new file mode 100644
index 00000000000..34b7f74a345
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-issue-52649.yml
@@ -0,0 +1,5 @@
+---
+title: Fix statement timeouts in RemoveRestrictedTodos migration
+merge_request: 22795
+author:
+type: other
diff --git a/db/post_migrate/20181107054254_remove_restricted_todos_again.rb b/db/post_migrate/20181107054254_remove_restricted_todos_again.rb
new file mode 100644
index 00000000000..644e0074c46
--- /dev/null
+++ b/db/post_migrate/20181107054254_remove_restricted_todos_again.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+# rescheduling of the revised RemoveRestrictedTodosWithCte background migration
+class RemoveRestrictedTodosAgain < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  disable_ddl_transaction!
+
+  MIGRATION = 'RemoveRestrictedTodos'.freeze
+  BATCH_SIZE = 1000
+  DELAY_INTERVAL = 5.minutes.to_i
+
+  class Project < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'projects'
+  end
+
+  def up
+    Project.where('EXISTS (SELECT 1 FROM todos WHERE todos.project_id = projects.id)')
+      .each_batch(of: BATCH_SIZE) do |batch, index|
+      range = batch.pluck('MIN(id)', 'MAX(id)').first
+
+      BackgroundMigrationWorker.perform_in(index * DELAY_INTERVAL, MIGRATION, range)
+    end
+  end
+
+  def down
+    # nothing to do
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index fbe09cf1611..df3067fae30 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181106135939) do
+ActiveRecord::Schema.define(version: 20181107054254) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/doc/user/markdown.md b/doc/user/markdown.md
index 96a509c4b21..93aa41e9a98 100644
--- a/doc/user/markdown.md
+++ b/doc/user/markdown.md
@@ -774,7 +774,7 @@ These details <em>will</em> remain <strong>hidden</strong> until expanded.
 </details>
 </p>
 
-**Note:** Markdown inside these tags is supported, as long as you have a blank link after the `</summary>` tag and before the `</details>` tag, as shown in the example.  _Redcarpet does not support Markdown inside these tags.  You can work around this by using HTML, for example you can use `<pre><code>` tags instead of [code fences](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#code-and-syntax-highlighting)._
+**Note:** Markdown inside these tags is supported, as long as you have a blank line after the `</summary>` tag and before the `</details>` tag, as shown in the example.  _Redcarpet does not support Markdown inside these tags.  You can work around this by using HTML, for example you can use `<pre><code>` tags instead of [code fences](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#code-and-syntax-highlighting)._
 
 ```html
 <details>
diff --git a/lib/gitlab/background_migration/remove_restricted_todos.rb b/lib/gitlab/background_migration/remove_restricted_todos.rb
index 9941c2fe6d9..47579d46c1b 100644
--- a/lib/gitlab/background_migration/remove_restricted_todos.rb
+++ b/lib/gitlab/background_migration/remove_restricted_todos.rb
@@ -67,7 +67,7 @@ module Gitlab
           .where('access_level >= ?', 20)
 
         confidential_issues = Issue.select(:id, :author_id).where(confidential: true, project_id: project_id)
-        confidential_issues.each_batch(of: 100) do |batch|
+        confidential_issues.each_batch(of: 100, order_hint: :confidential) do |batch|
           batch.each do |issue|
             assigned_users = IssueAssignee.select(:user_id).where(issue_id: issue.id)
 
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index 2c92458f777..9e59137a2c0 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -16,6 +16,11 @@ module Gitlab
       str.force_encoding(Encoding::UTF_8)
     end
 
+    # Append path to host, making sure there's one single / in between
+    def append_path(host, path)
+      "#{host.to_s.sub(%r{\/+$}, '')}/#{path.to_s.sub(%r{^\/+}, '')}"
+    end
+
     # A slugified version of the string, suitable for inclusion in URLs and
     # domain names. Rules:
     #
diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb
index 466e018d68c..8d0679e5699 100644
--- a/spec/helpers/events_helper_spec.rb
+++ b/spec/helpers/events_helper_spec.rb
@@ -2,18 +2,18 @@ require 'spec_helper'
 
 describe EventsHelper do
   describe '#event_commit_title' do
-    let(:message) { "foo & bar " + "A" * 70 + "\n" + "B" * 80 }
+    let(:message) { 'foo & bar ' + 'A' * 70 + '\n' + 'B' * 80 }
     subject { helper.event_commit_title(message) }
 
-    it "returns the first line, truncated to 70 chars" do
+    it 'returns the first line, truncated to 70 chars' do
       is_expected.to eq(message[0..66] + "...")
     end
 
-    it "is not html-safe" do
+    it 'is not html-safe' do
       is_expected.not_to be_a(ActiveSupport::SafeBuffer)
     end
 
-    it "handles empty strings" do
+    it 'handles empty strings' do
       expect(helper.event_commit_title("")).to eq("")
     end
 
@@ -22,7 +22,7 @@ describe EventsHelper do
     end
 
     it 'does not escape HTML entities' do
-      expect(helper.event_commit_title("foo & bar")).to eq("foo & bar")
+      expect(helper.event_commit_title('foo & bar')).to eq('foo & bar')
     end
   end
 
@@ -30,38 +30,54 @@ describe EventsHelper do
     let(:event) { create(:event) }
     let(:project) { create(:project, :public, :repository) }
 
-    it "returns project issue url" do
-      event.target = create(:issue)
+    context 'issue' do
+      before do
+        event.target = create(:issue)
+      end
 
-      expect(helper.event_feed_url(event)).to eq(project_issue_url(event.project, event.issue))
+      it 'returns the project issue url' do
+        expect(helper.event_feed_url(event)).to eq(project_issue_url(event.project, event.target))
+      end
+
+      it 'contains the project issue IID link' do
+        expect(helper.event_feed_title(event)).to include("##{event.target.iid}")
+      end
     end
 
-    it "returns project merge_request url" do
-      event.target = create(:merge_request)
+    context 'merge request' do
+      before do
+        event.target = create(:merge_request)
+      end
+
+      it 'returns the project merge request url' do
+        expect(helper.event_feed_url(event)).to eq(project_merge_request_url(event.project, event.target))
+      end
 
-      expect(helper.event_feed_url(event)).to eq(project_merge_request_url(event.project, event.merge_request))
+      it 'contains the project merge request IID link' do
+        expect(helper.event_feed_title(event)).to include("!#{event.target.iid}")
+      end
     end
 
-    it "returns project commit url" do
+    it 'returns project commit url' do
       event.target = create(:note_on_commit, project: project)
 
       expect(helper.event_feed_url(event)).to eq(project_commit_url(event.project, event.note_target))
     end
 
-    it "returns event note target url" do
+    it 'returns event note target url' do
       event.target = create(:note)
 
       expect(helper.event_feed_url(event)).to eq(event_note_target_url(event))
     end
 
-    it "returns project url" do
+    it 'returns project url' do
       event.project = project
       event.action = 1
 
       expect(helper.event_feed_url(event)).to eq(project_url(event.project))
     end
 
-    it "returns push event feed url" do
+    it 'returns push event feed url' do
       event = create(:push_event)
       create(:push_event_payload, event: event, action: :pushed)
 
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 4ba99009855..ad2c9d7f2af 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Utils do
   delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string,
-   :bytes_to_megabytes, to: :described_class
+   :bytes_to_megabytes, :append_path, to: :described_class
 
   describe '.slugify' do
     {
@@ -106,4 +106,25 @@ describe Gitlab::Utils do
       expect(bytes_to_megabytes(bytes)).to eq(1)
     end
   end
+
+  describe '.append_path' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:host, :path, :result) do
+      'http://test/'  | '/foo/bar'  |  'http://test/foo/bar'
+      'http://test/'  | '//foo/bar' |  'http://test/foo/bar'
+      'http://test//' | '/foo/bar'  |  'http://test/foo/bar'
+      'http://test'   | 'foo/bar'   |  'http://test/foo/bar'
+      'http://test//' | ''          |  'http://test/'
+      'http://test//' | nil         |  'http://test/'
+      ''              | '/foo/bar'  |  '/foo/bar'
+      nil             | '/foo/bar'  |  '/foo/bar'
+    end
+
+    with_them do
+      it 'makes sure there is only one slash as path separator' do
+        expect(append_path(host, path)).to eq(result)
+      end
+    end
+  end
 end
diff --git a/spec/models/concerns/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb
index 951690a217b..17224c09693 100644
--- a/spec/models/concerns/each_batch_spec.rb
+++ b/spec/models/concerns/each_batch_spec.rb
@@ -14,40 +14,45 @@ describe EachBatch do
       5.times { create(:user, updated_at: 1.day.ago) }
     end
 
-    it 'yields an ActiveRecord::Relation when a block is given' do
-      model.each_batch do |relation|
-        expect(relation).to be_a_kind_of(ActiveRecord::Relation)
+    shared_examples 'each_batch handling' do |kwargs|
+      it 'yields an ActiveRecord::Relation when a block is given' do
+        model.each_batch(kwargs) do |relation|
+          expect(relation).to be_a_kind_of(ActiveRecord::Relation)
+        end
       end
-    end
 
-    it 'yields a batch index as the second argument' do
-      model.each_batch do |_, index|
-        expect(index).to eq(1)
+      it 'yields a batch index as the second argument' do
+        model.each_batch(kwargs) do |_, index|
+          expect(index).to eq(1)
+        end
       end
-    end
 
-    it 'accepts a custom batch size' do
-      amount = 0
+      it 'accepts a custom batch size' do
+        amount = 0
 
-      model.each_batch(of: 1) { amount += 1 }
+        model.each_batch(kwargs.merge({ of: 1 })) { amount += 1 }
 
-      expect(amount).to eq(5)
-    end
+        expect(amount).to eq(5)
+      end
 
-    it 'does not include ORDER BYs in the yielded relations' do
-      model.each_batch do |relation|
-        expect(relation.to_sql).not_to include('ORDER BY')
+      it 'does not include ORDER BYs in the yielded relations' do
+        model.each_batch do |relation|
+          expect(relation.to_sql).not_to include('ORDER BY')
+        end
       end
-    end
 
-    it 'allows updating of the yielded relations' do
-      time = Time.now
+      it 'allows updating of the yielded relations' do
+        time = Time.now
 
-      model.each_batch do |relation|
-        relation.update_all(updated_at: time)
-      end
+        model.each_batch do |relation|
+          relation.update_all(updated_at: time)
+        end
 
-      expect(model.where(updated_at: time).count).to eq(5)
+        expect(model.where(updated_at: time).count).to eq(5)
+      end
     end
+
+    it_behaves_like 'each_batch handling', {}
+    it_behaves_like 'each_batch handling', { order_hint: :updated_at }
   end
 end