Merge remote-tracking branch 'origin/master' into remove-wip

21 files changed, 259 insertions, 121 deletions

diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index 005a5c4661c..50c95ced8a7 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -3,7 +3,7 @@ module Ci
     def execute(project, opts)
       sha = opts[:sha] || ref_sha(project, opts[:ref])
 
-      commit = project.ci_commits.ordered.find_by(sha: sha)
+      commit = project.ci_commits.find_by(sha: sha)
       image_name = image_for_commit(commit)
 
       image_path = Rails.root.join('public/ci', image_name)
diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb
index 43d1c766e35..a3c950ede1f 100644
--- a/app/services/commits/revert_service.rb
+++ b/app/services/commits/revert_service.rb
@@ -9,7 +9,8 @@ module Commits
       @commit = params[:commit]
       @create_merge_request = params[:create_merge_request].present?
 
-      validate and commit
+      check_push_permissions unless @create_merge_request
+      commit
     rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
            ValidationError, ReversionError => ex
       error(ex.message)
@@ -17,39 +18,39 @@ module Commits
 
     def commit
       revert_into = @create_merge_request ? @commit.revert_branch_name : @target_branch
+      revert_tree_id = repository.check_revert_content(@commit, @target_branch)
 
-      if @create_merge_request
-        # Temporary branch exists and contains the revert commit
-        return success if repository.find_branch(revert_into)
+      if revert_tree_id
+        create_target_branch(revert_into) if @create_merge_request
 
-        create_target_branch
-      end
-
-      unless repository.revert(current_user, @commit, revert_into)
+        repository.revert(current_user, @commit, revert_into, revert_tree_id)
+        success
+      else
         error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically.
                      It may have already been reverted, or a more recent commit may have updated some of its content."
         raise ReversionError, error_msg
       end
-
-      success
     end
 
     private
 
-    def create_target_branch
+    def create_target_branch(new_branch)
+      # Temporary branch exists and contains the revert commit
+      return success if repository.find_branch(new_branch)
+
       result = CreateBranchService.new(@project, current_user)
-                                  .execute(@commit.revert_branch_name, @target_branch, source_project: @source_project)
+                                  .execute(new_branch, @target_branch, source_project: @source_project)
 
       if result[:status] == :error
         raise ReversionError, "There was an error creating the source branch: #{result[:message]}"
       end
     end
 
-    def validate
+    def check_push_permissions
       allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch)
 
       unless allowed
-        raise_error('You are not allowed to push into this branch')
+        raise ValidationError.new('You are not allowed to push into this branch')
       end
 
       true
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index ec581658fc1..e2bccbdbcc3 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -1,7 +1,7 @@
 require 'securerandom'
 
 # Compare 2 branches for one repo or between repositories
-# and return Gitlab::CompareResult object that responds to commits and diffs
+# and return Gitlab::Git::Compare object that responds to commits and diffs
 class CompareService
   def execute(source_project, source_branch, target_project, target_branch, diff_options = {})
     source_commit = source_project.commit(source_branch)
@@ -20,12 +20,10 @@ class CompareService
       )
     end
 
