Merge branch 'jk-epic-changes-ce-port' into 'master'

CE port of code changed for epics

See merge request gitlab-org/gitlab-ce!15144

21 files changed, 247 insertions, 189 deletions

diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 4079072a930..b1ed973d178 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -7,6 +7,54 @@ module IssuableActions
     before_action :authorize_admin_issuable!, only: :bulk_update
   end
 
+  def show
+    respond_to do |format|
+      format.html do
+        render show_view
+      end
+      format.json do
+        render json: serializer.represent(issuable, serializer: params[:serializer])
+      end
+    end
+  end
+
+  def update
+    @issuable = update_service.execute(issuable)
+
+    respond_to do |format|
+      format.html do
+        recaptcha_check_with_fallback { render :edit }
+      end
+
+      format.json do
+        render_entity_json
+      end
+    end
+
+  rescue ActiveRecord::StaleObjectError
+    render_conflict_response
+  end
+
+  def realtime_changes
+    Gitlab::PollingInterval.set_header(response, interval: 3_000)
+
+    response = {
+      title: view_context.markdown_field(issuable, :title),
+      title_text: issuable.title,
+      description: view_context.markdown_field(issuable, :description),
+      description_text: issuable.description,
+      task_status: issuable.task_status
+    }
+
+    if issuable.edited?
+      response[:updated_at] = issuable.updated_at
+      response[:updated_by_name] = issuable.last_edited_by.name
+      response[:updated_by_path] = user_path(issuable.last_edited_by)
+    end
+
+    render json: response
+  end
+
   def destroy
     issuable.destroy
     destroy_method = "destroy_#{issuable.class.name.underscore}".to_sym
@@ -68,6 +116,10 @@ module IssuableActions
     end
   end
 
+  def authorize_update_issuable!
+    render_404 unless can?(current_user, :"update_#{resource_name}", issuable)
+  end
+
   def bulk_update_params
     permitted_keys = [
       :issuable_ids,
@@ -92,4 +144,24 @@ module IssuableActions
   def resource_name
     @resource_name ||= controller_name.singularize
   end
+
+  def render_entity_json
+    if @issuable.valid?
+      render json: serializer.represent(@issuable)
+    else
+      render json: { errors: @issuable.errors.full_messages }, status: :unprocessable_entity
+    end
+  end
+
+  def show_view
+    'show'
+  end
+
+  def serializer
+    raise NotImplementedError
+  end
+
+  def update_service
+    raise NotImplementedError
+  end
 end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 6a5e4538717..d4e763aa5b8 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -16,7 +16,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_create_issue!, only: [:new, :create]
 
   # Allow modify issue
-  before_action :authorize_update_issue!, only: [:edit, :update, :move]
+  before_action :authorize_update_issuable!, only: [:edit, :update, :move]
 
   # Allow create a new branch and empty WIP merge request from current issue
   before_action :authorize_create_merge_request!, only: [:create_merge_request]
@@ -67,18 +67,6 @@ class Projects::IssuesController < Projects::ApplicationController
     respond_with(@issue)
   end
 
-  def show
-    @noteable = @issue
-    @note     = @project.notes.new(noteable: @issue)
-
-    respond_to do |format|
-      format.html
-      format.json do
-        render json: serializer.represent(@issue, serializer: params[:serializer])
-      end
-    end
-  end
-
   def discussions
     notes = @issue.notes
       .inc_relations_for_view
@@ -120,25 +108,6 @@ class Projects::IssuesController < Projects::ApplicationController
     end
   end
 
-  def update
-    update_params = issue_params.merge(spammable_params)
-
-    @issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue)
-
-    respond_to do |format|
-      format.html do
-        recaptcha_check_with_fallback { render :edit }
-      end
-
-      format.json do
-        render_issue_json
-      end
-    end
-
-  rescue ActiveRecord::StaleObjectError
-    render_conflict_response
-  end
-
   def move
     params.require(:move_to_project_id)
 
@@ -196,26 +165,6 @@ class Projects::IssuesController < Projects::ApplicationController
     end
   end
 
-  def realtime_changes
-    Gitlab::PollingInterval.set_header(response, interval: 3_000)
-
-    response = {
-      title: view_context.markdown_field(@issue, :title),
-      title_text: @issue.title,
-      description: view_context.markdown_field(@issue, :description),
-      description_text: @issue.description,
-      task_status: @issue.task_status
-    }
-
-    if @issue.edited?
-      response[:updated_at] = @issue.updated_at
-      response[:updated_by_name] = @issue.last_edited_by.name
-      response[:updated_by_path] = user_path(@issue.last_edited_by)
-    end
-
-    render json: response
-  end
-
   def create_merge_request
     result = ::MergeRequests::CreateFromIssueService.new(project, current_user, issue_iid: issue.iid).execute
 
@@ -231,7 +180,8 @@ class Projects::IssuesController < Projects::ApplicationController
   def issue
     return @issue if defined?(@issue)
     # The Sortable default scope causes performance issues when used with find_by
-    @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take!
+    @issuable = @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take!
+    @note = @project.notes.new(noteable: @issuable)
 
     return render_404 unless can?(current_user, :read_issue, @issue)
 
@@ -246,14 +196,6 @@ class Projects::IssuesController < Projects::ApplicationController
     project_issue_path(@project, @issue)
   end
 
-  def authorize_update_issue!
-    render_404 unless can?(current_user, :update_issue, @issue)
-  end
-
-  def authorize_admin_issues!
-    render_404 unless can?(current_user, :admin_issue, @project)
-  end
-
   def authorize_create_merge_request!
     render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user)
   end
@@ -305,4 +247,9 @@ class Projects::IssuesController < Projects::ApplicationController
   def serializer
     IssueSerializer.new(current_user: current_user, project: issue.project)
   end
+
+  def update_service
+    update_params = issue_params.merge(spammable_params)
+    Issues::UpdateService.new(project, current_user, update_params)
+  end
 end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 2b0294c8387..17cac69e588 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -9,7 +9,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   skip_before_action :merge_request, only: [:index, :bulk_update]
   skip_before_action :ensure_ref_fetched, only: [:index, :bulk_update]
 
-  before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :remove_wip, :sort]
+  before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
 
   before_action :authenticate_user!, only: [:assign_related_issues]
 
@@ -256,14 +256,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   alias_method :issuable, :merge_request
   alias_method :awardable, :merge_request
 
-  def authorize_update_merge_request!
-    return render_404 unless can?(current_user, :update_merge_request, @merge_request)
-  end
-
-  def authorize_admin_merge_request!
-    return render_404 unless can?(current_user, :admin_merge_request, @merge_request)
-  end
-
   def validates_merge_request
     # Show git not found page
     # if there is no saved commits between source & target branch
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index d4a91e533c1..a77aa0ad2cc 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/app/helpers/gitlab_routing_helper.rb
@@ -71,11 +71,13 @@ module GitlabRoutingHelper
     project_commit_url(entity.project, entity.sha, *args)
   end
 
-  def preview_markdown_path(project, *args)
+  def preview_markdown_path(parent, *args)
+    return group_preview_markdown_path(parent) if parent.is_a?(Group)
+
     if @snippet.is_a?(PersonalSnippet)
       preview_markdown_snippets_path
     else
-      preview_markdown_project_path(project, *args)
+      preview_markdown_project_path(parent, *args)
     end
   end
 
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index d0069cd48cf..85407e38532 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -211,15 +211,13 @@ module IssuablesHelper
 
   def issuable_initial_data(issuable)
     data = {
-      endpoint: project_issue_path(@project, issuable),
-      canUpdate: can?(current_user, :update_issue, issuable),
-      canDestroy: can?(current_user, :destroy_issue, issuable),
+      endpoint: issuable_path(issuable),
+      canUpdate: can?(current_user, :"update_#{issuable.to_ability_name}", issuable),
+      canDestroy: can?(current_user, :"destroy_#{issuable.to_ability_name}", issuable),
       issuableRef: issuable.to_reference,
-      markdownPreviewPath: preview_markdown_path(@project),
+      markdownPreviewPath: preview_markdown_path(parent),
       markdownDocsPath: help_page_path('user/markdown'),
       issuableTemplates: issuable_templates(issuable),
-      projectPath: ref_project.path,
-      projectNamespace: ref_project.namespace.full_path,
       initialTitleHtml: markdown_field(issuable, :title),
       initialTitleText: issuable.title,
       initialDescriptionHtml: markdown_field(issuable, :description),
@@ -227,6 +225,12 @@ module IssuablesHelper
       initialTaskStatus: issuable.task_status
     }
 
