From f1bbd8119db3e494e662cd219a9882e5e17bf4bc Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 2 Dec 2015 22:03:43 -0200
Subject: [PATCH 01/55] Style warning about mentioning many people in a comment

---
 CHANGELOG                                |  1 +
 app/views/projects/_md_preview.html.haml | 18 +++++++++---------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index db812796b69..89d3f854ae1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ v 8.3.0 (unreleased)
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
   - Add ignore whitespace change option to commit view
   - Fire update hook from GitLab
+  - Style warning about mentioning many people in a comment
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)
diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml
index 8218cf11201..c4d3c9cc30a 100644
--- a/app/views/projects/_md_preview.html.haml
+++ b/app/views/projects/_md_preview.html.haml
@@ -8,17 +8,17 @@
         %a.js-md-preview-button(href="#md-preview-holder" tabindex="-1")
           Preview
 
-    - if defined?(referenced_users) && referenced_users
-      %span.referenced-users.pull-left.hide
-        = icon('exclamation-triangle')
-        You are about to add
-        %strong
-          %span.js-referenced-users-count 0
-          people
-        to the discussion. Proceed with caution.
-
   %div
     .md-write-holder
       = yield
     .md.md-preview-holder.hide
       .js-md-preview{class: (preview_class if defined?(preview_class))}
+
+  - if defined?(referenced_users) && referenced_users
+    %span.referenced-users.pull-left.hide
+      = icon('exclamation-triangle')
+      You are about to add
+      %strong
+        %span.js-referenced-users-count 0
+        people
+      to the discussion. Proceed with caution.

From 9cf67f72a581d4648b2b02b53403dd4318e4f1e9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Tue, 1 Dec 2015 12:00:11 +0100
Subject: [PATCH 02/55] Add validator for award-emoji note

---
 app/models/note.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/models/note.rb b/app/models/note.rb
index 1c6345e735c..40b46b85da1 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -41,6 +41,7 @@ class Note < ActiveRecord::Base
 
   validates :note, :project, presence: true
   validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award }
+  validates :note, format: { with: /\A[-_+[:alnum:]]*\z/ }, if: -> (n){ n.is_award }
   validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
   # Attachments are deprecated and are handled by Markdown uploader
   validates :attachment, file_size: { maximum: :max_attachment_size }

From 22d87b74a9de5d603f101699d7a3665db9627037 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Tue, 1 Dec 2015 14:45:24 +0100
Subject: [PATCH 03/55] Support award-emoji notes only when it a comment for an
 issue

---
 app/services/notes/create_service.rb | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index dbff58dfb9c..20a3ba30755 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -5,9 +5,9 @@ module Notes
       note.author = current_user
       note.system = false
 
-      if contains_emoji_only?(params[:note])
+      if award_emoji_note?
         note.is_award = true
-        note.note = emoji_name(params[:note])
+        note.note = emoji_name
       end
 
       if note.save
@@ -34,12 +34,23 @@ module Notes
       note.project.execute_services(note_data, :note_hooks)
     end
 
-    def contains_emoji_only?(note)
-      note =~ /\A:[-_+[:alnum:]]*:\s?\z/
+    private
+
+    def award_emoji_note?
+      # We support award-emojis only in issue discussion
+      issue_comment? && contains_emoji_only?
     end
 
-    def emoji_name(note)
-      note.match(/\A:([-_+[:alnum:]]*):\s?/)[1]
+    def contains_emoji_only?
+      params[:note] =~ /\A:[-_+[:alnum:]]*:\s?\z/
+    end
+
+    def issue_comment?
+      params[:noteable_type] == 'Issue'
+    end
+
+    def emoji_name
+      params[:note].match(/\A:([-_+[:alnum:]]*):\s?/)[1]
     end
   end
 end

From ba08811d07cd1804b027b1a37a5278723cdbeb2c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 2 Dec 2015 08:48:21 +0100
Subject: [PATCH 04/55] Move note emoji-award implementation to note model
 (feature envy)

---
 app/models/note.rb                   | 33 ++++++++++++++++++++++++++++
 app/services/notes/create_service.rb | 24 --------------------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/app/models/note.rb b/app/models/note.rb
index 40b46b85da1..8acb2cf7582 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -72,6 +72,7 @@ class Note < ActiveRecord::Base
 
   serialize :st_diff
   before_create :set_diff, if: ->(n) { n.line_code.present? }
+  before_validation :set_award!
 
   class << self
     def discussions_from_notes(notes)
@@ -349,4 +350,36 @@ class Note < ActiveRecord::Base
   def editable?
     !system?
   end
+
+  # Checks if note is an award added from an issue comment.
+  #
+  # If note is an award, this method sets is_award to true,
+  # and changes note content to award-emoji name.
+  #
+  # Awards are only supported for issue comments.
+  #
+  # Method is executed as a before_validation callback.
+  #
+  def set_award!
+    return unless issue_comment? && contains_emoji_only?
+
+    self.is_award = true
+    self.note = award_emoji_name
+  end
+
+  private
+
+  def issue_comment?
+    noteable.kind_of?(Issue)
+  end
+
+  def contains_emoji_only?
+    (note =~ /\A:[-_+[:alnum:]]*:\s?\z/) ? true : false
+  end
+
+  def award_emoji_name
+    return nil unless contains_emoji_only?
+
+    note.match(/\A:([-_+[:alnum:]]*):\s?/)[1]
+  end
 end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index 20a3ba30755..a8486e6a5a1 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -5,11 +5,6 @@ module Notes
       note.author = current_user
       note.system = false
 
-      if award_emoji_note?
-        note.is_award = true
-        note.note = emoji_name
-      end
-
       if note.save
         notification_service.new_note(note)
 
@@ -33,24 +28,5 @@ module Notes
       note.project.execute_hooks(note_data, :note_hooks)
       note.project.execute_services(note_data, :note_hooks)
     end
-
-    private
-
-    def award_emoji_note?
-      # We support award-emojis only in issue discussion
-      issue_comment? && contains_emoji_only?
-    end
-
-    def contains_emoji_only?
-      params[:note] =~ /\A:[-_+[:alnum:]]*:\s?\z/
-    end
-
-    def issue_comment?
-      params[:noteable_type] == 'Issue'
-    end
-
-    def emoji_name
-      params[:note].match(/\A:([-_+[:alnum:]]*):\s?/)[1]
-    end
   end
 end

From c92b6e66dc3a5bb1b07d1561fa09e8fd051c1956 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 2 Dec 2015 09:36:11 +0100
Subject: [PATCH 05/55] Scroll to awards after adding emoji-award comment

This makes it more intuitive, as user can see that something actually
happened after adding emoji-only comment in long discussions.
---
 app/assets/javascripts/awards_handler.coffee | 7 ++++++-
 app/assets/javascripts/notes.js.coffee       | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee
index 09b48fe5572..96fd8f8773e 100644
--- a/app/assets/javascripts/awards_handler.coffee
+++ b/app/assets/javascripts/awards_handler.coffee
@@ -88,4 +88,9 @@ class @AwardsHandler
         callback.call()
 
   findEmojiIcon: (emoji) ->
-    $(".icon[data-emoji='" + emoji + "']")
\ No newline at end of file
+    $(".icon[data-emoji='" + emoji + "']")
+
+  scrollToAwards: ->
+    $('body, html').animate({
+      scrollTop: $('.awards').offset().top - 80
+    }, 200)
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 7de7632201d..797234e6d9c 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -122,6 +122,7 @@ class @Notes
 
     if note.award
       awards_handler.addAwardToEmojiBar(note.note, note.emoji_path)
+      awards_handler.scrollToAwards()
 
   ###
   Check if note does not exists on page

From bfce5d716835f07b98b6d26ccc121d3ac8322aa9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 2 Dec 2015 10:13:29 +0100
Subject: [PATCH 06/55] Render json message with errors if note didn't pass
 validation

---
 app/controllers/projects/notes_controller.rb | 24 ++++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 5ac18446aa7..a7ff5fcd09a 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -131,16 +131,20 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def render_note_json(note)
-    render json: {
-      id: note.id,
-      discussion_id: note.discussion_id,
-      html: note_to_html(note),
-      award: note.is_award,
-      emoji_path: note.is_award ? view_context.image_url(::AwardEmoji.path_to_emoji_image(note.note)) : "",
-      note: note.note,
-      discussion_html: note_to_discussion_html(note),
-      discussion_with_diff_html: note_to_discussion_with_diff_html(note)
-    }
+    if note.valid?
+      render json: {
+        id: note.id,
+        discussion_id: note.discussion_id,
+        html: note_to_html(note),
+        award: note.is_award,
+        emoji_path: note.is_award ? view_context.image_url(::AwardEmoji.path_to_emoji_image(note.note)) : "",
+        note: note.note,
+        discussion_html: note_to_discussion_html(note),
+        discussion_with_diff_html: note_to_discussion_with_diff_html(note)
+      }
+    else
+      render json: { invalid: true, errors: note.errors }
+    end
   end
 
   def authorize_admin_note!

From a527f5c27ff92d2ee7e2d5e78dc20b6d1d982aa0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 2 Dec 2015 10:51:46 +0100
Subject: [PATCH 07/55] Notify user when award-emoji comment is invalid

---
 app/assets/javascripts/notes.js.coffee       | 4 ++++
 app/controllers/projects/notes_controller.rb | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 797234e6d9c..af0d62c8495 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -111,6 +111,10 @@ class @Notes
   Note: for rendering inline notes use renderDiscussionNote
   ###
   renderNote: (note) ->
+    unless note.valid
+      alert('You have already used this award emoji !') if note.award
+      return
+
     # render note if it not present in loaded list
     # or skip if rendered
     if @isNewNote(note) && !note.award
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index a7ff5fcd09a..88b949a27ab 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -133,6 +133,7 @@ class Projects::NotesController < Projects::ApplicationController
   def render_note_json(note)
     if note.valid?
       render json: {
+        valid: true,
         id: note.id,
         discussion_id: note.discussion_id,
         html: note_to_html(note),
@@ -143,7 +144,11 @@ class Projects::NotesController < Projects::ApplicationController
         discussion_with_diff_html: note_to_discussion_with_diff_html(note)
       }
     else
-      render json: { invalid: true, errors: note.errors }
+      render json: {
+        valid: false,
+        award: note.is_award,
+        errors: note.errors
+      }
     end
   end
 

From 4676ba1f7c7b37498d26815c1fbe0e02c2ffaeb8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 2 Dec 2015 14:11:32 +0000
Subject: [PATCH 08/55] Add test for award-emoji being added as regular comment

---
 features/project/issues/award_emoji.feature  |  6 +++-
 features/steps/project/issues/award_emoji.rb | 31 ++++++++++++--------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/features/project/issues/award_emoji.feature b/features/project/issues/award_emoji.feature
index a9bc8ffb9bb..2609f129d07 100644
--- a/features/project/issues/award_emoji.feature
+++ b/features/project/issues/award_emoji.feature
@@ -11,4 +11,8 @@ Feature: Award Emoji
     And I click to emoji in the picker
     Then I have award added
     And I can remove it by clicking to icon
-    
\ No newline at end of file
+
+  @javascript
+  Scenario: I add award emoji using regular comment
+  Given I leave comment with a single emoji
+  Then I have award added
diff --git a/features/steps/project/issues/award_emoji.rb b/features/steps/project/issues/award_emoji.rb
index 8f7a45dec0e..325eaf2ea6a 100644
--- a/features/steps/project/issues/award_emoji.rb
+++ b/features/steps/project/issues/award_emoji.rb
@@ -9,33 +9,40 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps
   end
 
   step 'I click to emoji-picker' do
-    page.within ".awards-controls" do
-      page.find(".add-award").click
+    page.within '.awards-controls' do
+      page.find('.add-award').click
     end
   end
 
   step 'I click to emoji in the picker' do
-    page.within ".awards-menu" do
-      page.first("img").click
+    page.within '.awards-menu' do
+      page.first('img').click
     end
   end
 
   step 'I can remove it by clicking to icon' do
-    page.within ".awards" do
-      page.first(".award").click
-      expect(page).to_not have_selector ".award"
+    page.within '.awards' do
+      page.first('.award').click
+      expect(page).to_not have_selector '.award'
     end
   end
 
   step 'I have award added' do
-    page.within ".awards" do
-      expect(page).to have_selector ".award"
-      expect(page.find(".award .counter")).to have_content "1"
+    page.within '.awards' do
+      expect(page).to have_selector '.award'
+      expect(page.find('.award .counter')).to have_content '1'
     end
   end
 
   step 'project "Shop" has issue "Bugfix"' do
-    @project = Project.find_by(name: "Shop")
-    @issue = create(:issue, title: "Bugfix", project: project)
+    @project = Project.find_by(name: 'Shop')
+    @issue = create(:issue, title: 'Bugfix', project: project)
+  end
+
+  step 'I leave comment with a single emoji' do
+    page.within('.js-main-target-form') do
+      fill_in 'note[note]', with: ':smile:'
+      click_button 'Add Comment'
+    end
   end
 end

From 6fc1b948560b9e19ac5c1412dd2deb98987aefe8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 2 Dec 2015 18:39:12 +0100
Subject: [PATCH 09/55] Show flash message instead of alert when note is
 invalid

---
 app/assets/javascripts/notes.js.coffee | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index af0d62c8495..ea190fa8b76 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -112,7 +112,8 @@ class @Notes
   ###
   renderNote: (note) ->
     unless note.valid
-      alert('You have already used this award emoji !') if note.award
+      if note.award
+        new Flash('You have already used this award emoji !', 'alert')
       return
 
     # render note if it not present in loaded list

From 70a076c059482e5c26cb723b722bc865142460ea Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Thu, 3 Dec 2015 08:51:14 +0100
Subject: [PATCH 10/55] Add new features to javascript flash message

---
 app/assets/javascripts/flash.js.coffee      | 19 +++++++++++++------
 app/assets/stylesheets/framework/flash.scss | 10 ++++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/flash.js.coffee b/app/assets/javascripts/flash.js.coffee
index b39ab0c4475..5e34b49c20d 100644
--- a/app/assets/javascripts/flash.js.coffee
+++ b/app/assets/javascripts/flash.js.coffee
@@ -1,12 +1,19 @@
 class @Flash
   constructor: (message, type)->
-    flash = $(".flash-container")
-    flash.html("")
+    @flash = $(".flash-container")
+    @flash.html("")
 
-    $('<div/>',
+    innerDiv = $('<div/>',
       class: "flash-#{type}",
       text: message
-    ).appendTo(".flash-container")
+    )
+    innerDiv.appendTo(".flash-container")
 
-    flash.click -> $(@).fadeOut()
-    flash.show()
+    @flash.click -> $(@).fadeOut()
+    @flash.show()
+
+  pinToTop: ->
+    @flash.addClass('flash-pinned')
+
+  raise: ->
+    @flash.addClass('flash-raised')
diff --git a/app/assets/stylesheets/framework/flash.scss b/app/assets/stylesheets/framework/flash.scss
index 82eb50ad4be..1b723021d76 100644
--- a/app/assets/stylesheets/framework/flash.scss
+++ b/app/assets/stylesheets/framework/flash.scss
@@ -15,3 +15,13 @@
     @extend .alert-danger;
   }
 }
+
+.flash-pinned {
+  position: fixed;
+  top: 80px;
+  width: 80%;
+}
+
+.flash-raised {
+  z-index: 1000;
+}

From 554f4684622564fe496acb25cacb2daed3b9f3ac Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Thu, 3 Dec 2015 08:51:37 +0100
Subject: [PATCH 11/55] Pin flash message to top if award note is invalid

---
 app/assets/javascripts/notes.js.coffee | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index ea190fa8b76..8eacd05d050 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -113,7 +113,9 @@ class @Notes
   renderNote: (note) ->
     unless note.valid
       if note.award
-        new Flash('You have already used this award emoji !', 'alert')
+        flash = new Flash('You have already used this award emoji !', 'alert')
+        flash.pinToTop()
+        flash.raise()
       return
 
     # render note if it not present in loaded list

From 2b577551177c631d229eca6844520e29478085f8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Thu, 3 Dec 2015 09:30:30 +0100
Subject: [PATCH 12/55] Add feature test for emoji-only diff notes

This specs is related to bug described in #3734 (award-emojis).
---
 features/project/commits/diff_comments.feature |  6 ++++++
 features/steps/shared/diff_note.rb             | 17 +++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/features/project/commits/diff_comments.feature b/features/project/commits/diff_comments.feature
index 4a2b870e082..d6e0c84537e 100644
--- a/features/project/commits/diff_comments.feature
+++ b/features/project/commits/diff_comments.feature
@@ -13,6 +13,12 @@ Feature: Project Commits Diff Comments
     Given I leave a diff comment like "Typo, please fix"
     Then I should see a diff comment saying "Typo, please fix"
 
+  @javascript
+  Scenario: I can add a diff comment with a single emoji
+    Given I open a diff comment form
+    And I write a diff comment like ":smile:"
+    Then I should see a diff comment with an emoji image
+
   @javascript
   Scenario: I get a temporary form for the first comment on a diff line
     Given I open a diff comment form
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index 72621911a37..dd466cde28d 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -87,6 +87,17 @@ module SharedDiffNote
     end
   end
 
+  step 'I write a diff comment like ":smile:"' do
+    page.within(diff_file_selector) do
+      click_diff_line(sample_commit.line_code)
+
+      page.within("form[rel$='#{sample_commit.line_code}']") do
+        fill_in 'note[note]', with: ':smile:'
+        click_button('Add Comment')
+      end
+    end
+  end
+
   step 'I submit the diff comment' do
     page.within(diff_file_selector) do
       click_button("Add Comment")
@@ -197,6 +208,12 @@ module SharedDiffNote
     end
   end
 
+  step 'I should see a diff comment with an emoji image' do
+    page.within("#{diff_file_selector} .note") do
+      expect(page).to have_xpath("//img[@alt=':smile:']")
+    end
+  end
+
   step 'I click side-by-side diff button' do
     find('#parallel-diff-btn').trigger('click')
   end

From f08f921d537c68ec0bbfb8a1ae7e905cded1bbad Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Thu, 3 Dec 2015 12:46:39 +0100
Subject: [PATCH 13/55] Support emoji awards also in merge requests

---
 app/models/note.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/models/note.rb b/app/models/note.rb
index 8acb2cf7582..30d15d93fed 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -361,7 +361,7 @@ class Note < ActiveRecord::Base
   # Method is executed as a before_validation callback.
   #
   def set_award!
-    return unless issue_comment? && contains_emoji_only?
+    return unless supports_awards? && contains_emoji_only?
 
     self.is_award = true
     self.note = award_emoji_name
@@ -369,8 +369,9 @@ class Note < ActiveRecord::Base
 
   private
 
-  def issue_comment?
-    noteable.kind_of?(Issue)
+  def supports_awards?
+    noteable.kind_of?(Issue) ||
+      noteable.is_a?(MergeRequest)
   end
 
   def contains_emoji_only?

From 7af67f619a9ce3dab0a66560bc57ccf606077779 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Thu, 3 Dec 2015 12:59:44 +0100
Subject: [PATCH 14/55] Combine new javascript Flash methods into one

---
 app/assets/javascripts/flash.js.coffee | 7 ++-----
 app/assets/javascripts/notes.js.coffee | 3 +--
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/flash.js.coffee b/app/assets/javascripts/flash.js.coffee
index 5e34b49c20d..9b59d4e57f7 100644
--- a/app/assets/javascripts/flash.js.coffee
+++ b/app/assets/javascripts/flash.js.coffee
@@ -12,8 +12,5 @@ class @Flash
     @flash.click -> $(@).fadeOut()
     @flash.show()
 
-  pinToTop: ->
-    @flash.addClass('flash-pinned')
-
-  raise: ->
-    @flash.addClass('flash-raised')
+  pin: ->
+    @flash.addClass('flash-pinned flash-raised')
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 8eacd05d050..4f559e86378 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -114,8 +114,7 @@ class @Notes
     unless note.valid
       if note.award
         flash = new Flash('You have already used this award emoji !', 'alert')
-        flash.pinToTop()
-        flash.raise()
+        flash.pin()
       return
 
     # render note if it not present in loaded list

From 83d8185f5afee7553bf5756ce61b6b855d48c1e5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Thu, 3 Dec 2015 13:03:50 +0100
Subject: [PATCH 15/55] Make method `supports_award?` public in `Note`

---
 app/models/note.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/note.rb b/app/models/note.rb
index 30d15d93fed..03640be7c93 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -367,13 +367,13 @@ class Note < ActiveRecord::Base
     self.note = award_emoji_name
   end
 
-  private
-
   def supports_awards?
     noteable.kind_of?(Issue) ||
       noteable.is_a?(MergeRequest)
   end
 
+  private
+
   def contains_emoji_only?
     (note =~ /\A:[-_+[:alnum:]]*:\s?\z/) ? true : false
   end

From 5145706c82613d64462fe736850d09799224cd77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Wed, 25 Nov 2015 19:20:40 -0500
Subject: [PATCH 16/55] Run custom Git hooks when creating or deleting branches
 through the UI. #1156

---
 CHANGELOG                               |   1 +
 app/models/repository.rb                |  54 +++++++------
 app/services/create_branch_service.rb   |   5 +-
 app/services/delete_branch_service.rb   |   4 +-
 app/services/files/base_service.rb      |   2 +-
 app/services/git_hooks_service.rb       |  32 ++++++++
 spec/models/repository_spec.rb          | 100 ++++++++++++++++++++++++
 spec/services/git_hooks_service_spec.rb |  38 +++++++++
 8 files changed, 207 insertions(+), 29 deletions(-)
 create mode 100644 app/services/git_hooks_service.rb
 create mode 100644 spec/services/git_hooks_service_spec.rb

diff --git a/CHANGELOG b/CHANGELOG
index db812796b69..882648b36a2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ v 8.3.0 (unreleased)
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
   - Add ignore whitespace change option to commit view
   - Fire update hook from GitLab
+  - Run custom Git hooks when branch is created or deleted. #1156
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index d247b0f5012..c304955b0b3 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1,7 +1,6 @@
 require 'securerandom'
 
 class Repository
-  class PreReceiveError < StandardError; end
   class CommitError < StandardError; end
 
   include Gitlab::ShellAdapter
@@ -108,10 +107,19 @@ class Repository
     tags.find { |tag| tag.name == name }
   end
 
-  def add_branch(branch_name, ref)
-    expire_branches_cache
+  def add_branch(user, branch_name, target)
+    oldrev = Gitlab::Git::BLANK_SHA
+    ref    = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
+    target = commit(target).try(:id)
 
-    gitlab_shell.add_branch(path_with_namespace, branch_name, ref)
+    return false unless target
+
+    GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
+      rugged.branches.create(branch_name, target)
+    end
+
+    expire_branches_cache
+    find_branch(branch_name)
   end
 
   def add_tag(tag_name, ref, message = nil)
@@ -120,10 +128,20 @@ class Repository
     gitlab_shell.add_tag(path_with_namespace, tag_name, ref, message)
   end
 
-  def rm_branch(branch_name)
+  def rm_branch(user, branch_name)
     expire_branches_cache
 
-    gitlab_shell.rm_branch(path_with_namespace, branch_name)
+    branch = find_branch(branch_name)
+    oldrev = branch.try(:target)
+    newrev = Gitlab::Git::BLANK_SHA
+    ref    = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
+
+    GitHooksService.new.execute(user, path_to_repo, oldrev, newrev, ref) do
+      rugged.branches.delete(branch_name)
+    end
+
+    expire_branches_cache
+    true
   end
 
   def rm_tag(tag_name)
@@ -550,7 +568,6 @@ class Repository
   def commit_with_hooks(current_user, branch)
     oldrev = Gitlab::Git::BLANK_SHA
     ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
-    gl_id = Gitlab::ShellEnv.gl_id(current_user)
     was_empty = empty?
 
     # Create temporary ref
@@ -569,15 +586,7 @@ class Repository
       raise CommitError.new('Failed to create commit')
     end
 
-    # Run GitLab pre-receive hook
-    pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo)
-    pre_receive_hook_status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref)
-
-    # Run GitLab update hook
-    update_hook = Gitlab::Git::Hook.new('update', path_to_repo)
-    update_hook_status = update_hook.trigger(gl_id, oldrev, newrev, ref)
-
-    if pre_receive_hook_status && update_hook_status
+    GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
       if was_empty
         # Create branch
         rugged.references.create(ref, newrev)
@@ -592,16 +601,11 @@ class Repository
           raise CommitError.new('Commit was rejected because branch received new push')
         end
       end
-
-      # Run GitLab post receive hook
-      post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo)
-      post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
-    else
-      # Remove tmp ref and return error to user
-      rugged.references.delete(tmp_ref)
-
-      raise PreReceiveError.new('Commit was rejected by git hook')
     end
+  rescue GitHooksService::PreReceiveError
+    # Remove tmp ref and return error to user
+    rugged.references.delete(tmp_ref)
+    raise
   end
 
   private
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index cf7ae4345f3..de18f3bc556 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -13,8 +13,7 @@ class CreateBranchService < BaseService
       return error('Branch already exists')
     end
 
-    repository.add_branch(branch_name, ref)
-    new_branch = repository.find_branch(branch_name)
+    new_branch = repository.add_branch(current_user, branch_name, ref)
 
     if new_branch
       push_data = build_push_data(project, current_user, new_branch)
@@ -27,6 +26,8 @@ class CreateBranchService < BaseService
     else
       error('Invalid reference name')
     end
+  rescue GitHooksService::PreReceiveError
+    error('Branch creation was rejected by Git hook')
   end
 
   def success(branch)
diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb
index b19b112a0c4..22bf9dd935e 100644
--- a/app/services/delete_branch_service.rb
+++ b/app/services/delete_branch_service.rb
@@ -24,7 +24,7 @@ class DeleteBranchService < BaseService
       return error('You dont have push access to repo', 405)
     end
 
-    if repository.rm_branch(branch_name)
+    if repository.rm_branch(current_user, branch_name)
       push_data = build_push_data(branch)
 
       EventCreateService.new.push(project, current_user, push_data)
@@ -35,6 +35,8 @@ class DeleteBranchService < BaseService
     else
       error('Failed to remove branch')
     end
+  rescue GitHooksService::PreReceiveError
+    error('Branch deletion was rejected by Git hook')
   end
 
   def error(message, return_code = 400)
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 008833eed80..f50aaf2eb52 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -26,7 +26,7 @@ module Files
       else
         error("Something went wrong. Your changes were not committed")
       end
-    rescue Repository::CommitError, Repository::PreReceiveError, ValidationError => ex
+    rescue Repository::CommitError, GitHooksService::PreReceiveError, ValidationError => ex
       error(ex.message)
     end
 
diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
new file mode 100644
index 00000000000..53f1fdef796
--- /dev/null
+++ b/app/services/git_hooks_service.rb
@@ -0,0 +1,32 @@
+class GitHooksService
+  PreReceiveError = Class.new(StandardError)
+
+  def execute(user, repo_path, oldrev, newrev, ref)
+    @repo_path  = repo_path
+    @user       = Gitlab::ShellEnv.gl_id(user)
+    @oldrev     = oldrev
+    @newrev     = newrev
+    @ref        = ref
+
+    pre_status = run_hook('pre-receive')
+
+    if pre_status
+      yield
+
+      run_hook('post-receive')
+    end
+  end
+
+  private
+
+  def run_hook(name)
+    hook = Gitlab::Git::Hook.new(name, @repo_path)
+    status = hook.trigger(@user, @oldrev, @newrev, @ref)
+
+    if !status && (name != 'post-receive')
+      raise PreReceiveError.new("Git operation was rejected by #{name} hook")
+    end
+
+    status
+  end
+end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 319fa0a7c8d..c746b8db621 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -4,6 +4,7 @@ describe Repository do
   include RepoHelpers
 
   let(:repository) { create(:project).repository }
+  let(:user) { create(:user) }
 
   describe :branch_names_contains do
     subject { repository.branch_names_contains(sample_commit.id) }
@@ -99,5 +100,104 @@ describe Repository do
       it { expect(subject.startline).to eq(186) }
       it { expect(subject.data.lines[2]).to eq("  - Feature: Replace teams with group membership\n") }
     end
+
   end
+
+  describe :add_branch do
+    context 'when pre hooks were successful' do
+      it 'should run without errors' do
+        hook = double(trigger: true)
+        expect(Gitlab::Git::Hook).to receive(:new).twice.and_return(hook)
+
+        expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error
+      end
+
+      it 'should create the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+
+        branch = repository.add_branch(user, 'new_feature', 'master')
+
+        expect(branch.name).to eq('new_feature')
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should get an error' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.add_branch(user, 'new_feature', 'master')
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+
+      it 'should not create the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.add_branch(user, 'new_feature', 'master')
+        end.to raise_error(GitHooksService::PreReceiveError)
+        expect(repository.find_branch('new_feature')).to be_nil
+      end
+    end
+  end
+
+  describe :rm_branch do
+    context 'when pre hooks were successful' do
+      it 'should run without errors' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+
+        expect { repository.rm_branch(user, 'feature') }.not_to raise_error
+      end
+
+      it 'should delete the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+
+        expect { repository.rm_branch(user, 'feature') }.not_to raise_error
+
+        expect(repository.find_branch('feature')).to be_nil
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should get an error' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.rm_branch(user, 'new_feature')
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+
+      it 'should not delete the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.rm_branch(user, 'feature')
+        end.to raise_error(GitHooksService::PreReceiveError)
+        expect(repository.find_branch('feature')).not_to be_nil
+      end
+    end
+  end
+
+  describe :commit_with_hooks do
+    context 'when pre hooks were successful' do
+      it 'should run without errors' do
+        expect_any_instance_of(GitHooksService).to receive(:execute).and_return(true)
+
+        expect do
+          repository.commit_with_hooks(user, 'feature') { sample_commit.id }
+        end.not_to raise_error
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should get an error' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.commit_with_hooks(user, 'feature') { sample_commit.id }
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+    end
+  end
+
 end
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
new file mode 100644
index 00000000000..21585cc4629
--- /dev/null
+++ b/spec/services/git_hooks_service_spec.rb
@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+describe GitHooksService do
+  include RepoHelpers
+
+  let(:user)    { create :user }
+  let(:project) { create :project }
+  let(:service) { GitHooksService.new }
+
+  before do
+    @blankrev = Gitlab::Git::BLANK_SHA
+    @oldrev = sample_commit.parent_id
+    @newrev = sample_commit.id
+    @ref = 'refs/heads/feature'
+    @repo_path = project.repository.path_to_repo
+  end
+
+  describe '#execute' do
+
+    context 'when pre hooks were successful' do
+      it 'should call post hooks' do
+        expect(service).to receive(:run_hook).with('pre-receive').and_return(true)
+        expect(service).to receive(:run_hook).with('post-receive').and_return(true)
+        expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq(true)
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should not call post hooks' do
+        expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
+        expect(service).not_to receive(:run_hook).with('post-receive')
+
+        service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+      end
+    end
+
+  end
+end

From 338eb2c41ea766779d6bb7798079a1dd3a50e11d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Tue, 1 Dec 2015 00:22:45 -0500
Subject: [PATCH 17/55] Call update hook from new GitHooksService class. #3069

---
 app/services/git_hooks_service.rb       |  4 +---
 spec/models/repository_spec.rb          |  2 +-
 spec/services/git_hooks_service_spec.rb | 23 +++++++++++++++++------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index 53f1fdef796..b7804ed472f 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -8,9 +8,7 @@ class GitHooksService
     @newrev     = newrev
     @ref        = ref
 
-    pre_status = run_hook('pre-receive')
-
-    if pre_status
+    if run_hook('pre-receive') && run_hook('update')
       yield
 
       run_hook('post-receive')
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index c746b8db621..fa261e64c35 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -107,7 +107,7 @@ describe Repository do
     context 'when pre hooks were successful' do
       it 'should run without errors' do
         hook = double(trigger: true)
-        expect(Gitlab::Git::Hook).to receive(:new).twice.and_return(hook)
+        expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
 
         expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error
       end
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
index 21585cc4629..bb639a5ae23 100644
--- a/spec/services/git_hooks_service_spec.rb
+++ b/spec/services/git_hooks_service_spec.rb
@@ -17,16 +17,17 @@ describe GitHooksService do
 
   describe '#execute' do
 
-    context 'when pre hooks were successful' do
-      it 'should call post hooks' do
-        expect(service).to receive(:run_hook).with('pre-receive').and_return(true)
-        expect(service).to receive(:run_hook).with('post-receive').and_return(true)
+    context 'when receive hooks were successful' do
+      it 'should call post-receive hook' do
+        hook = double(trigger: true)
+        expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
+
         expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq(true)
       end
     end
 
-    context 'when pre hooks failed' do
-      it 'should not call post hooks' do
+    context 'when pre-receive hook failed' do
+      it 'should not call post-receive hook' do
         expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
         expect(service).not_to receive(:run_hook).with('post-receive')
 
@@ -34,5 +35,15 @@ describe GitHooksService do
       end
     end
 
+    context 'when update hook failed' do
+      it 'should not call post-receive hook' do
+        expect(service).to receive(:run_hook).with('pre-receive').and_return(true)
+        expect(service).to receive(:run_hook).with('update').and_return(false)
+        expect(service).not_to receive(:run_hook).with('post-receive')
+
+        service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+      end
+    end
+
   end
 end

From 5e6a5270d52ea2f13ca9967913012691e257439b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Tue, 1 Dec 2015 12:31:44 -0500
Subject: [PATCH 18/55] Raise the exception from #execute instead of #run_hook.
 #1156 #3069

---
 app/services/git_hooks_service.rb       | 20 +++++++++-----------
 spec/services/git_hooks_service_spec.rb |  8 ++++++--
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index b7804ed472f..8f5c3393dfc 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -8,23 +8,21 @@ class GitHooksService
     @newrev     = newrev
     @ref        = ref
 
-    if run_hook('pre-receive') && run_hook('update')
-      yield
-
-      run_hook('post-receive')
+    %w(pre-receive update).each do |hook_name|
+      unless run_hook(hook_name)
+        raise PreReceiveError.new("Git operation was rejected by #{hook_name} hook")
+      end
     end
+
+    yield
+
+    run_hook('post-receive')
   end
 
   private
 
   def run_hook(name)
     hook = Gitlab::Git::Hook.new(name, @repo_path)
-    status = hook.trigger(@user, @oldrev, @newrev, @ref)
-
-    if !status && (name != 'post-receive')
-      raise PreReceiveError.new("Git operation was rejected by #{name} hook")
-    end
-
-    status
+    hook.trigger(@user, @oldrev, @newrev, @ref)
   end
 end
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
index bb639a5ae23..7e018d3c9fe 100644
--- a/spec/services/git_hooks_service_spec.rb
+++ b/spec/services/git_hooks_service_spec.rb
@@ -31,7 +31,9 @@ describe GitHooksService do
         expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
         expect(service).not_to receive(:run_hook).with('post-receive')
 
-        service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        expect do
+          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        end.to raise_error(GitHooksService::PreReceiveError)
       end
     end
 
@@ -41,7 +43,9 @@ describe GitHooksService do
         expect(service).to receive(:run_hook).with('update').and_return(false)
         expect(service).not_to receive(:run_hook).with('post-receive')
 
-        service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        expect do
+          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        end.to raise_error(GitHooksService::PreReceiveError)
       end
     end
 

From dbbd2b863b402e460ac1dc90f852fcae617a2351 Mon Sep 17 00:00:00 2001
From: Greg Smethells <smethells@icloud.com>
Date: Mon, 30 Nov 2015 14:47:44 -0600
Subject: [PATCH 19/55] sort milestones by due_date

---
 CHANGELOG                                     |  1 +
 app/controllers/concerns/global_milestones.rb |  2 ++
 app/finders/milestones_finder.rb              |  2 +-
 app/helpers/milestones_helper.rb              |  2 ++
 app/models/global_milestone.rb                | 31 ++++++++++++++++++-
 .../dashboard/milestones/_milestone.html.haml | 11 ++++---
 .../projects/milestones/_milestone.html.haml  |  6 +---
 app/views/shared/_milestone_expired.html.haml |  5 +++
 8 files changed, 49 insertions(+), 11 deletions(-)
 create mode 100644 app/views/shared/_milestone_expired.html.haml

diff --git a/CHANGELOG b/CHANGELOG
index db812796b69..aad58af6678 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ v 8.3.0 (unreleased)
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
   - Add ignore whitespace change option to commit view
   - Fire update hook from GitLab
+  - Fix: sort milestones by due date once again (Greg Smethells)
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)
diff --git a/app/controllers/concerns/global_milestones.rb b/app/controllers/concerns/global_milestones.rb
index b428249acd3..3e4c0e63601 100644
--- a/app/controllers/concerns/global_milestones.rb
+++ b/app/controllers/concerns/global_milestones.rb
@@ -2,8 +2,10 @@ module GlobalMilestones
   extend ActiveSupport::Concern
 
   def milestones
+    epoch = DateTime.parse('1970-01-01')
     @milestones = MilestonesFinder.new.execute(@projects, params)
     @milestones = GlobalMilestone.build_collection(@milestones)
+    @milestones = @milestones.sort_by { |x| x.due_date.nil? ? epoch : x.due_date }
     @milestones = Kaminari.paginate_array(@milestones).page(params[:page]).per(ApplicationController::PER_PAGE)
   end
 
diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb
index b704e878903..630c73c2a94 100644
--- a/app/finders/milestones_finder.rb
+++ b/app/finders/milestones_finder.rb
@@ -1,7 +1,7 @@
 class MilestonesFinder
   def execute(projects, params)
     milestones = Milestone.of_projects(projects)
-    milestones = milestones.order("due_date ASC")
+    milestones = milestones.reorder("due_date ASC")
 
     case params[:state]
     when 'closed' then milestones.closed
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index ad43892b639..a42cbcff182 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -28,7 +28,9 @@ module MilestonesHelper
         Milestone.where(project_id: @projects)
       end.active
 
+    epoch = DateTime.parse('1970-01-01')
     grouped_milestones = GlobalMilestone.build_collection(milestones)
+    grouped_milestones = grouped_milestones.sort_by { |x| x.due_date.nil? ? epoch : x.due_date }
     grouped_milestones.unshift(Milestone::None)
     grouped_milestones.unshift(Milestone::Any)
 
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 1321ccd963f..33ddb265fba 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -19,6 +19,14 @@ class GlobalMilestone
     @title.parameterize
   end
 
+  def expired?
+    if due_date
+      due_date.past?
+    else
+      false
+    end
+  end
+
   def projects
     milestones.map { |milestone| milestone.project }
   end
@@ -98,4 +106,25 @@ class GlobalMilestone
   def complete?
     total_items_count == closed_items_count
   end
-end
+
+  def due_date
+    return @due_date if defined?(@due_date)
+
+    @due_date =
+      if @milestones.all? { |x| x.due_date == @milestones.first.due_date }
+        @milestones.first.due_date
+      else
+        nil
+      end
+  end
+
+  def expires_at
+    if due_date
+      if due_date.past?
+        "expired at #{due_date.stamp("Aug 21, 2011")}"
+      else
+        "expires at #{due_date.stamp("Aug 21, 2011")}"
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/views/dashboard/milestones/_milestone.html.haml b/app/views/dashboard/milestones/_milestone.html.haml
index 55080d6b3fe..7c882a32702 100644
--- a/app/views/dashboard/milestones/_milestone.html.haml
+++ b/app/views/dashboard/milestones/_milestone.html.haml
@@ -16,7 +16,10 @@
       = milestone_progress_bar(milestone)
   .row
     .col-sm-6
-      - milestone.milestones.each do |milestone|
-        = link_to milestone_path(milestone) do
-          %span.label.label-gray
-            = milestone.project.name_with_namespace
+      .expiration
+        = render 'shared/milestone_expired', milestone: milestone
+      .projects
+        - milestone.milestones.each do |milestone|
+          = link_to milestone_path(milestone) do
+            %span.label.label-gray
+              = milestone.project.name_with_namespace
diff --git a/app/views/projects/milestones/_milestone.html.haml b/app/views/projects/milestones/_milestone.html.haml
index 334172b976f..d6a44c9f0a1 100644
--- a/app/views/projects/milestones/_milestone.html.haml
+++ b/app/views/projects/milestones/_milestone.html.haml
@@ -18,11 +18,7 @@
 
   .row
     .col-sm-6
-      - if milestone.expired? and not milestone.closed?
-        %span.cred (Expired)
-      - if milestone.expires_at
-        %span
-          = milestone.expires_at
+      = render 'shared/milestone_expired', milestone: milestone
     .col-sm-6
       - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
         = link_to edit_namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), class: "btn btn-xs edit-milestone-link btn-grouped" do
diff --git a/app/views/shared/_milestone_expired.html.haml b/app/views/shared/_milestone_expired.html.haml
new file mode 100644
index 00000000000..b8eef15fbec
--- /dev/null
+++ b/app/views/shared/_milestone_expired.html.haml
@@ -0,0 +1,5 @@
+- if milestone.expired? and not milestone.closed?
+  %span.cred (Expired)
+- if milestone.expires_at
+  %span
+    = milestone.expires_at

From 86ed2e43d58ef074ae3b1d80e0b18fe338ca9afd Mon Sep 17 00:00:00 2001
From: Phillip Berndt <phillip.berndt@googlemail.com>
Date: Thu, 3 Dec 2015 16:04:39 +0100
Subject: [PATCH 20/55] time_ago_with_tooltip javascript snippet: Don't
 reinitialize older elements

see bug #3758
---
 app/helpers/application_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 3230ff1b004..21f962df206 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -209,7 +209,7 @@ module ApplicationHelper
       title: time.in_time_zone.stamp('Aug 21, 2011 9:23pm'),
       data: { toggle: 'tooltip', placement: placement, container: 'body' }
 
-    element += javascript_tag "$('.js-timeago').timeago()" unless skip_js
+    element += javascript_tag "$('.js-timeago').last().timeago()" unless skip_js
 
     element
   end

From 11b54edf36c8d44f02004de166cf8b809df721ad Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Thu, 3 Dec 2015 09:36:45 -0600
Subject: [PATCH 21/55] Filter current user to the top of issue assignee list

---
 app/views/shared/issuable/_context.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/issuable/_context.html.haml b/app/views/shared/issuable/_context.html.haml
index f1646b4ce64..ced16f820cb 100644
--- a/app/views/shared/issuable/_context.html.haml
+++ b/app/views/shared/issuable/_context.html.haml
@@ -9,7 +9,7 @@
         none
     .issuable-context-selectbox
       - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
-        = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true)
+        = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true, first_user: true)
 
   %div.prepend-top-default.clearfix
     .issuable-context-title

From c204aca83b0c8308080a9f53b692f509a80ddffc Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 3 Dec 2015 15:30:10 -0200
Subject: [PATCH 22/55] Improve style of the warning when mentioning many
 people in a comment

---
 .../stylesheets/framework/markdown_area.scss      |  8 +++-----
 app/views/projects/_md_preview.html.haml          | 15 ++++++++-------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss
index cc660529cb4..11b609f3b79 100644
--- a/app/assets/stylesheets/framework/markdown_area.scss
+++ b/app/assets/stylesheets/framework/markdown_area.scss
@@ -73,11 +73,9 @@
 }
 
 .referenced-users {
-  padding: 10px 0;
-  color: #999;
-  margin-left: 10px;
-  margin-top: 1px;
-  margin-right: 130px;
+  color: #4c4e54;
+  display: inline-block;
+  padding-top: 10px;
 }
 
 .md-preview-holder {
diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml
index c4d3c9cc30a..5bdceb9523a 100644
--- a/app/views/projects/_md_preview.html.haml
+++ b/app/views/projects/_md_preview.html.haml
@@ -15,10 +15,11 @@
       .js-md-preview{class: (preview_class if defined?(preview_class))}
 
   - if defined?(referenced_users) && referenced_users
-    %span.referenced-users.pull-left.hide
-      = icon('exclamation-triangle')
-      You are about to add
-      %strong
-        %span.js-referenced-users-count 0
-        people
-      to the discussion. Proceed with caution.
+    %div.clearfix
+      %span.referenced-users.hide
+        = icon('exclamation-triangle')
+        You are about to add
+        %strong
+          %span.js-referenced-users-count 0
+          people
+        to the discussion. Proceed with caution.

From 0decc7f941b96bf53e172de0340847d3b7d714b7 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 3 Dec 2015 19:40:31 -0200
Subject: [PATCH 23/55] Fix specs

---
 app/assets/stylesheets/framework/markdown_area.scss | 1 -
 app/views/projects/_md_preview.html.haml            | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss
index 11b609f3b79..2b044786738 100644
--- a/app/assets/stylesheets/framework/markdown_area.scss
+++ b/app/assets/stylesheets/framework/markdown_area.scss
@@ -74,7 +74,6 @@
 
 .referenced-users {
   color: #4c4e54;
-  display: inline-block;
   padding-top: 10px;
 }
 
diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml
index 5bdceb9523a..54c818baaf4 100644
--- a/app/views/projects/_md_preview.html.haml
+++ b/app/views/projects/_md_preview.html.haml
@@ -15,8 +15,8 @@
       .js-md-preview{class: (preview_class if defined?(preview_class))}
 
   - if defined?(referenced_users) && referenced_users
-    %div.clearfix
-      %span.referenced-users.hide
+    %div.referenced-users.hide
+      %span
         = icon('exclamation-triangle')
         You are about to add
         %strong

From 496870ddec632ae13cff8b5e8f6ca0bff0fa3ec7 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <brodock@gmail.com>
Date: Thu, 3 Dec 2015 21:24:39 -0200
Subject: [PATCH 24/55] Migrate from Sidetiq to Sidekiq-cron

Updated Sidekiq to 3.5.x
---
 Gemfile                               |  4 +--
 Gemfile.lock                          | 46 +++++++++++++++++----------
 app/workers/stuck_ci_builds_worker.rb |  3 --
 config/initializers/sidekiq.rb        |  6 ++++
 config/routes.rb                      |  1 +
 config/schedule.yml                   | 10 ++++++
 6 files changed, 49 insertions(+), 21 deletions(-)
 create mode 100644 config/schedule.yml

diff --git a/Gemfile b/Gemfile
index 67640bb9ae0..954c5e13503 100644
--- a/Gemfile
+++ b/Gemfile
@@ -120,8 +120,8 @@ gem 'acts-as-taggable-on', '~> 3.4'
 
 # Background jobs
 gem 'sinatra', '~> 1.4.4', require: nil
-gem 'sidekiq', '3.3.0'
-gem 'sidetiq', '~> 0.6.3'
+gem 'sidekiq', '~> 3.5.0'
+gem 'sidekiq-cron', '~> 0.3.0'
 
 # HTTP requests
 gem "httparty", '~> 0.13.3'
diff --git a/Gemfile.lock b/Gemfile.lock
index cd1855758b2..67807bed4c4 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -116,8 +116,23 @@ GEM
       activemodel (>= 3.2.0)
       activesupport (>= 3.2.0)
       json (>= 1.7)
-    celluloid (0.16.0)
-      timers (~> 4.0.0)
+    celluloid (0.17.2)
+      celluloid-essentials
+      celluloid-extras
+      celluloid-fsm
+      celluloid-pool
+      celluloid-supervision
+      timers (>= 4.1.1)
+    celluloid-essentials (0.20.5)
+      timers (>= 4.1.1)
+    celluloid-extras (0.20.5)
+      timers (>= 4.1.1)
+    celluloid-fsm (0.20.5)
+      timers (>= 4.1.1)
+    celluloid-pool (0.20.5)
+      timers (>= 4.1.1)
+    celluloid-supervision (0.20.5)
+      timers (>= 4.1.1)
     charlock_holmes (0.7.3)
     chunky_png (1.3.5)
     cliver (0.3.2)
@@ -369,7 +384,6 @@ GEM
       multi_xml (>= 0.5.2)
     httpclient (2.7.0.1)
     i18n (0.7.0)
-    ice_cube (0.11.1)
     ice_nine (0.11.1)
     inflecto (0.0.2)
     ipaddress (0.8.0)
@@ -640,6 +654,7 @@ GEM
       sexp_processor (~> 4.1)
     rubyntlm (0.5.2)
     rubypants (0.2.0)
+    rufus-scheduler (3.1.10)
     rugged (0.23.3)
     safe_yaml (1.0.4)
     sanitize (2.1.0)
@@ -667,16 +682,15 @@ GEM
       rack
     shoulda-matchers (2.8.0)
       activesupport (>= 3.0.0)
-    sidekiq (3.3.0)
-      celluloid (>= 0.16.0)
-      connection_pool (>= 2.0.0)
-      json
-      redis (>= 3.0.6)
-      redis-namespace (>= 1.3.1)
-    sidetiq (0.6.3)
-      celluloid (>= 0.14.1)
-      ice_cube (= 0.11.1)
-      sidekiq (>= 3.0.0)
+    sidekiq (3.5.3)
+      celluloid (~> 0.17.2)
+      connection_pool (~> 2.2, >= 2.2.0)
+      json (~> 1.0)
+      redis (~> 3.2, >= 3.2.1)
+      redis-namespace (~> 1.5, >= 1.5.2)
+    sidekiq-cron (0.3.1)
+      rufus-scheduler (>= 2.0.24)
+      sidekiq (>= 2.17.3)
     simple_oauth (0.1.9)
     simplecov (0.10.0)
       docile (~> 1.1.0)
@@ -742,7 +756,7 @@ GEM
     thor (0.19.1)
     thread_safe (0.3.5)
     tilt (1.4.1)
-    timers (4.0.4)
+    timers (4.1.1)
       hitimes
     timfel-krb5-auth (0.8.3)
     tinder (1.10.1)
@@ -936,8 +950,8 @@ DEPENDENCIES
   settingslogic (~> 2.0.9)
   sham_rack
   shoulda-matchers (~> 2.8.0)
-  sidekiq (= 3.3.0)
-  sidetiq (~> 0.6.3)
+  sidekiq (~> 3.5.0)
+  sidekiq-cron (~> 0.3.0)
   simplecov (~> 0.10.0)
   sinatra (~> 1.4.4)
   six (~> 0.2.0)
diff --git a/app/workers/stuck_ci_builds_worker.rb b/app/workers/stuck_ci_builds_worker.rb
index 4e5eddbaba1..ca594e77e7c 100644
--- a/app/workers/stuck_ci_builds_worker.rb
+++ b/app/workers/stuck_ci_builds_worker.rb
@@ -1,11 +1,8 @@
 class StuckCiBuildsWorker
   include Sidekiq::Worker
-  include Sidetiq::Schedulable
 
   BUILD_STUCK_TIMEOUT = 1.day
 
-  recurrence { daily }
-
   def perform
     Rails.logger.info 'Cleaning stuck builds'
 
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index e856499732e..6e5701e33da 100644
--- a/config/initializers/sidekiq.rb
+++ b/config/initializers/sidekiq.rb
@@ -17,6 +17,12 @@ Sidekiq.configure_server do |config|
     chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS']
     chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS']
   end
+
+  # Sidekiq-cron: load recurring jobs from schedule.yml
+  schedule_file = 'config/schedule.yml'
+  if File.exists?(schedule_file)
+    Sidekiq::Cron::Job.load_from_hash YAML.load_file(schedule_file)
+  end
 end
 
 Sidekiq.configure_client do |config|
diff --git a/config/routes.rb b/config/routes.rb
index 5c114452a3f..6aa822bf859 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,4 +1,5 @@
 require 'sidekiq/web'
+require 'sidekiq/cron/web'
 require 'api/api'
 
 Rails.application.routes.draw do
diff --git a/config/schedule.yml b/config/schedule.yml
new file mode 100644
index 00000000000..993a95fef56
--- /dev/null
+++ b/config/schedule.yml
@@ -0,0 +1,10 @@
+# Here is a list of jobs that are scheduled to run periodically.
+# We use a UNIX cron notation to specify execution schedule.
+#
+# Please read here for more information:
+# https://github.com/ondrejbartas/sidekiq-cron#adding-cron-job
+
+stuck_ci_builds_worker:
+  cron: "0 0 * * *"
+  class: "StuckCiBuildsWorker"
+  queue: "default"

From 5c1b49f494f07bf37ba3c60f3b9f70d1842d8b60 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Fri, 4 Dec 2015 16:23:21 +0200
Subject: [PATCH 25/55] Add added, modified and removed properties to commit
 object in webhook

---
 CHANGELOG                                 |  3 +++
 app/models/commit.rb                      | 24 ++++++++++++++++-
 doc/web_hooks/web_hooks.md                | 14 ++++++----
 lib/gitlab/push_data_builder.rb           | 32 ++---------------------
 spec/lib/gitlab/push_data_builder_spec.rb |  9 +++----
 spec/models/commit_spec.rb                | 11 ++++++++
 6 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 58a0a3c8944..d8d878c45c9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,9 @@ v 8.3.0 (unreleased)
   - Add API endpoint to fetch merge request commits list
   - Expose events API with comment information and author info
 
+v 8.2.3
+  - Webhook payload has an added, modified and removed properties for each commit
+
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)
   - Ensure cached application settings are refreshed at startup (Stan Hu)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 492f6be1ce3..912b4dedf51 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -146,7 +146,10 @@ class Commit
       author: {
         name: author_name,
         email: author_email
-      }
+      },
+      added: repo_changes[:added],
+      modified: repo_changes[:modified],
+      removed: repo_changes[:removed]
     }
   end
 
@@ -196,4 +199,23 @@ class Commit
   def status
     ci_commit.try(:status) || :not_found
   end
+
+  private
+
+  def repo_changes
+    changes = { added: [], modified: [], removed: [] }
+
+    diffs.each do |diff|
+      case true
+      when diff.deleted_file
+        changes[:removed] << diff.old_path
+      when diff.renamed_file, diff.new_file
+        changes[:added] << diff.new_path
+      else
+        changes[:modified] << diff.new_path
+      end
+    end
+
+    changes
+  end
 end
diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md
index 7d838187a26..03746dd9df3 100644
--- a/doc/web_hooks/web_hooks.md
+++ b/doc/web_hooks/web_hooks.md
@@ -57,6 +57,9 @@ X-Gitlab-Event: Push Hook
         "name": "Jordi Mallach",
         "email": "jordi@softcatala.org"
       }
+      "added": ["CHANGELOG"],
+      "modified": ["app/controller/application.rb"],
+      "removed": []
     },
     {
       "id": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
@@ -66,13 +69,14 @@ X-Gitlab-Event: Push Hook
       "author": {
         "name": "GitLab dev user",
         "email": "gitlabdev@dv6700.(none)"
-      }
+      },
+      "added": ["CHANGELOG"],
+      "modified": ["app/controller/application.rb"],
+      "removed": []
     }
   ],
-  "total_commits_count": 4,
-  "added": ["CHANGELOG"],
-  "modified": ["app/controller/application.rb"],
-  "removed": []
+  "total_commits_count": 4
+  
 }
 ```
 
diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb
index fa068d50763..cdcdb02a052 100644
--- a/lib/gitlab/push_data_builder.rb
+++ b/lib/gitlab/push_data_builder.rb
@@ -18,10 +18,7 @@ module Gitlab
       #     homepage: String,
       #   },
       #   commits: Array,
-      #   total_commits_count: Fixnum,
-      #   added: ["CHANGELOG"],
-      #   modified: [],
-      #   removed: ["tmp/file.txt"]
+      #   total_commits_count: Fixnum
       # }
       #
       def build(project, user, oldrev, newrev, ref, commits = [], message = nil)
@@ -37,7 +34,6 @@ module Gitlab
 
         type = Gitlab::Git.tag_ref?(ref) ? "tag_push" : "push"
 
-        repo_changes = repo_changes(project, newrev, oldrev)
         # Hash to be passed as post_receive_data
         data = {
           object_kind: type,
@@ -60,10 +56,7 @@ module Gitlab
             visibility_level: project.visibility_level
           },
           commits: commit_attrs,
-          total_commits_count: commits_count,
-          added: repo_changes[:added],
-          modified: repo_changes[:modified],
-          removed: repo_changes[:removed]
+          total_commits_count: commits_count
         }
 
         data
@@ -94,27 +87,6 @@ module Gitlab
           newrev
         end
       end
-
-      def repo_changes(project, newrev, oldrev)
-        changes = { added: [], modified: [], removed: [] }
-        compare_result = CompareService.new.
-          execute(project, newrev, project, oldrev)
-
-        if compare_result
-          compare_result.diffs.each do |diff|
-            case true
-            when diff.deleted_file
-              changes[:removed] << diff.old_path
-            when diff.renamed_file, diff.new_file
-              changes[:added] << diff.new_path
-            else
-              changes[:modified] << diff.new_path
-            end
-          end
-        end
-
-        changes
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/push_data_builder_spec.rb
index 02710742625..2170399ab5c 100644
--- a/spec/lib/gitlab/push_data_builder_spec.rb
+++ b/spec/lib/gitlab/push_data_builder_spec.rb
@@ -17,9 +17,9 @@ describe 'Gitlab::PushDataBuilder' do
     it { expect(data[:repository][:git_ssh_url]).to eq(project.ssh_url_to_repo) }
     it { expect(data[:repository][:visibility_level]).to eq(project.visibility_level) }
     it { expect(data[:total_commits_count]).to eq(3) }
-    it { expect(data[:added]).to eq(["gitlab-grack"]) }
-    it { expect(data[:modified]).to eq([".gitmodules", "files/ruby/popen.rb", "files/ruby/regex.rb"]) }
-    it { expect(data[:removed]).to eq([]) }
+    it { expect(data[:commits].first[:added]).to eq(["gitlab-grack"]) }
+    it { expect(data[:commits].first[:modified]).to eq([".gitmodules"]) }
+    it { expect(data[:commits].first[:removed]).to eq([]) }
   end
 
   describe :build do
@@ -38,8 +38,5 @@ describe 'Gitlab::PushDataBuilder' do
     it { expect(data[:ref]).to eq('refs/tags/v1.1.0') }
     it { expect(data[:commits]).to be_empty }
     it { expect(data[:total_commits_count]).to be_zero }
-    it { expect(data[:added]).to eq([]) }
-    it { expect(data[:modified]).to eq([]) }
-    it { expect(data[:removed]).to eq([]) }
   end
 end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 90be9324951..b417bc98fa7 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -100,4 +100,15 @@ eos
     # Include the subject in the repository stub.
     let(:extra_commits) { [subject] }
   end
+
+  describe '#hook_attrs' do
+    let(:data) { commit.hook_attrs }
+
+    it { expect(data).to be_a(Hash) }
+    it { expect(data[:message]).to include('Add submodule from gitlab.com') }
+    it { expect(data[:timestamp]).to eq('2014-02-27T11:01:38+02:00') }
+    it { expect(data[:added]).to eq(["gitlab-grack"]) }
+    it { expect(data[:modified]).to eq([".gitmodules"]) }
+    it { expect(data[:removed]).to eq([]) }
+  end
 end

From a120b78940b6c7150f405091d620b34c0fccbd28 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 1 Dec 2015 16:15:01 -0800
Subject: [PATCH 26/55] Handle and report SSL errors in Web hook test. Check
 for status 200 for success.

If a Web hook test fails due to an SSL error or some other error, report
the result back to the user instead of an Error 500.

Closes #3656

Handle response
---
 CHANGELOG                                    |  1 +
 app/controllers/projects/hooks_controller.rb |  5 ++-
 app/models/hooks/web_hook.rb                 | 36 +++++++++++---------
 spec/models/hooks/web_hook_spec.rb           |  6 ++++
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 58a0a3c8944..faee22405dd 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.3.0 (unreleased)
+  - Handle and report SSL errors in Web hook test (Stan Hu)
   - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera)
   - Fix 500 error when update group member permission
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index c7569541899..6a62880cb71 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -25,13 +25,12 @@ class Projects::HooksController < Projects::ApplicationController
 
   def test
     if !@project.empty_repo?
-      status = TestHookService.new.execute(hook, current_user)
+      status, message = TestHookService.new.execute(hook, current_user)
 
       if status
         flash[:notice] = 'Hook successfully executed.'
       else
-        flash[:alert] = 'Hook execution failed. '\
-                        'Ensure hook URL is correct and service is up.'
+        flash[:alert] = "Hook execution failed: #{message}"
       end
     else
       flash[:alert] = 'Hook execution failed. Ensure the project has commits.'
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index d6c6f415c4a..2caf26cc8c9 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -37,31 +37,33 @@ class WebHook < ActiveRecord::Base
   def execute(data, hook_name)
     parsed_url = URI.parse(url)
     if parsed_url.userinfo.blank?
-      WebHook.post(url,
-                   body: data.to_json,
-                   headers: {
-                     "Content-Type" => "application/json",
-                     "X-Gitlab-Event" => hook_name.singularize.titleize
-                   },
-                   verify: enable_ssl_verification)
+      response = WebHook.post(url,
+                              body: data.to_json,
+                              headers: {
+                                  "Content-Type" => "application/json",
+                                  "X-Gitlab-Event" => hook_name.singularize.titleize
+                              },
+                              verify: enable_ssl_verification)
     else
       post_url = url.gsub("#{parsed_url.userinfo}@", "")
       auth = {
         username: URI.decode(parsed_url.user),
         password: URI.decode(parsed_url.password),
       }
-      WebHook.post(post_url,
-                   body: data.to_json,
-                   headers: {
-                     "Content-Type" => "application/json",
-                     "X-Gitlab-Event" => hook_name.singularize.titleize
-                   },
-                   verify: enable_ssl_verification,
-                   basic_auth: auth)
+      response = WebHook.post(post_url,
+                              body: data.to_json,
+                              headers: {
+                                  "Content-Type" => "application/json",
+                                  "X-Gitlab-Event" => hook_name.singularize.titleize
+                              },
+                              verify: enable_ssl_verification,
+                              basic_auth: auth)
     end
-  rescue SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e
+
+    [response.code == 200, ActionView::Base.full_sanitizer.sanitize(response.to_s)]
+  rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e
     logger.error("WebHook Error => #{e}")
-    false
+    [false, e.to_s]
   end
 
   def async_execute(data, hook_name)
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 2fdc49f02ee..35042788c65 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -71,5 +71,11 @@ describe ProjectHook do
 
       expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError)
     end
+
+    it "handles SSL exceptions" do
+      expect(WebHook).to receive(:post).and_raise(OpenSSL::SSL::SSLError.new('SSL error'))
+
+      expect(@project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error'])
+    end
   end
 end

From ba3c702073f2f57eebbeabb2926fbd367aad87ea Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 1 Dec 2015 16:45:40 -0800
Subject: [PATCH 27/55] Fix spec

---
 features/steps/project/hooks.rb | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/features/steps/project/hooks.rb b/features/steps/project/hooks.rb
index df4a23a3716..be4db770948 100644
--- a/features/steps/project/hooks.rb
+++ b/features/steps/project/hooks.rb
@@ -70,8 +70,6 @@ class Spinach::Features::ProjectHooks < Spinach::FeatureSteps
 
   step 'I should see hook service down error message' do
     expect(page).to have_selector '.flash-alert',
-                              text: 'Hook execution failed. '\
-                                    'Ensure hook URL is correct and '\
-                                    'service is up.'
+                              text: 'Hook execution failed: Exception from'
   end
 end

From 12fdc13ad386299b04431c43c7d4ab307698b1a8 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 4 Dec 2015 17:31:01 -0200
Subject: [PATCH 28/55] Fix 500 error when creating a merge request that
 removes a submodule

---
 CHANGELOG                                     |  1 +
 app/views/projects/diffs/_file.html.haml      |  3 +--
 spec/controllers/commit_controller_spec.rb    | 20 ++++++++++++++++
 .../merge_requests_controller_spec.rb         | 24 +++++++++++++++++++
 spec/support/test_env.rb                      |  3 ++-
 5 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 6b61cbc11e9..9b26f526fd8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -11,6 +11,7 @@ v 8.3.0 (unreleased)
   - Add API endpoint to fetch merge request commits list
   - Expose events API with comment information and author info
   - Fix: Ensure "Remove Source Branch" button is not shown when branch is being deleted. #3583
+  - Fix 500 error when creating a merge request that removes a submodule
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index b3392d00e01..b77e9f9f403 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -3,9 +3,8 @@
     - if diff_file.diff.submodule?
       %span
         = icon('archive fw')
-        - submodule_item = project.repository.blob_at(@commit.id, diff_file.file_path)
         %strong
-          = submodule_link(submodule_item, @commit.id, project.repository)
+          = submodule_link(blob, @commit.id, project.repository)
     - else
       %span
         = blob_icon blob.mode, blob.name
diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb
index 5337a69e84b..7793bf1e421 100644
--- a/spec/controllers/commit_controller_spec.rb
+++ b/spec/controllers/commit_controller_spec.rb
@@ -110,6 +110,26 @@ describe Projects::CommitController do
         expect(response.body).to match(/^diff --git/)
       end
     end
+
+    context 'commit that removes a submodule' do
+      render_views
+
+      let(:fork_project) { create(:forked_project_with_submodules) }
+      let(:commit) { fork_project.commit('remove-submodule') }
+
+      before do
+        fork_project.team << [user, :master]
+      end
+
+      it 'renders it' do
+        get(:show,
+            namespace_id: fork_project.namespace.to_param,
+            project_id: fork_project.to_param,
+            id: commit.id)
+
+        expect(response).to be_success
+      end
+    end
   end
 
   describe "#branches" do
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 3e5e1fa87ae..6aaec224f6e 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -10,6 +10,30 @@ describe Projects::MergeRequestsController do
     project.team << [user, :master]
   end
 
+  describe '#new' do
+    context 'merge request that removes a submodule' do
+      render_views
+
+      let(:fork_project) { create(:forked_project_with_submodules) }
+
+      before do
+        fork_project.team << [user, :master]
+      end
+
+      it 'renders it' do
+        get :new,
+            namespace_id: fork_project.namespace.to_param,
+            project_id: fork_project.to_param,
+            merge_request: {
+              source_branch: 'remove-submodule',
+              target_branch: 'master'
+            }
+
+        expect(response).to be_success
+      end
+    end
+  end
+
   describe "#show" do
     shared_examples "export merge as" do |format|
       it "should generally work" do
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 787670e9297..78b9a0f42fa 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -21,7 +21,8 @@ module TestEnv
   # We currently only need a subset of the branches
   FORKED_BRANCH_SHA = {
     'add-submodule-version-bump' => '3f547c08',
-    'master' => '5937ac0'
+    'master' => '5937ac0',
+    'remove-submodule' => '2a33e0c0'
   }
 
   # Test environment

From d800a949d2d5497e8aff3ae28ec8520e5b99cdb8 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 3 Dec 2015 23:33:52 -0800
Subject: [PATCH 29/55] Fix Error 500 when creating global milestones with
 Unicode characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two issues:

1. The constraints in the resources were incorrect. Here's what it was before:
```
group_milestone  GET /groups/:group_id/milestones/:id(.:format)  groups/milestones#show {:id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/, :group_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/}
```

In this case, id is actually the title of the milestone, which can be anything at the moment.

After:

```
group_milestone  GET /groups/:group_id/milestones/:id(.:format)  groups/milestones#show {:id=>/[^\/]+/, :group_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/}
```

2. `parameterize` would strip all Unicode characters, leaving a blank string. Rails would report something like:

ActionView::Template::Error (No route matches {:action=>"show", :controller=>"groups/milestones", :group_id=>#<Group id: 48, name: "ops-dev", path: "ops-dev", owner_id: nil, created_at: "2015-11-15 08:55:30", updated_at: "2015-12-02 06:23:26", type: "Group", description: "", avatar: "sha1.c71e73d51af1865c1bbbf6208e10044d46c9bb93.png", public: false>, :id=>"", :title=>"肯定不是中文的问题"} missing required keys: [:id]):

This change uses the babosa library to create a better slug, which surprisingly
isn't actually used by the global milestone controllers. Instead, they use the
title passed as a query string for some reason.

Closes https://github.com/gitlabhq/gitlabhq/issues/9881

Fix constraints
---
 CHANGELOG                                     |  1 +
 Gemfile                                       |  1 +
 Gemfile.lock                                  |  2 ++
 .../groups/milestones_controller.rb           |  2 +-
 app/models/global_milestone.rb                |  2 +-
 config/routes.rb                              |  2 +-
 .../groups/milestones_controller_spec.rb      | 27 +++++++++++++++++++
 7 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 spec/controllers/groups/milestones_controller_spec.rb

diff --git a/CHANGELOG b/CHANGELOG
index 7b2f1528656..228848c13f2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ v 8.3.0 (unreleased)
 
 v 8.2.3
   - Fix application settings cache not expiring after changes (Stan Hu)
+  - Fix Error 500s when creating global milestones with Unicode characters (Stan Hu)
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)
diff --git a/Gemfile b/Gemfile
index 67640bb9ae0..860e6bee47d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -171,6 +171,7 @@ gem "underscore-rails", "~> 1.4.4"
 
 # Sanitize user input
 gem "sanitize", '~> 2.0'
+gem 'babosa', '~> 1.0.2'
 
 # Protect against bruteforcing
 gem "rack-attack", '~> 4.3.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index cd1855758b2..be3e39d8e84 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -73,6 +73,7 @@ GEM
       descendants_tracker (~> 0.0.4)
       ice_nine (~> 0.11.0)
       thread_safe (~> 0.3, >= 0.3.1)
+    babosa (1.0.2)
     bcrypt (3.1.10)
     benchmark-ips (2.3.0)
     better_errors (1.0.1)
@@ -823,6 +824,7 @@ DEPENDENCIES
   asciidoctor (~> 1.5.2)
   attr_encrypted (~> 1.3.4)
   awesome_print (~> 1.2.0)
+  babosa (~> 1.0.2)
   benchmark-ips
   better_errors (~> 1.0.1)
   binding_of_caller (~> 0.7.2)
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index 10233222ee1..0c2a350bc39 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -46,7 +46,7 @@ class Groups::MilestonesController < Groups::ApplicationController
   end
 
   def milestone_path(title)
-    group_milestone_path(@group, title.parameterize, title: title)
+    group_milestone_path(@group, title.to_slug.to_s, title: title)
   end
 
   def projects
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 1321ccd963f..85aa71662fe 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -16,7 +16,7 @@ class GlobalMilestone
   end
 
   def safe_title
-    @title.parameterize
+    @title.to_slug.to_s
   end
 
   def projects
diff --git a/config/routes.rb b/config/routes.rb
index 5c114452a3f..fdd387fd184 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -368,7 +368,7 @@ Rails.application.routes.draw do
       end
 
       resource :avatar, only: [:destroy]
-      resources :milestones, only: [:index, :show, :update, :new, :create]
+      resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
     end
   end
 
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
new file mode 100644
index 00000000000..eb0c6ac6d80
--- /dev/null
+++ b/spec/controllers/groups/milestones_controller_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe Groups::MilestonesController do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+  let(:project2) { create(:empty_project, group: group) }
+  let(:user)    { create(:user) }
+  let(:title) { '肯定不是中文的问题' }
+
+  before do
+    sign_in(user)
+    group.add_owner(user)
+    project.team << [user, :master]
+    controller.instance_variable_set(:@group, group)
+  end
+
+  describe "#create" do
+    it "should create group milestone with Chinese title" do
+      post :create,
+           group_id: group.id,
+           milestone: { project_ids: [project.id, project2.id], title: title }
+
+      expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title))
+      expect(Milestone.where(title: title).count).to eq(2)
+    end
+  end
+end

From 4fab178850f512ad15715b6f723ace6fce7882fc Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 4 Dec 2015 22:56:10 -0800
Subject: [PATCH 30/55] Fix spec that broke due to fact that iid is needed, not
 id, for MilestonesController

---
 spec/controllers/projects/milestones_controller_spec.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 8127efabe6e..d173bb350f1 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::MilestonesController do
   let(:user)    { create(:user) }
   let(:milestone) { create(:milestone, project: project) }
   let(:issue) { create(:issue, project: project, milestone: milestone) }
-  let(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
+  let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
 
   before do
     sign_in(user)
@@ -15,10 +15,9 @@ describe Projects::MilestonesController do
 
   describe "#destroy" do
     it "should remove milestone" do
-      merge_request.reload
       expect(issue.milestone_id).to eq(milestone.id)
 
-      delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.id, format: :js
+      delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid, format: :js
       expect(response).to be_success
 
       expect(Event.first.action).to eq(Event::DESTROYED)

From 176d6e2a8ff97a33d533495aa3a2775dbb87284f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Sat, 5 Dec 2015 22:09:52 +0100
Subject: [PATCH 31/55] Refactor note awards to reuse `emoji_pattern` and
 improve validator

---
 app/models/note.rb | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/app/models/note.rb b/app/models/note.rb
index 03640be7c93..2bee19479c5 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -39,9 +39,11 @@ class Note < ActiveRecord::Base
   delegate :name, to: :project, prefix: true
   delegate :name, :email, to: :author, prefix: true
 
+  before_validation :set_award!
+
   validates :note, :project, presence: true
   validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award }
-  validates :note, format: { with: /\A[-_+[:alnum:]]*\z/ }, if: -> (n){ n.is_award }
+  validates :note, inclusion: { in: Emoji.emojis_names }, if: ->(n) { n.is_award }
   validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
   # Attachments are deprecated and are handled by Markdown uploader
   validates :attachment, file_size: { maximum: :max_attachment_size }
@@ -72,7 +74,6 @@ class Note < ActiveRecord::Base
 
   serialize :st_diff
   before_create :set_diff, if: ->(n) { n.line_code.present? }
-  before_validation :set_award!
 
   class << self
     def discussions_from_notes(notes)
@@ -351,36 +352,31 @@ class Note < ActiveRecord::Base
     !system?
   end
 
-  # Checks if note is an award added from an issue comment.
+  # Checks if note is an award added as a comment
   #
-  # If note is an award, this method sets is_award to true,
-  # and changes note content to award-emoji name.
-  #
-  # Awards are only supported for issue comments.
+  # If note is an award, this method sets is_award to true
+  #   and changes content of the note to award name.
   #
   # Method is executed as a before_validation callback.
   #
   def set_award!
-    return unless supports_awards? && contains_emoji_only?
-
+    return unless awards_supported? && contains_emoji_only?
     self.is_award = true
     self.note = award_emoji_name
   end
 
-  def supports_awards?
-    noteable.kind_of?(Issue) ||
-      noteable.is_a?(MergeRequest)
-  end
-
   private
 
+  def awards_supported?
+    noteable.kind_of?(Issue) || noteable.is_a?(MergeRequest)
+  end
+
   def contains_emoji_only?
-    (note =~ /\A:[-_+[:alnum:]]*:\s?\z/) ? true : false
+    emoji_only_pattern = /\A#{Gitlab::Markdown::EmojiFilter.emoji_pattern}\s?\Z/
+    (note =~ emoji_only_pattern) ? true : false
   end
 
   def award_emoji_name
-    return nil unless contains_emoji_only?
-
-    note.match(/\A:([-_+[:alnum:]]*):\s?/)[1]
+    note.match(Gitlab::Markdown::EmojiFilter.emoji_pattern)[1]
   end
 end

From bfe91b692a89f7a5ee8a0b044fabf5ec397b2904 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Sat, 5 Dec 2015 22:18:13 +0100
Subject: [PATCH 32/55] Remove space before exclamation mark in award alert

[ci skip]
---
 app/assets/javascripts/notes.js.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 4f559e86378..dd6cbcfc70b 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -113,7 +113,7 @@ class @Notes
   renderNote: (note) ->
     unless note.valid
       if note.award
-        flash = new Flash('You have already used this award emoji !', 'alert')
+        flash = new Flash('You have already used this award emoji!', 'alert')
         flash.pin()
       return
 

From ee134d09e7cedb57cd021314d5de459a4adf3d4d Mon Sep 17 00:00:00 2001
From: Sytse Sijbrandij <sytse@gitlab.com>
Date: Sat, 5 Dec 2015 15:06:32 -0800
Subject: [PATCH 33/55] Move release cycle comments to the documentation.

---
 README.md             | 2 +-
 doc/release/README.md | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 52e2d977620..4cc9b350635 100644
--- a/README.md
+++ b/README.md
@@ -82,7 +82,7 @@ There are a lot of [third-party applications integrating with GitLab](https://ab
 
 ## GitLab release cycle
 
-Since 2011 a minor or major version of GitLab is released on the 22nd of every month. Patch and security releases are published when needed. New features are detailed on the [blog](https://about.gitlab.com/blog/) and in the [changelog](CHANGELOG). For more information about the release process see the [release documentation](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/release). Features that will likely be in the next releases can be found on the [feature request forum](http://feedback.gitlab.com/forums/176466-general) with the status [started](http://feedback.gitlab.com/forums/176466-general/status/796456) and [completed](http://feedback.gitlab.com/forums/176466-general/status/796457).
+For more information about the release process see the [release documentation](http://doc.gitlab.com/ce/release/).
 
 ## Upgrading
 
diff --git a/doc/release/README.md b/doc/release/README.md
index 1342b90f3b3..52eca7c02a6 100644
--- a/doc/release/README.md
+++ b/doc/release/README.md
@@ -1,4 +1,8 @@
-GitLab has the following updates:
+## Release cycle
+
+Since 2011 a minor or major version of GitLab is released on the 22nd of every month. Patch and security releases are published when needed. New features are detailed on the [blog](https://about.gitlab.com/blog/) and in the [changelog](CHANGELOG).  Features that will likely be in the next releases can be found on the [direction page](https://about.gitlab.com/direction/).
+
+## Release process documentation
 
 - [Monthly release](monthly.md), every month on the 22nd.
 - [Patch release](patch.md), if there are serious regressions.

From 1c4213acd5dde6ce44a70b79dd766e9e7f8b59b4 Mon Sep 17 00:00:00 2001
From: Vyacheslav Stetskevych <stetskevych@gmail.com>
Date: Sun, 6 Dec 2015 03:10:29 +0200
Subject: [PATCH 34/55] Fix gitlab-ssl nginx config to work when multiple
 server_names are served over https

---
 lib/support/nginx/gitlab-ssl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl
index 016f7a536fb..79fe1474821 100644
--- a/lib/support/nginx/gitlab-ssl
+++ b/lib/support/nginx/gitlab-ssl
@@ -56,7 +56,7 @@ server {
   listen [::]:80 ipv6only=on default_server;
   server_name YOUR_SERVER_FQDN; ## Replace this with something like gitlab.example.com
   server_tokens off; ## Don't show the nginx version number, a security best practice
-  return 301 https://$server_name$request_uri;
+  return 301 https://$http_host$request_uri;
   access_log  /var/log/nginx/gitlab_access.log;
   error_log   /var/log/nginx/gitlab_error.log;
 }

From 631a30276e30354cfde6b759527abbb26ff6cf96 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 5 Dec 2015 17:36:19 -0800
Subject: [PATCH 35/55] Fix API setting of 'public' attribute to false will
 make a project private

Closes #3864
---
 CHANGELOG                          |  1 +
 lib/api/projects.rb                |  8 ++++++--
 spec/requests/api/projects_spec.rb | 12 ++++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 7b2f1528656..65be6b05478 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.3.0 (unreleased)
+  - Fix API setting of 'public' attribute to false will make a project private (Stan Hu)
   - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera)
   - Fix 500 error when update group member permission
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 2b4ada6e2eb..6928fe0eb9d 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -7,8 +7,12 @@ module API
       helpers do
         def map_public_to_visibility_level(attrs)
           publik = attrs.delete(:public)
-          publik = parse_boolean(publik)
-          attrs[:visibility_level] = Gitlab::VisibilityLevel::PUBLIC if !attrs[:visibility_level].present? && publik == true
+          if publik.present? && !attrs[:visibility_level].present?
+            publik = parse_boolean(publik)
+            # Since setting the public attribute to private could mean either
+            # private or internal, use the more conservative option, private.
+            attrs[:visibility_level] = (publik == true) ? Gitlab::VisibilityLevel::PUBLIC : Gitlab::VisibilityLevel::PRIVATE
+          end
           attrs
         end
       end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index c59ee7af8ab..24b765f4979 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -742,6 +742,18 @@ describe API::API, api: true  do
         end
       end
 
+      it 'should update visibility_level from public to private' do
+        project3.update_attributes({ visibility_level: Gitlab::VisibilityLevel::PUBLIC })
+
+        project_param = { public: false }
+        put api("/projects/#{project3.id}", user), project_param
+        expect(response.status).to eq(200)
+        project_param.each_pair do |k, v|
+          expect(json_response[k.to_s]).to eq(v)
+        end
+        expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE)
+      end
+
       it 'should not update name to existing name' do
         project_param = { name: project3.name }
         put api("/projects/#{project.id}", user), project_param

From 893d08c0dc6a1eba14db7694636707f30b28a7f4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Mon, 7 Dec 2015 11:00:03 +0100
Subject: [PATCH 36/55] Simplify `contains_emoji_only?` method in `Note`

---
 app/models/note.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/models/note.rb b/app/models/note.rb
index 2bee19479c5..239a0f77f8e 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -372,8 +372,7 @@ class Note < ActiveRecord::Base
   end
 
   def contains_emoji_only?
-    emoji_only_pattern = /\A#{Gitlab::Markdown::EmojiFilter.emoji_pattern}\s?\Z/
-    (note =~ emoji_only_pattern) ? true : false
+    note =~ /\A#{Gitlab::Markdown::EmojiFilter.emoji_pattern}\s?\Z/
   end
 
   def award_emoji_name

From f5ec1ebe2caa0d8b4ccccb671ae6a4cc99cddbe0 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 7 Dec 2015 12:07:13 +0100
Subject: [PATCH 37/55] Remove changelog entry issue number

---
 CHANGELOG | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 882648b36a2..00a24d4317b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,7 +6,7 @@ v 8.3.0 (unreleased)
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
   - Add ignore whitespace change option to commit view
   - Fire update hook from GitLab
-  - Run custom Git hooks when branch is created or deleted. #1156
+  - Run custom Git hooks when branch is created or deleted.
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)

From 839a8b924972d87e37085333adf1b74b41bbd26c Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 7 Dec 2015 12:07:40 +0100
Subject: [PATCH 38/55] Move changelog item

---
 CHANGELOG | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 9c00aa7a657..2cb341d882d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -11,10 +11,10 @@ v 8.3.0 (unreleased)
   - Add API endpoint to fetch merge request commits list
   - Expose events API with comment information and author info
   - Fix: Ensure "Remove Source Branch" button is not shown when branch is being deleted. #3583
+  - Run custom Git hooks when branch is created or deleted.
 
 v 8.2.3
   - Fix application settings cache not expiring after changes (Stan Hu)
-  - Run custom Git hooks when branch is created or deleted.
 
 v 8.2.2
   - Fix 404 in redirection after removing a project (Stan Hu)

From 5df2c4419c5019b5003ddfa6adb59c84c3d9910c Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Mon, 7 Dec 2015 14:11:15 +0200
Subject: [PATCH 39/55] fox specs

---
 app/models/commit.rb            | 37 ++++++++++++++++++++-------------
 app/models/merge_request.rb     |  2 +-
 lib/gitlab/push_data_builder.rb |  4 +++-
 spec/models/commit_spec.rb      |  2 +-
 4 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/app/models/commit.rb b/app/models/commit.rb
index 912b4dedf51..fecadfeec8e 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -135,10 +135,10 @@ class Commit
     description.present?
   end
 
-  def hook_attrs
+  def hook_attrs(with_changed_files = false)
     path_with_namespace = project.path_with_namespace
 
-    {
+    data = {
       id: id,
       message: safe_message,
       timestamp: committed_date.xmlschema,
@@ -146,11 +146,18 @@ class Commit
       author: {
         name: author_name,
         email: author_email
-      },
-      added: repo_changes[:added],
-      modified: repo_changes[:modified],
-      removed: repo_changes[:removed]
+      }
     }
+
+    if with_changed_files
+      data.merge!({
+        added: repo_changes[:added],
+        modified: repo_changes[:modified],
+        removed: repo_changes[:removed]
+      })
+    end
+
+    data
   end
 
   # Discover issues should be closed when this commit is pushed to a project's
@@ -205,14 +212,16 @@ class Commit
   def repo_changes
     changes = { added: [], modified: [], removed: [] }
 
-    diffs.each do |diff|
-      case true
-      when diff.deleted_file
-        changes[:removed] << diff.old_path
-      when diff.renamed_file, diff.new_file
-        changes[:added] << diff.new_path
-      else
-        changes[:modified] << diff.new_path
+    if diffs.any?
+      diffs.each do |diff|
+        case true
+        when diff.deleted_file
+          changes[:removed] << diff.old_path
+        when diff.renamed_file, diff.new_file
+          changes[:added] << diff.new_path
+        else
+          changes[:modified] << diff.new_path
+        end
       end
     end
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1b3d6079d2c..92a82d44c76 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -291,7 +291,7 @@ class MergeRequest < ActiveRecord::Base
       work_in_progress: work_in_progress?
     }
 
-    unless last_commit.nil?
+    if last_commit
       attrs.merge!(last_commit: last_commit.hook_attrs)
     end
 
diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb
index cdcdb02a052..5842b740e8e 100644
--- a/lib/gitlab/push_data_builder.rb
+++ b/lib/gitlab/push_data_builder.rb
@@ -30,7 +30,9 @@ module Gitlab
 
         # For performance purposes maximum 20 latest commits
         # will be passed as post receive hook data.
-        commit_attrs = commits_limited.map(&:hook_attrs)
+        commit_attrs = commits_limited.map do |commit|
+          commit.hook_attrs(true)
+        end
 
         type = Gitlab::Git.tag_ref?(ref) ? "tag_push" : "push"
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index b417bc98fa7..6728722b503 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -102,7 +102,7 @@ eos
   end
 
   describe '#hook_attrs' do
-    let(:data) { commit.hook_attrs }
+    let(:data) { commit.hook_attrs(true) }
 
     it { expect(data).to be_a(Hash) }
     it { expect(data[:message]).to include('Add submodule from gitlab.com') }

From ff08ce9ca4bef1a4f81f7a4b323614a639efe959 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 7 Dec 2015 13:45:00 +0100
Subject: [PATCH 40/55] Satisfy Rubocop

---
 app/models/global_milestone.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 33ddb265fba..dd9f88704a6 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -127,4 +127,4 @@ class GlobalMilestone
       end
     end
   end
-end
\ No newline at end of file
+end

From 3c97cbc74cf87856ed7b1af197358d4e3adb1240 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Mon, 7 Dec 2015 15:13:06 +0200
Subject: [PATCH 41/55] fixes after review

---
 app/models/commit.rb            | 25 +++++++++----------------
 lib/gitlab/push_data_builder.rb |  2 +-
 spec/models/commit_spec.rb      |  2 +-
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/app/models/commit.rb b/app/models/commit.rb
index fecadfeec8e..14883c96f5f 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -135,7 +135,7 @@ class Commit
     description.present?
   end
 
-  def hook_attrs(with_changed_files = false)
+  def hook_attrs(with_changed_files: false)
     path_with_namespace = project.path_with_namespace
 
     data = {
@@ -150,11 +150,7 @@ class Commit
     }
 
     if with_changed_files
-      data.merge!({
-        added: repo_changes[:added],
-        modified: repo_changes[:modified],
-        removed: repo_changes[:removed]
-      })
+      data.merge!(repo_changes)
     end
 
     data
@@ -212,16 +208,13 @@ class Commit
   def repo_changes
     changes = { added: [], modified: [], removed: [] }
 
-    if diffs.any?
-      diffs.each do |diff|
-        case true
-        when diff.deleted_file
-          changes[:removed] << diff.old_path
-        when diff.renamed_file, diff.new_file
-          changes[:added] << diff.new_path
-        else
-          changes[:modified] << diff.new_path
-        end
+    diffs.each do |diff|
+      if diff.deleted_file
+        changes[:removed] << diff.old_path
+      elsif diff.renamed_file || diff.new_file
+        changes[:added] << diff.new_path
+      else
+        changes[:modified] << diff.new_path
       end
     end
 
diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb
index 5842b740e8e..4f9cdef3869 100644
--- a/lib/gitlab/push_data_builder.rb
+++ b/lib/gitlab/push_data_builder.rb
@@ -31,7 +31,7 @@ module Gitlab
         # For performance purposes maximum 20 latest commits
         # will be passed as post receive hook data.
         commit_attrs = commits_limited.map do |commit|
-          commit.hook_attrs(true)
+          commit.hook_attrs(with_changed_files: true)
         end
 
         type = Gitlab::Git.tag_ref?(ref) ? "tag_push" : "push"
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 6728722b503..0b1e2bf74d0 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -102,7 +102,7 @@ eos
   end
 
   describe '#hook_attrs' do
-    let(:data) { commit.hook_attrs(true) }
+    let(:data) { commit.hook_attrs(with_changed_files: true) }
 
     it { expect(data).to be_a(Hash) }
     it { expect(data[:message]).to include('Add submodule from gitlab.com') }

From 8a0507d523293289ae4b7d61bb3fce8a873e3dcc Mon Sep 17 00:00:00 2001
From: "Michael A. Smith" <msmith3@ebay.com>
Date: Mon, 7 Dec 2015 10:29:42 -0500
Subject: [PATCH 42/55] Update Docker Syntax

---
 doc/ci/docker/using_docker_images.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md
index 64e52eba3a2..1feae62b1c7 100644
--- a/doc/ci/docker/using_docker_images.md
+++ b/doc/ci/docker/using_docker_images.md
@@ -190,7 +190,7 @@ This will create two service containers (MySQL and PostgreSQL).
 
 1. Create a build container and execute script in its context:
 ```