-    Gitlab::CompareResult.new(
-      Gitlab::Git::Compare.new(
-        target_project.repository.raw_repository,
-        target_branch,
-        source_sha,
-      ), diff_options
+    Gitlab::Git::Compare.new(
+      target_project.repository.raw_repository,
+      target_branch,
+      source_sha,
     )
   end
 end
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index 31b407efeb1..69d5c42a877 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -33,7 +33,6 @@ class CreateCommitBuildsService
     unless commit.skip_ci?
       # Create builds for commit
       tag = Gitlab::Git.tag_ref?(origin_ref)
-      commit.update_committed!
       commit.create_builds(ref, tag, user)
     end
 
diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb
index 173e50c9206..ce79287e35a 100644
--- a/app/services/delete_user_service.rb
+++ b/app/services/delete_user_service.rb
@@ -5,18 +5,22 @@ class DeleteUserService
     @current_user = current_user
   end
 
-  def execute(user)
-    if user.solo_owned_groups.present?
+  def execute(user, options = {})
+    if !options[:delete_solo_owned_groups] && user.solo_owned_groups.present?
       user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
-      user
-    else
-      user.personal_projects.each do |project|
-        # Skip repository removal because we remove directory with namespace
-        # that contain all this repositories
-        ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
-      end
+      return user
+    end
+
+    user.solo_owned_groups.each do |group|
+      DestroyGroupService.new(group, current_user).execute
+    end
 
-      user.destroy
+    user.personal_projects.each do |project|
+      # Skip repository removal because we remove directory with namespace
+      # that contain all this repositories
+      ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
     end
+
+    user.destroy
   end
 end
diff --git a/app/services/destroy_group_service.rb b/app/services/destroy_group_service.rb
index 9189de390a2..3c42ac61be4 100644
--- a/app/services/destroy_group_service.rb
+++ b/app/services/destroy_group_service.rb
@@ -6,12 +6,12 @@ class DestroyGroupService
   end
 
   def execute
-    @group.projects.each do |project|
+    group.projects.each do |project|
       # Skip repository removal because we remove directory with namespace
       # that contain all this repositories
       ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
     end
 
-    @group.destroy
+    group.destroy
   end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 9ba200f7bde..14e2a2c0699 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -12,11 +12,12 @@ class GitPushService < BaseService
   #  1. Creates the push event
   #  2. Updates merge requests
   #  3. Recognizes cross-references from commit messages
-  #  4. Executes the project's web hooks
+  #  4. Executes the project's webhooks
   #  5. Executes the project's services
+  #  6. Checks if the project's main language has changed
   #
   def execute
-    @project.repository.after_push_commit(branch_name)
+    @project.repository.after_push_commit(branch_name, params[:newrev])
 
     if push_remove_branch?
       @project.repository.after_remove_branch
@@ -42,9 +43,24 @@ class GitPushService < BaseService
       @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev])
       process_commit_messages
     end
+    # Checks if the main language has changed in the project and if so
+    # it updates it accordingly
+    update_main_language
     # Update merge requests that may be affected by this push. A new branch
     # could cause the last commit of a merge request to change.
     update_merge_requests
+
+    perform_housekeeping
+  end
+
+  def update_main_language
+    current_language = @project.repository.main_language
+
+    unless current_language == @project.main_language
+      return @project.update_attributes(main_language: current_language)
+    end
+
+    true
   end
 
   protected
@@ -59,6 +75,13 @@ class GitPushService < BaseService
     ProjectCacheWorker.perform_async(@project.id)
   end
 
+  def perform_housekeeping
+    housekeeping = Projects::HousekeepingService.new(@project)
+    housekeeping.increment!
+    housekeeping.execute if housekeeping.needed?
+  rescue Projects::HousekeepingService::LeaseTaken
+  end
+
   def process_default_branch
     @push_commits = project.repository.commits(params[:newrev])
 
@@ -66,7 +89,7 @@ class GitPushService < BaseService
     project.change_head(branch_name)
 
     # Set protection on the default branch if configured
-    if (current_application_settings.default_branch_protection != PROTECTION_NONE)
+    if current_application_settings.default_branch_protection != PROTECTION_NONE
       developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
       @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push })
     end
@@ -96,7 +119,9 @@ class GitPushService < BaseService
         # a different branch.
         closed_issues = commit.closes_issues(current_user)
         closed_issues.each do |issue|
-          Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit)
+          if can?(current_user, :update_issue, issue)
+            Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit)
+          end
         end
       end
 
diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb
index a62c5fc4fc4..c88c7672805 100644
--- a/app/services/git_tag_push_service.rb
+++ b/app/services/git_tag_push_service.rb
@@ -2,7 +2,7 @@ class GitTagPushService
   attr_accessor :project, :user, :push_data
 
   def execute(project, user, oldrev, newrev, ref)
-    project.repository.before_create_tag
+    project.repository.before_push_tag
 
     @project, @user = project, user
     @push_data = build_push_data(oldrev, newrev, ref)
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index ca87dca4a70..18f76d3f650 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -11,7 +11,10 @@ class IssuableBaseService < BaseService
       issuable, issuable.project, current_user, issuable.milestone)
   end
 
-  def create_labels_note(issuable, added_labels, removed_labels)
+  def create_labels_note(issuable, old_labels)
+    added_labels = issuable.labels - old_labels
+    removed_labels = old_labels - issuable.labels
+
     SystemNoteService.change_label(
       issuable, issuable.project, current_user, added_labels, removed_labels)
   end
@@ -71,20 +74,19 @@ class IssuableBaseService < BaseService
     end
   end
 
-  def has_changes?(issuable, options = {})
+  def has_changes?(issuable, old_labels: [])
     valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
 
     attrs_changed = valid_attrs.any? do |attr|
       issuable.previous_changes.include?(attr.to_s)
     end
 
-    old_labels = options[:old_labels]
-    labels_changed = old_labels && issuable.labels != old_labels
+    labels_changed = issuable.labels != old_labels
 
     attrs_changed || labels_changed
   end
 
-  def handle_common_system_notes(issuable, options = {})
+  def handle_common_system_notes(issuable, old_labels: [])
     if issuable.previous_changes.include?('title')
       create_title_change_note(issuable, issuable.previous_changes['title'].first)
     end
@@ -93,9 +95,6 @@ class IssuableBaseService < BaseService
       create_task_status_note(issuable)
     end
 
-    old_labels = options[:old_labels]
-    if old_labels && (issuable.labels != old_labels)
-      create_labels_note(issuable, issuable.labels - old_labels, old_labels - issuable.labels)
-    end
+    create_labels_note(issuable, old_labels) if issuable.labels != old_labels
   end
 end
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index 51ef9dfe610..3563cbaa997 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -4,8 +4,8 @@ module Issues
       update(issue)
     end
 
-    def handle_changes(issue, options = {})
-      if has_changes?(issue, options)
+    def handle_changes(issue, old_labels: [])
+      if has_changes?(issue, old_labels: old_labels)
         todo_service.mark_pending_todos_as_done(issue, current_user)
       end
 
@@ -23,6 +23,11 @@ module Issues
         notification_service.reassigned_issue(issue, current_user)
         todo_service.reassigned_issue(issue, current_user)
       end
+
+      added_labels = issue.labels - old_labels
+      if added_labels.present?
+        notification_service.relabeled_issue(issue, added_labels, current_user)
+      end
     end
 
     def reopen_service
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index c0700d953dd..fa34753c4fd 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -5,9 +5,7 @@ module MergeRequests
 
       # Set MR attributes
       merge_request.can_be_created = false
-      merge_request.compare_failed = false
       merge_request.compare_commits = []
-      merge_request.compare_diffs = []
       merge_request.source_project = project unless merge_request.source_project
       merge_request.target_project ||= (project.forked_from_project || project)
       merge_request.target_branch ||= merge_request.target_project.default_branch
@@ -21,35 +19,23 @@ module MergeRequests
         return build_failed(merge_request, message)
       end
 
