Commit b3df5148 authored by Marius Bobin's avatar Marius Bobin Committed by Mayra Cabrera

Send results only for external_pipelines

On GitHub integrations we might have multiple pipelines for the
same SHA. One pipeline triggered by the commit and a different
one from the pull request event. On GitHub, we update the status
of the pull request with the status of the pipeline that finishes
last. This commit cancel sending messages if we also have a
external_pull_request_event pipeline for the same sha.
parent 8b2e5366
......@@ -68,7 +68,7 @@ class GithubService < Service
end
def execute(data)
return if disabled? || invalid?
return if disabled? || invalid? || irrelevant_result?(data)
status_message = StatusMessage.from_pipeline_data(project, self, data)
......@@ -99,6 +99,31 @@ class GithubService < Service
private
def irrelevant_result?(data)
!external_pull_request_pipeline?(data) &&
external_pull_request_pipelines_exist_for_sha?(data)
end
def external_pull_request_pipeline?(data)
id = data.dig(:object_attributes, :id)
external_pull_request_pipelines.id_in(id).exists?
end
def external_pull_request_pipelines_exist_for_sha?(data)
sha = data.dig(:object_attributes, :sha)
return false if sha.nil?
external_pull_request_pipelines.for_sha(sha).exists?
end
def external_pull_request_pipelines
@external_pull_request_pipelines ||= project
.ci_pipelines
.external_pull_request_event
end
def remote_project
RemoteProject.new(repository_url)
end
......
---
title: Prefer sending external pull request pipeline statuses over general statuses
to GitHub
merge_request: 20364
author:
type: fixed
......@@ -230,6 +230,50 @@ describe GithubService do
end
end
context 'when an external pull request pipeline exists' do
let(:external_pr) { create(:external_pull_request, project: project) }
let!(:external_pipeline) do
create(:ci_pipeline,
source: :external_pull_request_event,
external_pull_request: external_pr,
project: project)
end
it 'does not send notification' do
expect(subject).not_to receive(:update_status)
expect(GithubService::StatusMessage).not_to receive(:from_pipeline_data)
expect(subject.execute(pipeline_sample_data)).to be_nil
end
it 'sends notification if the sha is not present' do
pipeline_sample_data[:object_attributes].delete(:sha)
expect(subject).to receive(:update_status)
expect(GithubService::StatusMessage).to receive(:from_pipeline_data)
subject.execute(pipeline_sample_data)
end
end
context 'when the pipeline is an external pull request pipeline' do
let(:external_pr) { create(:external_pull_request, project: project) }
before do
pipeline.update!(
source: :external_pull_request_event,
external_pull_request: external_pr)
end
it 'sends notification' do
expect(subject).to receive(:update_status)
expect(GithubService::StatusMessage).to receive(:from_pipeline_data)
subject.execute(pipeline_sample_data)
end
end
context 'without a license' do
it 'does nothing' do
stub_licensed_features(github_project_service_integration: false)
......
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