Merge branch '8-8-stable' of gitlab.com:gitlab-org/gitlab-ce into 8-8-stable-ee

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.8.0 (unreleased)
+  - Implement GFM references for milestones (Alejandro Rodríguez)
   - Snippets tab under user profile. !4001 (Long Nguyen)
   - Fix error when using link to uploads in global snippets
+  - Fix Error 500 when attempting to retrieve project license when HEAD points to non-existent ref
   - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
   - Use a case-insensitive comparison in sanitizing URI schemes
   - Toggle sign-up confirmation emails in application settings
   - Make it possible to prevent tagged runner from picking untagged jobs
+  - Added `InlineDiffFilter` to the markdown parser. (Adam Butler)
+  - Added inline diff styling for `change_title` system notes. (Adam Butler)
   - Project#open_branches has been cleaned up and no longer loads entire records into memory.
   - Escape HTML in commit titles in system note messages
   - Fix creation of Ci::Commit object which can lead to pending, failed in some scenarios
@@ -40,6 +44,7 @@ v 8.8.0 (unreleased)
   - Added button to toggle whitespaces changes on diff view
   - Backport GitHub Enterprise import support from EE
   - Create tags using Rugged for performance reasons. !3745
+  - Allow guests to set notification level in projects
   - API: Expose Issue#user_notes_count. !3126 (Anton Popov)
   - Don't show forks button when user can't view forks
   - Fix atom feed links and rendering
@@ -65,6 +70,7 @@ v 8.8.0 (unreleased)
   - All Grape API helpers are now instrumented
   - Improve Issue formatting for the Slack Service (Jeroen van Baarsen)
   - Fixed advice on invalid permissions on upload path !2948 (Ludovic Perrine)
+  - Allows MR authors to have the source branch removed when merging the MR. !2801 (Jeroen Jacobs)
 
 v 8.7.6
   - Fix links on wiki pages for relative url setups. !4131 (Artem Sidorenko)

VERSION

-8.8.0-rc2-ee
\ No newline at end of file
+8.8.0-rc2-ee

app/assets/javascripts/gfm_auto_complete.js.coffee

@@ -18,6 +18,10 @@ GitLab.GfmAutoComplete =
   Issues:
     template: '<li><small>${id}</small> ${title}</li>'
 
+  # Milestones
+  Milestones:
+    template: '<li>${title}</li>'
+
   # Add GFM auto-completion to all input fields, that accept GFM input.
   setup: (wrap) ->
     @input = $('.js-gfm-input')
@@ -81,6 +85,19 @@ GitLab.GfmAutoComplete =
             title:  sanitize(i.title)
             search: "#{i.iid} #{i.title}"
 
+    @input.atwho
+      at: '%'
+      alias: 'milestones'
+      searchKey: 'search'
+      displayTpl: @Milestones.template
+      insertTpl: '${atwho-at}"${title}"'
+      callbacks:
+        beforeSave: (milestones) ->
+          $.map milestones, (m) ->
+            id:     m.iid
+            title:  sanitize(m.title)
+            search: "#{m.title}"
+
     @input.atwho
       at: '!'
       alias: 'mergerequests'
@@ -105,6 +122,8 @@ GitLab.GfmAutoComplete =
     @input.atwho 'load', '@', data.members
     # load issues
     @input.atwho 'load', 'issues', data.issues
+    # load milestones
+    @input.atwho 'load', 'milestones', data.milestones
     # load merge requests
     @input.atwho 'load', 'mergerequests', data.mergerequests
     # load emojis

app/assets/stylesheets/framework/files.scss

@@ -36,22 +36,6 @@
       }
     }
 
-    .filename {
-      &.old {
-        display: inline-block;
-        span.idiff {
-          background-color: #f8cbcb;
-        }
-      }
-
-      &.new {
-        display: inline-block;
-        span.idiff {
-          background-color: #a6f3a6;
-        }
-      }
-    }
-
     a:not(.btn) {
       color: $gl-dark-link-color;
     }

app/assets/stylesheets/framework/typography.scss

@@ -269,3 +269,11 @@ h1, h2, h3, h4 {
     text-align: right;
   }
 }
+
+.idiff.deletion {
+  background: $line-removed-dark;
+}
+
+.idiff.addition {
+  background: $line-added-dark;
+}

app/assets/stylesheets/framework/variables.scss

@@ -178,6 +178,7 @@ $table-border-gray: #f0f0f0;
 $line-target-blue: #eaf3fc;
 $line-select-yellow: #fcf8e7;
 $line-select-yellow-dark: #f0e2bd;
+
 /*
  * Fonts
  */

app/controllers/projects/merge_requests_controller.rb

@@ -376,7 +376,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     params.require(:merge_request).permit(
       :title, :assignee_id, :source_project_id, :source_branch,
       :target_project_id, :target_branch, :milestone_id, :approver_ids,
-      :state_event, :description, :task_num, label_ids: []
+      :state_event, :description, :task_num, :force_remove_source_branch,
+      label_ids: []
     )
   end
 

app/controllers/projects_controller.rb

@@ -102,13 +102,7 @@ class ProjectsController < Projects::ApplicationController
 
     respond_to do |format|
       format.html do
-        if current_user
-          @membership = @project.team.find_member(current_user.id)
-
-          if @membership
-            @notification_setting = current_user.notification_settings_for(@project)
-          end
-        end
+        @notification_setting = current_user.notification_settings_for(@project) if current_user
 
         if @project.repository_exists?
           if @project.empty_repo?
@@ -148,6 +142,7 @@ class ProjectsController < Projects::ApplicationController
     @suggestions = {
       emojis: AwardEmoji.urls,
       issues: autocomplete.issues,
+      milestones: autocomplete.milestones,
       mergerequests: autocomplete.merge_requests,
       members: participants
     }

app/controllers/registrations_controller.rb

@@ -38,7 +38,7 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def after_sign_up_path_for(user)
-    user.confirmed_at.present? ? dashboard_projects_path : users_almost_there_path
+    user.confirmed? ? dashboard_projects_path : users_almost_there_path
   end
 
   def after_inactive_sign_up_path_for(_resource)

app/helpers/diff_helper.rb

@@ -2,8 +2,8 @@ module DiffHelper
   def mark_inline_diffs(old_line, new_line)
     old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs
 
-    marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs)
-    marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs)
+    marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs, mode: :deletion)
+    marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs, mode: :addition)
 
     [marked_old_line, marked_new_line]
   end

app/models/merge_request.rb

@@ -290,6 +290,18 @@ class MergeRequest < ActiveRecord::Base
       last_commit == source_project.commit(source_branch)
   end
 
+  def should_remove_source_branch?
+    merge_params['should_remove_source_branch'].present?
+  end
+
+  def force_remove_source_branch?
+    merge_params['force_remove_source_branch'].present?
+  end
+
+  def remove_source_branch?
+    should_remove_source_branch? || force_remove_source_branch?
+  end
+
   def mr_and_commit_notes
     # Fetch comments only from last 100 commits
     commits_for_notes_limit = 100
@@ -430,7 +442,10 @@ class MergeRequest < ActiveRecord::Base
 
     self.merge_when_build_succeeds = false
     self.merge_user = nil
-    self.merge_params = nil
+    if merge_params
+      merge_params.delete('should_remove_source_branch')
+      merge_params.delete('commit_message')
+    end
 
     self.save
   end

app/models/milestone.rb

@@ -60,8 +60,27 @@ class Milestone < ActiveRecord::Base
     end
   end
 
+  def self.reference_prefix
+    '%'
+  end
+
   def self.reference_pattern
-    nil
+    # NOTE: The iid pattern only matches when all characters on the expression
+    # are digits, so it will match %2 but not %2.1 because that's probably a
+    # milestone name and we want it to be matched as such.
+    @reference_pattern ||= %r{
+      (#{Project.reference_pattern})?
+      #{Regexp.escape(reference_prefix)}
+      (?:
+        (?<milestone_iid>
+          \d+(?!\S\w)\b # Integer-based milestone iid, or
+        ) |
+        (?<milestone_name>
+          [^"\s]+\b |  # String-based single-word milestone title, or
+          "[^"]+"      # String-based multi-word milestone surrounded in quotes
+        )
+      )
+    }x
   end
 
   def self.link_reference_pattern
@@ -82,13 +101,26 @@ class Milestone < ActiveRecord::Base
     end
   end
 
-  def to_reference(from_project = nil)
-    escaped_title = self.title.gsub("]", "\\]")
-
-    h = Gitlab::Routing.url_helpers
-    url = h.namespace_project_milestone_url(self.project.namespace, self.project, self)
+  ##
+  # Returns the String necessary to reference this Milestone in Markdown
+  #
+  # format - Symbol format to use (default: :iid, optional: :name)
+  #
+  # Examples:
+  #
+  #   Milestone.first.to_reference                # => "%1"
+  #   Milestone.first.to_reference(format: :name) # => "%\"goal\""
+  #   Milestone.first.to_reference(project)       # => "gitlab-org/gitlab-ce%1"
+  #
+  def to_reference(from_project = nil, format: :iid)
+    format_reference = milestone_format_reference(format)
+    reference = "#{self.class.reference_prefix}#{format_reference}"
 
-    "[#{escaped_title}](#{url})"
+    if cross_project_reference?(from_project)
+      project.to_reference + reference
+    else
+      reference
+    end
   end
 
   def reference_link_text(from_project = nil)
@@ -160,4 +192,16 @@ class Milestone < ActiveRecord::Base
     issues.where(id: ids).
       update_all(["position = CASE #{conditions} ELSE position END", *pairs])
   end
+
+  private
+
+  def milestone_format_reference(format = :iid)
+    raise ArgumentError, 'Unknown format' unless [:iid, :name].include?(format)
+
+    if format == :name && !name.include?('"')
+      %("#{name}")
+    else
+      iid
+    end
+  end
 end

app/models/repository.rb

@@ -529,7 +529,7 @@ class Repository
   end
 
   def license_blob
-    return nil if !exists? || empty?
+    return nil unless head_exists?
 
     cache.fetch(:license_blob) do
       tree(:head).blobs.find do |file|
@@ -539,7 +539,7 @@ class Repository
   end
 
   def license_key
-    return nil if !exists? || empty?
+    return nil unless head_exists?
 
     cache.fetch(:license_key) do
       Licensee.license(path).try(:key)
@@ -547,7 +547,7 @@ class Repository
   end
 
   def gitlab_ci_yml
-    return nil if !exists? || empty?
+    return nil unless head_exists?
 
     @gitlab_ci_yml ||= tree(:head).blobs.find do |file|
       file.name == '.gitlab-ci.yml'
@@ -1140,7 +1140,7 @@ class Repository
   end
 
   def main_language
-    return if empty? || rugged.head_unborn?
+    return unless head_exists?
 
     Linguist::Repository.new(rugged, rugged.head.target_id).language
   end
@@ -1160,4 +1160,8 @@ class Repository
   def cache
     @cache ||= RepositoryCache.new(path_with_namespace)
   end
+
+  def head_exists?
+    exists? && !empty? && !rugged.head_unborn?
+  end
 end

app/services/merge_requests/create_service.rb

@@ -8,11 +8,14 @@ module MergeRequests
       @project = Project.find(params[:target_project_id]) if params[:target_project_id]
 
       filter_params
-      label_params = params[:label_ids]
-      merge_request = MergeRequest.new(params.except(:label_ids))
+      label_params = params.delete(:label_ids)
+      force_remove_source_branch = params.delete(:force_remove_source_branch)
+
+      merge_request = MergeRequest.new(params)
       merge_request.source_project = source_project
       merge_request.target_project ||= source_project
       merge_request.author = current_user
+      merge_request.merge_params['force_remove_source_branch'] = force_remove_source_branch
 
       if merge_request.save
         merge_request.update_attributes(label_ids: label_params)

app/services/merge_requests/merge_service.rb

@@ -67,10 +67,14 @@ module MergeRequests
     def after_merge
       MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
 
-      if params[:should_remove_source_branch].present?
-        DeleteBranchService.new(@merge_request.source_project, current_user).
+      if params[:should_remove_source_branch].present? || @merge_request.force_remove_source_branch?
+        DeleteBranchService.new(@merge_request.source_project, branch_deletion_user).
           execute(merge_request.source_branch)
       end
     end
+
+    def branch_deletion_user
+      @merge_request.force_remove_source_branch? ? @merge_request.author : current_user
+    end
   end
 end

app/services/merge_requests/update_service.rb

@@ -11,6 +11,8 @@ module MergeRequests
       params.except!(:target_project_id)
       params.except!(:source_branch)
 
+      merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
+
       update(merge_request)
     end
 

app/services/projects/autocomplete_service.rb

@@ -4,6 +4,10 @@ module Projects
       @project.issues.visible_to_user(current_user).opened.select([:iid, :title])
     end
 
+    def milestones
+      @project.milestones.active.reorder(due_date: :asc, title: :asc).select([:iid, :title])
+    end
+
     def merge_requests
       @project.merge_requests.opened.select([:iid, :title])
     end

app/services/system_note_service.rb

@@ -169,7 +169,14 @@ class SystemNoteService
   #
   # Returns the created Note object
   def self.change_title(noteable, project, author, old_title)
-    body = "Title changed from **#{old_title}** to **#{noteable.title}**"
+    new_title = noteable.title.dup
+
+    old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_title, new_title).inline_diffs
+
+    marked_old_title = Gitlab::Diff::InlineDiffMarker.new(old_title).mark(old_diffs, mode: :deletion, markdown: true)
+    marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true)
+
+    body = "Changed title: **#{marked_old_title}** → **#{marked_new_title}**"
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
 

app/views/projects/diffs/_file.html.haml

@@ -11,11 +11,9 @@
       = link_to "#diff-#{i}" do
         - if diff_file.renamed_file
           - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
-          .filename.old
-            = old_path
+          = old_path
           →
-          .filename.new
-            = new_path
+          = new_path
         - else
           %span
             = diff_file.new_path

app/views/projects/merge_requests/widget/open/_accept.html.haml

@@ -25,7 +25,10 @@
         - else
           = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do
             Accept Merge Request
-      - if @merge_request.can_remove_source_branch?(current_user)
+      - if @merge_request.force_remove_source_branch?
+        .accept-control
+          The source branch will be removed.
+      - elsif @merge_request.can_remove_source_branch?(current_user)
         .accept-control.checkbox
           = label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
             = check_box_tag :should_remove_source_branch

app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml

@@ -2,7 +2,6 @@
   Set by #{link_to_member(@project, @merge_request.merge_user, avatar: true)}
   to be merged automatically when the build succeeds.
 %div
-  - should_remove_source_branch = @merge_request.merge_params["should_remove_source_branch"].present?
   %p
     = succeed '.' do
       - if @project.merge_requests_ff_only_enabled
@@ -10,12 +9,12 @@
       - else
         The changes will be merged into
       %span.label-branch= @merge_request.target_branch
-    - if should_remove_source_branch
+    - if @merge_request.remove_source_branch?
       The source branch will be removed.
     - else
       The source branch will not be removed.
 
-  - remove_source_branch_button = @merge_request.can_remove_source_branch?(current_user) && !should_remove_source_branch && @merge_request.merge_user == current_user
+  - remove_source_branch_button = !@merge_request.remove_source_branch? && @merge_request.can_remove_source_branch?(current_user) && @merge_request.merge_user == current_user
   - user_can_cancel_automatic_merge = @merge_request.can_cancel_merge_when_build_succeeds?(current_user)
   - if remove_source_branch_button || user_can_cancel_automatic_merge
     .clearfix.prepend-top-10

app/views/projects/merge_requests/widget/open/_not_allowed.html.haml

-%h4 
+%h4
   Ready to be merged automatically
 %p
   Ask someone with write access to this repository to merge this request.
+  - if @merge_request.force_remove_source_branch?
+    The source branch will be removed.

app/views/shared/issuable/_form.html.haml

@@ -161,6 +161,13 @@
       - if @merge_request.new_record?
          
         = link_to 'Change branches', mr_change_branches_path(@merge_request)
+  - if @merge_request.can_remove_source_branch?(current_user)
+    .form-group
+      .col-sm-10.col-sm-offset-2
+        .checkbox
+          = label_tag 'merge_request[force_remove_source_branch]' do
+            = check_box_tag 'merge_request[force_remove_source_branch]', '1', @merge_request.force_remove_source_branch?
+            Remove source branch when merge request is accepted.
 
 - is_footer = !(issuable.is_a?(MergeRequest) && issuable.new_record?)
 .row-content-block{class: (is_footer ? "footer-block" : "middle-block")}

db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb

@@ -7,7 +7,9 @@ class AddDefaultGroupVisibilityToApplicationSettings < ActiveRecord::Migration
     add_column :application_settings, :default_group_visibility, :integer
     # Unfortunately, this can't be a `default`, since we don't want the configuration specific
     # `allowed_visibility_level` to end up in schema.rb
-    execute("UPDATE application_settings SET default_group_visibility = #{allowed_visibility_level}")
+
+    visibility_level = allowed_visibility_level || Gitlab::VisibilityLevel::PRIVATE
+    execute("UPDATE application_settings SET default_group_visibility = #{visibility_level}")
   end
 
   def down

doc/markdown/markdown.md

