From c9f202b2efdecda2e9fa5290ba5de413ab345a7d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 11 Feb 2016 15:08:10 +0100 Subject: [PATCH] Fix broken link in CI build notification emails Closes #13199 --- app/mailers/emails/builds.rb | 13 +-- app/views/notify/build_fail_email.html.haml | 3 +- .../notify/build_success_email.html.haml | 2 +- spec/mailers/notify_spec.rb | 82 ++++++++++--------- 4 files changed, 52 insertions(+), 48 deletions(-) diff --git a/app/mailers/emails/builds.rb b/app/mailers/emails/builds.rb index 64c1ce8cfab..2f86d1be576 100644 --- a/app/mailers/emails/builds.rb +++ b/app/mailers/emails/builds.rb @@ -3,26 +3,27 @@ module Emails def build_fail_email(build_id, to) @build = Ci::Build.find(build_id) @project = @build.project + add_project_headers - add_build_headers - headers['X-GitLab-Build-Status'] = "failed" + add_build_headers('failed') mail(to: to, subject: subject("Build failed for #{@project.name}", @build.short_sha)) end def build_success_email(build_id, to) @build = Ci::Build.find(build_id) @project = @build.project + add_project_headers - add_build_headers - headers['X-GitLab-Build-Status'] = "success" + add_build_headers('success') mail(to: to, subject: subject("Build success for #{@project.name}", @build.short_sha)) end private - def add_build_headers + + def add_build_headers(status) headers['X-GitLab-Build-Id'] = @build.id headers['X-GitLab-Build-Ref'] = @build.ref + headers['X-GitLab-Build-Status'] = status.to_s end - end end diff --git a/app/views/notify/build_fail_email.html.haml b/app/views/notify/build_fail_email.html.haml index f4e9749e5c7..81d65037312 100644 --- a/app/views/notify/build_fail_email.html.haml +++ b/app/views/notify/build_fail_email.html.haml @@ -1,9 +1,10 @@ - content_for :header do %h1{style: "background: #c40834; color: #FFF; font: normal 20px Helvetica, Arial, sans-serif; margin: 0; padding: 5px 10px; line-height: 32px; font-size: 16px;"} GitLab (build failed) + %h3 Project: - = link_to ci_project_url(@project) do + = link_to namespace_project_url(@project.namespace, @project) do = @project.name %p diff --git a/app/views/notify/build_success_email.html.haml b/app/views/notify/build_success_email.html.haml index 8b004d34cca..5d247eb4cf2 100644 --- a/app/views/notify/build_success_email.html.haml +++ b/app/views/notify/build_success_email.html.haml @@ -4,7 +4,7 @@ %h3 Project: - = link_to ci_project_url(@project) do + = link_to namespace_project_url(@project.namespace, @project) do = @project.name %p diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 82bd057b16c..c22ff7f8cea 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -13,7 +13,6 @@ describe Notify do let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to } let(:recipient) { create(:user, email: 'recipient@example.com') } let(:project) { create(:project) } - let(:build) { create(:ci_build) } before(:each) do ActionMailer::Base.deliveries.clear @@ -48,13 +47,6 @@ describe Notify do end end - shared_examples 'an email with X-GitLab headers containing build details' do - it 'has X-GitLab-Build* headers' do - is_expected.to have_header 'X-GitLab-Build-Id', /#{build.id}/ - is_expected.to have_header 'X-GitLab-Build-Ref', /#{build.ref}/ - end - end - shared_examples 'an email that contains a header with author username' do it 'has X-GitLab-Author header containing author\'s username' do is_expected.to have_header 'X-GitLab-Author', user.username @@ -971,49 +963,59 @@ describe Notify do end end - describe 'build success' do - before { build.success } + describe 'build notification email' do + let(:build) { create(:ci_build) } + let(:project) { build.project } - subject { Notify.build_success_email(build.id, 'wow@example.com') } + shared_examples 'build email' do + it 'contains name of project' do + is_expected.to have_body_text build.project_name + end - it_behaves_like 'an email with X-GitLab headers containing build details' - it_behaves_like 'an email with X-GitLab headers containing project details' do - let(:project) { build.project } + it 'contains link to project' do + is_expected.to have_body_text namespace_project_path(project.namespace, project) + end end - it 'has header indicating build status' do - is_expected.to have_header 'X-GitLab-Build-Status', 'success' + shared_examples 'an email with X-GitLab headers containing build details' do + it 'has X-GitLab-Build* headers' do + is_expected.to have_header 'X-GitLab-Build-Id', /#{build.id}/ + is_expected.to have_header 'X-GitLab-Build-Ref', /#{build.ref}/ + end end - it 'has the correct subject' do - should have_subject /Build success for/ + describe 'build success' do + subject { Notify.build_success_email(build.id, 'wow@example.com') } + before { build.success } + + it_behaves_like 'build email' + it_behaves_like 'an email with X-GitLab headers containing build details' + it_behaves_like 'an email with X-GitLab headers containing project details' + + it 'has header indicating build status' do + is_expected.to have_header 'X-GitLab-Build-Status', 'success' + end + + it 'has the correct subject' do + is_expected.to have_subject /Build success for/ + end end - it 'contains name of project' do - should have_body_text build.project_name - end - end + describe 'build fail' do + subject { Notify.build_fail_email(build.id, 'wow@example.com') } + before { build.drop } - describe 'build fail' do - before { build.drop } + it_behaves_like 'build email' + it_behaves_like 'an email with X-GitLab headers containing build details' + it_behaves_like 'an email with X-GitLab headers containing project details' - subject { Notify.build_fail_email(build.id, 'wow@example.com') } + it 'has header indicating build status' do + is_expected.to have_header 'X-GitLab-Build-Status', 'failed' + end - it_behaves_like 'an email with X-GitLab headers containing build details' - it_behaves_like 'an email with X-GitLab headers containing project details' do - let(:project) { build.project } - end - - it 'has header indicating build status' do - is_expected.to have_header 'X-GitLab-Build-Status', 'failed' - end - - it 'has the correct subject' do - should have_subject /Build failed for/ - end - - it 'contains name of project' do - should have_body_text build.project_name + it 'has the correct subject' do + is_expected.to have_subject /Build failed for/ + end end end end