Skip to content

Commit

Permalink
Merge pull request #11596 from dmarcoux/scm-closed-merged-reopened-to…
Browse files Browse the repository at this point in the history
…ken-workflow

Handle closed, merged and reopened pull/merge requests in OBS workflows
  • Loading branch information
vpereira committed Sep 13, 2021
2 parents 8f34fcf + 72a7b57 commit 528a66a
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 46 deletions.
58 changes: 56 additions & 2 deletions src/api/app/models/token/workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,67 @@ def call(options)

yaml_file = Workflows::YAMLDownloader.new(scm_webhook.payload, token: self).call
workflows = Workflows::YAMLToWorkflowsService.new(yaml_file: yaml_file, scm_webhook: scm_webhook, token: self).call

case
when scm_webhook.closed_merged_pull_request?
destroy_target_projects(workflows)
when scm_webhook.reopened_pull_request?
restore_target_projects(workflows)
when scm_webhook.new_pull_request?, scm_webhook.updated_pull_request?
call_steps(workflows)
end
rescue Octokit::Unauthorized, Gitlab::Error::Unauthorized => e
raise Token::Errors::SCMTokenInvalid, e.message
end

private

def destroy_target_projects(workflows)
workflows.each do |workflow|
# Do not process steps for which there's nothing to do
workflow_steps = workflow.steps.reject { |step| step.instance_of?(::Workflow::Step::ConfigureRepositories) }
target_packages = workflow_steps.map(&:target_package).uniq.compact
delete_subscriptions(target_packages)

target_project_names = workflow_steps.map(&:target_project_name).uniq.compact
destroy_all_target_projects(target_project_names)
end
end

# We want the callbacks to run after destroy, so we can't use delete_all
def destroy_all_target_projects(target_project_names)
Project.where(name: target_project_names).destroy_all
end

def delete_subscriptions(packages)
EventSubscription.where(channel: 'scm', token: self, package: packages).delete_all
end

def restore_target_projects(workflows)
token_user_login = user.login

workflows.each do |workflow|
# Do not process steps for which there's nothing to do
workflow_steps = workflow.steps.reject { |step| step.instance_of?(::Workflow::Step::ConfigureRepositories) }
target_project_names = workflow_steps.map(&:target_project_name).uniq.compact
target_project_names.each do |target_project_name|
Project.restore(target_project_name, user: token_user_login)
end

target_packages = workflow_steps.map(&:target_package).uniq.compact
target_packages.each do |target_package|
# FIXME: We shouldn't rely on a workflow step to be able to create/update subscriptions
workflow_steps.first.create_or_update_subscriptions(target_package, workflow.filters)
end
end
end

def call_steps(workflows)
workflows.each do |workflow|
workflow.steps.each do |step|
step.call({ workflow_filters: workflow.filters })
end
end
rescue Octokit::Unauthorized, Gitlab::Error::Unauthorized => e
raise Token::Errors::SCMTokenInvalid, e.message
end
end

Expand Down
45 changes: 23 additions & 22 deletions src/api/app/models/workflow/step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,29 @@ def call(_options)
raise AbstractMethodCalled
end

def target_project_name
"home:#{@token.user.login}:#{source_project_name}:PR-#{scm_webhook.payload[:pr_number]}"
end

def target_package
Package.find_by_project_and_name(target_project_name, target_package_name)
end

# FIXME: Remove this and use create_subscriptions and update_subscriptions as soon as BranchPackageStep and Token::Workflow are refactored
# This method is public since it has to be called from Token::Workflow. This shouldn't be needed.
def create_or_update_subscriptions(package, workflow_filters)
['Event::BuildFail', 'Event::BuildSuccess'].each do |build_event|
subscription = EventSubscription.find_or_create_by!(eventtype: build_event,
receiver_role: 'reader', # We pass a valid value, but we don't need this.
user: @token.user,
channel: 'scm',
enabled: true,
token: @token,
package: package)
subscription.update!(payload: scm_webhook.payload.merge({ workflow_filters: workflow_filters }))
end
end

protected

def validate_step_instructions
Expand All @@ -28,10 +51,6 @@ def source_project_name
step_instructions[:source_project]
end

def target_project_name
"home:#{@token.user.login}:#{source_project_name}:PR-#{scm_webhook.payload[:pr_number]}"
end

def source_package_name
step_instructions[:source_package]
end
Expand All @@ -44,10 +63,6 @@ def target_package_name

private

def target_package
Package.find_by_project_and_name(target_project_name, target_package_name)
end

def remote_source?
Project.find_remote_project(source_project_name).present?
end
Expand Down Expand Up @@ -84,20 +99,6 @@ def branch_request_content_gitlab
object_attributes: { source: { default_branch: scm_webhook.payload[:commit_sha] } } }.to_json
end

# FIXME: remove this and use create_subscriptions and update_subscriptions as soon as BranchPackageStep is refactored
def create_or_update_subscriptions(package, workflow_filters)
['Event::BuildFail', 'Event::BuildSuccess'].each do |build_event|
subscription = EventSubscription.find_or_create_by!(eventtype: build_event,
receiver_role: 'reader', # We pass a valid value, but we don't need this.
user: @token.user,
channel: 'scm',
enabled: true,
token: @token,
package: package)
subscription.update!(payload: scm_webhook.payload.merge({ workflow_filters: workflow_filters }))
end
end