@@ -8,6 +8,7 @@
 * [Multiple underscores in words](#multiple-underscores-in-words)
 * [URL auto-linking](#url-auto-linking)
 * [Code and Syntax Highlighting](#code-and-syntax-highlighting)
+* [Inline Diff](#inline-diff)
 * [Emoji](#emoji)
 * [Special GitLab references](#special-gitlab-references)
 * [Task lists](#task-lists)
@@ -153,6 +154,19 @@ s = "There is no highlighting for this."
 But let's throw in a <b>tag</b>.
 ```
 
+## Inline Diff
+
+With inline diffs tags you can display {+ additions +} or [- deletions -].
+
+The wrapping tags can be either curly braces or square brackets [+ additions +] or {- deletions -}.
+
+However the wrapping tags cannot be mixed as such:
+
+- {+ additions +]
+- [+ additions +}
+- {- deletions -]
+- [- deletions -}
+
 ## Emoji
 
 	Sometimes you want to :monkey: around a bit and add some :star2: to your :speech_balloon:. Well we have a gift for you:
@@ -185,20 +199,23 @@ GFM will turn that reference into a link so you can navigate between them easily
 
 GFM will recognize the following:
 
-| input                  | references                 |
-|:-----------------------|:---------------------------|
-| `@user_name`           | specific user              |
-| `@group_name`          | specific group             |
-| `@all`                 | entire team                |
-| `#123`                 | issue                      |
-| `!123`                 | merge request              |
-| `$123`                 | snippet                    |
-| `~123`                 | label by ID                |
-| `~bug`                 | one-word label by name     |
-| `~"feature request"`   | multi-word label by name   |
-| `9ba12248`             | specific commit            |
-| `9ba12248...b19a04f5`  | commit range comparison    |
-| `[README](doc/README)` | repository file references |
+| input                  | references                   |
+|:-----------------------|:---------------------------  |
+| `@user_name`           | specific user                |
+| `@group_name`          | specific group               |
+| `@all`                 | entire team                  |
+| `#123`                 | issue                        |
+| `!123`                 | merge request                |
+| `$123`                 | snippet                      |
+| `~123`                 | label by ID                  |
+| `~bug`                 | one-word label by name       |
+| `~"feature request"`   | multi-word label by name     |
+| `%123`                 | milestone by ID              |
+| `%v1.23`               | one-word milestone by name   |
+| `%"release candidate"` | multi-word milestone by name |
+| `9ba12248`             | specific commit              |
+| `9ba12248...b19a04f5`  | commit range comparison      |
+| `[README](doc/README)` | repository file references   |
 
 GFM also recognizes certain cross-project references:
 
@@ -206,6 +223,7 @@ GFM also recognizes certain cross-project references:
 |:----------------------------------------|:------------------------|
 | `namespace/project#123`                 | issue                   |
 | `namespace/project!123`                 | merge request           |
+| `namespace/project%123`                 | milestone               |
 | `namespace/project$123`                 | snippet                 |
 | `namespace/project@9ba12248`            | specific commit         |
 | `namespace/project@9ba12248...b19a04f5` | commit range comparison |

doc/workflow/notifications.md

@@ -69,7 +69,7 @@ In all of the below cases, the notification will be sent to:
 
     ...with notification level "Participating" or higher
 
-- Watchers: project members with notification level "Watch"
+- Watchers: users with notification level "Watch"
 - Subscribers: anyone who manually subscribed to the issue/merge request
 
 | Event                  | Sent to |

lib/banzai/filter/inline_diff_filter.rb

+module Banzai
+  module Filter
+    class InlineDiffFilter < HTML::Pipeline::Filter
+      IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set
+
+      def call
+        search_text_nodes(doc).each do |node|
+          next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
+
+          content = node.to_html
+          content = content.gsub(/(?:\[\-(.*?)\-\]|\{\-(.*?)\-\})/, '<span class="idiff left right deletion">\1\2</span>')
+          content = content.gsub(/(?:\[\+(.*?)\+\]|\{\+(.*?)\+\})/, '<span class="idiff left right addition">\1\2</span>')
+
+          next if html == content
+
+          node.replace(content)
+        end
+        doc
+      end
+    end
+  end
+end

lib/banzai/filter/milestone_reference_filter.rb

@@ -10,11 +10,53 @@ module Banzai
         project.milestones.find_by(iid: id)
       end
 
-      def url_for_object(issue, project)
+      def references_in(text, pattern = Milestone.reference_pattern)
+        # We'll handle here the references that follow the `reference_pattern`.
+        # Other patterns (for example, the link pattern) are handled by the
+        # default implementation.
+        return super(text, pattern) if pattern != Milestone.reference_pattern
+
+        text.gsub(pattern) do |match|
+          milestone = find_milestone($~[:project], $~[:milestone_iid], $~[:milestone_name])
+
+          if milestone
+            yield match, milestone.iid, $~[:project], $~
+          else
+            match
+          end
+        end
+      end
+
+      def find_milestone(project_ref, milestone_id, milestone_name)
+        project = project_from_ref(project_ref)
+        return unless project
+
+        milestone_params = milestone_params(milestone_id, milestone_name)
+        project.milestones.find_by(milestone_params)
+      end
+
+      def milestone_params(iid, name)
+        if name
+          { name: name.tr('"', '') }
+        else
+          { iid: iid.to_i }
+        end
+      end
+
+      def url_for_object(milestone, project)
         h = Gitlab::Routing.url_helpers
         h.namespace_project_milestone_url(project.namespace, project, milestone,
                                         only_path: context[:only_path])
       end
+
+      def object_link_text(object, matches)
+        if context[:project] == object.project
+          super
+        else
+          "#{escape_once(super)} <i>in #{escape_once(object.project.path_with_namespace)}</i>".
+            html_safe
+        end
+      end
     end
   end
 end

lib/banzai/pipeline/gfm_pipeline.rb

@@ -23,7 +23,8 @@ module Banzai
           Filter::LabelReferenceFilter,
           Filter::MilestoneReferenceFilter,
 
-          Filter::TaskListFilter
+          Filter::TaskListFilter,
+          Filter::InlineDiffFilter
         ]
       end
 

lib/gitlab/diff/inline_diff_marker.rb

 module Gitlab
   module Diff
     class InlineDiffMarker
+      MARKDOWN_SYMBOLS = {
+        addition: "+",
+        deletion: "-"
+      }
+
       attr_accessor :raw_line, :rich_line
 
       def initialize(raw_line, rich_line = raw_line)
@@ -8,7 +13,7 @@ module Gitlab
         @rich_line = ERB::Util.html_escape(rich_line)
       end
 
-      def mark(line_inline_diffs)
+      def mark(line_inline_diffs, mode: nil, markdown: false)
         return rich_line unless line_inline_diffs
 
         marker_ranges = []
@@ -20,13 +25,22 @@ module Gitlab
         end
 
         offset = 0
-        # Mark each range
-        marker_ranges.each_with_index do |range, i|
-          class_names = ["idiff"]
-          class_names << "left"   if i == 0
-          class_names << "right"  if i == marker_ranges.length - 1
 
-          offset = insert_around_range(rich_line, range, "<span class='#{class_names.join(" ")}'>", "</span>", offset)
+        # Mark each range
+        marker_ranges.each_with_index do |range, index|
+          before_content =
+            if markdown
+              "{#{MARKDOWN_SYMBOLS[mode]}"
+            else
+              "<span class='#{html_class_names(marker_ranges, mode, index)}'>"
+            end
+          after_content =
+            if markdown
+              "#{MARKDOWN_SYMBOLS[mode]}}"
+            else
+              "</span>"
+            end
+          offset = insert_around_range(rich_line, range, before_content, after_content, offset)
         end
 
         rich_line.html_safe
@@ -34,6 +48,14 @@ module Gitlab
 
       private
 
+      def html_class_names(marker_ranges, mode, index)
+        class_names = ["idiff"]
+        class_names << "left"  if index == 0
+        class_names << "right" if index == marker_ranges.length - 1
+        class_names << mode if mode
+        class_names.join(" ")
+      end
+
       # Mapping of character positions in the raw line, to the rich (highlighted) line
       def position_mapping
         @position_mapping ||= begin

spec/controllers/projects/notification_settings_controller_spec.rb

@@ -34,5 +34,19 @@ describe Projects::NotificationSettingsController do
         expect(response.status).to eq 200
       end
     end
+
+    context 'not authorized' do
+      let(:private_project) { create(:project, :private) }
+      before { sign_in(user) }
+
+      it 'returns 404' do
+        put :update,
+            namespace_id: private_project.namespace.to_param,
+            project_id: private_project.to_param,
+            notification_setting: { level: :participating }
+
+        expect(response.status).to eq(404)
+      end
+    end
   end
 end

spec/controllers/projects_controller_spec.rb

@@ -8,6 +8,40 @@ describe ProjectsController do
   let(:txt)     { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
 
   describe "GET show" do
+    context "user not project member" do
+      before { sign_in(user) }
+
+      context "user does not have access to project" do
+        let(:private_project) { create(:project, :private) }
+
+        it "does not initialize notification setting" do
+          get :show, namespace_id: private_project.namespace.path, id: private_project.path
+          expect(assigns(:notification_setting)).to be_nil
+        end
+      end
+
+      context "user has access to project" do
+        context "and does not have notification setting" do
+          it "initializes notification as disabled" do
+            get :show, namespace_id: public_project.namespace.path, id: public_project.path
+            expect(assigns(:notification_setting).level).to eq("global")
+          end
+        end
+
+        context "and has notification setting" do
+          before do
+            setting = user.notification_settings_for(public_project)
+            setting.level = :watch
+            setting.save
+          end
+
+          it "shows current notification setting" do
+            get :show, namespace_id: public_project.namespace.path, id: public_project.path
+            expect(assigns(:notification_setting).level).to eq("watch")
+          end
+        end
+      end
+    end
 
     context "rendering default project view" do
       render_views

spec/features/markdown_spec.rb

@@ -278,6 +278,10 @@ describe 'GitLab Markdown', feature: true do
     it 'includes GollumTagsFilter' do
       expect(doc).to parse_gollum_tags
     end
+
+    it 'includes InlineDiffFilter' do
+      expect(doc).to parse_inline_diffs
+    end
   end
 
   # Fake a `current_user` helper

spec/fixtures/markdown.md.erb

@@ -216,10 +216,14 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
 
 #### MilestoneReferenceFilter
 
-- Milestone: <%= milestone.to_reference %>
+- Milestone by ID: <%= simple_milestone.to_reference %>
+- Milestone by name: <%= Milestone.reference_prefix %><%= simple_milestone.name %>
+- Milestone by name in quotes: <%= milestone.to_reference(format: :name) %>
 - Milestone in another project: <%= xmilestone.to_reference(project) %>
-- Ignored in code: `<%= milestone.to_reference %>`
-- Link to milestone by URL: [Milestone](<%= urls.namespace_project_milestone_url(milestone.project.namespace, milestone.project, milestone) %>)
+- Ignored in code: `<%= simple_milestone.to_reference %>`
+- Ignored in links: [Link to <%= simple_milestone.to_reference %>](#milestone-link)
+- Milestone by URL: <%= urls.namespace_project_milestone_url(milestone.project.namespace, milestone.project, milestone) %>
+- Link to milestone by URL: [Milestone](<%= milestone.to_reference %>)
 
 ### Task Lists
 
@@ -239,3 +243,16 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
 - [[link-text|http://example.com/pdfs/gollum.pdf]]
 - [[images/example.jpg]]
 - [[http://example.com/images/example.jpg]]
+
+### Inline Diffs
+
+With inline diffs tags you can display {+ additions +} or [- deletions -].
+
+The wrapping tags can be either curly braces or square brackets [+ additions +] or {- deletions -}.
+
+However the wrapping tags can not be mixed as such -
+
+- {+ additions +]
+- [+ additions +}
+- {- delletions -]
+- [- delletions -}

spec/helpers/diff_helper_spec.rb

@@ -93,9 +93,9 @@ describe DiffHelper do
     it "returns strings with marked inline diffs" do
       marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
 
-      expect(marked_old_line).to eq("abc <span class='idiff left right'>&#39;def&#39;</span>")
+      expect(marked_old_line).to eq("abc <span class='idiff left right deletion'>&#39;def&#39;</span>")
       expect(marked_old_line).to be_html_safe
-      expect(marked_new_line).to eq("abc <span class='idiff left right'>&quot;def&quot;</span>")
+      expect(marked_new_line).to eq("abc <span class='idiff left right addition'>&quot;def&quot;</span>")
       expect(marked_new_line).to be_html_safe
     end
   end

spec/lib/banzai/filter/inline_diff_filter_spec.rb

+require 'spec_helper'
+
+describe Banzai::Filter::InlineDiffFilter, lib: true do
+  include FilterSpecHelper
+
+  it 'adds inline diff span tags for deletions when using square brackets' do
+    doc = "START [-something deleted-] END"
+    expect(filter(doc).to_html).to eq('START <span class="idiff left right deletion">something deleted</span> END')
+  end
+
+  it 'adds inline diff span tags for deletions when using curley braces' do
+    doc = "START {-something deleted-} END"
+    expect(filter(doc).to_html).to eq('START <span class="idiff left right deletion">something deleted</span> END')
+  end
+
+  it 'does not add inline diff span tags when a closing tag is not provided' do
+    doc = "START [- END"
+    expect(filter(doc).to_html).to eq(doc)
+  end
+
+  it 'adds inline span tags for additions when using square brackets' do
+    doc = "START [+something added+] END"
+    expect(filter(doc).to_html).to eq('START <span class="idiff left right addition">something added</span> END')
+  end
+
+  it 'adds inline span tags for additions  when using curley braces' do
+    doc = "START {+something added+} END"
+    expect(filter(doc).to_html).to eq('START <span class="idiff left right addition">something added</span> END')
+  end
+
+  it 'does not add inline diff span tags when a closing addition tag is not provided' do
+    doc = "START {+ END"
+    expect(filter(doc).to_html).to eq(doc)
+  end
+
+  it 'does not add inline diff span tags when the tags do not match' do
+    examples = [
+      "{+ additions +]",
+      "[+ additions +}",
+      "{- delletions -]",
+      "[- delletions -}"
+    ]
+
+    examples.each do |doc|
+      expect(filter(doc).to_html).to eq(doc)
+    end
+  end
+
+  it 'prevents user-land html being injected' do
+    doc = "START {+&lt;script&gt;alert('I steal cookies')&lt;/script&gt;+} END"
+    expect(filter(doc).to_html).to eq("START <span class=\"idiff left right addition\">&lt;script&gt;alert('I steal cookies')&lt;/script&gt;</span> END")
+  end
+
+  it 'preserves content inside pre tags' do
+    doc = "<pre>START {+something added+} END</pre>"
+    expect(filter(doc).to_html).to eq(doc)
+  end
+
+  it 'preserves content inside code tags' do
+    doc = "<code>START {+something added+} END</code>"
+    expect(filter(doc).to_html).to eq(doc)
+  end
+
+  it 'preserves content inside tt tags' do
+    doc = "<tt>START {+something added+} END</tt>"
+    expect(filter(doc).to_html).to eq(doc)
+  end
+end

spec/lib/banzai/filter/milestone_reference_filter_spec.rb

@@ -3,8 +3,9 @@ require 'spec_helper'
 describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
   include FilterSpecHelper
 
-  let(:project) { create(:project, :public) }
-  let(:milestone)   { create(:milestone, project: project) }
+  let(:project)   { create(:project, :public) }
+  let(:milestone) { create(:milestone, project: project) }
+  let(:reference) { milestone.to_reference }
 
   it 'requires project context' do
     expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
@@ -17,11 +18,42 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
     end
   end
 
-  context 'internal reference' do
-    # Convert the Markdown link to only the URL, since these tests aren't run through the regular Markdown pipeline.
-    # Milestone reference behavior in the full Markdown pipeline is tested elsewhere.
-    let(:reference) { milestone.to_reference.gsub(/\[([^\]]+)\]\(([^)]+)\)/, '\2') }
+  it 'includes default classes' do
+    doc = reference_filter("Milestone #{reference}")
+    expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone'
+  end
+
+  it 'includes a data-project attribute' do
+    doc = reference_filter("Milestone #{reference}")
+    link = doc.css('a').first
+
+    expect(link).to have_attribute('data-project')
+    expect(link.attr('data-project')).to eq project.id.to_s
+  end
+
+  it 'includes a data-milestone attribute' do
+    doc = reference_filter("See #{reference}")
+    link = doc.css('a').first
+
+    expect(link).to have_attribute('data-milestone')
+    expect(link.attr('data-milestone')).to eq milestone.id.to_s
+  end
+
+  it 'supports an :only_path context' do
+    doc = reference_filter("Milestone #{reference}", only_path: true)
+    link = doc.css('a').first.attr('href')
 
+    expect(link).not_to match %r(https?://)
+    expect(link).to eq urls.
+      namespace_project_milestone_path(project.namespace, project, milestone)
+  end
+
+  it 'adds to the results hash' do
+    result = reference_pipeline_result("Milestone #{reference}")
+    expect(result[:references][:milestone]).to eq [milestone]
+  end
+
+  context 'Integer-based references' do
     it 'links to a valid reference' do
       doc = reference_filter("See #{reference}")
 
@@ -30,29 +62,82 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
     end
 
     it 'links with adjacent text' do
-      doc = reference_filter("milestone (#{reference}.)")
-      expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(milestone.title)}<\/a>\.\)/)
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
     end
 
-    it 'includes a title attribute' do
-      doc = reference_filter("milestone #{reference}")
-      expect(doc.css('a').first.attr('title')).to eq "Milestone: #{milestone.title}"
+    it 'ignores invalid milestone IIDs' do
+      exp = act = "Milestone #{invalidate_reference(reference)}"
+
+      expect(reference_filter(act).to_html).to eq exp
     end
+  end
+
+  context 'String-based single-word references' do
+    let(:milestone) { create(:milestone, name: 'gfm', project: project) }
+    let(:reference) { "#{Milestone.reference_prefix}#{milestone.name}" }
 
-    it 'escapes the title attribute' do
-      milestone.update_attribute(:title, %{"></a>whatever<a title="})
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(project.namespace, project, milestone)
+      expect(doc.text).to eq 'See gfm'
+    end
 
-      doc = reference_filter("milestone #{reference}")
-      expect(doc.text).to eq "milestone \">whatever"
+    it 'links with adjacent text' do
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
     end
 
-    it 'includes default classes' do
-      doc = reference_filter("milestone #{reference}")
-      expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone'
+    it 'ignores invalid milestone names' do
+      exp = act = "Milestone #{Milestone.reference_prefix}#{milestone.name.reverse}"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  context 'String-based multi-word references in quotes' do
+    let(:milestone) { create(:milestone, name: 'gfm references', project: project) }
+    let(:reference) { milestone.to_reference(format: :name) }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(project.namespace, project, milestone)
+      expect(doc.text).to eq 'See gfm references'
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
+    end
+
+    it 'ignores invalid milestone names' do
+      exp = act = %(Milestone #{Milestone.reference_prefix}"#{milestone.name.reverse}")
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  describe 'referencing a milestone in a link href' do
+    let(:reference) { %Q{<a href="#{milestone.to_reference}">Milestone</a>} }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(project.namespace, project, milestone)
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>Milestone</a>\.\)))
     end
 
     it 'includes a data-project attribute' do
-      doc = reference_filter("milestone #{reference}")
+      doc = reference_filter("Milestone #{reference}")
       link = doc.css('a').first
 
       expect(link).to have_attribute('data-project')
@@ -68,8 +153,34 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
     end
 
     it 'adds to the results hash' do
-      result = reference_pipeline_result("milestone #{reference}")
+      result = reference_pipeline_result("Milestone #{reference}")
       expect(result[:references][:milestone]).to eq [milestone]
     end
   end
+
+  describe 'cross project milestone references' do
+    let(:another_project)  { create(:empty_project, :public) }
+    let(:project_path) { another_project.path_with_namespace }
+    let(:milestone) { create(:milestone, project: another_project) }
+    let(:reference) { milestone.to_reference(project) }
+
+    let!(:result) { reference_filter("See #{reference}") }
+
+    it 'points to referenced project milestone page' do
+      expect(result.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(another_project.namespace,
+                                        another_project,
+                                        milestone)
+    end
+
+    it 'contains cross project content' do
+      expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_path}"
+    end
+
+    it 'escapes the name attribute' do
+      allow_any_instance_of(Milestone).to receive(:title).and_return(%{"></a>whatever<a title="})
+      doc = reference_filter("See #{reference}")
+      expect(doc.css('a').first.text).to eq "#{milestone.name} in #{project_path}"
+    end
+  end
 end

spec/models/merge_request_spec.rb

@@ -284,13 +284,18 @@ describe MergeRequest, models: true do
   end
 
   describe "#reset_merge_when_build_succeeds" do
-    let(:merge_if_green) { create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user) }
+    let(:merge_if_green) do
+      create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user),
+                             merge_params: { "should_remove_source_branch" => "1", "commit_message" => "msg" }
+    end
 
     it "sets the item to false" do
       merge_if_green.reset_merge_when_build_succeeds
       merge_if_green.reload
 
       expect(merge_if_green.merge_when_build_succeeds).to be_falsey
+      expect(merge_if_green.merge_params["should_remove_source_branch"]).to be_nil
+      expect(merge_if_green.merge_params["commit_message"]).to be_nil
     end
   end
 

spec/models/repository_spec.rb

@@ -176,6 +176,15 @@ describe Repository, models: true do
       repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
     end
 
+    it 'handles when HEAD points to non-existent ref' do
+      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
+      rugged = double('rugged')
+      expect(rugged).to receive(:head_unborn?).and_return(true)
+      expect(repository).to receive(:rugged).and_return(rugged)
+
+      expect(repository.license_blob).to be_nil
+    end
+
     it 'looks in the root_ref only' do
       repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'markdown')
       repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'markdown', false)
@@ -204,6 +213,15 @@ describe Repository, models: true do
       repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
     end
 
+    it 'handles when HEAD points to non-existent ref' do
+      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
+      rugged = double('rugged')
+      expect(rugged).to receive(:head_unborn?).and_return(true)
+      expect(repository).to receive(:rugged).and_return(rugged)
+
+      expect(repository.license_key).to be_nil
+    end
+
     it 'returns nil when no license is detected' do
       expect(repository.license_key).to be_nil
     end

spec/services/issues/update_service_spec.rb

@@ -75,10 +75,10 @@ describe Issues::UpdateService, services: true do
       end
 
       it 'creates system note about title change' do
-        note = find_note('Title changed')
+        note = find_note('Changed title:')
 
         expect(note).not_to be_nil
-        expect(note.note).to eq 'Title changed from **Old title** to **New title**'
+        expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
       end
 
       it 'creates system note about confidentiality change' do

spec/services/merge_requests/create_service_spec.rb

@@ -12,7 +12,8 @@ describe MergeRequests::CreateService, services: true do
           title: 'Awesome merge_request',
           description: 'please fix',
           source_branch: 'feature',
-          target_branch: 'master'
+          target_branch: 'master',
+          force_remove_source_branch: '1'
         }
       end
 
@@ -29,6 +30,7 @@ describe MergeRequests::CreateService, services: true do
       it { expect(@merge_request).to be_valid }
       it { expect(@merge_request.title).to eq('Awesome merge_request') }
       it { expect(@merge_request.assignee).to be_nil }
+      it { expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') }
 
       it 'should execute hooks with default action' do
         expect(service).to have_received(:execute_hooks).with(@merge_request)

spec/services/merge_requests/merge_service_spec.rb

@@ -38,6 +38,21 @@ describe MergeRequests::MergeService, services: true do
       end
     end
 
+    context 'remove source branch by author' do
+      let(:service) do
+        merge_request.merge_params['force_remove_source_branch'] = '1'
+        merge_request.save!
+        MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message')
+      end
+
+      it 'removes the source branch' do
+        expect(DeleteBranchService).to receive(:new).
+          with(merge_request.source_project, merge_request.author).
+          and_call_original
+        service.execute(merge_request)
+      end
+    end
+
     context "error handling" do
       let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
 

spec/services/merge_requests/update_service_spec.rb

@@ -39,7 +39,8 @@ describe MergeRequests::UpdateService, services: true do
           assignee_id: user2.id,
           state_event: 'close',
           label_ids: [label.id],
-          target_branch: 'target'
+          target_branch: 'target',
+          force_remove_source_branch: '1'
         }
       end
 
@@ -61,6 +62,7 @@ describe MergeRequests::UpdateService, services: true do
       it { expect(@merge_request.labels.count).to eq(1) }
       it { expect(@merge_request.labels.first.title).to eq(label.name) }
       it { expect(@merge_request.target_branch).to eq('target') }
+      it { expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') }
 
       it 'should execute hooks with update action' do
         expect(service).to have_received(:execute_hooks).
@@ -90,10 +92,10 @@ describe MergeRequests::UpdateService, services: true do
       end
 
       it 'creates system note about title change' do
-        note = find_note('Title changed')
+        note = find_note('Changed title:')
 
         expect(note).not_to be_nil
-        expect(note.note).to eq 'Title changed from **Old title** to **New title**'
+        expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
       end
 
       it 'creates system note about branch change' do

spec/services/notification_service_spec.rb

@@ -66,6 +66,7 @@ describe NotificationService, services: true do
           should_email(@subscriber)
           should_email(@watcher_and_subscriber)
           should_email(@subscribed_participant)
+          should_not_email(@u_guest_watcher)
           should_not_email(note.author)
           should_not_email(@u_participating)
           should_not_email(@u_disabled)
@@ -100,6 +101,7 @@ describe NotificationService, services: true do
           should_email(note.noteable.author)
           should_email(note.noteable.assignee)
           should_email(@u_mentioned)
+          should_not_email(@u_guest_watcher)
           should_not_email(@u_watcher)
           should_not_email(note.author)
           should_not_email(@u_participating)
@@ -160,6 +162,7 @@ describe NotificationService, services: true do
             should_email(member)
           end
 
+          should_email(@u_guest_watcher)
           should_email(note.noteable.author)
           should_email(note.noteable.assignee)
           should_not_email(note.author)
@@ -201,6 +204,7 @@ describe NotificationService, services: true do
             should_email(member)
           end
 
+          should_email(@u_guest_watcher)
           should_email(note.noteable.author)
           should_not_email(note.author)
           should_email(@u_mentioned)
@@ -224,6 +228,7 @@ describe NotificationService, services: true do
         it do
           notification.new_note(note)
 
+          should_email(@u_guest_watcher)
           should_email(@u_committer)
           should_email(@u_watcher)
           should_not_email(@u_mentioned)
@@ -236,6 +241,7 @@ describe NotificationService, services: true do
           note.update_attribute(:note, '@mention referenced')
           notification.new_note(note)
 
+          should_email(@u_guest_watcher)
           should_email(@u_committer)
           should_email(@u_watcher)
           should_email(@u_mentioned)
@@ -269,6 +275,7 @@ describe NotificationService, services: true do
 
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_not_email(@u_mentioned)
         should_not_email(@u_participating)
@@ -328,6 +335,7 @@ describe NotificationService, services: true do
 
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -342,6 +350,7 @@ describe NotificationService, services: true do
 
         should_email(@u_mentioned)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -356,6 +365,7 @@ describe NotificationService, services: true do
         expect(issue.assignee).to be @u_mentioned
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -370,6 +380,7 @@ describe NotificationService, services: true do
         expect(issue.assignee).to be @u_mentioned
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -383,6 +394,7 @@ describe NotificationService, services: true do
 
         expect(issue.assignee).to be @u_mentioned
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(issue.assignee)
@@ -411,6 +423,7 @@ describe NotificationService, services: true do
         should_not_email(issue.assignee)
         should_not_email(issue.author)
         should_not_email(@u_watcher)
+        should_not_email(@u_guest_watcher)
         should_not_email(@u_participant_mentioned)
         should_not_email(@subscriber)
         should_not_email(@watcher_and_subscriber)
@@ -459,6 +472,7 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(issue.author)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -475,6 +489,7 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(issue.author)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -502,6 +517,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@watcher_and_subscriber)
         should_email(@u_participant_mentioned)
+        should_email(@u_guest_watcher)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
       end
@@ -525,6 +541,7 @@ describe NotificationService, services: true do
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -566,6 +583,7 @@ describe NotificationService, services: true do
 
         should_email(merge_request.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -584,6 +602,7 @@ describe NotificationService, services: true do
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -599,6 +618,7 @@ describe NotificationService, services: true do
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -620,6 +640,7 @@ describe NotificationService, services: true do
 
         should_email(@u_watcher)
         should_email(@u_participating)
+        should_not_email(@u_guest_watcher)
         should_not_email(@u_disabled)
       end
     end
@@ -635,6 +656,8 @@ describe NotificationService, services: true do
     @u_not_mentioned = create(:user, username: 'regular', notification_level: :participating)
     @u_outsider_mentioned = create(:user, username: 'outsider')
 
+    create_guest_watcher
+
     project.team << [@u_watcher, :master]
     project.team << [@u_participating, :master]
     project.team << [@u_participant_mentioned, :master]
@@ -644,6 +667,13 @@ describe NotificationService, services: true do
     project.team << [@u_not_mentioned, :master]
   end
 
+  def create_guest_watcher
+    @u_guest_watcher = create(:user, username: 'guest_watching')
+    setting = @u_guest_watcher.notification_settings_for(project)
+    setting.level = :watch
+    setting.save
+  end
+
   def add_users_with_subscription(project, issuable)
     @subscriber = create :user
     @unsubscriber = create :user

spec/services/system_note_service_spec.rb

@@ -241,7 +241,7 @@ describe SystemNoteService, services: true do
 
       it 'sets the note text' do
         expect(subject.note).
-          to eq "Title changed from **Old title** to **#{noteable.title}**"
+          to eq "Changed title: **{-Old title-}** → **{+#{noteable.title}+}**"
       end
     end
   end

spec/support/markdown_feature.rb

@@ -63,8 +63,12 @@ class MarkdownFeature
     @label ||= create(:label, name: 'awaiting feedback', project: project)
   end
 
+  def simple_milestone
+    @simple_milestone ||= create(:milestone, name: 'gfm-milestone', project: project)
+  end
+
   def milestone
-    @milestone ||= create(:milestone, project: project)
+    @milestone ||= create(:milestone, name: 'next goal', project: project)
   end
 
   # Cross-references -----------------------------------------------------------

spec/support/matchers/markdown_matchers.rb

@@ -154,7 +154,7 @@ module MarkdownMatchers
     set_default_markdown_messages
 
     match do |actual|
-      expect(actual).to have_selector('a.gfm.gfm-milestone', count: 3)
+      expect(actual).to have_selector('a.gfm.gfm-milestone', count: 6)
     end
   end
 
@@ -168,6 +168,16 @@ module MarkdownMatchers
       expect(actual).to have_selector('input[checked]', count: 3)
     end
   end
+
+  # InlineDiffFilter
+  matcher :parse_inline_diffs do
+    set_default_markdown_messages
+
+    match do |actual|
+      expect(actual).to have_selector('span.idiff.addition', count: 2)
+      expect(actual).to have_selector('span.idiff.deletion', count: 2)
+    end
+  end
 end
 
 # Monkeypatch the matcher DSL so that we can reduce some noisy duplication for