From 5e3c9475a9332d7104a791f7bdfef30e069dd848 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 16 Mar 2016 10:24:44 +0100
Subject: [PATCH] Add minor improvements in code related to issue move
---
.../javascripts/issuable_form.js.coffee | 8 ++---
app/helpers/issues_helper.rb | 8 +++--
app/services/issues/move_service.rb | 31 +++++++------------
app/services/system_note_service.rb | 10 +++---
app/views/shared/issuable/_form.html.haml | 2 +-
lib/gitlab/gfm/reference_unfolder.rb | 27 ++++++++++++++--
spec/services/system_note_service_spec.rb | 2 +-
7 files changed, 50 insertions(+), 38 deletions(-)
diff --git a/app/assets/javascripts/issuable_form.js.coffee b/app/assets/javascripts/issuable_form.js.coffee
index 9b939985b33..d60832f90ed 100644
--- a/app/assets/javascripts/issuable_form.js.coffee
+++ b/app/assets/javascripts/issuable_form.js.coffee
@@ -30,11 +30,11 @@ class @IssuableForm
"description"
]
- handleSubmit: (e) =>
- @resetAutosave
-
+ handleSubmit: =>
if (parseInt(@issueMoveField?.val()) ? 0) > 0
- e.preventDefault() unless confirm(ISSUE_MOVE_CONFIRM_MSG)
+ return false unless confirm(ISSUE_MOVE_CONFIRM_MSG)
+
+ @resetAutosave()
resetAutosave: =>
@titleField.data("autosave").reset()
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 62479b8d1ce..b67057ebc7c 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -60,12 +60,14 @@ module IssuesHelper
def project_options(issuable, current_user, ability: :read_project)
projects = current_user.authorized_projects
projects = projects.select do |project|
- current_user.can?(ability, project) && project != issuable.project
+ current_user.can?(ability, project)
end
- projects.unshift(OpenStruct.new(id: 0, name_with_namespace: 'No project'))
+ no_project = OpenStruct.new(id: 0, name_with_namespace: 'No project')
+ projects.unshift(no_project)
+ projects.delete(issuable.project)
- options_from_collection_for_select(projects, :id, :name_with_namespace, 0)
+ options_from_collection_for_select(projects, :id, :name_with_namespace)
end
def status_box_class(item)
diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index 4abcd203407..ce31830f2d0 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -7,7 +7,7 @@ module Issues
@issue_new = nil
@project_old = @project
- if new_project_id
+ if new_project_id.to_i > 0
@project_new = Project.find(new_project_id)
end
@@ -19,7 +19,7 @@ module Issues
def execute
return unless move?
- # Using trasaction because of a high resources footprint
+ # Using transaction because of a high resources footprint
# on rewriting notes (unfolding references)
#
ActiveRecord::Base.transaction do
@@ -54,10 +54,11 @@ module Issues
def create_new_issue
new_params = { id: nil, iid: nil, milestone: nil, label_ids: [],
project: @project_new, author: @issue_old.author,
- description: rewrite_references(@issue_old) }
+ description: unfold_references(@issue_old.description) }
+ new_params = @issue_old.serializable_hash.merge(new_params)
create_service = CreateService.new(@project_new, @current_user,
- params.merge(new_params))
+ new_params)
@issue_new = create_service.execute(set_author: false)
end
@@ -66,7 +67,7 @@ module Issues
@issue_old.notes.find_each do |note|
new_note = note.dup
new_params = { project: @project_new, noteable: @issue_new,
- note: rewrite_references(new_note) }
+ note: unfold_references(new_note.note) }
new_note.update(new_params)
end
@@ -78,30 +79,20 @@ module Issues
end
def add_moved_from_note
- SystemNoteService.noteable_moved(:from, @issue_new, @project_new,
- @issue_old, @current_user)
+ SystemNoteService.noteable_moved(@issue_new, @project_new,
+ @issue_old, @current_user, direction: :from)
end
def add_moved_to_note
- SystemNoteService.noteable_moved(:to, @issue_old, @project_old,
- @issue_new, @current_user)
+ SystemNoteService.noteable_moved(@issue_old, @project_old,
+ @issue_new, @current_user, direction: :to)
end
- def rewrite_references(noteable)
- content = noteable_content(noteable).dup
+ def unfold_references(content)
unfolder = Gitlab::Gfm::ReferenceUnfolder.new(content, @project_old)
unfolder.unfold(@project_new)
end
- def noteable_content(noteable)
- case noteable
- when Issue then noteable.description
- when Note then noteable.note
- else
- raise 'Unexpected noteable while moving an issue!'
- end
- end
-
def notify_participants
notification_service.issue_moved(@issue_old, @issue_new, @current_user)
end
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index a8d6555dec3..c679891b07c 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -3,7 +3,6 @@
# Used for creating system notes (e.g., when a user references a merge request
# from an issue, an issue's assignee changes, an issue is closed, etc.)
class SystemNoteService
- extend GitlabMarkdownHelper
# Called when commits are added to a Merge Request
#
# noteable - Noteable object
@@ -398,16 +397,15 @@ class SystemNoteService
#
# Example Note text:
#
- # "Moved to project_new/#11"
+ # "Moved to some_namespace/project_new#11"
#
# Returns the created Note object
- def self.noteable_moved(direction, noteable, project, noteable_ref, author)
+ def self.noteable_moved(noteable, project, noteable_ref, author, direction:)
unless [:to, :from].include?(direction)
- raise StandardError, "Invalid direction `#{direction}`"
+ raise ArgumentError, "Invalid direction `#{direction}`"
end
- cross_reference = cross_project_reference(noteable_ref.project, noteable_ref)
-
+ cross_reference = noteable_ref.to_reference(project)
body = "Moved #{direction} #{cross_reference}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 11b8b9d44af..d3eabe9ea64 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -70,7 +70,7 @@
- if issuable.is_a?(Issue) && can?(current_user, :admin_issue, issuable.project)
%hr
.form-group
- = f.label :move_to_project_id, 'Move', class: 'control-label'
+ = label_tag :move_to_project_id, 'Move', class: 'control-label'
.col-sm-10
- projects = project_options(issuable, current_user, ability: :admin_issue)
= select_tag(:move_to_project_id, projects, include_blank: true,
diff --git a/lib/gitlab/gfm/reference_unfolder.rb b/lib/gitlab/gfm/reference_unfolder.rb
index 57871c36eb4..0a68d6f977f 100644
--- a/lib/gitlab/gfm/reference_unfolder.rb
+++ b/lib/gitlab/gfm/reference_unfolder.rb
@@ -1,8 +1,30 @@
module Gitlab
module Gfm
##
- # Class than unfolds local references in text.
+ # Class that unfolds local references in text.
#
+ # The initializer takes text in Markdown and project this text is valid
+ # in context of.
+ #
+ # `unfold` method tries to find all local references and unfold each of
+ # those local references to cross reference format.
+ #
+ # Examples:
+ #
+ # 'Hello, this issue is related to #123 and
+ # other issues labeled with ~"label"', will be converted to:
+ #
+ # 'Hello, this issue is related to gitlab-org/gitlab-ce#123 and
+ # other issue labeled with gitlab-org/gitlab-ce~"label"'.
+ #
+ # It does respect markdown lexical rules, so text in code block will not be
+ # replaced, see another example:
+ #
+ # 'Merge request for issue #1234, see also link:
+ # http://gitlab.com/some/link/#1234, and code `puts #1234`' =>
+ #
+ # 'Merge request for issue gitlab-org/gitlab-ce#1234, se also link:
+ # http://gitlab.com/some/link/#1234, and code `puts #1234`'
#
class ReferenceUnfolder
def initialize(text, project)
@@ -66,8 +88,7 @@ module Gitlab
end
def markdown(text)
- helper = Class.new.extend(GitlabMarkdownHelper)
- helper.markdown(text, project: @project, no_original_data: true)
+ Banzai.render(text, project: @project, no_original_data: true)
end
end
end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 7efb9f2ddba..7c93ce304f9 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -446,7 +446,7 @@ describe SystemNoteService, services: true do
let(:new_noteable) { create(:issue, project: new_project) }
subject do
- described_class.noteable_moved(direction, noteable, project, new_noteable, author)
+ described_class.noteable_moved(noteable, project, new_noteable, author, direction: direction)
end
shared_examples 'cross project mentionable' do
--
GitLab