Merge branch 'master' into fix-merge-request-that-removes-submodule

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 v 8.3.0 (unreleased)
-  - Fix application settings cache not expiring after changes (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)
+  - Recognize issue/MR/snippet/commit links as references
   - Add ignore whitespace change option to commit view
   - Fire update hook from GitLab
+  - Style warning about mentioning many people in a comment
+  - Fix: sort milestones by due date once again (Greg Smethells)
   - Don't show project fork event as "imported"
   - 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
+  - Run custom Git hooks when branch is created or deleted.
+
+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.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)
@@ -21,6 +31,7 @@ v 8.2.2
   - Prevent "413 Request entity too large" errors when pushing large files with LFS
   - Fix invalid links within projects dashboard header
   - Make current user the first user in assignee dropdown in issues detail page (Stan Hu)
+  - Fix: duplicate email notifications on issue comments
 v 8.2.1
   - Forcefully update builds that didn't want to update with state machine

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'

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)

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)

app/assets/javascripts/flash.js.coffee

 class @Flash
   constructor: (message, type)->
-    flash = $(".flash-container")
-    flash.html("")
-    $('<div/>',
+    @flash = $(".flash-container")
+    @flash.html("")
+    innerDiv = $('<div/>',
       class: "flash-#{type}",
       text: message
-    ).appendTo(".flash-container")
-    flash.click -> $(@).fadeOut()
-    flash.show()
+    )
+    innerDiv.appendTo(".flash-container")
+    @flash.click -> $(@).fadeOut()
+    @flash.show()
+
+  pin: ->
+    @flash.addClass('flash-pinned flash-raised')

app/assets/javascripts/notes.js.coffee

@@ -111,6 +111,12 @@ class @Notes
   Note: for rendering inline notes use renderDiscussionNote
   ###
   renderNote: (note) ->
+    unless note.valid
+      if note.award
+        flash = new Flash('You have already used this award emoji!', 'alert')
+        flash.pin()
+      return
+
     # render note if it not present in loaded list
     # or skip if rendered
     if @isNewNote(note) && !note.award
@@ -122,6 +128,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

app/assets/javascripts/user.js.coffee

@@ -2,3 +2,9 @@ class @User
   constructor: ->
     $('.profile-groups-avatars').tooltip("placement": "top")
     new ProjectsList()
+
+    $('.hide-project-limit-message').on 'click', (e) ->
+      path = '/'
+      $.cookie('hide_project_limit_message', 'false', { path: path })
+      $(@).parents('.project-limit-message').remove()
+      e.preventDefault()

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;
+}

app/assets/stylesheets/framework/markdown_area.scss

@@ -73,11 +73,8 @@
 }
 .referenced-users {
-  padding: 10px 0;
-  color: #999;
-  margin-left: 10px;
-  margin-top: 1px;
-  margin-right: 130px;
+  color: #4c4e54;
+  padding-top: 10px;
 }
 .md-preview-holder {

app/assets/stylesheets/pages/projects.scss

@@ -5,7 +5,7 @@
     font-weight: normal;
   }
 }
-.no-ssh-key-message {
+.no-ssh-key-message, .project-limit-message {
   background-color: #f28d35;
   margin-bottom: 16px;
 }

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

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

app/controllers/profiles_controller.rb

@@ -70,6 +70,7 @@ class ProfilesController < Profiles::ApplicationController
       :email,
       :hide_no_password,
       :hide_no_ssh_key,
+      :hide_project_limit,
       :linkedin,
       :location,
       :name,

app/controllers/projects/notes_controller.rb

@@ -131,16 +131,25 @@ 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: {
+        valid: true,
+        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: {
+        valid: false,
+        award: note.is_award,
+        errors: note.errors
+      }
+    end
   end
   def authorize_admin_note!

app/finders/milestones_finder.rb

 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

app/helpers/issues_helper.rb

@@ -87,7 +87,11 @@ module IssuesHelper
   end
   def merge_requests_sentence(merge_requests)
-    merge_requests.map(&:to_reference).to_sentence(last_word_connector: ', or ')
+    # Sorting based on the `!123` or `group/project!123` reference will sort
+    # local merge requests first.
+    merge_requests.map do |merge_request|
+      merge_request.to_reference(@project)
+    end.sort.to_sentence(last_word_connector: ', or ')
   end
   def url_to_emoji(name)

app/helpers/merge_requests_helper.rb

@@ -39,7 +39,11 @@ module MergeRequestsHelper
   end
   def issues_sentence(issues)
-    issues.map(&:to_reference).to_sentence
+    # Sorting based on the `#123` or `group/project#123` reference will sort
+    # local issues first.
+    issues.map do |issue|
+      issue.to_reference(@project)
+    end.sort.to_sentence
   end
   def mr_change_branches_path(merge_request)

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)

app/models/commit.rb

@@ -78,11 +78,23 @@ class Commit
     }x
   end