-$ cat build_script | docker run -n build -i -l mysql:service-mysql -l postgres:service-postgres ruby:2.1 /bin/bash
+$ docker run --name build -i --link=service-mysql:mysql --link=service-postgres:postgres ruby:2.1 /bin/bash < build_script
 ```
 This will create build container that has two service containers linked.
 The build_script is piped using STDIN to bash interpreter which executes the build script in container. 

From 2cec90254f5753ac1a8b92931613aaa6c9f19cf7 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Dec 2015 19:37:05 +0100
Subject: [PATCH 43/55] Dont use cached collection for Repository find_branch
 and find_tag methods

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
 CHANGELOG                | 1 +
 app/models/repository.rb | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 13207552e8e..f1ee21ec368 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ v 8.3.0 (unreleased)
   - Expose events API with comment information and author info
   - Fix: Ensure "Remove Source Branch" button is not shown when branch is being deleted. #3583
   - Run custom Git hooks when branch is created or deleted.
+  - Fix bug when simultaneously accepting multiple MRs results in MRs that are of "merged" status, but not merged to the target branch
 
 v 8.2.3
   - Fix application settings cache not expiring after changes (Stan Hu)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index c304955b0b3..1d43307e1e7 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -100,11 +100,11 @@ class Repository
   end
 
   def find_branch(name)
-    branches.find { |branch| branch.name == name }
+    raw_repository.branches.find { |branch| branch.name == name }
   end
 
   def find_tag(name)
-    tags.find { |tag| tag.name == name }
+    raw_repository.tags.find { |tag| tag.name == name }
   end
 
   def add_branch(user, branch_name, target)

From 74d73bd953c0a083cbf8aaf919a7034067542382 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Dec 2015 20:09:30 +0100
Subject: [PATCH 44/55] Fix random failing test

Make sure we wait till page reloads after request was merged. Otherwise
we get request running which fails next test

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
 features/project/merge_requests/accept.feature      | 6 ++++--
 features/steps/project/merge_requests/acceptance.rb | 4 ++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/features/project/merge_requests/accept.feature b/features/project/merge_requests/accept.feature
index 3e6e59a3808..9bc2b7c8eca 100644
--- a/features/project/merge_requests/accept.feature
+++ b/features/project/merge_requests/accept.feature
@@ -8,10 +8,12 @@ Feature: Project Merge Requests Acceptance
     Given I am on the Merge Request detail page
     When I click on "Remove source branch" option
     And I click on Accept Merge Request
-    Then I should not see the Remove Source Branch button
+    Then I should see merge request merged
+    And I should not see the Remove Source Branch button
 
   @javascript
   Scenario: Accepting the Merge Request without removing the source branch
     Given I am on the Merge Request detail page
     When I click on Accept Merge Request
-    Then I should see the Remove Source Branch button
+    Then I should see merge request merged
+    And I should see the Remove Source Branch button
diff --git a/features/steps/project/merge_requests/acceptance.rb b/features/steps/project/merge_requests/acceptance.rb
index 6adecaa8385..383c055c4ef 100644
--- a/features/steps/project/merge_requests/acceptance.rb
+++ b/features/steps/project/merge_requests/acceptance.rb
@@ -32,4 +32,8 @@ class Spinach::Features::ProjectMergeRequestsAcceptance < Spinach::FeatureSteps
   step 'I am signed in as a developer of the project' do
     login_as(@user)
   end
+
+  step 'I should see merge request merged' do
+    expect(page).to have_content('The changes were merged into')
+  end
 end

From a1d3b8d7fa61f757978e237dd95de8eeb186212c Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 7 Dec 2015 16:50:20 -0500
Subject: [PATCH 45/55] Fix spec failure introduced by
 9d03bc6fa31f123e070bab4a58b67dbb008e75e9

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

diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 0a64b70d6a6..5568f06639c 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -278,7 +278,7 @@ describe ApplicationHelper do
       el = element.next_element
 
       expect(el.name).to eq 'script'
-      expect(el.text).to include "$('.js-timeago').timeago()"
+      expect(el.text).to include "$('.js-timeago').last().timeago()"
     end
 
     it 'allows the script tag to be excluded' do

From d5ea93469b4ec95916361c61876c949f60539211 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 1 Dec 2015 18:45:36 -0500
Subject: [PATCH 46/55] Add custom UrlValidator

---
 app/models/application_setting.rb             |  4 +--
 app/models/ci/web_hook.rb                     |  3 +-
 app/models/hooks/web_hook.rb                  |  3 +-
 app/models/project.rb                         |  2 +-
 app/models/project_services/bamboo_service.rb |  7 ++--
 .../project_services/drone_ci_service.rb      | 29 +++++++--------
 .../project_services/external_wiki_service.rb |  6 ++--
 .../project_services/teamcity_service.rb      | 10 +++---
 app/validators/url_validator.rb               | 36 +++++++++++++++++++
 spec/models/application_setting_spec.rb       | 16 +++++++++
 10 files changed, 79 insertions(+), 37 deletions(-)
 create mode 100644 app/validators/url_validator.rb

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 5ddcf3d9a0b..1880ad9f33c 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -43,12 +43,12 @@ class ApplicationSetting < ActiveRecord::Base
 
   validates :home_page_url,
     allow_blank: true,
-    format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" },
+    url: true,
     if: :home_page_url_column_exist
 
   validates :after_sign_out_path,
     allow_blank: true,
-    format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }
+    url: true
 
   validates :admin_notification_email,
     allow_blank: true,
diff --git a/app/models/ci/web_hook.rb b/app/models/ci/web_hook.rb
index 7ca16a1bde8..0dc15eb6683 100644
--- a/app/models/ci/web_hook.rb
+++ b/app/models/ci/web_hook.rb
@@ -20,8 +20,7 @@ module Ci
     # HTTParty timeout
     default_timeout 10
 
-    validates :url, presence: true,
-                    format: { with: URI::regexp(%w(http https)), message: "should be a valid url" }
+    validates :url, presence: true, url: true
 
     def execute(data)
       parsed_url = URI.parse(url)
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 2caf26cc8c9..715ec5908b7 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -31,8 +31,7 @@ class WebHook < ActiveRecord::Base
   # HTTParty timeout
   default_timeout Gitlab.config.gitlab.webhook_timeout
 
-  validates :url, presence: true,
-                  format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }
+  validates :url, presence: true, url: true
 
   def execute(data, hook_name)
     parsed_url = URI.parse(url)
diff --git a/app/models/project.rb b/app/models/project.rb
index 6010770a5f2..af034a6692b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -152,7 +152,7 @@ class Project < ActiveRecord::Base
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
   validates :import_url,
-    format: { with: /\A#{URI.regexp(%w(ssh git http https))}\z/, message: 'should be a valid url' },
+    url: { protocols: %w(ssh git http https) },
     if: :external_import?
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index d31b12f539e..0a61ad96a0e 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -23,10 +23,7 @@ class BambooService < CiService
 
   prop_accessor :bamboo_url, :build_key, :username, :password
 
-  validates :bamboo_url,
-    presence: true,
-    format: { with: /\A#{URI.regexp}\z/ },
-    if: :activated?
+  validates :bamboo_url, presence: true, url: true, if: :activated?
   validates :build_key, presence: true, if: :activated?
   validates :username,
     presence: true,
@@ -84,7 +81,7 @@ class BambooService < CiService
   def supported_events
     %w(push)
   end
-  
+
   def build_info(sha)
     url = URI.parse("#{bamboo_url}/rest/api/latest/result?label=#{sha}")
 
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 06c3922593c..08e5ccb3855 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -19,14 +19,11 @@
 #
 
 class DroneCiService < CiService
-  
+
   prop_accessor :drone_url, :token, :enable_ssl_verification
-  validates :drone_url,
-    presence: true,
-    format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }, if: :activated?
-  validates :token,
-    presence: true,
-    if: :activated?
+
+  validates :drone_url, presence: true, url: true, if: :activated?
+  validates :token, presence: true, if: :activated?
 
   after_save :compose_service_hook, if: :activated?
 
@@ -58,16 +55,16 @@ class DroneCiService < CiService
   end
 
   def merge_request_status_path(iid, sha = nil, ref = nil)
-    url = [drone_url, 
-           "gitlab/#{project.namespace.path}/#{project.path}/pulls/#{iid}", 
+    url = [drone_url,
+           "gitlab/#{project.namespace.path}/#{project.path}/pulls/#{iid}",
            "?access_token=#{token}"]
 
     URI.join(*url).to_s
   end
 
   def commit_status_path(sha, ref)
-    url = [drone_url, 
-           "gitlab/#{project.namespace.path}/#{project.path}/commits/#{sha}", 
+    url = [drone_url,
+           "gitlab/#{project.namespace.path}/#{project.path}/commits/#{sha}",
            "?branch=#{URI::encode(ref.to_s)}&access_token=#{token}"]
 
     URI.join(*url).to_s
@@ -114,15 +111,15 @@ class DroneCiService < CiService
   end
 
   def merge_request_page(iid, sha, ref)
-    url = [drone_url, 
+    url = [drone_url,
            "gitlab/#{project.namespace.path}/#{project.path}/redirect/pulls/#{iid}"]
 
     URI.join(*url).to_s
   end
 
   def commit_page(sha, ref)
-    url = [drone_url, 
-           "gitlab/#{project.namespace.path}/#{project.path}/redirect/commits/#{sha}", 
+    url = [drone_url,
+           "gitlab/#{project.namespace.path}/#{project.path}/redirect/commits/#{sha}",
            "?branch=#{URI::encode(ref.to_s)}"]
 
     URI.join(*url).to_s
@@ -163,10 +160,10 @@ class DroneCiService < CiService
   end
 
   def push_valid?(data)
-    opened_merge_requests = project.merge_requests.opened.where(source_project_id: project.id, 
+    opened_merge_requests = project.merge_requests.opened.where(source_project_id: project.id,
                                                                 source_branch: Gitlab::Git.ref_name(data[:ref]))
 
-    opened_merge_requests.empty? && data[:total_commits_count] > 0 && 
+    opened_merge_requests.empty? && data[:total_commits_count] > 0 &&
       !Gitlab::Git.blank_ref?(data[:after])
   end
 
diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb
index 9c46af7e721..74c57949b4d 100644
--- a/app/models/project_services/external_wiki_service.rb
+++ b/app/models/project_services/external_wiki_service.rb
@@ -22,10 +22,8 @@ class ExternalWikiService < Service
   include HTTParty
 
   prop_accessor :external_wiki_url
-  validates :external_wiki_url,
-            presence: true,
-            format: { with: /\A#{URI.regexp}\z/ },
-            if: :activated?
+
+  validates :external_wiki_url, presence: true, url: true, if: :activated?
 
   def title
     'External Wiki'
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index 0b022461250..29d4236745a 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -23,16 +23,16 @@ class TeamcityService < CiService
 
   prop_accessor :teamcity_url, :build_type, :username, :password
 
-  validates :teamcity_url,
-    presence: true,
-    format: { with: /\A#{URI.regexp}\z/ }, if: :activated?
+  validates :teamcity_url, presence: true, url: true, if: :activated?
   validates :build_type, presence: true, if: :activated?
   validates :username,
     presence: true,
-    if: ->(service) { service.password? }, if: :activated?
+    if: ->(service) { service.password? },
+    if: :activated?
   validates :password,
     presence: true,
-    if: ->(service) { service.username? }, if: :activated?
+    if: ->(service) { service.username? },
+    if: :activated?
 
   attr_accessor :response
 
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
new file mode 100644
index 00000000000..2848b9cd33d
--- /dev/null
+++ b/app/validators/url_validator.rb
@@ -0,0 +1,36 @@
+# UrlValidator
+#
+# Custom validator for URLs.
+#
+# By default, only URLs for the HTTP(S) protocols will be considered valid.
+# Provide a `:protocols` option to configure accepted protocols.
+#
+# Example:
+#
+#   class User < ActiveRecord::Base
+#     validates :personal_url, url: true
+#
+#     validates :ftp_url, url: { protocols: %w(ftp) }
+#
+#     validates :git_url, url: { protocols: %w(http https ssh git) }
+#   end
+#
+class UrlValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless valid_url?(value)
+      record.errors.add(attribute, "must be a valid URL")
+    end
+  end
+
+  private
+
+  def default_options
+    @default_options ||= { protocols: %w(http https) }
+  end
+
+  def valid_url?(value)
+    options = default_options.merge(self.options)
+
+    value =~ /\A#{URI.regexp(options[:protocols])}\z/
+  end
+end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index dfbac7b4004..b67b84959d9 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -36,6 +36,22 @@ describe ApplicationSetting, models: true do
 
   it { expect(setting).to be_valid }
 
+  describe 'validations' do
+    let(:http)  { 'http://example.com' }
+    let(:https) { 'https://example.com' }
+    let(:ftp)   { 'ftp://example.com' }
+
+    it { is_expected.to allow_value(nil).for(:home_page_url) }
+    it { is_expected.to allow_value(http).for(:home_page_url) }
+    it { is_expected.to allow_value(https).for(:home_page_url) }
+    it { is_expected.not_to allow_value(ftp).for(:home_page_url) }
+
+    it { is_expected.to allow_value(nil).for(:after_sign_out_path) }
+    it { is_expected.to allow_value(http).for(:after_sign_out_path) }
+    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) }
+  end
+
   context 'restricted signup domains' do
     it 'set single domain' do
       setting.restricted_signup_domains_raw = 'example.com'

From b3200c8c44f2351d88d5d78d7ded3ac06001bd7c Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 1 Dec 2015 18:46:00 -0500
Subject: [PATCH 47/55] Move EmailValidator to app/validators

---
 {lib => app/validators}/email_validator.rb | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename {lib => app/validators}/email_validator.rb (100%)

diff --git a/lib/email_validator.rb b/app/validators/email_validator.rb
similarity index 100%
rename from lib/email_validator.rb
rename to app/validators/email_validator.rb

From e48391b813d3e5079238aa3f0662e7a46e1b4a54 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 1 Dec 2015 18:53:44 -0500
Subject: [PATCH 48/55] Add custom ColorValidator

---
 app/models/broadcast_message.rb         |  8 ++++----
 app/models/label.rb                     |  4 +---
 app/validators/color_validator.rb       | 20 ++++++++++++++++++++
 features/steps/admin/labels.rb          |  2 +-
 features/steps/project/issues/labels.rb |  2 +-
 spec/models/broadcast_message_spec.rb   | 15 +++++++++++++++
 spec/requests/api/labels_spec.rb        | 10 +++++-----
 7 files changed, 47 insertions(+), 14 deletions(-)
 create mode 100644 app/validators/color_validator.rb

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 05f5e979695..ad514706160 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -16,12 +16,12 @@
 class BroadcastMessage < ActiveRecord::Base
   include Sortable
 
-  validates :message, presence: true
+  validates :message,   presence: true
   validates :starts_at, presence: true
-  validates :ends_at, presence: true
+  validates :ends_at,   presence: true
 
-  validates :color, format: { with: /\A\#[0-9A-Fa-f]{3}{1,2}+\Z/ }, allow_blank: true
-  validates :font,  format: { with: /\A\#[0-9A-Fa-f]{3}{1,2}+\Z/ }, allow_blank: true
+  validates :color, allow_blank: true, color: true
+  validates :font,  allow_blank: true, color: true
 
   def self.current
     where("ends_at > :now AND starts_at < :now", now: Time.zone.now).last
diff --git a/app/models/label.rb b/app/models/label.rb
index bef6063fe88..220da10a6ab 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -27,9 +27,7 @@ class Label < ActiveRecord::Base
   has_many :label_links, dependent: :destroy
   has_many :issues, through: :label_links, source: :target, source_type: 'Issue'
 
-  validates :color,
-            format: { with: /\A#[0-9A-Fa-f]{6}\Z/ },
-            allow_blank: false
+  validates :color, color: true, allow_blank: false
   validates :project, presence: true, unless: Proc.new { |service| service.template? }
 
   # Don't allow '?', '&', and ',' for label titles
diff --git a/app/validators/color_validator.rb b/app/validators/color_validator.rb
new file mode 100644
index 00000000000..571d0007aa2
--- /dev/null
+++ b/app/validators/color_validator.rb
@@ -0,0 +1,20 @@
+# ColorValidator
+#
+# Custom validator for web color codes. It requires the leading hash symbol and
+# will accept RGB triplet or hexadecimal formats.
+#
+# Example:
+#
+#   class User < ActiveRecord::Base
+#     validates :background_color, allow_blank: true, color: true
+#   end
+#
+class ColorValidator < ActiveModel::EachValidator
+  PATTERN = /\A\#[0-9A-Fa-f]{3}{1,2}+\Z/.freeze
+
+  def validate_each(record, attribute, value)
+    unless value =~ PATTERN
+      record.errors.add(attribute, "must be a valid color code")
+    end
+  end
+end
diff --git a/features/steps/admin/labels.rb b/features/steps/admin/labels.rb
index 2ea5dffdc66..55ddcc25085 100644
--- a/features/steps/admin/labels.rb
+++ b/features/steps/admin/labels.rb
@@ -71,7 +71,7 @@ class Spinach::Features::AdminIssuesLabels < Spinach::FeatureSteps
 
   step 'I should see label color error message' do
     page.within '.label-form' do
-      expect(page).to have_content 'Color is invalid'
+      expect(page).to have_content 'Color must be a valid color code'
     end
   end
 
diff --git a/features/steps/project/issues/labels.rb b/features/steps/project/issues/labels.rb
index e273bb391b3..2ab8956867b 100644
--- a/features/steps/project/issues/labels.rb
+++ b/features/steps/project/issues/labels.rb
@@ -55,7 +55,7 @@ class Spinach::Features::ProjectIssuesLabels < Spinach::FeatureSteps
 
   step 'I should see label color error message' do
     page.within '.label-form' do
-      expect(page).to have_content 'Color is invalid'
+      expect(page).to have_content 'Color must be a valid color code'
     end
   end
 
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index d80748f23a4..2b325f44f64 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -20,6 +20,21 @@ describe BroadcastMessage do
 
   it { is_expected.to be_valid }
 
+  describe 'validations' do
+    let(:triplet) { '#000' }
+    let(:hex)     { '#AABBCC' }
+
+    it { is_expected.to allow_value(nil).for(:color) }
+    it { is_expected.to allow_value(triplet).for(:color) }
+    it { is_expected.to allow_value(hex).for(:color) }
+    it { is_expected.not_to allow_value('000').for(:color) }
+
+    it { is_expected.to allow_value(nil).for(:font) }
+    it { is_expected.to allow_value(triplet).for(:font) }
+    it { is_expected.to allow_value(hex).for(:font) }
+    it { is_expected.not_to allow_value('000').for(:font) }
+  end
+
   describe :current do
     it "should return last message if time match" do
       broadcast_message = create(:broadcast_message, starts_at: Time.now.yesterday, ends_at: Time.now.tomorrow)
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index aff109a9424..667f0dbea5c 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -47,7 +47,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAA'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
 
     it 'should return 400 for too long color code' do
@@ -55,7 +55,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAAFFFF'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
 
     it 'should return 400 for invalid name' do
@@ -151,12 +151,12 @@ describe API::API, api: true  do
       expect(json_response['message']['title']).to eq(['is invalid'])
     end
 
-    it 'should return 400 for invalid name' do
+    it 'should return 400 when color code is too short' do
       put api("/projects/#{project.id}/labels", user),
           name: 'label1',
           color: '#FF'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
 
     it 'should return 400 for too long color code' do
@@ -164,7 +164,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAAFFFF'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
   end
 end

From 96e51a0304022664c06a025f4a54c4a41c25edd2 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 1 Dec 2015 19:21:45 -0500
Subject: [PATCH 49/55] Minor EmailValidator refactor

---
 app/validators/email_validator.rb | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/app/validators/email_validator.rb b/app/validators/email_validator.rb
index f509f0a5843..b35af100803 100644
--- a/app/validators/email_validator.rb
+++ b/app/validators/email_validator.rb
@@ -1,3 +1,5 @@
+# EmailValidator
+#
 # Based on https://github.com/balexand/email_validator
 #
 # Extended to use only strict mode with following allowed characters:
@@ -6,15 +8,10 @@
 # See http://www.remote.org/jochen/mail/info/chars.html
 #
 class EmailValidator < ActiveModel::EachValidator
-  @@default_options = {}
-
-  def self.default_options
-    @@default_options
-  end
+  PATTERN = /\A\s*([-a-z0-9+._']{1,64})@((?:[-a-z0-9]+\.)+[a-z]{2,})\s*\z/i.freeze
 
   def validate_each(record, attribute, value)
-    options = @@default_options.merge(self.options)
-    unless value =~ /\A\s*([-a-z0-9+._']{1,64})@((?:[-a-z0-9]+\.)+[a-z]{2,})\s*\z/i
+    unless value =~ PATTERN
       record.errors.add(attribute, options[:message] || :invalid)
     end
   end

From ad6a771dc680b52e4b46c73f20bc39340d08bf32 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 1 Dec 2015 19:30:01 -0500
Subject: [PATCH 50/55] Add custom LineCodeValidator

---
 app/models/note.rb                    |  2 +-
 app/models/sent_notification.rb       |  2 +-
 app/validators/line_code_validator.rb | 12 ++++++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)
 create mode 100644 app/validators/line_code_validator.rb

diff --git a/app/models/note.rb b/app/models/note.rb
index 239a0f77f8e..8d433c57ceb 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -44,7 +44,7 @@ class Note < ActiveRecord::Base
   validates :note, :project, presence: true
   validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award }
   validates :note, inclusion: { in: Emoji.emojis_names }, if: ->(n) { n.is_award }
-  validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
+  validates :line_code, line_code: true, allow_blank: true
   # Attachments are deprecated and are handled by Markdown uploader
   validates :attachment, file_size: { maximum: :max_attachment_size }
 
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index d8fe65b06f6..f36eda1531b 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -21,7 +21,7 @@ class SentNotification < ActiveRecord::Base
   validates :reply_key, uniqueness: true
   validates :noteable_id, presence: true, unless: :for_commit?
   validates :commit_id, presence: true, if: :for_commit?
-  validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
+  validates :line_code, line_code: true, allow_blank: true
 
   class << self
     def reply_key
diff --git a/app/validators/line_code_validator.rb b/app/validators/line_code_validator.rb
new file mode 100644
index 00000000000..ed29e5aeb67
--- /dev/null
+++ b/app/validators/line_code_validator.rb
@@ -0,0 +1,12 @@
+# LineCodeValidator
+#
+# Custom validator for GitLab line codes.
+class LineCodeValidator < ActiveModel::EachValidator
+  PATTERN = /\A[a-z0-9]+_\d+_\d+\z/.freeze
+
+  def validate_each(record, attribute, value)
+    unless value =~ PATTERN
+      record.errors.add(attribute, "must be a valid line code")
+    end
+  end
+end

From 9321d382bd5a0697e0e15a5065ec274e75541851 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 7 Dec 2015 16:17:12 -0500
Subject: [PATCH 51/55] Add custom NamespaceValidator

---
 app/models/namespace.rb               |  8 +++-----
 app/models/user.rb                    |  6 ++----
 app/validators/namespace_validator.rb | 22 ++++++++++++++++++++++
 spec/models/user_spec.rb              | 18 +++++++++++++++++-
 spec/requests/api/users_spec.rb       |  4 ++--
 5 files changed, 46 insertions(+), 12 deletions(-)
 create mode 100644 app/validators/namespace_validator.rb

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 20b92e68d61..e07c676a9f3 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -30,12 +30,10 @@ class Namespace < ActiveRecord::Base
 
   validates :description, length: { within: 0..255 }
   validates :path,
-    uniqueness: { case_sensitive: false },
-    presence: true,
     length: { within: 1..255 },
-    exclusion: { in: Gitlab::Blacklist.path },
-    format: { with: Gitlab::Regex.namespace_regex,
-              message: Gitlab::Regex.namespace_regex_message }
+    namespace: true,
+    presence: true,
+    uniqueness: { case_sensitive: false }
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 719b49b16fe..cfed797e725 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -148,11 +148,9 @@ class User < ActiveRecord::Base
   validates :bio, length: { maximum: 255 }, allow_blank: true
   validates :projects_limit, presence: true, numericality: { greater_than_or_equal_to: 0 }
   validates :username,
+    namespace: true,
     presence: true,
-    uniqueness: { case_sensitive: false },
-    exclusion: { in: Gitlab::Blacklist.path },
-    format: { with: Gitlab::Regex.namespace_regex,
-              message: Gitlab::Regex.namespace_regex_message }
+    uniqueness: { case_sensitive: false }
 
   validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true
   validate :namespace_uniq, if: ->(user) { user.username_changed? }
diff --git a/app/validators/namespace_validator.rb b/app/validators/namespace_validator.rb
new file mode 100644
index 00000000000..4ab1706abda
--- /dev/null
+++ b/app/validators/namespace_validator.rb
@@ -0,0 +1,22 @@
+# NamespaceValidator
+#
+# Custom validator for GitLab namespace values.
+#
+# Values are checked for formatting and exclusion from `Gitlab::Blacklist.path`.
+class NamespaceValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless value =~ Gitlab::Regex.namespace_regex
+      record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
+    end
+
+    if blacklisted?(value)
+      record.errors.add(attribute, "#{value} is a reserved name")
+    end
+  end
+
+  private
+
+  def blacklisted?(value)
+    Gitlab::Blacklist.path.include?(value)
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 4631b12faf1..a0f78d3b336 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -91,7 +91,23 @@ describe User do
   end
 
   describe 'validations' do
-    it { is_expected.to validate_presence_of(:username) }
+    describe 'username' do
+      it 'validates presence' do
+        expect(subject).to validate_presence_of(:username)
+      end
+
+      it 'rejects blacklisted names' do
+        user = build(:user, username: 'dashboard')
+
+        expect(user).not_to be_valid
+        expect(user.errors.values).to eq [['dashboard is a reserved name']]
+      end
+
+      it 'validates uniqueness' do
+        expect(subject).to validate_uniqueness_of(:username)
+      end
+    end
+
     it { is_expected.to validate_presence_of(:projects_limit) }
     it { is_expected.to validate_numericality_of(:projects_limit) }
     it { is_expected.to allow_value(0).for(:projects_limit) }
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index a9ef2fe5885..2f609c63330 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -153,7 +153,7 @@ describe API::API, api: true  do
       expect(json_response['message']['projects_limit']).
         to eq(['must be greater than or equal to 0'])
       expect(json_response['message']['username']).
-        to eq([Gitlab::Regex.send(:namespace_regex_message)])
+        to eq([Gitlab::Regex.namespace_regex_message])
     end
 
     it "shouldn't available for non admin users" do
@@ -296,7 +296,7 @@ describe API::API, api: true  do
       expect(json_response['message']['projects_limit']).
         to eq(['must be greater than or equal to 0'])
       expect(json_response['message']['username']).
-        to eq([Gitlab::Regex.send(:namespace_regex_message)])
+        to eq([Gitlab::Regex.namespace_regex_message])
     end
 
     context "with existing user" do

From 175f482c3cd584ba73c66e65aa180c1107e72913 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 7 Dec 2015 16:17:24 -0500
Subject: [PATCH 52/55] Add custom NamespaceNameValidator

---
 app/models/namespace.rb                    |  6 +++---
 app/validators/namespace_name_validator.rb | 10 ++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 app/validators/namespace_name_validator.rb

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index e07c676a9f3..1c4e101cc10 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -23,10 +23,10 @@ class Namespace < ActiveRecord::Base
 
   validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
   validates :name,
-    presence: true, uniqueness: true,
     length: { within: 0..255 },
-    format: { with: Gitlab::Regex.namespace_name_regex,
-              message: Gitlab::Regex.namespace_name_regex_message }
+    namespace_name: true,
+    presence: true,
+    uniqueness: true
 
   validates :description, length: { within: 0..255 }
   validates :path,
diff --git a/app/validators/namespace_name_validator.rb b/app/validators/namespace_name_validator.rb
new file mode 100644
index 00000000000..2e51af2982d
--- /dev/null
+++ b/app/validators/namespace_name_validator.rb
@@ -0,0 +1,10 @@
+# NamespaceNameValidator
+#
+# Custom validator for GitLab namespace name strings.
+class NamespaceNameValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless value =~ Gitlab::Regex.namespace_name_regex
+      record.errors.add(attribute, Gitlab::Regex.namespace_name_regex_message)
+    end
+  end
+end

From 2379c8beeac600c3352e33fda0c2b4f4f39c8b84 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 7 Dec 2015 16:29:39 -0500
Subject: [PATCH 53/55] Inline Gitlab::Blacklist in NamespaceValidator

---
 app/validators/namespace_validator.rb | 36 ++++++++++++++++++++++++---
 lib/gitlab/blacklist.rb               | 34 -------------------------
 2 files changed, 32 insertions(+), 38 deletions(-)
 delete mode 100644 lib/gitlab/blacklist.rb

diff --git a/app/validators/namespace_validator.rb b/app/validators/namespace_validator.rb
index 4ab1706abda..10e35ce665a 100644
--- a/app/validators/namespace_validator.rb
+++ b/app/validators/namespace_validator.rb
@@ -2,21 +2,49 @@
 #
 # Custom validator for GitLab namespace values.
 #
-# Values are checked for formatting and exclusion from `Gitlab::Blacklist.path`.
+# Values are checked for formatting and exclusion from a list of reserved path
+# names.
 class NamespaceValidator < ActiveModel::EachValidator
+  RESERVED = %w(
+    admin
+    all
+    assets
+    ci
+    dashboard
+    files
+    groups
+    help
+    hooks
+    issues
+    merge_requests
+    notes
+    profile
+    projects
+    public
+    repository
+    s
+    search
+    services
+    snippets
+    teams
+    u
+    unsubscribes
+    users
+  ).freeze
+
   def validate_each(record, attribute, value)
     unless value =~ Gitlab::Regex.namespace_regex
       record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
     end
 
-    if blacklisted?(value)
+    if reserved?(value)
       record.errors.add(attribute, "#{value} is a reserved name")
     end
   end
 
   private
 
-  def blacklisted?(value)
-    Gitlab::Blacklist.path.include?(value)
+  def reserved?(value)
+    RESERVED.include?(value)
   end
 end
diff --git a/lib/gitlab/blacklist.rb b/lib/gitlab/blacklist.rb
deleted file mode 100644
index 43145e0ee1b..00000000000
--- a/lib/gitlab/blacklist.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-module Gitlab
-  module Blacklist
-    extend self
-
-    def path
-      %w(
-        admin
-        dashboard
-        files
-        groups
-        help
-        profile
-        projects
-        search
-        public
-        assets
-        u
-        s
-        teams
-        merge_requests
-        issues
-        users
-        snippets
-        services
-        repository
-        hooks
-        notes
-        unsubscribes
-        all
-        ci
-      )
-    end
-  end
-end

From bdb4945dcf3c899c6a88fd42657a1b3f04baced1 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Dec 2015 23:08:22 +0100
Subject: [PATCH 54/55] Fix random failing test - delete attachment

Make sure we wait for AJAX request to finish before end test and cleanup
database

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
 app/assets/javascripts/notes.js.coffee        |  4 ++--
 spec/features/notes_on_merge_requests_spec.rb |  5 +++--
 spec/support/wait_for_ajax.rb                 | 11 +++++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 spec/support/wait_for_ajax.rb

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index dd6cbcfc70b..533d00bfb0c 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -369,8 +369,8 @@ class @Notes
     note = $(this).closest(".note")
     note.find(".note-attachment").remove()
     note.find(".note-body > .note-text").show()
-    note.find(".js-note-attachment-delete").hide()
-    note.find(".note-edit-form").hide()
+    note.find(".note-header").show()
+    note.find(".current-note-edit-form").remove()
 
   ###
   Called when clicking on the "reply" button for a diff line.
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index d7cb3b2e86e..f0fc6916c4d 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe 'Comments', feature: true do
   include RepoHelpers
+  include WaitForAjax
 
   describe 'On a merge request', js: true, feature: true do
     let!(:merge_request) { create(:merge_request) }
@@ -123,8 +124,8 @@ describe 'Comments', feature: true do
         it 'removes the attachment div and resets the edit form' do
           find('.js-note-attachment-delete').click
           is_expected.not_to have_css('.note-attachment')
-          expect(find('.current-note-edit-form', visible: false)).
-            not_to be_visible
+          is_expected.not_to have_css('.current-note-edit-form')
+          wait_for_ajax
         end
       end
     end
diff --git a/spec/support/wait_for_ajax.rb b/spec/support/wait_for_ajax.rb
new file mode 100644
index 00000000000..692d219e9f1
--- /dev/null
+++ b/spec/support/wait_for_ajax.rb
@@ -0,0 +1,11 @@
+module WaitForAjax
+  def wait_for_ajax
+    Timeout.timeout(Capybara.default_wait_time) do
+      loop until finished_all_ajax_requests?
+    end
+  end
+
+  def finished_all_ajax_requests?
+    page.evaluate_script('jQuery.active').zero?
+  end
+end

From 2e270c0705895f7535ca3f62abc1c1a9cbff769d Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Tue, 1 Dec 2015 16:02:26 +0200
Subject: [PATCH 55/55] add explicit reference to rouge 1.10.1

---
 Gemfile      | 3 ++-
 Gemfile.lock | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Gemfile b/Gemfile
index 25e30e0211a..fc4d565fc84 100644
--- a/Gemfile
+++ b/Gemfile
@@ -99,7 +99,7 @@ gem 'org-ruby',      '~> 0.9.12'
 gem 'creole',        '~> 0.5.0'
 gem 'wikicloth',     '0.8.1'
 gem 'asciidoctor',   '~> 1.5.2'
-gem 'net-ssh',       '~> 3.0.1'
+gem 'rouge',         '~> 1.10.1'
 
 # Diffs
 gem 'diffy', '~> 3.0.3'
@@ -205,6 +205,7 @@ gem 'raphael-rails',      '~> 2.1.2'
 gem 'request_store',      '~> 1.2.0'
 gem 'select2-rails',      '~> 3.5.9'
 gem 'virtus',             '~> 1.0.1'
+gem 'net-ssh',            '~> 3.0.1'
 
 group :development do
   gem "foreman"
diff --git a/Gemfile.lock b/Gemfile.lock
index 9e7b93be3ab..5d70788d981 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -940,6 +940,7 @@ DEPENDENCIES
   request_store (~> 1.2.0)
   rerun (~> 0.10.0)
   responders (~> 2.0)
+  rouge (~> 1.10.1)
   rqrcode-rails3 (~> 0.1.7)
   rspec-rails (~> 3.3.0)
   rubocop (~> 0.28.0)