Commit 3fca30d2 authored by Kamil Trzciński's avatar Kamil Trzciński

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
parents bfcdb47c db8f70d5
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.6.0 (unreleased) 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) - 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) - Make HTTP(s) label consistent on clone bar (Stan Hu)
- Add confidential issues - Add confidential issues
......
class @IssuableForm 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 wipRegex: /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i
constructor: (@form) -> constructor: (@form) ->
GitLab.GfmAutoComplete.setup() GitLab.GfmAutoComplete.setup()
new UsersSelect() new UsersSelect()
...@@ -7,12 +9,13 @@ class @IssuableForm ...@@ -7,12 +9,13 @@ class @IssuableForm
@titleField = @form.find("input[name*='[title]']") @titleField = @form.find("input[name*='[title]']")
@descriptionField = @form.find("textarea[name*='[description]']") @descriptionField = @form.find("textarea[name*='[description]']")
@issueMoveField = @form.find("#move_to_project_id")
return unless @titleField.length && @descriptionField.length return unless @titleField.length && @descriptionField.length
@initAutosave() @initAutosave()
@form.on "submit", @resetAutosave @form.on "submit", @handleSubmit
@form.on "click", ".btn-cancel", @resetAutosave @form.on "click", ".btn-cancel", @resetAutosave
@initWip() @initWip()
...@@ -30,6 +33,12 @@ class @IssuableForm ...@@ -30,6 +33,12 @@ class @IssuableForm
"description" "description"
] ]
handleSubmit: =>
if (parseInt(@issueMoveField?.val()) ? 0) > 0
return false unless confirm(@issueMoveConfirmMsg)
@resetAutosave()
resetAutosave: => resetAutosave: =>
@titleField.data("autosave").reset() @titleField.data("autosave").reset()
@descriptionField.data("autosave").reset() @descriptionField.data("autosave").reset()
......
...@@ -90,6 +90,12 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -90,6 +90,12 @@ class Projects::IssuesController < Projects::ApplicationController
def update def update
@issue = Issues::UpdateService.new(project, current_user, issue_params).execute(issue) @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| respond_to do |format|
format.js format.js
format.html do format.html do
......
...@@ -57,6 +57,19 @@ module IssuesHelper ...@@ -57,6 +57,19 @@ module IssuesHelper
options_from_collection_for_select(milestones, 'id', 'title', object.milestone_id) options_from_collection_for_select(milestones, 'id', 'title', object.milestone_id)
end 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) def status_box_class(item)
if item.respond_to?(:expired?) && item.expired? if item.respond_to?(:expired?) && item.expired?
'status-box-expired' 'status-box-expired'
......
...@@ -36,6 +36,14 @@ module Emails ...@@ -36,6 +36,14 @@ module Emails
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
end 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 private
def setup_issue_mail(issue_id, recipient_id) def setup_issue_mail(issue_id, recipient_id)
......
...@@ -209,4 +209,13 @@ module Issuable ...@@ -209,4 +209,13 @@ module Issuable
Taskable.get_updated_tasks(old_content: previous_changes['description'].first, Taskable.get_updated_tasks(old_content: previous_changes['description'].first,
new_content: description) new_content: description)
end 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 end
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
# state :string(255) # state :string(255)
# iid :integer # iid :integer
# updated_by_id :integer # updated_by_id :integer
# moved_to_id :integer
# #
require 'carrierwave/orm/activerecord' require 'carrierwave/orm/activerecord'
...@@ -31,6 +32,8 @@ class Issue < ActiveRecord::Base ...@@ -31,6 +32,8 @@ class Issue < ActiveRecord::Base
ActsAsTaggableOn.strict_case_match = true ActsAsTaggableOn.strict_case_match = true
belongs_to :project belongs_to :project
belongs_to :moved_to, class_name: 'Issue'
validates :project, presence: true validates :project, presence: true
scope :of_group, scope :of_group,
...@@ -137,6 +140,18 @@ class Issue < ActiveRecord::Base ...@@ -137,6 +140,18 @@ class Issue < ActiveRecord::Base
end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) } end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
end 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 def to_branch_name
"#{title.parameterize}-#{iid}" "#{title.parameterize}-#{iid}"
end end
......
...@@ -435,7 +435,7 @@ class User < ActiveRecord::Base ...@@ -435,7 +435,7 @@ class User < ActiveRecord::Base
Group.where("namespaces.id IN (#{union.to_sql})") Group.where("namespaces.id IN (#{union.to_sql})")
end end
# Returns the groups a user is authorized to access. # Returns projects user is authorized to access.
def authorized_projects def authorized_projects
Project.where("projects.id IN (#{projects_union.to_sql})") Project.where("projects.id IN (#{projects_union.to_sql})")
end end
......
...@@ -120,7 +120,7 @@ class GitPushService < BaseService ...@@ -120,7 +120,7 @@ class GitPushService < BaseService
closed_issues = commit.closes_issues(current_user) closed_issues = commit.closes_issues(current_user)
closed_issues.each do |issue| closed_issues.each do |issue|
if can?(current_user, :update_issue, 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 end
end end
......
module Issues module Issues
class CloseService < Issues::BaseService 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 if project.jira_tracker? && project.jira_service.active
project.jira_service.execute(commit, issue) project.jira_service.execute(commit, issue)
todo_service.close_issue(issue, current_user) todo_service.close_issue(issue, current_user)
...@@ -9,8 +9,8 @@ module Issues ...@@ -9,8 +9,8 @@ module Issues
if project.default_issues_tracker? && issue.close if project.default_issues_tracker? && issue.close
event_service.close_issue(issue, current_user) event_service.close_issue(issue, current_user)
create_note(issue, commit) create_note(issue, commit) if system_note
notification_service.close_issue(issue, current_user) notification_service.close_issue(issue, current_user) if notifications
todo_service.close_issue(issue, current_user) todo_service.close_issue(issue, current_user)
execute_hooks(issue, 'close') execute_hooks(issue, 'close')
end end
......
...@@ -4,7 +4,7 @@ module Issues ...@@ -4,7 +4,7 @@ module Issues
filter_params filter_params
label_params = params[:label_ids] label_params = params[:label_ids]
issue = project.issues.new(params.except(:label_ids)) issue = project.issues.new(params.except(:label_ids))
issue.author = current_user issue.author = params[:author] || current_user
if issue.save if issue.save
issue.update_attributes(label_ids: label_params) issue.update_attributes(label_ids: label_params)
......
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
...@@ -22,7 +22,7 @@ module MergeRequests ...@@ -22,7 +22,7 @@ module MergeRequests
closed_issues = merge_request.closes_issues(current_user) closed_issues = merge_request.closes_issues(current_user)
closed_issues.each do |issue| closed_issues.each do |issue|
if can?(current_user, :update_issue, 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 end
end end
......
...@@ -236,6 +236,16 @@ class NotificationService ...@@ -236,6 +236,16 @@ class NotificationService
end end
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 protected
# Get project users with WATCH notification level # Get project users with WATCH notification level
......
...@@ -411,4 +411,26 @@ class SystemNoteService ...@@ -411,4 +411,26 @@ class SystemNoteService
body = "Marked the task **#{new_task.source}** as #{status_label}" body = "Marked the task **#{new_task.source}** as #{status_label}"
create_note(noteable: noteable, project: project, author: author, note: body) create_note(noteable: noteable, project: project, author: author, note: body)
end 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 end
%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
Issue was moved to another project.
New issue location:
<%= namespace_project_issue_url(@new_project.namespace, @new_project, @new_issue) %>
...@@ -85,6 +85,19 @@ ...@@ -85,6 +85,19 @@
- if can? current_user, :admin_label, issuable.project - 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 = 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) - if issuable.is_a?(MergeRequest)
%hr %hr
- if @merge_request.new_record? - if @merge_request.new_record?
......
class AddMovedToToIssue < ActiveRecord::Migration
def change
add_reference :issues, :moved_to, references: :issues
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -416,6 +416,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do ...@@ -416,6 +416,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do
t.string "state" t.string "state"
t.integer "iid" t.integer "iid"
t.integer "updated_by_id" t.integer "updated_by_id"
t.integer "moved_to_id"
t.boolean "confidential", default: false t.boolean "confidential", default: false
end end
......
...@@ -47,6 +47,7 @@ module Banzai ...@@ -47,6 +47,7 @@ module Banzai
# Returns a String # Returns a String
def data_attribute(attributes = {}) def data_attribute(attributes = {})
attributes[:reference_filter] = self.class.name.demodulize 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(" ") attributes.map { |key, value| %Q(data-#{key.to_s.dasherize}="#{escape_once(value)}") }.join(" ")
end end
......
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
module Gitlab module Gitlab
# Extract possible GFM references from an arbitrary String for further processing. # Extract possible GFM references from an arbitrary String for further processing.
class ReferenceExtractor < Banzai::ReferenceExtractor class ReferenceExtractor < Banzai::ReferenceExtractor
REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range)
attr_accessor :project, :current_user, :author attr_accessor :project, :current_user, :author
def initialize(project, current_user = nil, author = nil) def initialize(project, current_user = nil, author = nil)
...@@ -17,7 +18,7 @@ module Gitlab ...@@ -17,7 +18,7 @@ module Gitlab
super(text, context.merge(project: project)) super(text, context.merge(project: project))
end end
%i(user label milestone merge_request snippet commit commit_range).each do |type| REFERABLES.each do |type|
define_method("#{type}s") do define_method("#{type}s") do
@references[type] ||= references(type, reference_context) @references[type] ||= references(type, reference_context)
end end
...@@ -31,6 +32,21 @@ module Gitlab ...@@ -31,6 +32,21 @@ module Gitlab
end end
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 private
def reference_context def reference_context
......
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
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
...@@ -124,4 +124,24 @@ describe Gitlab::ReferenceExtractor, lib: true do ...@@ -124,4 +124,24 @@ describe Gitlab::ReferenceExtractor, lib: true do
expect(extracted).to match_array([issue]) expect(extracted).to match_array([issue])
end end
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 end
...@@ -158,6 +158,33 @@ describe Notify do ...@@ -158,6 +158,33 @@ describe Notify do
is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/ is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
end end
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 end
context 'for merge requests' do context 'for merge requests' do
......
...@@ -130,6 +130,56 @@ describe Issue, models: true do ...@@ -130,6 +130,56 @@ describe Issue, models: true do
end end
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 describe '#related_branches' do
it "selects the right branches" do it "selects the right branches" do
allow(subject.project.repository).to receive(:branch_names). allow(subject.project.repository).to receive(:branch_names).
......
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
...@@ -453,6 +453,59 @@ describe SystemNoteService, services: true do ...@@ -453,6 +453,59 @@ describe SystemNoteService, services: true do
end end
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 include JiraServiceHelper
describe 'JIRA integration' do describe 'JIRA integration' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment