diff options
| -rw-r--r-- | app/services/issuable/clone/attributes_rewriter.rb | 62 | ||||
| -rw-r--r-- | app/services/issuable/clone/base_service.rb | 60 | ||||
| -rw-r--r-- | app/services/issuable/clone/content_rewriter.rb | 65 | ||||
| -rw-r--r-- | app/services/issues/move_service.rb | 157 | ||||
| -rw-r--r-- | app/uploaders/file_uploader.rb | 6 | ||||
| -rw-r--r-- | lib/gitlab/gfm/reference_rewriter.rb | 22 | ||||
| -rw-r--r-- | lib/gitlab/gfm/uploads_rewriter.rb | 5 | ||||
| -rw-r--r-- | lib/gitlab/quick_actions/command_definition.rb | 16 | ||||
| -rw-r--r-- | lib/gitlab/quick_actions/dsl.rb | 6 | ||||
| -rw-r--r-- | spec/lib/gitlab/quick_actions/command_definition_spec.rb | 13 | ||||
| -rw-r--r-- | spec/lib/gitlab/quick_actions/dsl_spec.rb | 8 | ||||
| -rw-r--r-- | spec/services/issuable/clone/attributes_rewriter_spec.rb | 79 | ||||
| -rw-r--r-- | spec/services/issuable/clone/content_rewriter_spec.rb | 153 | ||||
| -rw-r--r-- | spec/services/issues/move_service_spec.rb | 268 | ||||
| -rw-r--r-- | spec/uploaders/file_uploader_spec.rb | 27 | ||||
| -rw-r--r-- | spec/uploaders/namespace_file_uploader_spec.rb | 58 |
16 files changed, 594 insertions, 411 deletions
diff --git a/app/services/issuable/clone/attributes_rewriter.rb b/app/services/issuable/clone/attributes_rewriter.rb new file mode 100644 index 00000000000..0300cc0d8d3 --- /dev/null +++ b/app/services/issuable/clone/attributes_rewriter.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Issuable + module Clone + class AttributesRewriter < ::Issuable::Clone::BaseService + def initialize(current_user, original_entity, new_entity) + @current_user = current_user + @original_entity = original_entity + @new_entity = new_entity + end + + def execute + new_entity.update(milestone: cloneable_milestone, labels: cloneable_labels) + copy_resource_label_events + end + + private + + def cloneable_milestone + title = original_entity.milestone&.title + return unless title + + params = { title: title, project_ids: new_entity.project&.id, group_ids: group&.id } + + milestones = MilestonesFinder.new(params).execute + milestones.first + end + + def cloneable_labels + params = { + project_id: new_entity.project&.id, + group_id: group&.id, + title: original_entity.labels.select(:title), + include_ancestor_groups: true + } + + params[:only_group_labels] = true if new_parent.is_a?(Group) + + LabelsFinder.new(current_user, params).execute + end + + def copy_resource_label_events + original_entity.resource_label_events.find_in_batches do |batch| + events = batch.map do |event| + entity_key = new_entity.is_a?(Issue) ? 'issue_id' : 'epic_id' + # rubocop: disable CodeReuse/ActiveRecord + event.attributes + .except('id', 'reference', 'reference_html') + .merge(entity_key => new_entity.id, 'action' => ResourceLabelEvent.actions[event.action]) + # rubocop: enable CodeReuse/ActiveRecord + end + + Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events) + end + end + + def entity_key + new_entity.class.name.parameterize('_').foreign_key + end + end + end +end diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb new file mode 100644 index 00000000000..42dd9c666f5 --- /dev/null +++ b/app/services/issuable/clone/base_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Issuable + module Clone + class BaseService < IssuableBaseService + attr_reader :original_entity, :new_entity + + alias_method :old_project, :project + + def execute(original_entity, new_project = nil) + @original_entity = original_entity + + # Using transaction because of a high resources footprint + # on rewriting notes (unfolding references) + # + ActiveRecord::Base.transaction do + @new_entity = create_new_entity + + update_new_entity + update_old_entity + create_notes + end + end + + private + + def update_new_entity + rewriters = [ContentRewriter, AttributesRewriter] + + rewriters.each do |rewriter| + rewriter.new(current_user, original_entity, new_entity).execute + end + end + + def update_old_entity + close_issue + end + + def create_notes + add_note_from + add_note_to + end + + def close_issue + close_service = Issues::CloseService.new(old_project, current_user) + close_service.execute(original_entity, notifications: false, system_note: false) + end + + def new_parent + new_entity.project ? new_entity.project : new_entity.group + end + + def group + if new_entity.project&.group && current_user.can?(:read_group, new_entity.project.group) + new_entity.project.group + end + end + end + end +end diff --git a/app/services/issuable/clone/content_rewriter.rb b/app/services/issuable/clone/content_rewriter.rb new file mode 100644 index 00000000000..e1e0b75085d --- /dev/null +++ b/app/services/issuable/clone/content_rewriter.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module Issuable + module Clone + class ContentRewriter < ::Issuable::Clone::BaseService + def initialize(current_user, original_entity, new_entity) + @current_user = current_user + @original_entity = original_entity + @new_entity = new_entity + @project = original_entity.project + end + + def execute + rewrite_description + rewrite_award_emoji(original_entity, new_entity) + rewrite_notes + end + + private + + def rewrite_description + new_entity.update(description: rewrite_content(original_entity.description)) + end + + def rewrite_notes + original_entity.notes_with_associations.find_each do |note| + new_note = note.dup + new_params = { + project: new_entity.project, noteable: new_entity, + note: rewrite_content(new_note.note), + created_at: note.created_at, + updated_at: note.updated_at + } + + if note.system_note_metadata + new_params[:system_note_metadata] = note.system_note_metadata.dup + end + + new_note.update(new_params) + + rewrite_award_emoji(note, new_note) + end + end + + def rewrite_content(content) + return unless content + + rewriters = [Gitlab::Gfm::ReferenceRewriter, Gitlab::Gfm::UploadsRewriter] + + rewriters.inject(content) do |text, klass| + rewriter = klass.new(text, old_project, current_user) + rewriter.rewrite(new_parent) + end + end + + def rewrite_award_emoji(old_awardable, new_awardable) + old_awardable.award_emoji.each do |award| + new_award = award.dup + new_award.awardable = new_awardable + new_award.save + end + end + end + end +end diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index d2bdba1e627..41b6a96b005 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -1,165 +1,66 @@ # frozen_string_literal: true module Issues - class MoveService < Issues::BaseService + class MoveService < Issuable::Clone::BaseService MoveError = Class.new(StandardError) - def execute(issue, new_project) - @old_issue = issue - @old_project = @project - @new_project = new_project + def execute(issue, target_project) + @target_project = target_project - unless issue.can_move?(current_user, new_project) + unless issue.can_move?(current_user, @target_project) raise MoveError, 'Cannot move issue due to insufficient permissions!' end - if @project == new_project + if @project == @target_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 = create_new_issue - - update_new_issue - update_old_issue - end + super notify_participants - @new_issue + new_entity end private - def update_new_issue - rewrite_notes - copy_resource_label_events - rewrite_issue_award_emoji - add_note_moved_from - end + def update_old_entity + super - def update_old_issue - add_note_moved_to - close_issue mark_as_moved end - def create_new_issue - new_params = { id: nil, iid: nil, label_ids: cloneable_label_ids, - milestone_id: cloneable_milestone_id, - project: @new_project, author: @old_issue.author, - description: rewrite_content(@old_issue.description), - assignee_ids: @old_issue.assignee_ids } - - new_params = @old_issue.serializable_hash.symbolize_keys.merge(new_params) - CreateService.new(@new_project, @current_user, new_params).execute - end - - # rubocop: disable CodeReuse/ActiveRecord - def cloneable_label_ids - params = { - project_id: @new_project.id, - title: @old_issue.labels.pluck(:title), - include_ancestor_groups: true - } + def create_new_entity + new_params = { + id: nil, + iid: nil, + project: @target_project, + author: original_entity.author, + assignee_ids: original_entity.assignee_ids + } - LabelsFinder.new(current_user, params).execute.pluck(:id) + new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params) + CreateService.new(@target_project, @current_user, new_params).execute end - # rubocop: enable CodeReuse/ActiveRecord - - def cloneable_milestone_id - title = @old_issue.milestone&.title - return unless title - - if @new_project.group && can?(current_user, :read_group, @new_project.group) - group_id = @new_project.group.id - end - - params = - { title: title, project_ids: @new_project.id, group_ids: group_id } - milestones = MilestonesFinder.new(params).execute - milestones.first&.id - end - - def rewrite_notes - @old_issue.notes_with_associations.find_each do |note| - new_note = note.dup - new_params = { project: @new_project, noteable: @new_issue, - note: rewrite_content(new_note.note), - created_at: note.created_at, - updated_at: note.updated_at } - - new_note.update(new_params) - - rewrite_award_emoji(note, new_note) - end - end - - # rubocop: disable CodeReuse/ActiveRecord - def copy_resource_label_events - @old_issue.resource_label_events.find_in_batches do |batch| - events = batch.map do |event| - event.attributes - .except('id', 'reference', 'reference_html') - .merge('issue_id' => @new_issue.id, 'action' => ResourceLabelEvent.actions[event.action]) - end - - Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events) - end - end - # rubocop: enable CodeReuse/ActiveRecord - - def rewrite_issue_award_emoji - rewrite_award_emoji(@old_issue, @new_issue) - end - - def rewrite_award_emoji(old_awardable, new_awardable) - old_awardable.award_emoji.each do |award| - new_award = award.dup - new_award.awardable = new_awardable - new_award.save - end - end - - def rewrite_content(content) - return unless content - - rewriters = [Gitlab::Gfm::ReferenceRewriter, - Gitlab::Gfm::UploadsRewriter] - - rewriters.inject(content) do |text, klass| - rewriter = klass.new(text, @old_project, @current_user) - rewriter.rewrite(@new_project) - end + def mark_as_moved + original_entity.update(moved_to: new_entity) end - def close_issue - close_service = CloseService.new(@old_project, @current_user) - close_service.execute(@old_issue, notifications: false, system_note: false) + def notify_participants + notification_service.async.issue_moved(original_entity, new_entity, @current_user) end - def add_note_moved_from - SystemNoteService.noteable_moved(@new_issue, @new_project, - @old_issue, @current_user, + def add_note_from + SystemNoteService.noteable_moved(new_entity, @target_project, + original_entity, current_user, direction: :from) end - def add_note_moved_to - SystemNoteService.noteable_moved(@old_issue, @old_project, - @new_issue, @current_user, + def add_note_to + SystemNoteService.noteable_moved(original_entity, old_project, + new_entity, current_user, direction: :to) end - - def mark_as_moved - @old_issue.update(moved_to: @new_issue) - end - - def notify_participants - notification_service.async.issue_moved(@old_issue, @new_issue, @current_user) - end end end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index ffc1e5f75ca..e90599f2505 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -149,9 +149,9 @@ class FileUploader < GitlabUploader # return a new uploader with a file copy on another project def self.copy_to(uploader, to_project) - moved = uploader.dup.tap do |u| - u.model = to_project - end + moved = self.new(to_project) + moved.object_store = uploader.object_store + moved.filename = uploader.filename moved.copy_file(uploader.file) moved diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb index 455814a9159..641446b52a5 100644 --- a/lib/gitlab/gfm/reference_rewriter.rb +++ b/lib/gitlab/gfm/reference_rewriter.rb @@ -31,19 +31,19 @@ module Gitlab class ReferenceRewriter RewriteError = Class.new(StandardError) - def initialize(text, source_project, current_user) + def initialize(text, source_parent, current_user) @text = text - @source_project = source_project + @source_parent = source_parent @current_user = current_user @original_html = markdown(text) @pattern = Gitlab::ReferenceExtractor.references_pattern end - def rewrite(target_project) + def rewrite(target_parent) return @text unless needs_rewrite? @text.gsub(@pattern) do |reference| - unfold_reference(reference, Regexp.last_match, target_project) + unfold_reference(reference, Regexp.last_match, target_parent) end end @@ -53,14 +53,14 @@ module Gitlab private - def unfold_reference(reference, match, target_project) + def unfold_reference(reference, match, target_parent) before = @text[0...match.begin(0)] after = @text[match.end(0)..-1] referable = find_referable(reference) return reference unless referable - cross_reference = build_cross_reference(referable, target_project) + cross_reference = build_cross_reference(referable, target_parent) return reference if reference == cross_reference if cross_reference.nil? @@ -72,17 +72,17 @@ module Gitlab end def find_referable(reference) - extractor = Gitlab::ReferenceExtractor.new(@source_project, + extractor = Gitlab::ReferenceExtractor.new(@source_parent, @current_user) extractor.analyze(reference) extractor.all.first end - def build_cross_reference(referable, target_project) + def build_cross_reference(referable, target_parent) if referable.respond_to?(:project) - referable.to_reference(target_project) + referable.to_reference(target_parent) else - referable.to_reference(@source_project, target_project: target_project) + referable.to_reference(@source_parent, target_project: target_parent) end end @@ -91,7 +91,7 @@ module Gitlab end def markdown(text) - Banzai.render(text, project: @source_project, no_original_data: true) + Banzai.render(text, project: @source_parent, no_original_data: true) end end end diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index f7e66697da3..b767c8a278d 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -16,14 +16,15 @@ module Gitlab @pattern = FileUploader::MARKDOWN_PATTERN end - def rewrite(target_project) + def rewrite(target_parent) return @text unless needs_rewrite? @text.gsub(@pattern) do |markdown| file = find_file(@source_project, $~[:secret], $~[:file]) break markdown unless file.try(:exists?) - moved = FileUploader.copy_to(file, target_project) + klass = target_parent.is_a?(Namespace) ? NamespaceFileUploader : FileUploader + moved = klass.copy_to(file, target_parent) moved.markdown_link end end diff --git a/lib/gitlab/quick_actions/command_definition.rb b/lib/gitlab/quick_actions/command_definition.rb index 96415271316..c682eb22890 100644 --- a/lib/gitlab/quick_actions/command_definition.rb +++ b/lib/gitlab/quick_actions/command_definition.rb @@ -2,13 +2,14 @@ module Gitlab module QuickActions class CommandDefinition attr_accessor :name, :aliases, :description, :explanation, :params, - :condition_block, :parse_params_block, :action_block + :condition_block, :parse_params_block, :action_block, :warning def initialize(name, attributes = {}) @name = name @aliases = attributes[:aliases] || [] @description = attributes[:description] || '' + @warning = attributes[:warning] || '' @explanation = attributes[:explanation] || '' @params = attributes[:params] || [] @condition_block = attributes[:condition_block] @@ -33,11 +34,13 @@ module Gitlab def explain(context, arg) return unless available?(context) - if explanation.respond_to?(:call) - execute_block(explanation, context, arg) - else - explanation - end + message = if explanation.respond_to?(:call) + execute_block(explanation, context, arg) + else + explanation + end + + warning.empty? ? message : "#{message} (#{warning})" end def execute(context, arg) @@ -61,6 +64,7 @@ module Gitlab name: name, aliases: aliases, description: desc, + warning: warning, params: prms } end diff --git a/lib/gitlab/quick_actions/dsl.rb b/lib/gitlab/quick_actions/dsl.rb index d82dccd0db5..192c7ec2ff5 100644 --- a/lib/gitlab/quick_actions/dsl.rb +++ b/lib/gitlab/quick_actions/dsl.rb @@ -31,6 +31,10 @@ module Gitlab @description = block_given? ? block : text end + def warning(message = '') + @warning = message + end + # Allows to define params for the next quick action. # These params are shown in the autocomplete menu. # @@ -133,6 +137,7 @@ module Gitlab name, aliases: aliases, description: @description, + warning: @warning, explanation: @explanation, params: @params, condition_block: @condition_block, @@ -150,6 +155,7 @@ module Gitlab @explanation = nil @params = nil @condition_block = nil + @warning = nil @parse_params_block = nil end end diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb index b03c1e23ca3..5dae82a63b4 100644 --- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb +++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb @@ -210,6 +210,19 @@ describe Gitlab::QuickActions::CommandDefinition do end end + context 'when warning is set' do + before do + subject.explanation = 'Explanation' + subject.warning = 'dangerous!' + end + + it 'returns this static string' do + result = subject.explain({}, nil) + + expect(result).to eq 'Explanation (dangerous!)' + end + end + context 'when the explanation is dynamic' do before do subject.explanation = proc { |arg| "Dynamic #{arg}" } diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb index 067a30fd7e2..fd4df8694ba 100644 --- a/spec/lib/gitlab/quick_actions/dsl_spec.rb +++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb @@ -12,6 +12,7 @@ describe Gitlab::QuickActions::Dsl do params 'The first argument' explanation 'Static explanation' + warning 'Possible problem!' command :explanation_with_aliases, :once, :first do |arg| arg end @@ -64,6 +65,7 @@ describe Gitlab::QuickActions::Dsl do expect(no_args_def.condition_block).to be_nil expect(no_args_def.action_block).to be_a_kind_of(Proc) expect(no_args_def.parse_params_block).to be_nil + expect(no_args_def.warning).to eq('') expect(explanation_with_aliases_def.name).to eq(:explanation_with_aliases) expect(explanation_with_aliases_def.aliases).to eq([:once, :first]) @@ -73,6 +75,7 @@ describe Gitlab::QuickActions::Dsl do expect(explanation_with_aliases_def.condition_block).to be_nil expect(explanation_with_aliases_def.action_block).to be_a_kind_of(Proc) expect(explanation_with_aliases_def.parse_params_block).to be_nil + expect(explanation_with_aliases_def.warning).to eq('Possible problem!') expect(dynamic_description_def.name).to eq(:dynamic_description) expect(dynamic_description_def.aliases).to eq([]) @@ -82,6 +85,7 @@ describe Gitlab::QuickActions::Dsl do expect(dynamic_description_def.condition_block).to be_nil expect(dynamic_description_def.action_block).to be_a_kind_of(Proc) expect(dynamic_description_def.parse_params_block).to be_nil + expect(dynamic_description_def.warning).to eq('') expect(cc_def.name).to eq(:cc) expect(cc_def.aliases).to eq([]) @@ -91,6 +95,7 @@ describe Gitlab::QuickActions::Dsl do expect(cc_def.condition_block).to be_nil expect(cc_def.action_block).to be_nil expect(cc_def.parse_params_block).to be_nil + expect(cc_def.warning).to eq('') expect(cond_action_def.name).to eq(:cond_action) expect(cond_action_def.aliases).to eq([]) @@ -100,6 +105,7 @@ describe Gitlab::QuickActions::Dsl do expect(cond_action_def.condition_block).to be_a_kind_of(Proc) expect(cond_action_def.action_block).to be_a_kind_of(Proc) expect(cond_action_def.parse_params_block).to be_nil + expect(cond_action_def.warning).to eq('') expect(with_params_parsing_def.name).to eq(:with_params_parsing) expect(with_params_parsing_def.aliases).to eq([]) @@ -109,6 +115,7 @@ describe Gitlab::QuickActions::Dsl do expect(with_params_parsing_def.condition_block).to be_nil expect(with_params_parsing_def.action_block).to be_a_kind_of(Proc) expect(with_params_parsing_def.parse_params_block).to be_a_kind_of(Proc) + expect(with_params_parsing_def.warning).to eq('') expect(substitution_def.name).to eq(:something) expect(substitution_def.aliases).to eq([]) @@ -118,6 +125,7 @@ describe Gitlab::QuickActions::Dsl do expect(substitution_def.condition_block).to be_nil expect(substitution_def.action_block.call('text')).to eq('text Some complicated thing you want in here') expect(substitution_def.parse_params_block).to be_nil + expect(substitution_def.warning).to eq('') end end end diff --git a/spec/services/issuable/clone/attributes_rewriter_spec.rb b/spec/services/issuable/clone/attributes_rewriter_spec.rb new file mode 100644 index 00000000000..20bda6984bd --- /dev/null +++ b/spec/services/issuable/clone/attributes_rewriter_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Issuable::Clone::AttributesRewriter do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project1) { create(:project, :public, group: group) } + let(:project2) { create(:project, :public, group: group) } + let(:original_issue) { create(:issue, project: project1) } + let(:new_issue) { create(:issue, project: project2) } + + subject { described_class.new(user, original_issue, new_issue) } + + context 'setting labels' do + it 'sets labels present in the new project and group labels' do + project1_label_1 = create(:label, title: 'label1', project: project1) + project1_label_2 = create(:label, title: 'label2', project: project1) + project2_label_1 = create(:label, title: 'label1', project: project2) + group_label = create(:group_label, title: 'group_label', group: group) + create(:label, title: 'label3', project: project2) + + original_issue.update(labels: [project1_label_1, project1_label_2, group_label]) + + subject.execute + + expect(new_issue.reload.labels).to match_array([project2_label_1, group_label]) + end + + it 'does not set any labels when not used on the original issue' do + subject.execute + + expect(new_issue.reload.labels).to be_empty + end + + it 'copies the resource label events' do + resource_label_events = create_list(:resource_label_event, 2, issue: original_issue) + + subject.execute + + expected = resource_label_events.map(&:label_id) + + expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected) + end + end + + context 'setting milestones' do + it 'sets milestone to nil when old issue milestone is not in the new project' do + milestone = create(:milestone, title: 'milestone', project: project1) + + original_issue.update(milestone: milestone) + + subject.execute + + expect(new_issue.reload.milestone).to be_nil + end + + it 'copies the milestone when old issue milestone title is in the new project' do + milestone_project1 = create(:milestone, title: 'milestone', project: project1) + milestone_project2 = create(:milestone, title: 'milestone', project: project2) + + original_issue.update(milestone: milestone_project1) + + subject.execute + + expect(new_issue.reload.milestone).to eq(milestone_project2) + end + + it 'copies the milestone when old issue milestone is a group milestone' do + milestone = create(:milestone, title: 'milestone', group: group) + + original_issue.update(milestone: milestone) + + subject.execute + + expect(new_issue.reload.milestone).to eq(milestone) + end + end +end diff --git a/spec/services/issuable/clone/content_rewriter_spec.rb b/spec/services/issuable/clone/content_rewriter_spec.rb new file mode 100644 index 00000000000..4d3cb0bd254 --- /dev/null +++ b/spec/services/issuable/clone/content_rewriter_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Issuable::Clone::ContentRewriter do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project1) { create(:project, :public, group: group) } + let(:project2) { create(:project, :public, group: group) } + + let(:other_issue) { create(:issue, project: project1) } + let(:merge_request) { create(:merge_request) } + + subject { described_class.new(user, original_issue, new_issue)} + + let(:description) { 'Simple text' } + let(:original_issue) { create(:issue, description: description, project: project1) } + let(:new_issue) { create(:issue, project: project2) } + + context 'rewriting award emojis' do + it 'copies the award emojis' do + create(:award_emoji, awardable: original_issue, name: 'thumbsup') + create(:award_emoji, awardable: original_issue, name: 'thumbsdown') + + expect { subject.execute }.to change { AwardEmoji.count }.by(2) + + expect(new_issue.award_emoji.map(&:name)).to match_array(%w(thumbsup thumbsdown)) + end + end + + context 'rewriting description' do + before do + subject.execute + end + + context 'when description is a simple text' do + it 'does not rewrite the description' do + expect(new_issue.reload.description).to eq(original_issue.description) + end + end + + context 'when description contains a local reference' do + let(:description) { "See ##{other_issue.iid}" } + + it 'rewrites the local reference correctly' do + expected_description = "See #{project1.path}##{other_issue.iid}" + + expect(new_issue.reload.description).to eq(expected_description) + end + end + + context 'when description contains a cross reference' do + let(:description) { "See #{merge_request.project.full_path}!#{merge_request.iid}" } + + it 'rewrites the cross reference correctly' do + expected_description = "See #{merge_request.project.full_path}!#{merge_request.iid}" + + expect(new_issue.reload.description).to eq(expected_description) + end + end + + context 'when description contains a user reference' do + let(:description) { "FYU #{user.to_reference}" } + + it 'works with a user reference' do + expect(new_issue.reload.description).to eq("FYU #{user.to_reference}") + end + end + + context 'when description contains uploads' do + let(:uploader) { build(:file_uploader, project: project1) } + let(:description) { "Text and #{uploader.markdown_link}" } + + it 'rewrites uploads in the description' do + upload = Upload.last + + expect(new_issue.description).not_to eq(description) + expect(new_issue.description).to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/) + expect(upload.secret).not_to eq(uploader.secret) + expect(new_issue.description).to include(upload.secret) + expect(new_issue.description).to include(upload.path) + end + end + end + + context 'rewriting notes' do + context 'simple notes' do + let!(:notes) do + [ + create(:note, noteable: original_issue, project: project1, + created_at: 2.weeks.ago, updated_at: 1.week.ago), + create(:note, noteable: original_issue, project: project1), + create(:note, system: true, noteable: original_issue, project: project1) + ] + end + let!(:system_note_metadata) { create(:system_note_metadata, note: notes.last) } + let!(:award_emoji) { create(:award_emoji, awardable: notes.first, name: 'thumbsup')} + + before do + subject.execute + end + + it 'rewrites existing notes in valid order' do + expect(new_issue.notes.order('id ASC').pluck(:note).first(3)).to eq(notes.map(&:note)) + end + + it 'copies all the issue notes' do + expect(new_issue.notes.count).to eq(3) + end + + it 'does not change the note attributes' do + subject.execute + + new_note = new_issue.notes.first + + expect(new_note.note).to eq(notes.first.note) + expect(new_note.author).to eq(notes.first.author) + end + + it 'copies the award emojis' do + subject.execute + + new_note = new_issue.notes.first + new_note.award_emoji.first.name = 'thumbsup' + end + + it 'copies system_note_metadata for system note' do + new_note = new_issue.notes.last + + expect(new_note.system_note_metadata.action).to eq(system_note_metadata.action) + expect(new_note.system_note_metadata.id).not_to eq(system_note_metadata.id) + end + end + + context 'notes with reference' do + let(:text) do + "See ##{other_issue.iid} and #{merge_request.project.full_path}!#{merge_request.iid}" + end + let!(:note) { create(:note, noteable: original_issue, note: text, project: project1) } + + it 'rewrites the references correctly' do + subject.execute + + new_note = new_issue.notes.first + + expected_text = "See #{other_issue.project.path}##{other_issue.iid} and #{merge_request.project.full_path}!#{merge_request.iid}" + + expect(new_note.note).to eq(expected_text) + expect(new_note.author).to eq(note.author) + end + end + end +end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index b5767583952..1e088bc7d9b 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -10,11 +10,9 @@ describe Issues::MoveService do let(:sub_group_2) { create(:group, :private, parent: group) } let(:old_project) { create(:project, namespace: sub_group_1) } let(:new_project) { create(:project, namespace: sub_group_2) } - let(:milestone1) { create(:milestone, project_id: old_project.id, title: 'v9.0') } let(:old_issue) do - create(:issue, title: title, description: description, - project: old_project, author: author, milestone: milestone1) + create(:issue, title: title, description: description, project: old_project, author: author) end subject(:move_service) do @@ -25,16 +23,6 @@ describe Issues::MoveService do before do old_project.add_reporter(user) new_project.add_reporter(user) - - labels = Array.new(2) { |x| "label%d" % (x + 1) } - - labels.each do |label| - old_issue.labels << create(:label, - project_id: old_project.id, - title: label) - - new_project.labels << create(:label, title: label) - end end end @@ -48,91 +36,6 @@ describe Issues::MoveService do context 'issue movable' do include_context 'user can move issue' - context 'move to new milestone' do - let(:new_issue) { move_service.execute(old_issue, new_project) } - - context 'project milestone' do - let!(:milestone2) do - create(:milestone, project_id: new_project.id, title: 'v9.0') - end - - it 'assigns milestone to new issue' do - expect(new_issue.reload.milestone.title).to eq 'v9.0' - expect(new_issue.reload.milestone).to eq(milestone2) - end - end - - context 'group milestones' do - let!(:group) { create(:group, :private) } - let!(:group_milestone_1) do - create(:milestone, group_id: group.id, title: 'v9.0_group') - end - - before do - old_issue.update(milestone: group_milestone_1) - old_project.update(namespace: group) - new_project.update(namespace: group) - - group.add_users([user], GroupMember::DEVELOPER) - end - - context 'when moving to a project of the same group' do - it 'keeps the same group milestone' do - expect(new_issue.reload.project).to eq(new_project) - expect(new_issue.reload.milestone).to eq(group_milestone_1) - end - end - - context 'when moving to a project of a different group' do - let!(:group_2) { create(:group, :private) } - - let!(:group_milestone_2) do - create(:milestone, group_id: group_2.id, title: 'v9.0_group') - end - - before do - old_issue.update(milestone: group_milestone_1) - new_project.update(namespace: group_2) - - group_2.add_users([user], GroupMember::DEVELOPER) - end - - it 'assigns to new group milestone of same title' do - expect(new_issue.reload.project).to eq(new_project) - expect(new_issue.reload.milestone).to eq(group_milestone_2) - end - end - end - end - - context 'issue with group labels', :nested_groups do - it 'assigns group labels to new issue' do - label = create(:group_label, group: group) - label_issue = create(:labeled_issue, description: description, project: old_project, - milestone: milestone1, labels: [label]) - old_project.add_reporter(user) - new_project.add_reporter(user) - - new_issue = move_service.execute(label_issue, new_project) - - expect(new_issue).to have_attributes( - project: new_project, - labels: include(label) - ) - end - end - - context 'issue with resource label events' do - it 'assigns resource label events to new issue' do - old_issue.resource_label_events = create_list(:resource_label_event, 2, issue: old_issue) - - new_issue = move_service.execute(old_issue, new_project) - - expected = old_issue.resource_label_events.map(&:label_id) - expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected) - end - end - context 'generic issue' do include_context 'issue move executed' @@ -140,18 +43,6 @@ describe Issues::MoveService do expect(new_issue.project).to eq new_project end - it 'assign labels to new issue' do - expected_label_titles = new_issue.reload.labels.map(&:title) - expect(expected_label_titles).to include 'label1' - expect(expected_label_titles).to include 'label2' - expect(expected_label_titles.size).to eq 2 - - new_issue.labels.each do |label| - expect(new_project.labels).to include(label) - expect(old_project.labels).not_to include(label) - end - end - it 'rewrites issue title' do expect(new_issue.title).to eq title end @@ -203,140 +94,25 @@ describe Issues::MoveService do 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(:award_names) { %w(thumbsup thumbsdown facepalm) } - let(:notes_contents) { notes_params.map { |n| n[:note] } } - - before do - note_params = { noteable: old_issue, project: old_project, author: author } - notes_params.each_with_index do |note, index| - new_note = create(:note, note_params.merge(note)) - award_emoji_params = { awardable: new_note, name: award_names[index] } - create(:award_emoji, award_emoji_params) - 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 'creates new emojis for the new notes' do - expect(all_notes.map(&:award_emoji).to_a.flatten.map(&:name)).to eq award_names - 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 - end - - context 'note that has been updated' do - let!(:note) do - create(:note, noteable: old_issue, project: old_project, - author: author, updated_at: Date.yesterday, - created_at: Date.yesterday) - end - - include_context 'issue move executed' - - it 'preserves time when note has been created at' do - expect(new_issue.notes.first.created_at).to eq note.created_at - end + context 'issue with assignee' do + let(:assignee) { create(:user) } - it 'preserves time when note has been updated at' do - expect(new_issue.notes.first.updated_at).to eq note.updated_at - end - end - - context 'issue with assignee' do - let(:assignee) { create(:user) } - - before do - old_issue.assignees = [assignee] - end - - it 'preserves assignee with access to the new issue' do - new_project.add_reporter(assignee) - - new_issue = move_service.execute(old_issue, new_project) - - expect(new_issue.assignees).to eq([assignee]) - end - - it 'ignores assignee without access to the new issue' do - new_issue = move_service.execute(old_issue, new_project) - - expect(new_issue.assignees).to be_empty - 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(new_project)}!1" - end - end - - context 'issue description with uploads' do - let(:uploader) { build(:file_uploader, project: old_project) } - let(:description) { "Text and #{uploader.markdown_link}" } - - include_context 'issue move executed' - - it 'rewrites uploads in description' do - expect(new_issue.description).not_to eq description - expect(new_issue.description) - .to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/) - expect(new_issue.description).not_to include uploader.secret - end + before do + old_issue.assignees = [assignee] end - end - describe 'rewriting references' do - include_context 'issue move executed' + it 'preserves assignee with access to the new issue' do + new_project.add_reporter(assignee) - context 'issue references' do - let(:another_issue) { create(:issue, project: old_project) } - let(:description) { "Some description #{another_issue.to_reference}" } + new_issue = move_service.execute(old_issue, new_project) - it 'rewrites referenced issues creating cross project reference' do - expect(new_issue.description) - .to eq "Some description #{another_issue.to_reference(new_project)}" - end + expect(new_issue.assignees).to eq([assignee]) end - context "user references" do - let(:another_issue) { create(:issue, project: old_project) } - let(:description) { "Some description #{user.to_reference}" } + it 'ignores assignee without access to the new issue' do + new_issue = move_service.execute(old_issue, new_project) - it "doesn't throw any errors for issues containing user references" do - expect(new_issue.description) - .to eq "Some description #{user.to_reference}" - end + expect(new_issue.assignees).to be_empty end end @@ -416,25 +192,5 @@ describe Issues::MoveService do it { expect { move }.to raise_error(StandardError, /permissions/) } end end - - context 'movable issue with no assigned labels' do - before do - old_project.add_reporter(user) - new_project.add_reporter(user) - - labels = Array.new(2) { |x| "label%d" % (x + 1) } - - labels.each do |label| - new_project.labels << create(:label, title: label) - end - end - - include_context 'issue move executed' - - it 'does not assign labels to new issue' do - expected_label_titles = new_issue.reload.labels.map(&:title) - expect(expected_label_titles.size).to eq 0 - end - end end end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index 7e24efda5dd..c74e0bf1955 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -81,19 +81,24 @@ describe FileUploader do end describe 'copy_to' do + let(:new_project) { create(:project) } + let(:moved) { described_class.copy_to(subject, new_project) } + shared_examples 'returns a valid uploader' do describe 'returned uploader' do - let(:new_project) { create(:project) } - let(:moved) { described_class.copy_to(subject, new_project) } - it 'generates a new secret' do expect(subject).to be expect(described_class).to receive(:generate_secret).once.and_call_original expect(moved).to be end - it 'create new upload' do - expect(moved.upload).not_to eq(subject.upload) + it 'creates new upload correctly' do + upload = moved.upload + + expect(upload).not_to eq(subject.upload) + expect(upload.model).to eq(new_project) + expect(upload.uploader).to eq('FileUploader') + expect(upload.secret).not_to eq(subject.upload.secret) end it 'copies the file' do @@ -111,6 +116,12 @@ describe FileUploader do end include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.upload.path).to eq("#{moved.upload.secret}/dk.png") + expect(moved.file.path).to end_with("public/uploads/#{new_project.disk_path}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end end context 'files are stored remotely' do @@ -121,6 +132,12 @@ describe FileUploader do end include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.upload.path).to eq("#{new_project.disk_path}/#{moved.upload.secret}/dk.png") + expect(moved.file.path).to eq("#{new_project.disk_path}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end end end diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb index 799c6db57fa..d09725ee4be 100644 --- a/spec/uploaders/namespace_file_uploader_spec.rb +++ b/spec/uploaders/namespace_file_uploader_spec.rb @@ -55,4 +55,62 @@ describe NamespaceFileUploader do it_behaves_like "migrates", to_store: described_class::Store::REMOTE it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL end + + describe 'copy_to' do + let(:group) { create(:group) } + let(:moved) { described_class.copy_to(subject, group) } + + shared_examples 'returns a valid uploader' do + it 'generates a new secret' do + expect(subject).to be + expect(described_class).to receive(:generate_secret).once.and_call_original + expect(moved).to be + end + + it 'creates new upload correctly' do + upload = moved.upload + + expect(upload).not_to eq(subject.upload) + expect(upload.model).to eq(group) + expect(upload.uploader).to eq('NamespaceFileUploader') + expect(upload.secret).not_to eq(subject.upload.secret) + end + + it 'copies the file' do + expect(subject.file).to exist + expect(moved.file).to exist + expect(subject.file).not_to eq(moved.file) + expect(subject.object_store).to eq(moved.object_store) + end + end + + context 'files are stored locally' do + before do + subject.store!(fixture_file_upload('spec/fixtures/dk.png')) + end + + include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.upload.path).to eq("#{moved.upload.secret}/dk.png") + expect(moved.file.path).to end_with("system/namespace/#{group.id}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end + end + + context 'files are stored remotely' do + before do + stub_uploads_object_storage + subject.store!(fixture_file_upload('spec/fixtures/dk.png')) + subject.migrate!(ObjectStorage::Store::REMOTE) + end + + include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.file.path).to eq("namespace/#{group.id}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end + end + end end |