Commit a2a00a5e authored by Douwe Maan's avatar Douwe Maan

Merge branch '51564-fix-commit-email-usage' into 'master'

Respect the user commit email in more places

Closes #51564

See merge request gitlab-org/gitlab-ce!21773
parents 139a14b6 5a883915
......@@ -184,11 +184,12 @@ class ProjectWiki
def commit_details(action, message = nil, title = nil)
commit_message = message || default_message(action, title)
git_user = Gitlab::Git::User.from_gitlab(@user)
Gitlab::Git::Wiki::CommitDetails.new(@user.id,
@user.username,
@user.name,
@user.email,
git_user.username,
git_user.name,
git_user.email,
commit_message)
end
......
......@@ -7,8 +7,10 @@ module Files
def initialize(*args)
super
@author_email = params[:author_email] || current_user&.email
@author_name = params[:author_name] || current_user&.name
git_user = Gitlab::Git::User.from_gitlab(current_user) if current_user.present?
@author_email = params[:author_email] || git_user&.email
@author_name = params[:author_name] || git_user&.name
@commit_message = params[:commit_message]
@last_commit_sha = params[:last_commit_sha]
......
---
title: Respect the user commit email in more places
merge_request: 21773
author:
type: fixed
......@@ -591,10 +591,6 @@ module Gitlab
end
end
def user_to_committer(user)
Gitlab::Git.committer_hash(email: user.email, name: user.name)
end
# Delete the specified branch from the repository
def delete_branch(branch_name)
wrapped_gitaly_errors do
......
......@@ -58,6 +58,14 @@ FactoryBot.define do
project_view :readme
end
trait :commit_email do
after(:create) do |user, evaluator|
additional = create(:email, :confirmed, user: user, email: "commit-#{user.email}")
user.update!(commit_email: additional.email)
end
end
factory :omniauth_user do
transient do
extern_uid '123456'
......
......@@ -2,12 +2,13 @@
require "spec_helper"
describe ProjectWiki do
let(:project) { create(:project, :wiki_repo) }
let(:user) { create(:user, :commit_email) }
let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
let(:repository) { project.repository }
let(:user) { project.owner }
let(:gitlab_shell) { Gitlab::Shell.new }
let(:project_wiki) { described_class.new(project, user) }
let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') }
let(:commit) { project_wiki.repository.head_commit }
subject { project_wiki }
......@@ -276,6 +277,14 @@ describe ProjectWiki do
expect(subject.pages.first.page.version.message).to eq("commit message")
end
it 'sets the correct commit email' do
subject.create_page('test page', 'content')
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
end
it 'updates project activity' do
subject.create_page('Test Page', 'This is content')
......@@ -320,6 +329,12 @@ describe ProjectWiki do
expect(@page.version.message).to eq("updated page")
end
it 'sets the correct commit email' do
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
end
it 'updates project activity' do
subject.update_page(
@gitlab_git_wiki_page,
......@@ -347,6 +362,14 @@ describe ProjectWiki do
expect(subject.pages.count).to eq(0)
end
it 'sets the correct commit email' do
subject.delete_page(@page)
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
end
it 'updates project activity' do
subject.delete_page(@page)
......@@ -420,7 +443,7 @@ describe ProjectWiki do
end
def commit_details
Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "test commit")
Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.commit_email, "test commit")
end
def create_page(name, content)
......
......@@ -3,7 +3,7 @@ require "spec_helper"
describe Files::CreateService do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:user) { create(:user) }
let(:user) { create(:user, :commit_email) }
let(:file_content) { 'Test file content' }
let(:branch_name) { project.default_branch }
let(:start_branch) { branch_name }
......@@ -20,6 +20,8 @@ describe Files::CreateService do
}
end
let(:commit) { repository.head_commit }
subject { described_class.new(project, user, commit_params) }
before do
......@@ -75,4 +77,16 @@ describe Files::CreateService do
end
end
end
context 'commit attribute' do
let(:file_path) { 'test-commit-attributes.txt' }
it 'uses the commit email' do
subject.execute
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
end
end
end
......@@ -4,10 +4,11 @@ describe Files::DeleteService do
subject { described_class.new(project, user, commit_params) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:user) { create(:user, :commit_email) }
let(:file_path) { 'files/ruby/popen.rb' }
let(:branch_name) { project.default_branch }
let(:last_commit_sha) { nil }
let(:commit) { project.repository.head_commit }
let(:commit_params) do
{
......@@ -34,6 +35,14 @@ describe Files::DeleteService do
expect(blob).to be_nil
end
it 'uses the commit email' do
subject.execute
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
end
end
before do
......
......@@ -4,11 +4,12 @@ describe Files::UpdateService do
subject { described_class.new(project, user, commit_params) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:user) { create(:user, :commit_email) }
let(:file_path) { 'files/ruby/popen.rb' }
let(:new_contents) { 'New Content' }
let(:branch_name) { project.default_branch }
let(:last_commit_sha) { nil }
let(:commit) { project.repository.commit }
let(:commit_params) do
{
......@@ -54,6 +55,14 @@ describe Files::UpdateService do
expect(results.data).to eq(new_contents)
end
it 'uses the commit email' do
subject.execute
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
end
end
context "when the last_commit_sha is not supplied" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment