diff --git a/Gemfile b/Gemfile index b722dc449061e19e5bb610250fe50fc11edeab49..645247dfcd1a7588d4ad1b659552dbb66dff432e 100644 --- a/Gemfile +++ b/Gemfile @@ -563,7 +563,7 @@ gem 'ssh_data', '~> 1.3' # rubocop:todo Gemfile/MissingFeatureCategory gem 'spamcheck', '~> 1.3.0' # rubocop:todo Gemfile/MissingFeatureCategory # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 16.10.1', feature_category: :gitaly +gem 'gitaly', '~> 16.11.0.pre.rc1', feature_category: :gitaly # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.4.0', feature_category: :deployment_management diff --git a/Gemfile.checksum b/Gemfile.checksum index c0b924520ecb4fdae979b9e00933a9fa9ea167c7..54a55aba551ba1dea485b6dc944e82610c15770c 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -206,7 +206,7 @@ {"name":"gettext","version":"3.4.9","platform":"ruby","checksum":"292864fe6a15c224cee4125a4a72fab426fdbb280e4cff3cfe44935f549b009a"}, {"name":"gettext_i18n_rails","version":"1.12.0","platform":"ruby","checksum":"6ac4817731a9e2ce47e1e83381ac34f9142263bc2911aaaafb2526d2f1afc1be"}, {"name":"git","version":"1.18.0","platform":"ruby","checksum":"c9b80462e4565cd3d7a9ba8440c41d2c52244b17b0dad0bfddb46de70630c465"}, -{"name":"gitaly","version":"16.10.1","platform":"ruby","checksum":"8f416e71d70fcea89941ca02ddd74b0938ec31e11d5d297bc99e8e3a8861df5a"}, +{"name":"gitaly","version":"16.11.0.pre.rc1","platform":"ruby","checksum":"24334f5f3fd5b6c3d278eea9fe2b6732dd08e87a4146cd4374615506b1a6e7ae"}, {"name":"gitlab","version":"4.19.0","platform":"ruby","checksum":"3f645e3e195dbc24f0834fbf83e8ccfb2056d8e9712b01a640aad418a6949679"}, {"name":"gitlab-chronic","version":"0.10.5","platform":"ruby","checksum":"f80f18dc699b708870a80685243331290bc10cfeedb6b99c92219722f729c875"}, {"name":"gitlab-dangerfiles","version":"4.7.0","platform":"ruby","checksum":"2576876a8dcb7290853fc3aef8048001cfe593b87318dd0016959d42e0e145ca"}, diff --git a/Gemfile.lock b/Gemfile.lock index e533578b2867eb9392838fff8feea4f6b943289f..3afeb66a4726c9886dd51db8d40ed2dc394b56b3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -687,7 +687,7 @@ GEM git (1.18.0) addressable (~> 2.8) rchardet (~> 1.8) - gitaly (16.10.1) + gitaly (16.11.0.pre.rc1) grpc (~> 1.0) gitlab (4.19.0) httparty (~> 0.20) @@ -1907,7 +1907,7 @@ DEPENDENCIES gdk-toogle (~> 0.9) gettext (~> 3.4, >= 3.4.9) gettext_i18n_rails (~> 1.12.0) - gitaly (~> 16.10.1) + gitaly (~> 16.11.0.pre.rc1) gitlab-backup-cli! gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 4.7.0) diff --git a/app/models/commit.rb b/app/models/commit.rb index 9704bfeb6f2a8f40156d45b653d30afd53cddb5e..12864962ecb1bc359d6d2ade1a3e1f139fc0ed6c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -42,6 +42,8 @@ class Commit MAX_DIFF_FILES_SETTING_UPPER_BOUND = 3_000 DIFF_SAFE_LIMIT_FACTOR = 10 + CO_AUTHORED_TRAILER = "Co-authored-by" + cache_markdown_field :title, pipeline: :single_line cache_markdown_field :full_title, pipeline: :single_line, limit: 1.kilobyte cache_markdown_field :description, pipeline: :commit_description, limit: 1.megabyte diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb index ef133bc1663b4b71a6c4655d48ad7c669f5f0449..d635cdc1170cd68acc352b56576e34f695fd4022 100644 --- a/app/models/commit_collection.rb +++ b/app/models/commit_collection.rb @@ -156,8 +156,22 @@ def load_tags private def committers_emails(with_merge_commits: false) - return commits.filter_map(&:committer_email).uniq if with_merge_commits + return committer_emails_for(commits) if with_merge_commits - without_merge_commits.filter_map(&:committer_email).uniq + committer_emails_for(without_merge_commits) + end + + def committer_emails_for(commits) + if ::Feature.enabled?(:web_ui_commit_author_change, project) + commits.each(&:signature) # preload signatures + end + + commits.filter_map do |commit| + if ::Feature.enabled?(:web_ui_commit_author_change, project) && commit.signature&.verified_system? + commit.author_email + else + commit.committer_email + end + end.uniq end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 724b5c17cf4540bb5d5fd319d1b534f33ac2bf73..6ccee7bcc37020ab65b118bf36b3bc67fdbf3daf 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -956,7 +956,8 @@ def revert( def cherry_pick( user, commit, branch_name, message, - start_branch_name: nil, start_project: project, dry_run: false) + start_branch_name: nil, start_project: project, + author_name: nil, author_email: nil, dry_run: false) with_cache_hooks do raw_repository.cherry_pick( @@ -966,6 +967,8 @@ def cherry_pick( message: message, start_branch_name: start_branch_name, start_repository: start_project.repository.raw_repository, + author_name: author_name, + author_email: author_email, dry_run: dry_run ) end diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index 0b97aae9972d8ce5218592c6cc896a37a05cc85c..696e975af1a3e7aa7a570d3aab5b4ba418d0e218 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -9,25 +9,18 @@ def initialize(*args) @message = params[:message] end + def commit_message + raise NotImplementedError + end + private + attr_reader :commit + def commit_change(action) - raise NotImplementedError unless repository.respond_to?(action) - - # rubocop:disable GitlabSecurity/PublicSend - message = - @message || @commit.public_send(:"#{action}_message", current_user) - - repository.public_send( - action, - current_user, - @commit, - @branch_name, - message, - start_project: @start_project, - start_branch_name: @start_branch, - dry_run: @dry_run - ) + message = @message || commit_message + + yield message rescue Gitlab::Git::Repository::CreateTreeError => ex type = @commit.change_type_title(current_user) diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb index 2a634c5ec71ba1c8354408f92dfbd54f0783b8a1..40894cbb4365a1f4264e402b2a8408e90d538bb9 100644 --- a/app/services/commits/cherry_pick_service.rb +++ b/app/services/commits/cherry_pick_service.rb @@ -11,14 +11,28 @@ def initialize(*args) def create_commit! Gitlab::Git::CrossRepo.new(@project.repository, @source_project.repository).execute(@commit.id) do - commit_change(:cherry_pick).tap do |sha| - track_mr_picking(sha) + commit_sha = commit_change(:cherry_pick) do |message| + perform_cherry_pick(message) end + + track_mr_picking(commit_sha) + + commit_sha end end private + def commit_message + message = commit.cherry_pick_message(current_user) + + return message unless ::Feature.enabled?(:web_ui_commit_author_change, project) + + co_authored_trailer = "#{Commit::CO_AUTHORED_TRAILER}: #{commit.author_name} <#{commit.author_email}>" + + "#{message}\n\n#{co_authored_trailer}" + end + def track_mr_picking(pick_sha) merge_request = project.merge_requests.by_merge_commit_sha(@commit.sha).first return unless merge_request @@ -29,5 +43,19 @@ def track_mr_picking(pick_sha) author: current_user ).picked_into_branch(@branch_name, pick_sha) end + + def perform_cherry_pick(message) + author_kwargs = + if Feature.enabled?(:web_ui_commit_author_change, project) + { author_name: current_user.name, author_email: current_user.email } + else + {} + end + + repository.cherry_pick(current_user, @commit, @branch_name, message, + start_project: @start_project, start_branch_name: @start_branch, dry_run: @dry_run, + **author_kwargs + ) + end end end diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb index dddb8b24eacde23482c30c97369e2995fac43979..951fd62dc21a29e34da99fdc8fb8fbdc1da9684e 100644 --- a/app/services/commits/revert_service.rb +++ b/app/services/commits/revert_service.rb @@ -3,7 +3,17 @@ module Commits class RevertService < ChangeService def create_commit! - commit_change(:revert) + commit_change(:revert) do |message| + repository.revert(current_user, @commit, @branch_name, message, + start_project: @start_project, start_branch_name: @start_branch, dry_run: @dry_run + ) + end + end + + private + + def commit_message + commit.revert_message(current_user) end end end diff --git a/app/services/suggestions/apply_service.rb b/app/services/suggestions/apply_service.rb index a20eb6b79c568b5c041b819500bc6d63b4d11e8f..4895e2c2b91c0b7c651681c31b2088a8eb22cc9b 100644 --- a/app/services/suggestions/apply_service.rb +++ b/app/services/suggestions/apply_service.rb @@ -54,6 +54,13 @@ def multi_service author_email: author&.email } + if ::Feature.enabled?(:web_ui_commit_author_change, project) + params.merge!({ + author_name: current_user.name, + author_email: current_user.commit_email_or_default + }) + end + ::Files::MultiService.new(suggestion_set.source_project, current_user, params) end diff --git a/config/feature_flags/gitlab_com_derisk/web_ui_commit_author_change.yml b/config/feature_flags/gitlab_com_derisk/web_ui_commit_author_change.yml new file mode 100644 index 0000000000000000000000000000000000000000..793d7a27a7939fe0ccf033a89c01f1f4a981671f --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/web_ui_commit_author_change.yml @@ -0,0 +1,9 @@ +--- +name: web_ui_commit_author_change +feature_issue_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148889 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/455288 +milestone: '16.11' +group: group::source code +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/spec/requests/api/commits_spec.rb b/ee/spec/requests/api/commits_spec.rb index 126f5a881ab6bdd7e9bfc188698440320e028794..92b8d838f4326811d17ac1966aba052970a64cf0 100644 --- a/ee/spec/requests/api/commits_spec.rb +++ b/ee/spec/requests/api/commits_spec.rb @@ -158,8 +158,10 @@ expect(response).to have_gitlab_http_status(:created) expect(response).to match_response_schema("public_api/v4/commit/basic") expect(json_response["title"]).to eq(commit.title) - expect(json_response["message"]).to eq(commit.cherry_pick_message(user)) - expect(json_response["author_name"]).to eq(commit.author_name) + expect(json_response["message"]).to eq( + "#{commit.cherry_pick_message(user)}\n\nCo-authored-by: #{commit.author_name} <#{commit.author_email}>" + ) + expect(json_response["author_name"]).to eq(user.name) expect(json_response["committer_name"]).to eq(user.name) end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 5238bd9b14218ac4839bcfd79abb5803a506a4ac..7ddb89ca2a21db9216ba4ae0cd422943b99c898d 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -701,19 +701,9 @@ def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_rep end end - def cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:, dry_run: false) - args = { - user: user, - commit: commit, - branch_name: branch_name, - message: message, - start_branch_name: start_branch_name, - start_repository: start_repository, - dry_run: dry_run - } - + def cherry_pick(...) wrapped_gitaly_errors do - gitaly_operation_client.user_cherry_pick(**args) + gitaly_operation_client.user_cherry_pick(...) end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 1e71f0811a8db620c6b939d3060f687cb27a3612..014881535e2a8f067d7d9d0b0e88dc7927be246f 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -235,15 +235,33 @@ def user_ff_branch(user, source_sha:, target_branch:, target_sha: nil) raise Gitlab::Git::CommitError, e end - def user_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:, dry_run: false) - response = call_cherry_pick_or_revert(:cherry_pick, - user: user, - commit: commit, - branch_name: branch_name, - message: message, - start_branch_name: start_branch_name, - start_repository: start_repository, - dry_run: dry_run) + # rubocop:disable Metrics/ParameterLists + def user_cherry_pick( + user:, commit:, branch_name:, message:, + start_branch_name:, start_repository:, author_name: nil, author_email: nil, dry_run: false) + + request = Gitaly::UserCherryPickRequest.new( + repository: @gitaly_repo, + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, + commit: commit.to_gitaly_commit, + branch_name: encode_binary(branch_name), + message: encode_binary(message), + start_branch_name: encode_binary(start_branch_name.to_s), + start_repository: start_repository.gitaly_repository, + commit_author_name: encode_binary(author_name), + commit_author_email: encode_binary(author_email), + dry_run: dry_run, + timestamp: Google::Protobuf::Timestamp.new(seconds: Time.now.utc.to_i) + ) + + response = gitaly_client_call( + @repository.storage, + :operation_service, + :user_cherry_pick, + request, + remote_storage: start_repository.storage, + timeout: GitalyClient.long_timeout + ) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) rescue GRPC::BadStatus => e @@ -264,16 +282,29 @@ def user_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, raise e end end + # rubocop:enable Metrics/ParameterLists def user_revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:, dry_run: false) - response = call_cherry_pick_or_revert(:revert, - user: user, - commit: commit, - branch_name: branch_name, - message: message, - start_branch_name: start_branch_name, - start_repository: start_repository, - dry_run: dry_run) + request = Gitaly::UserRevertRequest.new( + repository: @gitaly_repo, + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, + commit: commit.to_gitaly_commit, + branch_name: encode_binary(branch_name), + message: encode_binary(message), + start_branch_name: encode_binary(start_branch_name.to_s), + start_repository: start_repository.gitaly_repository, + dry_run: dry_run, + timestamp: Google::Protobuf::Timestamp.new(seconds: Time.now.utc.to_i) + ) + + response = gitaly_client_call( + @repository.storage, + :operation_service, + :user_revert, + request, + remote_storage: start_repository.storage, + timeout: GitalyClient.long_timeout + ) if response.pre_receive_error.presence raise Gitlab::Git::PreReceiveError, response.pre_receive_error @@ -541,31 +572,6 @@ def user_commit_patches(user, branch_name, patches) private - def call_cherry_pick_or_revert(rpc, user:, commit:, branch_name:, message:, start_branch_name:, start_repository:, dry_run:) - request_class = "Gitaly::User#{rpc.to_s.camelcase}Request".constantize - - request = request_class.new( - repository: @gitaly_repo, - user: Gitlab::Git::User.from_gitlab(user).to_gitaly, - commit: commit.to_gitaly_commit, - branch_name: encode_binary(branch_name), - message: encode_binary(message), - start_branch_name: encode_binary(start_branch_name.to_s), - start_repository: start_repository.gitaly_repository, - dry_run: dry_run, - timestamp: Google::Protobuf::Timestamp.new(seconds: Time.now.utc.to_i) - ) - - gitaly_client_call( - @repository.storage, - :operation_service, - :"user_#{rpc}", - request, - remote_storage: start_repository.storage, - timeout: GitalyClient.long_timeout - ) - end - # rubocop:disable Metrics/ParameterLists def user_commit_files_request_header( user, branch_name, commit_message, actions, author_email, author_name, diff --git a/lib/gitlab/suggestions/commit_message.rb b/lib/gitlab/suggestions/commit_message.rb index c631d36db91cb51baaa68b7cde4aaaf5a6720b44..aedcdfae3a9156b52e5fbf297bd08896a9835638 100644 --- a/lib/gitlab/suggestions/commit_message.rb +++ b/lib/gitlab/suggestions/commit_message.rb @@ -44,7 +44,7 @@ def self.format_paths(paths) 'suggestions_count' => ->(user, suggestion_set) { suggestion_set.suggestions.size }, 'co_authored_by' => ->(user, suggestions_set) { suggestions_set.authors.without(user).map do |author| - "Co-authored-by: #{author.name} <#{author.commit_email_or_default}>" + "#{Commit::CO_AUTHORED_TRAILER}: #{author.name} <#{author.commit_email_or_default}>" end.join("\n") } }.freeze diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index e4c6cc7b27e3524fa17f9b040075f94a94dd5bc0..e462f5a8366f04547a2eecc2b2308e490c965c63 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -573,6 +573,9 @@ let(:branch_name) { 'master' } let(:cherry_pick_message) { 'Cherry-pick message' } let(:time) { Time.now.utc } + let(:author_name) { user.name } + let(:author_email) { user.email } + let(:dry_run) { false } let(:branch_update) do Gitaly::OperationBranchUpdate.new( @@ -591,7 +594,10 @@ start_branch_name: branch_name, start_repository: repository.gitaly_repository, message: cherry_pick_message, - timestamp: Google::Protobuf::Timestamp.new(seconds: time.to_i) + commit_author_name: author_name, + commit_author_email: author_email, + timestamp: Google::Protobuf::Timestamp.new(seconds: time.to_i), + dry_run: dry_run ) end @@ -604,7 +610,10 @@ branch_name: branch_name, message: cherry_pick_message, start_branch_name: branch_name, - start_repository: repository + start_repository: repository, + author_name: author_name, + author_email: author_email, + dry_run: dry_run ) end diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index f0061ff4defa90d62c179bcda261a8135c1ea249..9726b40978b161f4d66355eaec972a915709e2c3 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -56,6 +56,33 @@ expect(collection.committers).to be_empty end end + + context 'when a commit is signed by GitLab' do + let(:author_email) { 'author@gitlab.com' } + let(:committer_email) { 'committer@gitlab.com' } + let(:author) { create(:user, email: author_email.upcase) } + let(:committer) { create(:user, email: committer_email.upcase) } + + before do + allow(commit).to receive_message_chain(:signature, :verified_system?).and_return(true) + allow(commit).to receive(:author_email).and_return(author_email) + allow(commit).to receive(:committer_email).and_return(committer_email) + end + + it 'uses author email to identify committers' do + expect(collection.committers).to eq([author]) + end + + context 'when web_ui_commit_author_change feature flag is disabled' do + before do + stub_feature_flags(web_ui_commit_author_change: false) + end + + it 'users committer email to identify committers' do + expect(collection.committers).to eq([committer]) + end + end + end end describe '#committer_user_ids' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 9d1e28af13080747b1ce1d4684f59a88766aaf4c..67085da80e74b565798aa3723bb7ff14a59e475a 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -1929,8 +1929,10 @@ def push_params(branch_name) expect(response).to have_gitlab_http_status(:created) expect(response).to match_response_schema('public_api/v4/commit/basic') expect(json_response['title']).to eq(commit.title) - expect(json_response['message']).to eq(commit.cherry_pick_message(user)) - expect(json_response['author_name']).to eq(commit.author_name) + expect(json_response['message']).to eq( + "#{commit.cherry_pick_message(user)}\n\nCo-authored-by: #{commit.author_name} <#{commit.author_email}>" + ) + expect(json_response['author_name']).to eq(user.name) expect(json_response['committer_name']).to eq(user.name) end diff --git a/spec/services/commits/change_service_spec.rb b/spec/services/commits/change_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..96fdf96c533ac3853183487a24cb54544e1a5264 --- /dev/null +++ b/spec/services/commits/change_service_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Commits::ChangeService, feature_category: :source_code_management do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + subject(:service) do + described_class.new(project, user) + end + + describe '#commit_message' do + it 'raises NotImplementedError' do + expect { service.commit_message }.to raise_error(NotImplementedError) + end + end +end diff --git a/spec/services/commits/cherry_pick_service_spec.rb b/spec/services/commits/cherry_pick_service_spec.rb index 880ebea1c0942b4e1735a4270d9dc40ead199361..6eaddb05ea417592de733450a0ca858b2a54ea44 100644 --- a/spec/services/commits/cherry_pick_service_spec.rb +++ b/spec/services/commits/cherry_pick_service_spec.rb @@ -40,11 +40,21 @@ def cherry_pick(sha, branch_name, message: nil) describe '#execute' do shared_examples 'successful cherry-pick' do it 'picks the commit into the branch' do + source_commit = project.commit(merge_commit_sha) result = cherry_pick(merge_commit_sha, branch_name) expect(result[:status]).to eq(:success), result[:message] - head = repository.find_branch(branch_name).target + branch = repository.find_branch(branch_name) + head = branch.target expect(head).not_to eq(merge_base_sha) + + commit = branch.dereferenced_target + expect(commit.author_name).to eq(user.name) + expect(commit.author_email).to eq(user.email) + expect(commit.message).to include("(cherry picked from commit #{merge_commit_sha})") + expect(commit.message).to include( + "Co-authored-by: #{source_commit.author_name} <#{source_commit.author_email}>" + ) end it 'supports a custom commit message' do @@ -54,6 +64,24 @@ def cherry_pick(sha, branch_name, message: nil) expect(result[:status]).to eq(:success) expect(branch.dereferenced_target.message).to eq('foo') end + + context 'when web_ui_commit_author_change feature flag is disabled' do + before do + stub_feature_flags(web_ui_commit_author_change: false) + end + + it 'does not contain co authored by trailer' do + source_commit = project.commit(merge_commit_sha) + + result = cherry_pick(merge_commit_sha, branch_name) + expect(result[:status]).to eq(:success), result[:message] + + commit = repository.find_branch(branch_name).dereferenced_target + expect(commit.message).not_to include('Co-authored-by') + expect(commit.author_name).to eq(source_commit.author_name) + expect(commit.author_email).to eq(source_commit.author_email) + end + end end it_behaves_like 'successful cherry-pick' diff --git a/spec/services/commits/revert_service_spec.rb b/spec/services/commits/revert_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e931185d55798c8c81ac5aa6a9fa1ca24854d25a --- /dev/null +++ b/spec/services/commits/revert_service_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Commits::RevertService, feature_category: :source_code_management do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:user) { create(:user) } + + before do + project.add_maintainer(user) + end + + def revert(message: nil) + commit = project.commit + + described_class.new( + project, + user, + commit: commit, + start_branch: project.default_branch, + branch_name: project.default_branch, + message: message + ).execute + end + + describe '#execute' do + it 'reverts the commit from the branch' do + result = revert + expect(result[:status]).to eq(:success), result[:message] + + expect(project.commit.message).to include( + "Revert \"Merge branch 'branch-merged' into '#{project.default_branch}'\"" + ) + expect(project.commit.message).to include( + "This reverts commit b83d6e391c22777fca1ed3012fce84f633d7fed0" + ) + end + + it 'supports a custom commit message' do + result = revert(message: 'revert this commit') + + expect(result[:status]).to eq(:success) + expect(project.commit.message).to eq('revert this commit') + end + end +end diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb index 6b56e5cd5ea9c9cc81a4bfabe2d19d8a0c928a24..6a35a2457885ca631cc674940da2b783256f8597 100644 --- a/spec/services/suggestions/apply_service_spec.rb +++ b/spec/services/suggestions/apply_service_spec.rb @@ -68,15 +68,33 @@ def apply(suggestions, custom_message = nil) apply(suggestions) commit = project.repository.commit - author = suggestions.first.note.author expect(user.commit_email).not_to eq(user.email) - expect(commit.author_email).to eq(author.commit_email_or_default) + expect(commit.author_email).to eq(user.commit_email) expect(commit.committer_email).to eq(user.commit_email) - expect(commit.author_name).to eq(author.name) + expect(commit.author_name).to eq(user.name) expect(commit.committer_name).to eq(user.name) end + context 'when web_ui_commit_author_change feature flag is disabled' do + before do + stub_feature_flags(web_ui_commit_author_change: false) + end + + it 'created commit has users email and name' do + apply(suggestions) + + commit = project.repository.commit + author = suggestions.first.note.author + + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(author.commit_email_or_default) + expect(commit.committer_email).to eq(user.commit_email) + expect(commit.author_name).to eq(author.name) + expect(commit.committer_name).to eq(user.name) + end + end + it 'tracks apply suggestion event' do expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) .to receive(:track_apply_suggestion_action) @@ -354,9 +372,9 @@ def default_regex it 'created commit has authors info and commiters info' do expect(user.commit_email).not_to eq(user.email) expect(author).not_to eq(user) - expect(commit.author_email).to eq(author.commit_email_or_default) + expect(commit.author_email).to eq(user.commit_email) expect(commit.committer_email).to eq(user.commit_email) - expect(commit.author_name).to eq(author.name) + expect(commit.author_name).to eq(user.name) expect(commit.committer_name).to eq(user.name) end end @@ -374,7 +392,7 @@ def default_regex it 'uses first authors information' do expect(author_emails).to include(first_author.commit_email_or_default).exactly(3) - expect(commit.author_email).to eq(first_author.commit_email_or_default) + expect(commit.author_email).to eq(user.commit_email) end end