def create_subscriptions(package, workflow_filters)
['Event::BuildFail', 'Event::BuildSuccess'].each do |build_event|
EventSubscription.create!(eventtype: build_event,
Expand Down
21 changes: 9 additions & 12 deletions src/api/app/models/workflow/step/branch_package_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ class Workflow::Step::BranchPackageStep < ::Workflow::Step
def call(options = {})
return unless valid?

# FIXME: Support closed/merged/reopened PRs
return if scm_webhook.closed_merged_pull_request? || scm_webhook.reopened_pull_request?
workflow_filters = options.fetch(:workflow_filters, {})
branch_package(workflow_filters)
end

private

def branch_package(workflow_filters = {})
branched_package = find_or_create_branched_package

add_or_update_branch_request_file(package: branched_package)

workflow_filters = options.fetch(:workflow_filters, {})
create_or_update_subscriptions(branched_package, workflow_filters)

workflow_repositories(target_project_name, workflow_filters).each do |repository|
Expand All @@ -27,14 +30,6 @@ def call(options = {})
branched_package
end

private

def find_or_create_branched_package
return target_package if scm_webhook.updated_pull_request? && target_package.present?

branch
end

def check_source_access
return if remote_source?

Expand All @@ -49,7 +44,9 @@ def check_source_access
Pundit.authorize(@token.user, src_package, :create_branch?)
end

def branch
def find_or_create_branched_package
return target_package if scm_webhook.updated_pull_request? && target_package.present?

check_source_access

begin
Expand Down
3 changes: 0 additions & 3 deletions src/api/app/models/workflow/step/configure_repositories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ class Workflow::Step::ConfigureRepositories < Workflow::Step
def call(_options = {})
return unless valid?

# FIXME: Support closed/merged/reopened PRs
return if scm_webhook.closed_merged_pull_request? || scm_webhook.reopened_pull_request?

step_instructions[:repositories].each do |repository_instructions|
repository = Repository.includes(:architectures).find_or_create_by(name: repository_instructions[:name], project: Project.get_by_name(target_project_name))

Expand Down
11 changes: 4 additions & 7 deletions src/api/app/models/workflow/step/link_package_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,12 @@ class Workflow::Step::LinkPackageStep < ::Workflow::Step
def call(options = {})
return unless valid?

# FIXME: Support closed/merged/reopened PRs
return if scm_webhook.closed_merged_pull_request? || scm_webhook.reopened_pull_request?

workflow_filters = options.fetch(:workflow_filters, {})

# Updated PR
if scm_webhook.updated_pull_request? && target_package.present?
update_subscriptions(target_package, workflow_filters)
else # New PR
if scm_webhook.updated_pull_request?
create_target_package if target_package.blank?
create_or_update_subscriptions(target_package, workflow_filters)
elsif scm_webhook.new_pull_request?
create_target_package
create_subscriptions(target_package, workflow_filters)
end
Expand Down
74 changes: 74 additions & 0 deletions src/api/spec/models/token/workflow_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,46 @@
RSpec.describe Token::Workflow, vcr: true do
let(:token_user) { create(:confirmed_user, :with_home, login: 'Iggy') }
let(:workflow_token) { create(:workflow_token, user: token_user) }
let(:github_payload_closed) do
{
action: 'closed',
pull_request: {
head: {
repo: {
full_name: 'username/test_repo'
}
},
base: {
ref: 'main'
}
},
number: 4,
sender: {
url: 'https://api.github.com'
}
}
end

let(:github_payload_reopened) do
{
action: 'reopened',
pull_request: {
head: {
repo: {
full_name: 'username/test_repo'
}
},
base: {
ref: 'main'
}
},
number: 4,
sender: {
url: 'https://api.github.com'
}
}
end

let(:github_payload) do
{
action: 'opened',
Expand Down Expand Up @@ -72,6 +112,40 @@
end

describe '#call' do
context 'PR was reopened' do
let(:scm) { 'github' }
let(:event) { 'pull_request' }
let(:payload) { github_payload_reopened }
let(:workflows_yml_file) { File.expand_path(Rails.root.join('spec/support/files/workflows.yml')) }
let(:downloader) { instance_double(Workflows::YAMLDownloader) }

before do
allow(Workflows::YAMLDownloader).to receive(:new).and_return(downloader)
allow(downloader).to receive(:call).and_return(workflows_yml_file)
allow(Project).to receive(:restore)
subject
end

it { expect(Project).to have_received(:restore) }
end

context 'PR was closed' do
let(:scm) { 'github' }
let(:event) { 'pull_request' }
let(:payload) { github_payload_closed }
let(:workflows_yml_file) { File.expand_path(Rails.root.join('spec/support/files/workflows.yml')) }
let(:downloader) { instance_double(Workflows::YAMLDownloader) }

before do
allow(Workflows::YAMLDownloader).to receive(:new).and_return(downloader)
allow(downloader).to receive(:call).and_return(workflows_yml_file)
allow(workflow_token).to receive(:destroy_all_target_projects)
subject
end

it { expect(workflow_token).to have_received(:destroy_all_target_projects) }
end

context "when the webhook's event is not the expected one" do
context 'when the SCM is GitHub' do
it_behaves_like 'not-allowed event or action' do
Expand Down

0 comments on commit 528a66a

Please sign in to comment.