diff --git a/.rubocop_todo/rspec/change_by_zero.yml b/.rubocop_todo/rspec/change_by_zero.yml index 74acf4d504e9f04cf3fe5ecb513fc2c67a0e3522..c88bc67c044902a24e1119d865c98caeed7414aa 100644 --- a/.rubocop_todo/rspec/change_by_zero.yml +++ b/.rubocop_todo/rspec/change_by_zero.yml @@ -117,7 +117,6 @@ RSpec/ChangeByZero: - 'spec/workers/gitlab/bitbucket_import/stage/import_pull_requests_worker_spec.rb' - 'spec/workers/gitlab/bitbucket_server_import/stage/import_pull_requests_worker_spec.rb' - 'spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb' - - 'spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb' - 'spec/workers/packages/cleanup_package_file_worker_spec.rb' - 'spec/workers/packages/maven/metadata/sync_worker_spec.rb' - 'spec/workers/packages/rubygems/extraction_worker_spec.rb' diff --git a/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb b/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb index 573c73cd7dfe66275ff118797ac211c68fbfda43..b378d07d59cd6040a7c68a0c69079522ed02dd2e 100644 --- a/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb +++ b/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb @@ -14,11 +14,7 @@ def import(project) importer.execute - if Feature.enabled?(:bitbucket_server_convert_mentions_to_users, project.creator) - ImportUsersWorker.perform_async(project.id) - else - ImportPullRequestsWorker.perform_async(project.id) - end + ImportPullRequestsWorker.perform_async(project.id) end def importer_class diff --git a/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb b/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb index 4c323a117556870818d9c393d9a60c94c15dd43b..7d2602283e3c9d59271b2ea1bbbae23f93b8feed 100644 --- a/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb +++ b/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb @@ -10,13 +10,7 @@ class ImportUsersWorker private - def import(project) - importer = importer_class.new(project) - - importer.execute - - ImportPullRequestsWorker.perform_async(project.id) - end + def import(project); end def importer_class Importers::UsersImporter diff --git a/config/feature_flags/development/bitbucket_server_convert_mentions_to_users.yml b/config/feature_flags/development/bitbucket_server_convert_mentions_to_users.yml deleted file mode 100644 index 90ea8d56c0acca704cb4ef12d497d529b46d05b7..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/bitbucket_server_convert_mentions_to_users.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: bitbucket_server_convert_mentions_to_users -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139097 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/434453 -milestone: '16.7' -type: development -group: group::import and integrate -default_enabled: false diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb index 9712a8d25514623f85e3789322b21d8495276231..7e2aba7b61645ebcb6152126ce72ab32dc2dde8d 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb @@ -5,14 +5,13 @@ module BitbucketServerImport module Importers class PullRequestImporter include Loggable + include Gitlab::Import::UsernameMentionRewriter include ::Import::PlaceholderReferences::Pusher def initialize(project, hash) @project = project @formatter = Gitlab::ImportFormatter.new @user_finder = UserFinder.new(project) - @mentions_converter = Gitlab::Import::MentionsConverter.new('bitbucket_server', project) - # Object should behave as a object so we can remove object.is_a?(Hash) check # This will be fixed in https://gitlab.com/gitlab-org/gitlab/-/issues/412328 @object = hash.with_indifferent_access @@ -55,21 +54,14 @@ def execute private - attr_reader :object, :project, :formatter, :user_finder, :mentions_converter + attr_reader :object, :project, :formatter, :user_finder def description description = '' description += author_line description += object[:description] if object[:description] - description = mentions_converter.convert(description) if convert_mentions? - - description - end - - def convert_mentions? - Feature.enabled?(:bitbucket_server_convert_mentions_to_users, project.creator) && - !user_mapping_enabled?(project) + wrap_mentions_in_backticks(description) end def author_line diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_importer.rb index fc9f1d81b9bf3ff59689ba9976f5deeb2672e0fc..0d7c00569656b4ae49275dd1100f00c8a336ace0 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_importer.rb @@ -7,6 +7,7 @@ module PullRequestNotes # Base class for importing pull request notes during project import from Bitbucket Server class BaseImporter include Loggable + include ::Gitlab::Import::UsernameMentionRewriter include ::Import::PlaceholderReferences::Pusher # @param project [Project] @@ -15,7 +16,6 @@ def initialize(project, merge_request) @project = project @user_finder = UserFinder.new(project) @formatter = Gitlab::ImportFormatter.new - @mentions_converter = Gitlab::Import::MentionsConverter.new('bitbucket_server', project) @merge_request = merge_request end @@ -25,7 +25,7 @@ def execute(_args) private - attr_reader :project, :user_finder, :merge_request, :mentions_converter + attr_reader :project, :user_finder, :merge_request end end end diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_note_diff_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_note_diff_importer.rb index 0e73959882f2247b2c6fa79a7621af12710ecf6b..1cffe9f8998dc149be58925b1c618d6e3331224e 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_note_diff_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/base_note_diff_importer.rb @@ -62,19 +62,19 @@ def pull_request_comment_attributes(comment) note = "*By #{comment[:author_username]} (#{comment[:author_email]})*\n\n" end - comment_note = mentions_converter.convert(comment[:note]) - note += # Provide some context for replying if comment[:parent_comment_note] - "> #{comment[:parent_comment_note].truncate(PARENT_COMMENT_CONTEXT_LENGTH)}\n\n#{comment_note}" + parent_comment_note = comment[:parent_comment_note].truncate(80, omission: ' ...') + + "> #{parent_comment_note}\n\n#{comment[:note]}" else - comment_note + comment[:note] end { project: project, - note: note, + note: wrap_mentions_in_backticks(note), author_id: author, created_at: comment[:created_at], updated_at: comment[:updated_at] @@ -98,7 +98,7 @@ def create_basic_fallback_note(merge_request, comment, position) note_text += " #{position.old_path}:#{position.old_line} -->" if position.old_line note_text += " #{position.new_path}:#{position.new_line}" if position.new_line - note_text += "*\n\n#{comment[:note]}" + note_text += "*\n\n#{wrap_mentions_in_backticks(comment[:note])}" attributes[:note] = note_text diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb index 89f46c98fd139d1abb1268ee8dd3c9ab15432a50..7f470e17f8d8837d80722ed4400e33540c7efdc3 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb @@ -5,6 +5,7 @@ module BitbucketServerImport module Importers class PullRequestNotesImporter include ::Gitlab::Import::MergeRequestHelpers + include ::Gitlab::Import::UsernameMentionRewriter include Loggable include ::Import::PlaceholderReferences::Pusher @@ -12,7 +13,6 @@ def initialize(project, hash) @project = project @user_finder = UserFinder.new(project) @formatter = Gitlab::ImportFormatter.new - @mentions_converter = Gitlab::Import::MentionsConverter.new('bitbucket_server', project) @object = hash.with_indifferent_access end @@ -30,7 +30,7 @@ def execute private - attr_reader :object, :project, :formatter, :user_finder, :mentions_converter + attr_reader :object, :project, :formatter, :user_finder def import_notes_in_batch(merge_request) activities = client.activities(project_key, repository_slug, merge_request.iid) @@ -221,23 +221,21 @@ def pull_request_comment_attributes(comment) note = "*By #{comment.author_username} (#{comment.author_email})*\n\n" end - comment_note = if convert_mentions? - mentions_converter.convert(comment.note) - else - comment.note - end + comment_note = comment.note note += # Provide some context for replying if comment.parent_comment - "> #{comment.parent_comment.note.truncate(80)}\n\n#{comment_note}" + parent_comment_note = comment.parent_comment.note.truncate(80, omission: ' ...') + + "> #{parent_comment_note}\n\n#{comment_note}" else comment_note end { project: project, - note: note, + note: wrap_mentions_in_backticks(note), author_id: author, created_at: comment.created_at, updated_at: comment.updated_at, @@ -245,11 +243,6 @@ def pull_request_comment_attributes(comment) } end - def convert_mentions? - Feature.enabled?(:bitbucket_server_convert_mentions_to_users, project.creator) && - !user_mapping_enabled?(project) - end - def author(comment) if user_mapping_enabled?(project) user_finder.uid( diff --git a/lib/gitlab/bitbucket_server_import/importers/users_importer.rb b/lib/gitlab/bitbucket_server_import/importers/users_importer.rb deleted file mode 100644 index 588c7be5dd4bd6db7030030607f28d7b39d275c6..0000000000000000000000000000000000000000 --- a/lib/gitlab/bitbucket_server_import/importers/users_importer.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BitbucketServerImport - module Importers - class UsersImporter - include Loggable - include Gitlab::Import::UserFromMention - - BATCH_SIZE = 100 - - def initialize(project) - @project = project - @project_id = project.id - end - - attr_reader :project, :project_id - - def execute - log_info(import_stage: 'import_users', message: 'starting') - - current = page_counter.current - - loop do - log_info( - import_stage: 'import_users', - message: "importing page #{current} using batch size #{BATCH_SIZE}" - ) - - users = client.users(project_key, page_offset: current, limit: BATCH_SIZE).to_a - - break if users.empty? - - cache_users(users) - - current += 1 - page_counter.set(current) - end - - page_counter.expire! - - log_info(import_stage: 'import_users', message: 'finished') - end - - private - - def cache_users(users) - users_hash = users.each_with_object({}) do |user, hash| - cache_key = source_user_cache_key('bitbucket_server', project_id, user.username) - hash[cache_key] = source_user_cache_value(user.email, type: :email) - end - - cache_multiple(users_hash) - end - - def client - @client ||= BitbucketServer::Client.new(project.import_data.credentials) - end - - def project_key - project.import_data.data['project_key'] - end - - def page_counter - @page_counter ||= Gitlab::Import::PageCounter.new(project, :users, 'bitbucket-server-importer') - end - end - end - end -end diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb index 67f2925cde3330013635cff9596af23dd7ba24dc..e4ad27086df21ab1da54f1cc7af36ceab1ee7d1b 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb @@ -53,6 +53,26 @@ ) end + describe 'when handling @ username mentions' do + let(:original_body) { "I said to @sam_allen.greg the code should follow @bob's advice. @.ali-ce/group#9?" } + let(:expected_body) do + "I said to `@sam_allen.greg` the code should follow `@bob`'s advice. `@.ali-ce/group#9`?" + end + + let(:pull_request_data) do + Gitlab::Json.parse(fixture_file('importers/bitbucket_server/pull_request.json')) + .merge({ "description" => original_body }) + end + + it 'inserts backticks around mentions' do + importer.execute + + merge_request = project.merge_requests.find_by_iid(pull_request.iid) + + expect(merge_request.description).to eq(expected_body) + end + end + describe 'refs/merge-requests/:iid/head creation' do before do project.repository.create_branch(pull_request.source_branch_name, 'master') @@ -164,22 +184,9 @@ ) end - context 'when the `bitbucket_server_convert_mentions_to_users` flag is disabled' do - before do - stub_feature_flags(bitbucket_server_convert_mentions_to_users: false) - end - - it 'does not convert mentions' do - expect_next(Gitlab::Import::MentionsConverter, 'bitbucket_server', project).not_to receive(:convert) - - importer.execute - end - end - context 'when alternate UCM flags are disabled' do before do stub_feature_flags( - bitbucket_server_convert_mentions_to_users: false, bitbucket_server_user_mapping: false ) end diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/inline_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/inline_spec.rb index 467dcc52f86bad5323df7f0ebd8d18a6d98cb3f8..89a2b045682c27bb7826c23bb3aeb9909e6cc13b 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/inline_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/inline_spec.rb @@ -11,7 +11,6 @@ let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:now) { Time.now.utc.change(usec: 0) } - let_it_be(:mentions_converter) { Gitlab::Import::MentionsConverter.new('bitbucket_server', project) } let_it_be(:reply) do { author_email: 'reply_author@example.org', @@ -45,10 +44,6 @@ let_it_be(:reply_source_user) { generate_source_user(project, reply[:author_username]) } let_it_be(:note_source_user) { generate_source_user(project, pr_inline_comment[:author_username]) } - before do - allow(Gitlab::Import::MentionsConverter).to receive(:new).and_return(mentions_converter) - end - def expect_log(stage:, message:, iid:, comment_id:) allow(Gitlab::BitbucketServerImport::Logger).to receive(:info).and_call_original expect(Gitlab::BitbucketServerImport::Logger) @@ -69,8 +64,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'imports the threaded discussion' do - expect(mentions_converter).to receive(:convert).and_call_original.twice - expect { importer.execute(pr_inline_comment) }.to change { Note.count }.by(2) expect(merge_request.discussions.count).to eq(1) @@ -101,6 +94,51 @@ def expect_log(stage:, message:, iid:, comment_id:) importer.execute(pr_inline_comment) end + context 'when note has @ username mentions' do + let(:original_text) { "Attention: @ali has worked on this. @fred's work from @.ali-ce/group#9?" } + let(:expected_text) { "Attention: `@ali` has worked on this. `@fred`'s work from `@.ali-ce/group#9`?" } + let(:mention_reply) do + { + author_email: 'reply_author@example.org', + author_username: 'reply_author', + note: original_text, + created_at: now, + updated_at: now, + parent_comment_note: nil + } + end + + let(:mention_comment) do + { + id: 7, + file_type: 'ADDED', + from_sha: 'c5f4288162e2e6218180779c7f6ac1735bb56eab', + to_sha: 'a4c2164330f2549f67c13f36a93884cf66e976be', + file_path: '.gitmodules', + old_pos: nil, + new_pos: 4, + note: original_text, + author_email: 'inline_note_author@example.org', + author_username: 'inline_note_author', + comments: [mention_reply], + created_at: now, + updated_at: now, + parent_comment_note: nil + } + end + + it 'inserts backticks around the mentions' do + importer.execute(mention_comment) + + notes = merge_request.notes.order(:id).to_a + start_note = notes.first + expect(start_note.note).to end_with(expected_text) + + reply_note = notes.last + expect(reply_note.note).to eq(expected_text) + end + end + context 'when note is invalid' do let(:invalid_comment) do pr_inline_comment.merge( @@ -111,8 +149,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'fallback to basic note' do - expect(mentions_converter).to receive(:convert).and_call_original.twice - expect { importer.execute(invalid_comment) }.to change { Note.count }.by(1) expect(merge_request.discussions.count).to eq(1) @@ -125,7 +161,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'logs its fallback' do - expect(mentions_converter).to receive(:convert).and_call_original.twice expect_log( stage: 'create_diff_note', message: 'creating standalone fallback for DiffNote', @@ -148,8 +183,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'imports the threaded discussion' do - expect(mentions_converter).to receive(:convert).and_call_original.twice - expect { importer.execute(pr_inline_comment) }.to change { Note.count }.by(2) expect(merge_request.discussions.count).to eq(1) @@ -162,16 +195,6 @@ def expect_log(stage:, message:, iid:, comment_id:) expect(reply_note.author_id).to eq(reply_author.id) end - context 'when converting mention is failed' do - it 'logs its exception' do - expect(mentions_converter).to receive(:convert).and_raise(StandardError) - expect(Gitlab::ErrorTracking).to receive(:log_exception) - .with(StandardError, include(import_stage: 'create_diff_note')) - - importer.execute(pr_inline_comment) - end - end - it 'does not push placeholder references' do importer.execute(pr_inline_comment) diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/standalone_notes_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/standalone_notes_spec.rb index 01fd4843f038863d6f189a3b0185ffa6ff717860..57421fab9abcd1a5c5f8fe0da3dd191eeb1fd9e1 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/standalone_notes_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes/standalone_notes_spec.rb @@ -8,7 +8,6 @@ let_it_be(:project) { create(:project, :repository, :bitbucket_server_import, :import_user_mapping_enabled) } let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:now) { Time.now.utc.change(usec: 0) } - let_it_be(:mentions_converter) { Gitlab::Import::MentionsConverter.new('bitbucket_server', project) } let_it_be(:author_details) do { author_name: 'John Notes', @@ -30,10 +29,6 @@ let_it_be(:source_user) { generate_source_user(project, pr_comment[:author_username]) } - before do - allow(Gitlab::Import::MentionsConverter).to receive(:new).and_return(mentions_converter) - end - def expect_log(stage:, message:, iid:, comment_id:) allow(Gitlab::BitbucketServerImport::Logger).to receive(:info).and_call_original expect(Gitlab::BitbucketServerImport::Logger) @@ -53,8 +48,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'imports the stand alone comments' do - expect(mentions_converter).to receive(:convert).and_call_original - expect { importer.execute(pr_comment) }.to change { Note.count }.by(1) expect(merge_request.notes.count).to eq(1) @@ -93,8 +86,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'imports multiple comments' do - expect(mentions_converter).to receive(:convert).and_call_original.twice - expect { importer.execute(pr_comment) }.to change { Note.count }.by(2) expect(merge_request.notes.count).to eq(2) @@ -115,6 +106,41 @@ def expect_log(stage:, message:, iid:, comment_id:) end end + context 'when note has @ username mentions' do + let(:original_text) { "Attention: @ali has worked on this. @fred's work from @.ali-ce/group#9?" } + let(:expected_text) { "Attention: `@ali` has worked on this. `@fred`'s work from `@.ali-ce/group#9`?" } + let(:mention_extra_comment) do + { + id: 6, + note: original_text, + comments: [], + created_at: now, + updated_at: now, + parent_comment_note: nil, + imported_from: 'bitbucket_server' + }.merge(author_details) + end + + let(:mention_comment) do + { + id: 5, + note: original_text, + comments: [mention_extra_comment], + created_at: now, + updated_at: now, + parent_comment_note: nil, + imported_from: 'bitbucket_server' + }.merge(author_details) + end + + it 'inserts backticks around the mentions' do + importer.execute(mention_comment) + + expect(merge_request.notes.first.note).to eq(expected_text) + expect(merge_request.notes.last.note).to eq(expected_text) + end + end + context 'when the note has a parent note' do let(:pr_comment) do { @@ -158,7 +184,6 @@ def expect_log(stage:, message:, iid:, comment_id:) end it 'logs its exception' do - expect(mentions_converter).to receive(:convert).and_call_original expect(Gitlab::ErrorTracking).to receive(:log_exception) .with(StandardError, include(import_stage: 'import_standalone_notes_comments')) diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer_spec.rb index f403d534c7750d71d4a9045f74d17d98cace89ee..c89fc2ad1a260cfed32302fce7d79d41c3f5eae3 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer_spec.rb @@ -13,7 +13,6 @@ let_it_be(:pull_request_data) { Gitlab::Json.parse(fixture_file('importers/bitbucket_server/pull_request.json')) } let_it_be(:pull_request) { BitbucketServer::Representation::PullRequest.new(pull_request_data) } let_it_be_with_reload(:merge_request) { create(:merge_request, iid: pull_request.iid, source_project: project) } - let(:mentions_converter) { Gitlab::Import::MentionsConverter.new('bitbucket_server', project) } let(:merge_event) do instance_double( @@ -97,10 +96,6 @@ def expect_log(stage:, message:) .to receive(:info).with(include(import_stage: stage, message: message)) end - before do - allow(Gitlab::Import::MentionsConverter).to receive(:new).and_return(mentions_converter) - end - subject(:importer) { described_class.new(project.reload, pull_request.to_hash) } describe '#execute', :clean_gitlab_redis_shared_state do @@ -167,7 +162,56 @@ def expect_log(stage:, message:) ) end - context 'when the note has a parent note' do + context 'when the note has @ mentions' do + let(:original_note_text) { "I said to @sam_allen.greg follow @bob's advice. @.ali-ce/group#9?" } + let(:expected_note_text) { "I said to `@sam_allen.greg` follow `@bob`'s advice. `@.ali-ce/group#9`?" } + let(:original_reply_text) { "@bachhus I don't agree. See @ali's evidence cc @.ali-ce/group#9?" } + let(:expected_reply_text) { "`@bachhus` I don't agree. See `@ali`'s evidence cc `@.ali-ce/group#9`?" } + + let(:pr_note) do + instance_double( + BitbucketServer::Representation::Comment, + id: 456, + note: original_note_text, + author_name: 'Note Author', + author_email: 'note_author@example.org', + author_username: 'note_author', + comments: [pr_note_reply], + created_at: now, + updated_at: now, + parent_comment: nil) + end + + let(:pr_note_reply) do + instance_double( + BitbucketServer::Representation::Comment, + note: original_reply_text, + author_name: 'Note Author', + author_email: 'note_author@example.org', + author_username: 'note_author', + comments: [], + created_at: now, + updated_at: now, + parent_comment: nil) + end + + it 'inserts backticks around the mentions' do + importer.execute + + expect(Note.first.note).to eq(expected_note_text) + expect(Note.last.note).to eq(expected_reply_text) + end + end + + context 'when the note has a parent note with @ mentions' do + let(:original_parent_text) do + "Attention: To inform @ali has worked on this. @fred's work from @.ali-ce/group#9?" + end + + let(:expected_parent_text) do + "Attention: To inform `@ali` has worked on this. `@fred`'s work from `@.ali-ce/gro` ..." + end + let(:pr_note) do instance_double( BitbucketServer::Representation::Comment, @@ -185,7 +229,7 @@ def expect_log(stage:, message:) let(:pr_parent_note) do instance_double( BitbucketServer::Representation::Comment, - note: 'Parent note', + note: original_parent_text, author_name: 'Note Author', author_email: 'note_author@example.org', author_username: 'note_author', @@ -196,10 +240,10 @@ def expect_log(stage:, message:) ) end - it 'adds the parent note before the actual note' do + it 'adds the parent note before the actual note with backticks inserted' do importer.execute - expect(Note.first.note).to include("> #{pr_parent_note.note}\n\n") + expect(Note.first.note).to include("> #{expected_parent_text}") end end @@ -223,18 +267,6 @@ def expect_log(stage:, message:) end end - context 'when the `bitbucket_server_convert_mentions_to_users` flag is disabled' do - before do - stub_feature_flags(bitbucket_server_convert_mentions_to_users: false) - end - - it 'does not convert mentions' do - expect(mentions_converter).not_to receive(:convert) - - importer.execute - end - end - it 'logs its progress' do expect_log(stage: 'import_standalone_pr_comments', message: 'starting') expect_log(stage: 'import_standalone_pr_comments', message: 'finished') @@ -376,18 +408,6 @@ def expect_log(stage:, message:) end end - context 'when the `bitbucket_server_convert_mentions_to_users` flag is disabled' do - before do - stub_feature_flags(bitbucket_server_convert_mentions_to_users: false) - end - - it 'does not convert mentions' do - expect(mentions_converter).not_to receive(:convert) - - importer.execute - end - end - it 'logs its progress' do expect_log(stage: 'import_inline_comments', message: 'starting') expect_log(stage: 'import_inline_comments', message: 'finished') diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb deleted file mode 100644 index 5519a829daab18a291b93caa211b239706d0a41a..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BitbucketServerImport::Importers::UsersImporter, feature_category: :importers do - let(:logger) { Gitlab::BitbucketServerImport::Logger } - - let_it_be(:project) do - create(:project, :with_import_url, :import_started, :empty_repo, - import_data_attributes: { - data: { 'project_key' => 'key', 'repo_slug' => 'slug' }, - credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' } - } - ) - end - - let(:user_1) do - BitbucketServer::Representation::User.new( - { 'user' => { 'emailAddress' => 'email1', 'slug' => 'username1' } } - ) - end - - let(:user_2) do - BitbucketServer::Representation::User.new( - { 'user' => { 'emailAddress' => 'email2', 'slug' => 'username2' } } - ) - end - - let(:user_3) do - BitbucketServer::Representation::User.new( - { 'user' => { 'emailAddress' => 'email3', 'slug' => 'username3' } } - ) - end - - before do - stub_const("#{described_class}::BATCH_SIZE", 2) - - allow_next_instance_of(BitbucketServer::Client) do |client| - allow(client).to receive(:users).with('key', limit: 2, page_offset: 1).and_return([user_1, user_2]) - allow(client).to receive(:users).with('key', limit: 2, page_offset: 2).and_return([user_3]) - allow(client).to receive(:users).with('key', limit: 2, page_offset: 3).and_return([]) - end - end - - subject(:importer) { described_class.new(project) } - - describe '#execute' do - it 'writes the username and email to cache for every user in batches' do - expect(logger).to receive(:info).with(hash_including(message: 'starting')) - expect(logger).to receive(:info).with(hash_including(message: 'importing page 1 using batch size 2')) - expect(logger).to receive(:info).with(hash_including(message: 'importing page 2 using batch size 2')) - expect(logger).to receive(:info).with(hash_including(message: 'importing page 3 using batch size 2')) - expect(logger).to receive(:info).with(hash_including(message: 'finished')) - - expect_next_instance_of(Gitlab::Import::PageCounter) do |page_counter| - expect(page_counter).to receive(:current).and_call_original.once - expect(page_counter).to receive(:set).with(2).and_call_original.once - expect(page_counter).to receive(:set).with(3).and_call_original.once - expect(page_counter).to receive(:expire!).and_call_original.once - end - - expect(Gitlab::Cache::Import::Caching).to receive(:write_multiple).and_call_original.twice - - importer.execute - - cache_key_prefix = "bitbucket_server/project/#{project.id}/source" - expect(Gitlab::Cache::Import::Caching.read("#{cache_key_prefix}/username1")).to eq( - { value: 'email1', type: :email }.to_json - ) - expect(Gitlab::Cache::Import::Caching.read("#{cache_key_prefix}/username2")).to eq( - { value: 'email2', type: :email }.to_json - ) - expect(Gitlab::Cache::Import::Caching.read("#{cache_key_prefix}/username3")).to eq( - { value: 'email3', type: :email }.to_json - ) - end - end -end diff --git a/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb b/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb index 1531b30089c9c2346cffc8232056c7fded81f6f4..7ea23041e791b3f442653457976c49c2a08e7e4a 100644 --- a/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb +++ b/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb @@ -18,25 +18,12 @@ end it 'schedules the next stage' do - expect(Gitlab::BitbucketServerImport::Stage::ImportUsersWorker).to receive(:perform_async) + expect(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async) .with(project.id) worker.perform(project.id) end - context 'when the bitbucket_server_convert_mentions_to_users flag is disabled' do - before do - stub_feature_flags(bitbucket_server_convert_mentions_to_users: false) - end - - it 'skips the user import and schedules the next stage' do - expect(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async) - .with(project.id) - - worker.perform(project.id) - end - end - it 'logs stage start and finish' do expect(Gitlab::BitbucketServerImport::Logger) .to receive(:info).with(hash_including(message: 'starting stage', project_id: project.id)) diff --git a/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb b/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb index 1141d08729d356d844f4f70ef83e6d6325967330..dcc72c187f7c60617dfdf2766fc95a85e317c57c 100644 --- a/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb +++ b/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb @@ -14,77 +14,9 @@ let(:worker) { described_class.new } - it_behaves_like Gitlab::BitbucketServerImport::StageMethods - describe '#perform' do - context 'when the import succeeds' do - before do - allow_next_instance_of(Gitlab::BitbucketServerImport::Importers::UsersImporter) do |importer| - allow(importer).to receive(:execute) - end - - allow(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async).and_return(nil) - end - - it_behaves_like 'an idempotent worker' do - let(:job_args) { [project.id] } - end - - it 'schedules the next stage' do - expect(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async) - .with(project.id) - - worker.perform(project.id) - end - - it 'logs stage start and finish' do - expect(Gitlab::BitbucketServerImport::Logger) - .to receive(:info).with(hash_including(message: 'starting stage', project_id: project.id)) - expect(Gitlab::BitbucketServerImport::Logger) - .to receive(:info).with(hash_including(message: 'stage finished', project_id: project.id)) - - worker.perform(project.id) - end - end - - context 'when project does not exists' do - it 'does not call importer' do - expect(Gitlab::BitbucketServerImport::Importers::UsersImporter).not_to receive(:new) - - worker.perform(-1) - end - end - - context 'when project import state is not `started`' do - it 'does not call importer' do - project = create(:project, :import_canceled) - - expect(Gitlab::BitbucketServerImport::Importers::UsersImporter).not_to receive(:new) - - worker.perform(project.id) - end - end - - context 'when the importer fails' do - it 'does not schedule the next stage and raises error' do - exception = StandardError.new('Error') - - allow_next_instance_of(Gitlab::BitbucketServerImport::Importers::UsersImporter) do |importer| - allow(importer).to receive(:execute).and_raise(exception) - end - - expect(Gitlab::Import::ImportFailureService) - .to receive(:track).with( - project_id: project.id, - exception: exception, - error_source: described_class.name, - fail_import: false - ).and_call_original - - expect { worker.perform(project.id) } - .to change { Gitlab::BitbucketServerImport::Stage::ImportUsersWorker.jobs.size }.by(0) - .and raise_error(exception) - end + it 'returns without calling the next import stage' do # no-oped. Will be removed in follow up https://gitlab.com/gitlab-org/gitlab/-/issues/508916 + expect { worker.perform(project.id) }.not_to raise_error end end end