+    if parent.is_a?(Group)
+      data[:groupPath] = parent.path
+    else
+      data.merge!(projectPath: ref_project.path, projectNamespace: ref_project.namespace.full_path)
+    end
+
     data.merge!(updated_at_by(issuable))
 
     data.to_json
@@ -263,12 +267,7 @@ module IssuablesHelper
   end
 
   def issuable_path(issuable, *options)
-    case issuable
-    when Issue
-      issue_path(issuable, *options)
-    when MergeRequest
-      merge_request_path(issuable, *options)
-    end
+    polymorphic_path(issuable, *options)
   end
 
   def issuable_url(issuable, *options)
@@ -369,4 +368,8 @@ module IssuablesHelper
       fullPath: @project.full_path
     }
   end
+
+  def parent
+    @project || @group
+  end
 end
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 9417033d1f6..98776eab424 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -49,7 +49,8 @@ module CacheMarkdownField
 
     # Always include a project key, or Banzai complains
     project = self.project if self.respond_to?(:project)
-    context = cached_markdown_fields[field].merge(project: project)
+    group = self.group if self.respond_to?(:group)
+    context = cached_markdown_fields[field].merge(project: project, group: group)
 
     # Banzai is less strict about authors, so don't always have an author key
     context[:author] = self.author if self.respond_to?(:author)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 27f4dedffd3..a928b9d6367 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -14,7 +14,6 @@ module Issuable
   include StripAttribute
   include Awardable
   include Taskable
-  include TimeTrackable
   include Importable
   include Editable
   include AfterCommitQueue
@@ -95,8 +94,6 @@ module Issuable
 
     strip_attributes :title
 
-    acts_as_paranoid
-
     after_save :record_metrics, unless: :imported?
 
     # We want to use optimistic lock for cases when only title or description are involved
diff --git a/app/models/epic.rb b/app/models/epic.rb
new file mode 100644
index 00000000000..0d5f21fb617
--- /dev/null
+++ b/app/models/epic.rb
@@ -0,0 +1,9 @@
+# Placeholder class for model that is implemented in EE
+# It will reserve (ee#3853) '&' as a reference prefix, but the table does not exists in CE
+class Epic < ActiveRecord::Base
+  prepend EE::Epic
+
+  # TODO: this will be implemented as part of #3853
+  def to_reference
+  end
+end
diff --git a/app/models/group.rb b/app/models/group.rb
index 07fb62bb249..4e8023cdb7f 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -180,6 +180,12 @@ class Group < Namespace
     add_user(user, :owner, current_user: current_user)
   end
 
+  def member?(user, min_access_level = Gitlab::Access::GUEST)
+    return false unless user
+
+    max_member_access_for_user(user) >= min_access_level
+  end
+
   def has_owner?(user)
     return false unless user
 
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 36e4108b9d6..fc590f9257e 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -10,6 +10,7 @@ class Issue < ActiveRecord::Base
   include FasterCacheKeys
   include RelativePositioning
   include CreatedAtFilterable
+  include TimeTrackable
 
   DueDateStruct = Struct.new(:title, :name).freeze
   NoDueDate     = DueDateStruct.new('No Due Date', '0').freeze
@@ -74,6 +75,8 @@ class Issue < ActiveRecord::Base
     end
   end
 