-      compare_result = CompareService.new.execute(
+      compare = CompareService.new.execute(
         merge_request.source_project,
         merge_request.source_branch,
         merge_request.target_project,
         merge_request.target_branch,
       )
 
-      commits = compare_result.commits
+      commits = compare.commits
 
       # At this point we decide if merge request can be created
       # If we have at least one commit to merge -> creation allowed
       if commits.present?
         merge_request.compare_commits = Commit.decorate(commits, merge_request.source_project)
         merge_request.can_be_created = true
-        merge_request.compare_failed = false
-
-        # Try to collect diff for merge request.
-        diffs = compare_result.diffs
-
-        if diffs.present?
-          merge_request.compare_diffs = diffs
-
-        elsif diffs == false
-          merge_request.can_be_created = false
-          merge_request.compare_failed = true
-        end
+        merge_request.compare = compare
       else
         merge_request.can_be_created = false
-        merge_request.compare_failed = false
       end
 
       commits = merge_request.compare_commits
@@ -61,6 +47,21 @@ module MergeRequests
         merge_request.title = merge_request.source_branch.titleize.humanize
       end
 
+      # When your branch name starts with an iid followed by a dash this pattern will
+      # be interpreted as the use wants to close that issue on this project
+      # Pattern example: 112-fix-mep-mep
+      # Will lead to appending `Closes #112` to the description
+      if match = merge_request.source_branch.match(/\A(\d+)-/)
+        iid = match[1]
+        closes_issue = "Closes ##{iid}"
+
+        if merge_request.description.present?
+          merge_request.description << closes_issue.prepend("\n")
+        else
+          merge_request.description = closes_issue
+        end
+      end
+
       merge_request
     end
 
diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index 8f25c5e2496..ebb67c7db65 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -21,7 +21,9 @@ module MergeRequests
 
       closed_issues = merge_request.closes_issues(current_user)
       closed_issues.each do |issue|
-        Issues::CloseService.new(project, current_user, {}).execute(issue, merge_request)
+        if can?(current_user, :update_issue, issue)
+          Issues::CloseService.new(project, current_user, {}).execute(issue, merge_request)
+        end
       end
     end
 
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 6319ad805b6..477c64e7377 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -14,8 +14,8 @@ module MergeRequests
       update(merge_request)
     end
 
-    def handle_changes(merge_request, options = {})
-      if has_changes?(merge_request, options)
+    def handle_changes(merge_request, old_labels: [])
+      if has_changes?(merge_request, old_labels: old_labels)
         todo_service.mark_pending_todos_as_done(merge_request, current_user)
       end
 
@@ -44,6 +44,15 @@ module MergeRequests
           merge_request.previous_changes.include?('source_branch')
         merge_request.mark_as_unchecked
       end
+
+      added_labels = merge_request.labels - old_labels
+      if added_labels.present?
+        notification_service.relabeled_merge_request(
+          merge_request,
+          added_labels,
+          current_user
+        )
+      end
     end
 
     def reopen_service
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index ca8a41d93b8..19a6779dea9 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -24,16 +24,17 @@ class NotificationService
     end
   end
 
-  # When create an issue we should send next emails:
+  # When create an issue we should send an email to:
   #
   #  * issue assignee if their notification level is not Disabled
   #  * project team members with notification level higher then Participating
+  #  * watchers of the issue's labels
   #
   def new_issue(issue, current_user)
     new_resource_email(issue, issue.project, 'new_issue_email')
   end
 
-  # When we close an issue we should send next emails:
+  # When we close an issue we should send an email to:
   #
   #  * issue author if their notification level is not Disabled
   #  * issue assignee if their notification level is not Disabled
@@ -43,7 +44,7 @@ class NotificationService
     close_resource_email(issue, issue.project, current_user, 'closed_issue_email')
   end
 
-  # When we reassign an issue we should send next emails:
+  # When we reassign an issue we should send an email to:
   #
   #  * issue old assignee if their notification level is not Disabled
   #  * issue new assignee if their notification level is not Disabled
@@ -52,16 +53,25 @@ class NotificationService
     reassign_resource_email(issue, issue.project, current_user, 'reassigned_issue_email')
   end
 
+  # When we add labels to an issue we should send an email to:
+  #
+  #  * watchers of the issue's labels
+  #
+  def relabeled_issue(issue, added_labels, current_user)
+    relabeled_resource_email(issue, added_labels, current_user, 'relabeled_issue_email')
+  end
 
-  # When create a merge request we should send next emails:
+  # When create a merge request we should send an email to:
   #
   #  * mr assignee if their notification level is not Disabled
+  #  * project team members with notification level higher then Participating
+  #  * watchers of the mr's labels
   #
   def new_merge_request(merge_request, current_user)
     new_resource_email(merge_request, merge_request.target_project, 'new_merge_request_email')
   end
 
-  # When we reassign a merge_request we should send next emails:
+  # When we reassign a merge_request we should send an email to:
   #
   #  * merge_request old assignee if their notification level is not Disabled
   #  * merge_request assignee if their notification level is not Disabled
@@ -70,6 +80,14 @@ class NotificationService
     reassign_resource_email(merge_request, merge_request.target_project, current_user, 'reassigned_merge_request_email')
   end
 
+  # When we add labels to a merge request we should send an email to:
+  #
+  #  * watchers of the mr's labels
+  #
+  def relabeled_merge_request(merge_request, added_labels, current_user)
+    relabeled_resource_email(merge_request, added_labels, current_user, 'relabeled_merge_request_email')
+  end
+
   def close_mr(merge_request, current_user)
     close_resource_email(merge_request, merge_request.target_project, current_user, 'closed_merge_request_email')
   end
@@ -91,7 +109,8 @@ class NotificationService
     reopen_resource_email(
       merge_request,
       merge_request.target_project,
-      current_user, 'merge_request_status_email',
+      current_user,
+      'merge_request_status_email',
       'reopened'
     )
   end
@@ -348,19 +367,23 @@ class NotificationService
   end
 
   def add_subscribed_users(recipients, target)
-    return recipients unless target.respond_to? :subscriptions
+    return recipients unless target.respond_to? :subscribers
 
-    subscriptions = target.subscriptions
+    recipients + target.subscribers
+  end
 
-    if subscriptions.any?
-      recipients + subscriptions.where(subscribed: true).map(&:user)
-    else
-      recipients
+  def add_labels_subscribers(recipients, target, labels: nil)
+    return recipients unless target.respond_to? :labels
+
+    (labels || target.labels).each do |label|
+      recipients += label.subscribers
     end
+
+    recipients
   end
 
   def new_resource_email(target, project, method)
-    recipients = build_recipients(target, project, target.author)
+    recipients = build_recipients(target, project, target.author, action: :new)
 
     recipients.each do |recipient|
       mailer.send(method, recipient.id, target.id).deliver_later
@@ -392,6 +415,15 @@ class NotificationService
     end
   end
 
+  def relabeled_resource_email(target, labels, current_user, method)
+    recipients = build_relabeled_recipients(target, current_user, labels: labels)
+    label_names = labels.map(&:name)
+
+    recipients.each do |recipient|
+      mailer.send(method, recipient.id, target.id, label_names, current_user.id).deliver_later
+    end
+  end
+
   def reopen_resource_email(target, project, current_user, method, status)
     recipients = build_recipients(target, project, current_user)
 
@@ -416,6 +448,11 @@ class NotificationService
 
     recipients = reject_muted_users(recipients, project)
     recipients = add_subscribed_users(recipients, target)
+
+    if action == :new
+      recipients = add_labels_subscribers(recipients, target)
+    end
+
     recipients = reject_unsubscribed_users(recipients, target)
 
     recipients.delete(current_user)
@@ -423,6 +460,13 @@ class NotificationService
     recipients.uniq
   end
 
+  def build_relabeled_recipients(target, current_user, labels:)
+    recipients = add_labels_subscribers([], target, labels: labels)
+    recipients = reject_unsubscribed_users(recipients, target)
+    recipients.delete(current_user)
+    recipients.uniq
+  end
+
   def mailer
     Notify
   end
diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index 7408e09ed1e..ba50305dbd5 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -1,11 +1,7 @@
 module Projects
   class AutocompleteService < BaseService
-    def initialize(project)
-      @project = project
-    end
-
     def issues
-      @project.issues.opened.select([:iid, :title])
+      @project.issues.visible_to_user(current_user).opened.select([:iid, :title])
     end
 
     def merge_requests
diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb
index 0db85ac2142..a0973c5d260 100644
--- a/app/services/projects/housekeeping_service.rb
+++ b/app/services/projects/housekeeping_service.rb
@@ -9,12 +9,39 @@ module Projects
   class HousekeepingService < BaseService
     include Gitlab::ShellAdapter
 
+    LEASE_TIMEOUT = 3600
+
+    class LeaseTaken < StandardError
+      def to_s
+        "Somebody already triggered housekeeping for this project in the past #{LEASE_TIMEOUT / 60} minutes"
+      end
+    end
+
     def initialize(project)
       @project = project
     end
 
     def execute
-      GitlabShellWorker.perform_async(:gc, @project.path_with_namespace)
+      raise LeaseTaken if !try_obtain_lease
+
+      GitlabShellOneShotWorker.perform_async(:gc, @project.path_with_namespace)
+    ensure
+      @project.update_column(:pushes_since_gc, 0)
+    end
+
+    def needed?
+      @project.pushes_since_gc >= 10
+    end
+
+    def increment!
+      @project.increment!(:pushes_since_gc)
+    end
+
+    private
+
+    def try_obtain_lease
+      lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT)
+      lease.try_obtain
     end
   end
 end
diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index e904cb6c6fc..aa9837038a6 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -10,9 +10,8 @@ module Search
       group = Group.find_by(id: params[:group_id]) if params[:group_id].present?
       projects = ProjectsFinder.new.execute(current_user)
       projects = projects.in_namespace(group.id) if group
-      project_ids = projects.pluck(:id)
 
-      Gitlab::SearchResults.new(project_ids, params[:search])
+      Gitlab::SearchResults.new(current_user, projects, params[:search])
     end
   end
 end
diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb
index f630c0a3790..4b500914cfb 100644
--- a/app/services/search/project_service.rb
+++ b/app/services/search/project_service.rb
@@ -7,7 +7,8 @@ module Search
     end
 
     def execute
-      Gitlab::ProjectSearchResults.new(project.id,
+      Gitlab::ProjectSearchResults.new(current_user,
+                                       project,
                                        params[:search],
                                        params[:repository_ref])
     end
diff --git a/app/services/search/snippet_service.rb b/app/services/search/snippet_service.rb
index 8ca0877321d..0b3e713e220 100644
--- a/app/services/search/snippet_service.rb
+++ b/app/services/search/snippet_service.rb
@@ -7,8 +7,9 @@ module Search
     end
 
     def execute
-      snippet_ids = Snippet.accessible_to(current_user).pluck(:id)
-      Gitlab::SnippetSearchResults.new(snippet_ids, params[:search])
+      snippets = Snippet.accessible_to(current_user)
+
+      Gitlab::SnippetSearchResults.new(snippets, params[:search])
     end
   end
 end
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 2404ac2ff4c..ce69ae8ada2 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -66,7 +66,7 @@ class SystemNoteService
   def self.change_label(noteable, project, author, added_labels, removed_labels)
     labels_count = added_labels.count + removed_labels.count
 
-    references     = ->(label) { label.to_reference(:id) }
+    references     = ->(label) { label.to_reference(format: :id) }
     added_labels   = added_labels.map(&references).join(' ')
     removed_labels = removed_labels.map(&references).join(' ')
 
@@ -219,6 +219,18 @@ class SystemNoteService
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
 
+  # Called when a branch is created from the 'new branch' button on a issue
+  # Example note text:
+  #
+  #   "Started branch `201-issue-branch-button`"
+  def self.new_issue_branch(issue, project, author, branch)
+    h = Gitlab::Application.routes.url_helpers
+    link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
+
+    body = "Started branch [`#{branch}`](#{link})"
+    create_note(noteable: issue, project: project, author: author, note: body)
+  end
+
   # Called when a Mentionable references a Noteable
   #
   # noteable  - Noteable object being referenced
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 4392e2d17fe..f2662922e90 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -103,24 +103,16 @@ class TodoService
   #  * mark all pending todos related to the target for the current user as done
   #
   def mark_pending_todos_as_done(target, user)
-    pending_todos(user, target.project, target).update_all(state: :done)
+    attributes = attributes_for_target(target)
+    pending_todos(user, attributes).update_all(state: :done)
   end
 
   private
 
-  def create_todos(project, target, author, users, action, note = nil)
+  def create_todos(users, attributes)
     Array(users).each do |user|
-      next if pending_todos(user, project, target).exists?
-
-      Todo.create(
-        project: project,
-        user_id: user.id,
-        author_id: author.id,
-        target_id: target.id,
-        target_type: target.class.name,
-        action: action,
-        note: note
-      )
+      next if pending_todos(user, attributes).exists?
+      Todo.create(attributes.merge(user_id: user.id))
     end
   end
 
@@ -130,8 +122,8 @@ class TodoService
   end
 
   def handle_note(note, author)
-    # Skip system notes, notes on commit, and notes on project snippet
-    return if note.system? || ['Commit', 'Snippet'].include?(note.noteable_type)
+    # Skip system notes, and notes on project snippet
+    return if note.system? || note.for_project_snippet?
 
     project = note.project
     target  = note.noteable
@@ -142,13 +134,39 @@ class TodoService
 
   def create_assignment_todo(issuable, author)
     if issuable.assignee && issuable.assignee != author
-      create_todos(issuable.project, issuable, author, issuable.assignee, Todo::ASSIGNED)
+      attributes = attributes_for_todo(issuable.project, issuable, author, Todo::ASSIGNED)
+      create_todos(issuable.assignee, attributes)
     end
   end
 
-  def create_mention_todos(project, issuable, author, note = nil)
-    mentioned_users = filter_mentioned_users(project, note || issuable, author)
-    create_todos(project, issuable, author, mentioned_users, Todo::MENTIONED, note)
+  def create_mention_todos(project, target, author, note = nil)
+    mentioned_users = filter_mentioned_users(project, note || target, author)
+    attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note)
+    create_todos(mentioned_users, attributes)
+  end
+
+  def attributes_for_target(target)
+    attributes = {
+      project_id: target.project.id,
+      target_id: target.id,
+      target_type: target.class.name,
+      commit_id: nil
+    }
+
+    if target.is_a?(Commit)
+      attributes.merge!(target_id: nil, commit_id: target.id)
+    end
+
+    attributes
+  end
+
+  def attributes_for_todo(project, target, author, action, note = nil)
+    attributes_for_target(target).merge!(
+      project_id: project.id,
+      author_id: author.id,
+      action: action,
+      note: note
+    )
   end
 
   def filter_mentioned_users(project, target, author)
@@ -160,11 +178,8 @@ class TodoService
     mentioned_users.uniq
   end
 
-  def pending_todos(user, project, target)
-    user.todos.pending.where(
-      project_id: project.id,
-      target_id: target.id,
-      target_type: target.class.name
-    )
+  def pending_todos(user, criteria = {})
+    valid_keys = [:project_id, :target_id, :target_type, :commit_id]
+    user.todos.pending.where(criteria.slice(*valid_keys))
   end
 end