summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2016-03-21 14:01:19 +0000
committerKamil Trzciński <ayufan@ayufan.eu>2016-03-21 14:01:19 +0000
commit3fca30d27f90a52bdbca746b1244c95e6c7db2d2 (patch)
treecfc9e2ac318ca4c8513c1d8530abc0ea4270c236
parentbfcdb47c403ae5054d1feb2b57e1e255ba01a9cb (diff)
parentdb8f70d5088cc1e0172fd6b94fc4628bd83aa4a4 (diff)
downloadgitlab-ce-3fca30d27f90a52bdbca746b1244c95e6c7db2d2.tar.gz
Merge branch 'feature/issue-move' into 'master'
Ability to move issue to another project Tasks: - [x] Create scaffold of service that will move issue to another project. - [x] Close old issue, add system note about moving issue to a new project. - [x] Create a new issue, add system note about issue being moved from old project. - [x] Check if issue can be moved to another project before executing service - [x] Check permissions when moving an issue (`:admin_issue` ability) - [x] Display select box for a new project when editing an issue - [x] Show only projects that issue can be moved into in that select box - [x] Add project select handler, helper and some permission filters to it - [x] Preserve as much information as possible, including author - [x] Prepare mechanisms that unfolds local references in issue description - [x] Rewrite issue description with references unfolding and add some specs for it - [x] Rewrite all system notes and comments attached to issue that is being moved - [x] Update `Label` so that is was able to create cross reference labels (separate MR) - [x] Add notifications about moving issue to another project - [x] Display confirmation alert/message when issue move has been requested - [x] Make it possible to undo selecting project where issue will be moved to - [x] Add column to issue, that will indicate if it has been moved to another project - [x] Do not allow to move issue that has been already moved - [x] Write top-to-bottom feature spec in RSpec instead of Spinach UI: ![issue_move_ui](/uploads/b3c6b563362c1fded9082cc0f51e5a74/issue_move_ui.png) ![issue_move_tooltip](/uploads/2ab913b06f52df1cafde9abe89bd9cb8/issue_move_tooltip.png) Closes #3024 See merge request !2831
-rw-r--r--CHANGELOG1
-rw-r--r--app/assets/javascripts/issuable_form.js.coffee11
-rw-r--r--app/controllers/projects/issues_controller.rb6
-rw-r--r--app/helpers/issues_helper.rb13
-rw-r--r--app/mailers/emails/issues.rb8
-rw-r--r--app/models/concerns/issuable.rb9
-rw-r--r--app/models/issue.rb15
-rw-r--r--app/models/user.rb2
-rw-r--r--app/services/git_push_service.rb2
-rw-r--r--app/services/issues/close_service.rb6
-rw-r--r--app/services/issues/create_service.rb2
-rw-r--r--app/services/issues/move_service.rb94
-rw-r--r--app/services/merge_requests/post_merge_service.rb2
-rw-r--r--app/services/notification_service.rb10
-rw-r--r--app/services/system_note_service.rb22
-rw-r--r--app/views/notify/issue_moved_email.html.haml6
-rw-r--r--app/views/notify/issue_moved_email.text.erb4
-rw-r--r--app/views/shared/issuable/_form.html.haml23
-rw-r--r--db/migrate/20160317092222_add_moved_to_to_issue.rb5
-rw-r--r--db/schema.rb3
-rw-r--r--lib/banzai/filter/reference_filter.rb1
-rw-r--r--lib/gitlab/gfm/reference_rewriter.rb79
-rw-r--r--lib/gitlab/reference_extractor.rb18
-rw-r--r--spec/features/issues/move_spec.rb87
-rw-r--r--spec/lib/gitlab/gfm/reference_rewriter_spec.rb89
-rw-r--r--spec/lib/gitlab/reference_extractor_spec.rb20
-rw-r--r--spec/mailers/notify_spec.rb27
-rw-r--r--spec/models/issue_spec.rb50
-rw-r--r--spec/services/issues/move_service_spec.rb213
-rw-r--r--spec/services/system_note_service_spec.rb53
30 files changed, 866 insertions, 15 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 7abdf014c2a..7109c63ace2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
Please view this file on the master branch, on stable branches it's out of date.
v 8.6.0 (unreleased)
+ - Add ability to move issue to another project
- Fix bug where wrong commit ID was being used in a merge request diff to show old image (Stan Hu)
- Make HTTP(s) label consistent on clone bar (Stan Hu)
- Add confidential issues
diff --git a/app/assets/javascripts/issuable_form.js.coffee b/app/assets/javascripts/issuable_form.js.coffee
index 6c1699c178c..7a788f761b7 100644
--- a/app/assets/javascripts/issuable_form.js.coffee
+++ b/app/assets/javascripts/issuable_form.js.coffee
@@ -1,5 +1,7 @@
class @IssuableForm
+ issueMoveConfirmMsg: 'Are you sure you want to move this issue to another project?'
wipRegex: /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i
+
constructor: (@form) ->
GitLab.GfmAutoComplete.setup()
new UsersSelect()
@@ -7,12 +9,13 @@ class @IssuableForm
@titleField = @form.find("input[name*='[title]']")
@descriptionField = @form.find("textarea[name*='[description]']")
+ @issueMoveField = @form.find("#move_to_project_id")
return unless @titleField.length && @descriptionField.length
@initAutosave()
- @form.on "submit", @resetAutosave
+ @form.on "submit", @handleSubmit
@form.on "click", ".btn-cancel", @resetAutosave
@initWip()
@@ -30,6 +33,12 @@ class @IssuableForm
"description"
]
+ handleSubmit: =>
+ if (parseInt(@issueMoveField?.val()) ? 0) > 0
+ return false unless confirm(@issueMoveConfirmMsg)
+
+ @resetAutosave()
+
resetAutosave: =>
@titleField.data("autosave").reset()
@descriptionField.data("autosave").reset()
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index fb8375116c6..a3c28a82d26 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -90,6 +90,12 @@ class Projects::IssuesController < Projects::ApplicationController
def update
@issue = Issues::UpdateService.new(project, current_user, issue_params).execute(issue)
+ if params[:move_to_project_id].to_i > 0
+ new_project = Project.find(params[:move_to_project_id])
+ move_service = Issues::MoveService.new(project, current_user)
+ @issue = move_service.execute(@issue, new_project)
+ end
+
respond_to do |format|
format.js
format.html do
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index e00d3204027..24b90fef4fe 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -57,6 +57,19 @@ module IssuesHelper
options_from_collection_for_select(milestones, 'id', 'title', object.milestone_id)
end
+ def project_options(issuable, current_user, ability: :read_project)
+ projects = current_user.authorized_projects
+ projects = projects.select do |project|
+ current_user.can?(ability, project)
+ end
+
+ 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)
+ end
+
def status_box_class(item)
if item.respond_to?(:expired?) && item.expired?
'status-box-expired'
diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index 5f9adb32e00..6f54c42146c 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -36,6 +36,14 @@ module Emails
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
end
+ def issue_moved_email(recipient, issue, new_issue, updated_by_user)
+ setup_issue_mail(issue.id, recipient.id)
+
+ @new_issue = new_issue
+ @new_project = new_issue.project
+ mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id))
+ end
+
private
def setup_issue_mail(issue_id, recipient_id)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 86ab84615ba..9ab72652190 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -209,4 +209,13 @@ module Issuable
Taskable.get_updated_tasks(old_content: previous_changes['description'].first,
new_content: description)
end
+
+ ##
+ # Method that checks if issuable can be moved to another project.
+ #
+ # Should be overridden if issuable can be moved.
+ #
+ def can_move?(*)
+ false
+ end
end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 5347d4fa1be..35e08fa915b 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -16,6 +16,7 @@
# state :string(255)
# iid :integer
# updated_by_id :integer
+# moved_to_id :integer
#
require 'carrierwave/orm/activerecord'
@@ -31,6 +32,8 @@ class Issue < ActiveRecord::Base
ActsAsTaggableOn.strict_case_match = true
belongs_to :project
+ belongs_to :moved_to, class_name: 'Issue'
+
validates :project, presence: true
scope :of_group,
@@ -137,6 +140,18 @@ class Issue < ActiveRecord::Base
end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
end
+ def moved?
+ !moved_to.nil?
+ end
+
+ def can_move?(user, to_project = nil)
+ if to_project
+ return false unless user.can?(:admin_issue, to_project)
+ end
+
+ !moved? && user.can?(:admin_issue, self.project)
+ end
+
def to_branch_name
"#{title.parameterize}-#{iid}"
end
diff --git a/app/models/user.rb b/app/models/user.rb
index c011af03591..9c315cfe966 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -435,7 +435,7 @@ class User < ActiveRecord::Base
Group.where("namespaces.id IN (#{union.to_sql})")
end
- # Returns the groups a user is authorized to access.
+ # Returns projects user is authorized to access.
def authorized_projects
Project.where("projects.id IN (#{projects_union.to_sql})")
end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 14e2a2c0699..c007d648dd6 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -120,7 +120,7 @@ class GitPushService < BaseService
closed_issues = commit.closes_issues(current_user)
closed_issues.each do |issue|
if can?(current_user, :update_issue, issue)
- Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit)
+ Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit: commit)
end
end
end
diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb
index 78254b49af3..859c934ea3b 100644
--- a/app/services/issues/close_service.rb
+++ b/app/services/issues/close_service.rb
@@ -1,6 +1,6 @@
module Issues
class CloseService < Issues::BaseService
- def execute(issue, commit = nil)
+ def execute(issue, commit: nil, notifications: true, system_note: true)
if project.jira_tracker? && project.jira_service.active
project.jira_service.execute(commit, issue)
todo_service.close_issue(issue, current_user)
@@ -9,8 +9,8 @@ module Issues
if project.default_issues_tracker? && issue.close
event_service.close_issue(issue, current_user)
- create_note(issue, commit)
- notification_service.close_issue(issue, current_user)
+ create_note(issue, commit) if system_note
+ notification_service.close_issue(issue, current_user) if notifications
todo_service.close_issue(issue, current_user)
execute_hooks(issue, 'close')
end
diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index 10787e8873c..e63e1af8766 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -4,7 +4,7 @@ module Issues
filter_params
label_params = params[:label_ids]
issue = project.issues.new(params.except(:label_ids))
- issue.author = current_user
+ issue.author = params[:author] || current_user
if issue.save
issue.update_attributes(label_ids: label_params)
diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
new file mode 100644
index 00000000000..3cfbafe1576
--- /dev/null
+++ b/app/services/issues/move_service.rb
@@ -0,0 +1,94 @@
+module Issues
+ class MoveService < Issues::BaseService
+ class MoveError < StandardError; end
+
+ def execute(issue, new_project)
+ @old_issue = issue
+ @old_project = @project
+ @new_project = new_project
+
+ unless issue.can_move?(current_user, new_project)
+ raise MoveError, 'Cannot move issue due to insufficient permissions!'
+ end
+
+ if @project == new_project
+ raise MoveError, 'Cannot move issue to project it originates from!'
+ end
+
+ # Using transaction because of a high resources footprint
+ # on rewriting notes (unfolding references)
+ #
+ ActiveRecord::Base.transaction do
+ # New issue tasks
+ #
+ @new_issue = create_new_issue
+
+ rewrite_notes
+ add_note_moved_from
+
+ # Old issue tasks
+ #
+ add_note_moved_to
+ close_issue
+ mark_as_moved
+ end
+
+ notify_participants
+
+ @new_issue
+ end
+
+ private
+
+ def create_new_issue
+ new_params = { id: nil, iid: nil, label_ids: [], milestone: nil,
+ project: @new_project, author: @old_issue.author,
+ description: unfold_references(@old_issue.description) }
+
+ new_params = @old_issue.serializable_hash.merge(new_params)
+ CreateService.new(@new_project, @current_user, new_params).execute
+ end
+
+ def rewrite_notes
+ @old_issue.notes.find_each do |note|
+ new_note = note.dup
+ new_params = { project: @new_project, noteable: @new_issue,
+ note: unfold_references(new_note.note),
+ created_at: note.created_at }
+
+ new_note.update(new_params)
+ end
+ end
+
+ def close_issue
+ close_service = CloseService.new(@old_project, @current_user)
+ close_service.execute(@old_issue, notifications: false, system_note: false)
+ end
+
+ def add_note_moved_from
+ SystemNoteService.noteable_moved(@new_issue, @new_project,
+ @old_issue, @current_user,
+ direction: :from)
+ end
+
+ def add_note_moved_to
+ SystemNoteService.noteable_moved(@old_issue, @old_project,
+ @new_issue, @current_user,
+ direction: :to)
+ end
+
+ def unfold_references(content)
+ rewriter = Gitlab::Gfm::ReferenceRewriter.new(content, @old_project,
+ @current_user)
+ rewriter.rewrite(@new_project)
+ end
+
+ def notify_participants
+ notification_service.issue_moved(@old_issue, @new_issue, @current_user)
+ end
+
+ def mark_as_moved
+ @old_issue.update(moved_to: @new_issue)
+ end
+ end
+end
diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index ebb67c7db65..064910f81f7 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -22,7 +22,7 @@ module MergeRequests
closed_issues = merge_request.closes_issues(current_user)
closed_issues.each do |issue|
if can?(current_user, :update_issue, issue)
- Issues::CloseService.new(project, current_user, {}).execute(issue, merge_request)
+ Issues::CloseService.new(project, current_user, {}).execute(issue, commit: merge_request)
end
end
end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 19a6779dea9..3bdf00a8291 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -236,6 +236,16 @@ class NotificationService
end
end
+ def issue_moved(issue, new_issue, current_user)
+ recipients = build_recipients(issue, issue.project, current_user)
+
+ recipients.map do |recipient|
+ email = mailer.issue_moved_email(recipient, issue, new_issue, current_user)
+ email.deliver_later
+ email
+ end
+ end
+
protected
# Get project users with WATCH notification level
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index c644cd0b951..e022a046c48 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -411,4 +411,26 @@ class SystemNoteService
body = "Marked the task **#{new_task.source}** as #{status_label}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
+
+ # Called when noteable has been moved to another project
+ #
+ # direction - symbol, :to or :from
+ # noteable - Noteable object
+ # noteable_ref - Referenced noteable
+ # author - User performing the move
+ #
+ # Example Note text:
+ #
+ # "Moved to some_namespace/project_new#11"
+ #
+ # Returns the created Note object
+ def self.noteable_moved(noteable, project, noteable_ref, author, direction:)
+ unless [:to, :from].include?(direction)
+ raise ArgumentError, "Invalid direction `#{direction}`"
+ end
+
+ cross_reference = noteable_ref.to_reference(project)
+ body = "Moved #{direction} #{cross_reference}"
+ create_note(noteable: noteable, project: project, author: author, note: body)
+ end
end
diff --git a/app/views/notify/issue_moved_email.html.haml b/app/views/notify/issue_moved_email.html.haml
new file mode 100644
index 00000000000..40f7d61fe19
--- /dev/null
+++ b/app/views/notify/issue_moved_email.html.haml
@@ -0,0 +1,6 @@
+%p
+ Issue was moved to another project.
+%p
+ New issue:
+ = link_to namespace_project_issue_url(@new_project.namespace, @new_project, @new_issue) do
+ = @new_issue.title
diff --git a/app/views/notify/issue_moved_email.text.erb b/app/views/notify/issue_moved_email.text.erb
new file mode 100644
index 00000000000..b3bd43c2055
--- /dev/null
+++ b/app/views/notify/issue_moved_email.text.erb
@@ -0,0 +1,4 @@
+Issue was moved to another project.
+
+New issue location:
+<%= namespace_project_issue_url(@new_project.namespace, @new_project, @new_issue) %>
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 80418e69d9c..1740b128ee4 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -85,13 +85,26 @@
- if can? current_user, :admin_label, issuable.project
= link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank
+- if issuable.can_move?(current_user)
+ %hr
+ .form-group
+ = 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,
+ class: 'select2', data: { placeholder: 'Select project' })
+ &nbsp;
+ %span{ data: { toggle: 'tooltip', placement: 'auto top' }, style: 'cursor: default',
+ title: 'Moving an issue will copy the discussion to a different project and close it here. All participants will be notified of the new location.' }
+ = icon('question-circle')
+
- if issuable.is_a?(MergeRequest)
%hr
- - if @merge_request.new_record?
- .form-group
- = f.label :source_branch, class: 'control-label'
- .col-sm-10
- = f.select(:source_branch, [@merge_request.source_branch], { }, { class: 'source_branch select2 span2', disabled: true })
+ - if @merge_request.new_record?
+ .form-group
+ = f.label :source_branch, class: 'control-label'
+ .col-sm-10
+ = f.select(:source_branch, [@merge_request.source_branch], { }, { class: 'source_branch select2 span2', disabled: true })
.form-group
= f.label :target_branch, class: 'control-label'
.col-sm-10
diff --git a/db/migrate/20160317092222_add_moved_to_to_issue.rb b/db/migrate/20160317092222_add_moved_to_to_issue.rb
new file mode 100644
index 00000000000..461e7fb3a9b
--- /dev/null
+++ b/db/migrate/20160317092222_add_moved_to_to_issue.rb
@@ -0,0 +1,5 @@
+class AddMovedToToIssue < ActiveRecord::Migration
+ def change
+ add_reference :issues, :moved_to, references: :issues
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5b2f5aa3ddd..36b37d3944c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160316204731) do
+ActiveRecord::Schema.define(version: 20160317092222) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -416,6 +416,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do
t.string "state"
t.integer "iid"
t.integer "updated_by_id"
+ t.integer "moved_to_id"
t.boolean "confidential", default: false
end
diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb
index 3637b1bac94..132f0a4bd93 100644
--- a/lib/banzai/filter/reference_filter.rb
+++ b/lib/banzai/filter/reference_filter.rb
@@ -47,6 +47,7 @@ module Banzai
# Returns a String
def data_attribute(attributes = {})
attributes[:reference_filter] = self.class.name.demodulize
+ attributes.delete(:original) if context[:no_original_data]
attributes.map { |key, value| %Q(data-#{key.to_s.dasherize}="#{escape_once(value)}") }.join(" ")
end
diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb
new file mode 100644
index 00000000000..a1c6ee7bd69
--- /dev/null
+++ b/lib/gitlab/gfm/reference_rewriter.rb
@@ -0,0 +1,79 @@
+module Gitlab
+ module Gfm
+ ##
+ # 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, assuming that the
+ # argument passed to this method is a project that references will be
+ # viewed from (see `Referable#to_reference method).
+ #
+ # 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 ReferenceRewriter
+ def initialize(text, source_project, current_user)
+ @text = text
+ @source_project = source_project
+ @current_user = current_user
+ @original_html = markdown(text)
+ end
+
+ def rewrite(target_project)
+ pattern = Gitlab::ReferenceExtractor.references_pattern
+
+ @text.gsub(pattern) do |reference|
+ unfold_reference(reference, Regexp.last_match, target_project)
+ end
+ end
+
+ private
+
+ def unfold_reference(reference, match, target_project)
+ before = @text[0...match.begin(0)]
+ after = @text[match.end(0)..-1]
+
+ referable = find_referable(reference)
+ return reference unless referable
+
+ cross_reference = referable.to_reference(target_project)
+ return reference if reference == cross_reference
+
+ new_text = before + cross_reference + after
+ substitution_valid?(new_text) ? cross_reference : reference
+ end
+
+ def find_referable(reference)
+ extractor = Gitlab::ReferenceExtractor.new(@source_project,
+ @current_user)
+ extractor.analyze(reference)
+ extractor.all.first
+ end
+
+ def substitution_valid?(substituted)
+ @original_html == markdown(substituted)
+ end
+
+ def markdown(text)
+ Banzai.render(text, project: @source_project, no_original_data: true)
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb
index 4d830aa45e1..13c4d64c99b 100644
--- a/lib/gitlab/reference_extractor.rb
+++ b/lib/gitlab/reference_extractor.rb
@@ -1,6 +1,7 @@
module Gitlab
# Extract possible GFM references from an arbitrary String for further processing.
class ReferenceExtractor < Banzai::ReferenceExtractor
+ REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range)
attr_accessor :project, :current_user, :author
def initialize(project, current_user = nil, author = nil)
@@ -17,7 +18,7 @@ module Gitlab
super(text, context.merge(project: project))
end
- %i(user label milestone merge_request snippet commit commit_range).each do |type|
+ REFERABLES.each do |type|
define_method("#{type}s") do
@references[type] ||= references(type, reference_context)
end
@@ -31,6 +32,21 @@ module Gitlab
end
end
+ def all
+ REFERABLES.each { |referable| send(referable.to_s.pluralize) }
+ @references.values.flatten
+ end
+
+ def self.references_pattern
+ return @pattern if @pattern
+
+ patterns = REFERABLES.map do |ref|
+ ref.to_s.classify.constantize.try(:reference_pattern)
+ end
+
+ @pattern = Regexp.union(patterns.compact)
+ end
+
private
def reference_context
diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb
new file mode 100644
index 00000000000..6fda0c31866
--- /dev/null
+++ b/spec/features/issues/move_spec.rb
@@ -0,0 +1,87 @@
+require 'rails_helper'
+
+feature 'issue move to another project' do
+ let(:user) { create(:user) }
+ let(:old_project) { create(:project) }
+ let(:text) { 'Some issue description' }
+
+ let(:issue) do
+ create(:issue, description: text, project: old_project, author: user)
+ end
+
+ background { login_as(user) }
+
+ context 'user does not have permission to move issue' do
+ background do
+ old_project.team << [user, :guest]
+
+ edit_issue(issue)
+ end
+
+ scenario 'moving issue to another project not allowed' do
+ expect(page).to have_no_select('move_to_project_id')
+ end
+ end
+
+ context 'user has permission to move issue' do
+ let!(:mr) { create(:merge_request, source_project: old_project) }
+ let(:new_project) { create(:project) }
+ let(:text) { 'Text with !1' }
+ let(:cross_reference) { old_project.to_reference }
+
+ background do
+ old_project.team << [user, :reporter]
+ new_project.team << [user, :reporter]
+
+ edit_issue(issue)
+ end
+
+ scenario 'moving issue to another project' do
+ select(new_project.name_with_namespace, from: 'move_to_project_id')
+ click_button('Save changes')
+
+ expect(current_url).to include project_path(new_project)
+
+ page.within('.issue') do
+ expect(page).to have_content("Text with #{cross_reference}!1")
+ expect(page).to have_content("Moved from #{cross_reference}#1")
+ expect(page).to have_content(issue.title)
+ end
+ end
+
+ context 'projects user does not have permission to move issue to exist' do
+ let!(:private_project) { create(:project, :private) }
+ let(:another_project) { create(:project) }
+ background { another_project.team << [user, :guest] }
+
+ scenario 'browsing projects in projects select' do
+ options = [ '', 'No project', new_project.name_with_namespace ]
+ expect(page).to have_select('move_to_project_id', options: options)
+ end
+ end
+
+ context 'issue has been already moved' do
+ let(:new_issue) { create(:issue, project: new_project) }
+ let(:issue) do
+ create(:issue, project: old_project, author: user, moved_to: new_issue)
+ end
+
+ scenario 'user wants to move issue that has already been moved' do
+ expect(page).to have_no_select('move_to_project_id')
+ end
+ end
+ end
+
+ def edit_issue(issue)
+ visit issue_path(issue)
+ page.within('.issuable-header') { click_link 'Edit' }
+ end
+
+ def issue_path(issue)
+ namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+ end
+
+ def project_path(project)
+ namespace_project_path(new_project.namespace, new_project)
+ end
+end
diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
new file mode 100644
index 00000000000..0a7ca3ec848
--- /dev/null
+++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
@@ -0,0 +1,89 @@
+require 'spec_helper'
+
+describe Gitlab::Gfm::ReferenceRewriter do
+ let(:text) { 'some text' }
+ let(:old_project) { create(:project) }
+ let(:new_project) { create(:project) }
+ let(:user) { create(:user) }
+
+ before { old_project.team << [user, :guest] }
+
+ describe '#rewrite' do
+ subject do
+ described_class.new(text, old_project, user).rewrite(new_project)
+ end
+
+ context 'multiple issues and merge requests referenced' do
+ let!(:issue_first) { create(:issue, project: old_project) }
+ let!(:issue_second) { create(:issue, project: old_project) }
+ let!(:merge_request) { create(:merge_request, source_project: old_project) }
+
+ context 'plain text description' do
+ let(:text) { 'Description that references #1, #2 and !1' }
+
+ it { is_expected.to include issue_first.to_reference(new_project) }
+ it { is_expected.to include issue_second.to_reference(new_project) }
+ it { is_expected.to include merge_request.to_reference(new_project) }
+ end
+
+ context 'description with ignored elements' do
+ let(:text) do
+ "Hi. This references #1, but not `#2`\n" +
+ '<pre>and not !1</pre>'
+ end
+
+ it { is_expected.to include issue_first.to_reference(new_project) }
+ it { is_expected.to_not include issue_second.to_reference(new_project) }
+ it { is_expected.to_not include merge_request.to_reference(new_project) }
+ end
+
+ context 'description ambigous elements' do
+ context 'url' do
+ let(:url) { 'http://gitlab.com/#1' }
+ let(:text) { "This references #1, but not #{url}" }
+
+ it { is_expected.to include url }
+ end
+
+ context 'code' do
+ let(:text) { "#1, but not `[#1]`" }
+ it { is_expected.to eq "#{issue_first.to_reference(new_project)}, but not `[#1]`" }
+ end
+
+ context 'code reverse' do
+ let(:text) { "not `#1`, but #1" }
+ it { is_expected.to eq "not `#1`, but #{issue_first.to_reference(new_project)}" }
+ end
+
+ context 'code in random order' do
+ let(:text) { "#1, `#1`, #1, `#1`" }
+ let(:ref) { issue_first.to_reference(new_project) }
+
+ it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" }
+ end
+
+ context 'description with labels' do
+ let!(:label) { create(:label, id: 123, name: 'test', project: old_project) }
+ let(:project_ref) { old_project.to_reference }
+
+ context 'label referenced by id' do
+ let(:text) { '#1 and ~123' }
+ it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} }
+ end
+
+ context 'label referenced by text' do
+ let(:text) { '#1 and ~"test"' }
+ it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} }
+ end
+ end
+ end
+
+ context 'reference contains milestone' do
+ let(:milestone) { create(:milestone) }
+ let(:text) { "milestone ref: #{milestone.to_reference}" }
+
+ it { is_expected.to eq text }
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 65af37e24f1..7c617723e6d 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -124,4 +124,24 @@ describe Gitlab::ReferenceExtractor, lib: true do
expect(extracted).to match_array([issue])
end
end
+
+ describe '#all' do
+ let(:issue) { create(:issue, project: project) }
+ let(:label) { create(:label, project: project) }
+ let(:text) { "Ref. #{issue.to_reference} and #{label.to_reference}" }
+
+ before do
+ project.team << [project.creator, :developer]
+ subject.analyze(text)
+ end
+
+ it 'returns all referables' do
+ expect(subject.all).to match_array([issue, label])
+ end
+ end
+
+ describe '.references_pattern' do
+ subject { described_class.references_pattern }
+ it { is_expected.to be_kind_of Regexp }
+ end
end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index f910424d85b..9b47acfe0cd 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -158,6 +158,33 @@ describe Notify do
is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
end
end
+
+ describe 'moved to another project' do
+ let(:new_issue) { create(:issue) }
+ subject { Notify.issue_moved_email(recipient, issue, new_issue, current_user) }
+
+ it_behaves_like 'an answer to an existing thread', 'issue'
+ it_behaves_like 'it should show Gmail Actions View Issue link'
+ it_behaves_like 'an unsubscribeable thread'
+
+ it 'contains description about action taken' do
+ is_expected.to have_body_text 'Issue was moved to another project'
+ end
+
+ it 'has the correct subject' do
+ is_expected.to have_subject /#{issue.title} \(##{issue.iid}\)/i
+ end
+
+ it 'contains link to new issue' do
+ new_issue_url = namespace_project_issue_path(new_issue.project.namespace,
+ new_issue.project, new_issue)
+ is_expected.to have_body_text new_issue_url
+ end
+
+ it 'contains a link to the original issue' do
+ is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
+ end
+ end
end
context 'for merge requests' do
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 540a62eb1f8..05e4233243d 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -130,6 +130,56 @@ describe Issue, models: true do
end
end
+ describe '#can_move?' do
+ let(:user) { create(:user) }
+ let(:issue) { create(:issue) }
+ subject { issue.can_move?(user) }
+
+ context 'user is not a member of project issue belongs to' do
+ it { is_expected.to eq false}
+ end
+
+ context 'user is reporter in project issue belongs to' do
+ let(:project) { create(:project) }
+ let(:issue) { create(:issue, project: project) }
+
+ before { project.team << [user, :reporter] }
+
+ it { is_expected.to eq true }
+
+ context 'checking destination project also' do
+ subject { issue.can_move?(user, to_project) }
+ let(:to_project) { create(:project) }
+
+ context 'destination project allowed' do
+ before { to_project.team << [user, :reporter] }
+ it { is_expected.to eq true }
+ end
+
+ context 'destination project not allowed' do
+ before { to_project.team << [user, :guest] }
+ it { is_expected.to eq false }
+ end
+ end
+ end
+ end
+
+ describe '#moved?' do
+ let(:issue) { create(:issue) }
+ subject { issue.moved? }
+
+ context 'issue not moved' do
+ it { is_expected.to eq false }
+ end
+
+ context 'issue already moved' do
+ let(:moved_to_issue) { create(:issue) }
+ let(:issue) { create(:issue, moved_to: moved_to_issue) }
+
+ it { is_expected.to eq true }
+ end
+ end
+
describe '#related_branches' do
it "selects the right branches" do
allow(subject.project.repository).to receive(:branch_names).
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
new file mode 100644
index 00000000000..14cc20e529a
--- /dev/null
+++ b/spec/services/issues/move_service_spec.rb
@@ -0,0 +1,213 @@
+require 'spec_helper'
+
+describe Issues::MoveService, services: true do
+ let(:user) { create(:user) }
+ let(:author) { create(:user) }
+ let(:title) { 'Some issue' }
+ let(:description) { 'Some issue description' }
+ let(:old_project) { create(:project) }
+ let(:new_project) { create(:project) }
+
+ let(:old_issue) do
+ create(:issue, title: title, description: description,
+ project: old_project, author: author)
+ end
+
+ let(:move_service) do
+ described_class.new(old_project, user)
+ end
+
+ shared_context 'user can move issue' do
+ before do
+ old_project.team << [user, :reporter]
+ new_project.team << [user, :reporter]
+ end
+ end
+
+ describe '#execute' do
+ shared_context 'issue move executed' do
+ let!(:new_issue) { move_service.execute(old_issue, new_project) }
+ end
+
+ context 'issue movable' do
+ include_context 'user can move issue'
+
+ context 'generic issue' do
+ include_context 'issue move executed'
+
+ it 'creates a new issue in a new project' do
+ expect(new_issue.project).to eq new_project
+ end
+
+ it 'rewrites issue title' do
+ expect(new_issue.title).to eq title
+ end
+
+ it 'rewrites issue description' do
+ expect(new_issue.description).to eq description
+ end
+
+ it 'adds system note to old issue at the end' do
+ expect(old_issue.notes.last.note).to match /^Moved to/
+ end
+
+ it 'adds system note to new issue at the end' do
+ expect(new_issue.notes.last.note).to match /^Moved from/
+ end
+
+ it 'closes old issue' do
+ expect(old_issue.closed?).to be true
+ end
+
+ it 'persists new issue' do
+ expect(new_issue.persisted?).to be true
+ end
+
+ it 'persists all changes' do
+ expect(old_issue.changed?).to be false
+ expect(new_issue.changed?).to be false
+ end
+
+ it 'preserves author' do
+ expect(new_issue.author).to eq author
+ end
+
+ it 'removes data that is invalid in new context' do
+ expect(new_issue.milestone).to be_nil
+ expect(new_issue.labels).to be_empty
+ end
+
+ it 'creates a new internal id for issue' do
+ expect(new_issue.iid).to be 1
+ end
+
+ it 'marks issue as moved' do
+ expect(old_issue.moved?).to eq true
+ expect(old_issue.moved_to).to eq new_issue
+ end
+ end
+
+ context 'issue with notes' do
+ context 'notes without references' do
+ let(:notes_params) do
+ [{ system: false, note: 'Some comment 1' },
+ { system: true, note: 'Some system note' },
+ { system: false, note: 'Some comment 2' }]
+ end
+
+ let(:notes_contents) { notes_params.map { |n| n[:note] } }
+
+ before do
+ note_params = { noteable: old_issue, project: old_project, author: author }
+ notes_params.each do |note|
+ create(:note, note_params.merge(note))
+ end
+ end
+
+ include_context 'issue move executed'
+
+ let(:all_notes) { new_issue.notes.order('id ASC') }
+ let(:system_notes) { all_notes.system }
+ let(:user_notes) { all_notes.user }
+
+ it 'rewrites existing notes in valid order' do
+ expect(all_notes.pluck(:note).first(3)).to eq notes_contents
+ end
+
+ it 'adds a system note about move after rewritten notes' do
+ expect(system_notes.last.note).to match /^Moved from/
+ end
+
+ it 'preserves orignal author of comment' do
+ expect(user_notes.pluck(:author_id)).to all(eq(author.id))
+ end
+
+ it 'preserves time when note has been created at' do
+ expect(old_issue.notes.first.created_at)
+ .to eq new_issue.notes.first.created_at
+ end
+ end
+
+ context 'notes with references' do
+ before do
+ create(:merge_request, source_project: old_project)
+ create(:note, noteable: old_issue, project: old_project, author: author,
+ note: 'Note with reference to merge request !1')
+ end
+
+ include_context 'issue move executed'
+ let(:new_note) { new_issue.notes.first }
+
+ it 'rewrites references using a cross reference to old project' do
+ expect(new_note.note)
+ .to eq "Note with reference to merge request #{old_project.to_reference}!1"
+ end
+ end
+ end
+
+ describe 'rewritting references' do
+ include_context 'issue move executed'
+
+ context 'issue reference' do
+ let(:another_issue) { create(:issue, project: old_project) }
+ let(:description) { "Some description #{another_issue.to_reference}" }
+
+ it 'rewrites referenced issues creating cross project reference' do
+ expect(new_issue.description)
+ .to eq "Some description #{old_project.to_reference}#{another_issue.to_reference}"
+ end
+ end
+ end
+
+ context 'moving to same project' do
+ let(:new_project) { old_project }
+
+ it 'raises error' do
+ expect { move_service.execute(old_issue, new_project) }
+ .to raise_error(StandardError, /Cannot move issue/)
+ end
+ end
+ end
+
+ describe 'move permissions' do
+ let(:move) { move_service.execute(old_issue, new_project) }
+
+ context 'user is reporter in both projects' do
+ include_context 'user can move issue'
+ it { expect { move }.to_not raise_error }
+ end
+
+ context 'user is reporter only in new project' do
+ before { new_project.team << [user, :reporter] }
+ it { expect { move }.to raise_error(StandardError, /permissions/) }
+ end
+
+ context 'user is reporter only in old project' do
+ before { old_project.team << [user, :reporter] }
+ it { expect { move }.to raise_error(StandardError, /permissions/) }
+ end
+
+ context 'user is reporter in one project and guest in another' do
+ before do
+ new_project.team << [user, :guest]
+ old_project.team << [user, :reporter]
+ end
+
+ it { expect { move }.to raise_error(StandardError, /permissions/) }
+ end
+
+ context 'issue has already been moved' do
+ include_context 'user can move issue'
+
+ let(:moved_to_issue) { create(:issue) }
+
+ let(:old_issue) do
+ create(:issue, project: old_project, author: author,
+ moved_to: moved_to_issue)
+ end
+
+ it { expect { move }.to raise_error(StandardError, /permissions/) }
+ end
+ end
+ end
+end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 8e6292014d4..240eae10052 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -453,6 +453,59 @@ describe SystemNoteService, services: true do
end
end
+ describe '.noteable_moved' do
+ let(:new_project) { create(:project) }
+ let(:new_noteable) { create(:issue, project: new_project) }
+
+ subject do
+ described_class.noteable_moved(noteable, project, new_noteable, author, direction: direction)
+ end
+
+ shared_examples 'cross project mentionable' do
+ include GitlabMarkdownHelper
+
+ it 'should contain cross reference to new noteable' do
+ expect(subject.note).to include cross_project_reference(new_project, new_noteable)
+ end
+
+ it 'should mention referenced noteable' do
+ expect(subject.note).to include new_noteable.to_reference
+ end
+
+ it 'should mention referenced project' do
+ expect(subject.note).to include new_project.to_reference
+ end
+ end
+
+ context 'moved to' do
+ let(:direction) { :to }
+
+ it_behaves_like 'cross project mentionable'
+
+ it 'should notify about noteable being moved to' do
+ expect(subject.note).to match /Moved to/
+ end
+ end
+
+ context 'moved from' do
+ let(:direction) { :from }
+
+ it_behaves_like 'cross project mentionable'
+
+ it 'should notify about noteable being moved from' do
+ expect(subject.note).to match /Moved from/
+ end
+ end
+
+ context 'invalid direction' do
+ let(:direction) { :invalid }
+
+ it 'should raise error' do
+ expect { subject }.to raise_error StandardError, /Invalid direction/
+ end
+ end
+ end
+
include JiraServiceHelper
describe 'JIRA integration' do