+  acts_as_paranoid
+
   def self.reference_prefix
     '#'
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index d45b9c805a4..3133dc9e7eb 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -6,6 +6,7 @@ class MergeRequest < ActiveRecord::Base
   include Sortable
   include IgnorableColumn
   include CreatedAtFilterable
+  include TimeTrackable
 
   ignore_column :locked_at
 
@@ -119,6 +120,8 @@ class MergeRequest < ActiveRecord::Base
 
   after_save :keep_around_commit
 
+  acts_as_paranoid
+
   def self.reference_prefix
     '!'
   end
diff --git a/app/models/note.rb b/app/models/note.rb
index 8939e590ef1..f9676361072 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -69,7 +69,7 @@ class Note < ActiveRecord::Base
   delegate :title, to: :noteable, allow_nil: true
 
   validates :note, presence: true
-  validates :project, presence: true, unless: :for_personal_snippet?
+  validates :project, presence: true, if: :for_project_noteable?
 
   # Attachments are deprecated and are handled by Markdown uploader
   validates :attachment, file_size: { maximum: :max_attachment_size }
@@ -114,7 +114,7 @@ class Note < ActiveRecord::Base
   after_initialize :ensure_discussion_id
   before_validation :nullify_blank_type, :nullify_blank_line_code
   before_validation :set_discussion_id, on: :create
-  after_save :keep_around_commit, unless: :for_personal_snippet?
+  after_save :keep_around_commit, if: :for_project_noteable?
   after_save :expire_etag_cache
   after_destroy :expire_etag_cache
 
@@ -208,6 +208,10 @@ class Note < ActiveRecord::Base
     noteable.is_a?(PersonalSnippet)
   end
 
+  def for_project_noteable?
+    !for_personal_snippet?
+  end
+
   def skip_project_check?
     for_personal_snippet?
   end
diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb
index 61c7a428745..3b5a4fd4f79 100644
--- a/app/serializers/issuable_entity.rb
+++ b/app/serializers/issuable_entity.rb
@@ -1,20 +1,16 @@
 class IssuableEntity < Grape::Entity
+  include RequestAwareEntity
+
   expose :id
   expose :iid
   expose :author_id
   expose :description
   expose :lock_version
   expose :milestone_id
-  expose :state
   expose :title
   expose :updated_by_id
   expose :created_at
   expose :updated_at
-  expose :deleted_at
-  expose :time_estimate
-  expose :total_time_spent
-  expose :human_time_estimate
-  expose :human_total_time_spent
   expose :milestone, using: API::Entities::Milestone
   expose :labels, using: LabelEntity
 end
diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb
index 10d3ad0214b..5f47592e4ad 100644
--- a/app/serializers/issue_entity.rb
+++ b/app/serializers/issue_entity.rb
@@ -1,6 +1,8 @@
 class IssueEntity < IssuableEntity
-  include RequestAwareEntity
+  include TimeTrackableEntity
 
+  expose :state
+  expose :deleted_at
   expose :branch_name
   expose :confidential
   expose :discussion_locked
diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb
index 297a459e394..b53a49fe59e 100644
--- a/app/serializers/merge_request_entity.rb
+++ b/app/serializers/merge_request_entity.rb
@@ -1,6 +1,8 @@
 class MergeRequestEntity < IssuableEntity
-  include RequestAwareEntity
+  include TimeTrackableEntity
 
+  expose :state
+  expose :deleted_at
   expose :in_progress_merge_commit_sha
   expose :merge_commit_sha
   expose :merge_error
diff --git a/app/serializers/time_trackable_entity.rb b/app/serializers/time_trackable_entity.rb
new file mode 100644
index 00000000000..e81cd7bec72
--- /dev/null
+++ b/app/serializers/time_trackable_entity.rb
@@ -0,0 +1,11 @@
+module TimeTrackableEntity
+  extend ActiveSupport::Concern
+  extend Grape
+
+  included do
+    expose :time_estimate
+    expose :total_time_spent
+    expose :human_time_estimate
+    expose :human_total_time_spent
+  end
+end
diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb
new file mode 100644
index 00000000000..92eaa5d5115
--- /dev/null
+++ b/app/services/issuable/common_system_notes_service.rb
@@ -0,0 +1,81 @@
+module Issuable
+  class CommonSystemNotesService < ::BaseService
+    attr_reader :issuable
+
+    def execute(issuable, old_labels)
+      @issuable = issuable
+
+      if issuable.previous_changes.include?('title')
+        create_title_change_note(issuable.previous_changes['title'].first)
+      end
+
+      handle_description_change_note
+
+      handle_time_tracking_note if issuable.is_a?(TimeTrackable)
+      create_labels_note(old_labels) if issuable.labels != old_labels
+      create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked')
+      create_milestone_note if issuable.previous_changes.include?('milestone_id')
+    end
+
+    private
+
+    def handle_time_tracking_note
+      if issuable.previous_changes.include?('time_estimate')
+        create_time_estimate_note
+      end
+
+      if issuable.time_spent?
+        create_time_spent_note
+      end
+    end
+
+    def handle_description_change_note
+      if issuable.previous_changes.include?('description')
+        if issuable.tasks? && issuable.updated_tasks.any?
+          create_task_status_note
+        else
+          # TODO: Show this note if non-task content was modified.
+          # https://gitlab.com/gitlab-org/gitlab-ce/issues/33577
+          create_description_change_note
+        end
+      end
+    end
+
+    def create_labels_note(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
+
+    def create_title_change_note(old_title)
+      SystemNoteService.change_title(issuable, issuable.project, current_user, old_title)
+    end
+
+    def create_description_change_note
+      SystemNoteService.change_description(issuable, issuable.project, current_user)
+    end
+
+    def create_task_status_note
+      issuable.updated_tasks.each do |task|
+        SystemNoteService.change_task_status(issuable, issuable.project, current_user, task)
+      end
+    end
+
+    def create_time_estimate_note
+      SystemNoteService.change_time_estimate(issuable, issuable.project, current_user)
+    end
+
+    def create_time_spent_note
+      SystemNoteService.change_time_spent(issuable, issuable.project, issuable.time_spent_user)
+    end
+
+    def create_milestone_note
+      SystemNoteService.change_milestone(issuable, issuable.project, current_user, issuable.milestone)
+    end
+
+    def create_discussion_lock_note
+      SystemNoteService.discussion_lock(issuable, current_user)
+    end
+  end
+end
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index d61a342ebad..68b49d880f7 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -1,56 +1,10 @@
 class IssuableBaseService < BaseService
   private
 
-  def create_milestone_note(issuable)
-    SystemNoteService.change_milestone(
-      issuable, issuable.project, current_user, issuable.milestone)
-  end
-
-  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
-
-  def create_title_change_note(issuable, old_title)
-    SystemNoteService.change_title(
-      issuable, issuable.project, current_user, old_title)
-  end
-
-  def create_description_change_note(issuable)
-    SystemNoteService.change_description(issuable, issuable.project, current_user)
-  end
-
-  def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
-    SystemNoteService.change_branch(
-      issuable, issuable.project, current_user, branch_type,
-      old_branch, new_branch)
-  end
-
-  def create_task_status_note(issuable)
-    issuable.updated_tasks.each do |task|
-      SystemNoteService.change_task_status(issuable, issuable.project, current_user, task)
-    end
-  end
-
-  def create_time_estimate_note(issuable)
-    SystemNoteService.change_time_estimate(issuable, issuable.project, current_user)
-  end
-
-  def create_time_spent_note(issuable)
-    SystemNoteService.change_time_spent(issuable, issuable.project, current_user)
-  end
-
-  def create_discussion_lock_note(issuable)
-    SystemNoteService.discussion_lock(issuable, current_user)
-  end
-
   def filter_params(issuable)
     ability_name = :"admin_#{issuable.to_ability_name}"
 
-    unless can?(current_user, ability_name, project)
+    unless can?(current_user, ability_name, issuable)
       params.delete(:milestone_id)
       params.delete(:labels)
       params.delete(:add_label_ids)
@@ -233,15 +187,14 @@ class IssuableBaseService < BaseService
 
       # We have to perform this check before saving the issuable as Rails resets
       # the changed fields upon calling #save.
-      update_project_counters = issuable.update_project_counter_caches?
+      update_project_counters = issuable.project && issuable.update_project_counter_caches?
 
       if issuable.with_transaction_returning_status { issuable.save }
         # We do not touch as it will affect a update on updated_at field
         ActiveRecord::Base.no_touching do
-          handle_common_system_notes(issuable, old_labels: old_labels)
+          Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels)
         end
 