+  def self.link_reference_pattern
+    super("commit", /(?<commit>\h{6,40})/)
+  end
+
   def to_reference(from_project = nil)
     if cross_project_reference?(from_project)
-      "#{project.to_reference}@#{id}"
+      project.to_reference + self.class.reference_prefix + self.id
     else
-      id
+      self.id
+    end
+  end
+
+  def reference_link_text(from_project = nil)
+    if cross_project_reference?(from_project)
+      project.to_reference + self.class.reference_prefix + self.short_id
+    else
+      self.short_id
     end
   end
@@ -135,10 +147,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,
@@ -148,6 +160,12 @@ class Commit
         email: author_email
       }
     }
+
+    if with_changed_files
+      data.merge!(repo_changes)
+    end
+
+    data
   end
   # Discover issues should be closed when this commit is pushed to a project's
@@ -196,4 +214,22 @@ class Commit
   def status
     ci_commit.try(:status) || :not_found
   end
+
+  private
+
+  def repo_changes
+    changes = { added: [], modified: [], removed: [] }
+
+    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
+
+    changes
+  end
 end

app/models/commit_range.rb

@@ -2,36 +2,38 @@
 #
 # Examples:
 #
-#   range = CommitRange.new('f3f85602...e86e1013')
+#   range = CommitRange.new('f3f85602...e86e1013', project)
 #   range.exclude_start?  # => false
 #   range.reference_title # => "Commits f3f85602 through e86e1013"
 #   range.to_s            # => "f3f85602...e86e1013"
 #
-#   range = CommitRange.new('f3f856029bc5f966c5a7ee24cf7efefdd20e6019..e86e1013709735be5bb767e2b228930c543f25ae')
+#   range = CommitRange.new('f3f856029bc5f966c5a7ee24cf7efefdd20e6019..e86e1013709735be5bb767e2b228930c543f25ae', project)
 #   range.exclude_start?  # => true
 #   range.reference_title # => "Commits f3f85602^ through e86e1013"
 #   range.to_param        # => {from: "f3f856029bc5f966c5a7ee24cf7efefdd20e6019^", to: "e86e1013709735be5bb767e2b228930c543f25ae"}
 #   range.to_s            # => "f3f85602..e86e1013"
 #
-#   # Assuming `project` is a Project with a repository containing both commits:
-#   range.project = project
+#   # Assuming the specified project has a repository containing both commits:
 #   range.valid_commits? # => true
 #
 class CommitRange
   include ActiveModel::Conversion
   include Referable
-  attr_reader :sha_from, :notation, :sha_to
+  attr_reader :commit_from, :notation, :commit_to
+  attr_reader :ref_from, :ref_to
   # Optional Project model
   attr_accessor :project
-  # See `exclude_start?`
-  attr_reader :exclude_start
-
-  # The beginning and ending SHAs can be between 6 and 40 hex characters, and
+  # The beginning and ending refs can be named or SHAs, and
   # the range notation can be double- or triple-dot.
-  PATTERN = /\h{6,40}\.{2,3}\h{6,40}/
+  REF_PATTERN = /[0-9a-zA-Z][0-9a-zA-Z_.-]*[0-9a-zA-Z\^]/
+  PATTERN = /#{REF_PATTERN}\.{2,3}#{REF_PATTERN}/
+
+  # In text references, the beginning and ending refs can only be SHAs
+  # between 6 and 40 hex characters.
+  STRICT_PATTERN = /\h{6,40}\.{2,3}\h{6,40}/
   def self.reference_prefix
     '@'
@@ -43,27 +45,40 @@ class CommitRange
   def self.reference_pattern
     %r{
       (?:#{Project.reference_pattern}#{reference_prefix})?
-      (?<commit_range>#{PATTERN})
+      (?<commit_range>#{STRICT_PATTERN})
     }x
   end
+  def self.link_reference_pattern
+    super("compare", /(?<commit_range>#{PATTERN})/)
+  end
+
   # Initialize a CommitRange
   #
   # range_string - The String commit range.
   # project      - An optional Project model.
   #
   # Raises ArgumentError if `range_string` does not match `PATTERN`.
-  def initialize(range_string, project = nil)
+  def initialize(range_string, project)
+    @project = project
+
     range_string.strip!
-    unless range_string.match(/\A#{PATTERN}\z/)
+    unless range_string =~ /\A#{PATTERN}\z/
       raise ArgumentError, "invalid CommitRange string format: #{range_string}"
     end
-    @exclude_start = !range_string.include?('...')
-    @sha_from, @notation, @sha_to = range_string.split(/(\.{2,3})/, 2)
-    @project = project
+    @ref_from, @notation, @ref_to = range_string.split(/(\.{2,3})/, 2)
+    if project.valid_repo?
+      @commit_from = project.commit(@ref_from)
+      @commit_to   = project.commit(@ref_to)
+    end
+
+    if valid_commits?
+      @ref_from = Commit.truncate_sha(sha_from) if sha_from.start_with?(@ref_from)
+      @ref_to   = Commit.truncate_sha(sha_to)   if sha_to.start_with?(@ref_to)
+    end
   end
   def inspect
@@ -71,15 +86,24 @@ class CommitRange
   end
   def to_s
-    "#{sha_from[0..7]}#{notation}#{sha_to[0..7]}"
+    sha_from + notation + sha_to
   end
+  alias_method :id, :to_s
+
   def to_reference(from_project = nil)
-    # Not using to_s because we want the full SHAs
-    reference = sha_from + notation + sha_to
+    if cross_project_reference?(from_project)
+      project.to_reference + self.class.reference_prefix + self.id
+    else
+      self.id
+    end
+  end
+
+  def reference_link_text(from_project = nil)
+    reference = ref_from + notation + ref_to
     if cross_project_reference?(from_project)
-      reference = project.to_reference + '@' + reference
+      reference = project.to_reference + self.class.reference_prefix + reference
     end
     reference
@@ -87,46 +111,58 @@ class CommitRange
   # Returns a String for use in a link's title attribute
   def reference_title
-    "Commits #{suffixed_sha_from} through #{sha_to}"
+    "Commits #{sha_start} through #{sha_to}"
   end
   # Return a Hash of parameters for passing to a URL helper
   #
   # See `namespace_project_compare_url`
   def to_param
-    { from: suffixed_sha_from, to: sha_to }
+    { from: sha_start, to: sha_to }
   end
   def exclude_start?
-    exclude_start
+    @notation == '..'
   end
   # Check if both the starting and ending commit IDs exist in a project's
   # repository
-  #
-  # project - An optional Project to check (default: `project`)
-  def valid_commits?(project = project)
-    return nil   unless project.present?
-    return false unless project.valid_repo?
-
-    commit_from.present? && commit_to.present?
+  def valid_commits?
+    commit_start.present? && commit_end.present?
   end
   def persisted?
     true
   end
-  def commit_from
-    @commit_from ||= project.repository.commit(suffixed_sha_from)
+  def sha_from
+    return nil unless @commit_from
+
+    @commit_from.id
+  end
+
+  def sha_to
+    return nil unless @commit_to
+
+    @commit_to.id
   end
-  def commit_to
-    @commit_to ||= project.repository.commit(sha_to)
+  def sha_start
+    return nil unless sha_from
+
+    exclude_start? ? sha_from + '^' : sha_from
   end
-  private
-  def suffixed_sha_from
-    sha_from + (exclude_start? ? '^' : '')
+  def commit_start
+    return nil unless sha_start
+    if exclude_start?
+      @commit_start ||= project.commit(sha_start)
+    else
+      commit_from
+    end
   end
+
+  alias_method :sha_end, :sha_to
+  alias_method :commit_end, :commit_to
 end