-        change_discussion_lock(issuable)
         handle_changes(
           issuable,
           old_labels: old_labels,
@@ -300,12 +253,6 @@ class IssuableBaseService < BaseService
     end
   end
 
-  def change_discussion_lock(issuable)
-    if issuable.previous_changes.include?('discussion_locked')
-      create_discussion_lock_note(issuable)
-    end
-  end
-
   def toggle_award(issuable)
     award = params.delete(:emoji_award)
     if award
@@ -328,35 +275,17 @@ class IssuableBaseService < BaseService
     attrs_changed || labels_changed || assignees_changed
   end
 
-  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
-
-    if issuable.previous_changes.include?('description')
-      if issuable.tasks? && issuable.updated_tasks.any?
-        create_task_status_note(issuable)
-      else
-        # TODO: Show this note if non-task content was modified.
-        # https://gitlab.com/gitlab-org/gitlab-ce/issues/33577
-        create_description_change_note(issuable)
-      end
-    end
-
-    if issuable.previous_changes.include?('time_estimate')
-      create_time_estimate_note(issuable)
-    end
-
-    if issuable.time_spent?
-      create_time_spent_note(issuable)
-    end
-
-    create_labels_note(issuable, old_labels) if issuable.labels != old_labels
-  end
-
   def invalidate_cache_counts(issuable, users: [])
     users.each do |user|
       user.public_send("invalidate_#{issuable.model_name.singular}_cache_counts") # rubocop:disable GitlabSecurity/PublicSend
     end
   end
+
+  # override if needed
+  def handle_changes(issuable, options)
+  end
+
+  # override if needed
+  def execute_hooks(issuable, action = 'open', params = {})
+  end
 end
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index e0339ddf9bb..1b7b5927c5a 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -27,10 +27,6 @@ module Issues
         todo_service.update_issue(issue, current_user, old_mentioned_users)
       end
 
-      if issue.previous_changes.include?('milestone_id')
-        create_milestone_note(issue)
-      end
-
       if issue.assignees != old_assignees
         create_assignee_note(issue, old_assignees)
         notification_service.reassigned_issue(issue, current_user, old_assignees)
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 2832d893e95..1f394cacc64 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -40,10 +40,6 @@ module MergeRequests
                                   merge_request.target_branch)
       end
 
-      if merge_request.previous_changes.include?('milestone_id')
-        create_milestone_note(merge_request)
-      end
-
       if merge_request.previous_changes.include?('assignee_id')
         create_assignee_note(merge_request)
         notification_service.reassigned_merge_request(merge_request, current_user)
@@ -111,5 +107,11 @@ module MergeRequests
                          end
       end
     end
+
+    def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
+      SystemNoteService.change_branch(
+        issuable, issuable.project, current_user, branch_type,
+        old_branch, new_branch)
+    end
   end
 end
diff --git a/app/views/projects/merge_requests/_mr_title.html.haml b/app/views/projects/merge_requests/_mr_title.html.haml
index cb723fe6a18..72d5c4961ec 100644
--- a/app/views/projects/merge_requests/_mr_title.html.haml
+++ b/app/views/projects/merge_requests/_mr_title.html.haml
@@ -34,7 +34,7 @@
             %li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }
               = link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, title: 'Close merge request'
             %li{ class: merge_request_button_visibility(@merge_request, false) }
-              = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen merge request'
+              = link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen merge request'
 
       - if can_update_merge_request
         = link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "hidden-xs hidden-sm btn btn-grouped issuable-edit"