Merge pull request #11601 from saraycp/refactor_branch_package

Refactor BranchPackageStep and LinkPackageStep
openSUSE · Sep 16, 2021 · ee90b19 · ee90b19
2 parents 04ffff7 + ae07c98
commit ee90b19
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 67 deletions.
diff --git a/src/api/app/models/workflow/step.rb b/src/api/app/models/workflow/step.rb
@@ -1,4 +1,3 @@
-# rubocop:disable Metrics/ClassLength
 class Workflow::Step
   include ActiveModel::Model
 
@@ -24,8 +23,6 @@ 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,
@@ -67,7 +64,7 @@ def remote_source?
     Project.find_remote_project(source_project_name).present?
   end
 
-  def add_or_update_branch_request_file(package:)
+  def add_branch_request_file(package:)
     branch_request_file = case scm_webhook.payload[:scm]
                           when 'github'
                             branch_request_content_github
@@ -99,29 +96,6 @@ def branch_request_content_gitlab
       object_attributes: { source: { default_branch: scm_webhook.payload[:commit_sha] } } }.to_json
   end
 
-  def create_subscriptions(package, workflow_filters)
-    ['Event::BuildFail', 'Event::BuildSuccess'].each do |build_event|
-      EventSubscription.create!(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,
-                                payload: scm_webhook.payload.merge({ workflow_filters: workflow_filters }))
-    end
-  end
-
-  def update_subscriptions(package, workflow_filters)
-    ['Event::BuildFail', 'Event::BuildSuccess'].each do |build_event|
-      subscription = EventSubscription.find_by(eventtype: build_event,
-                                               channel: 'scm',
-                                               token: @token,
-                                               package: package)
-      subscription.update!(payload: scm_webhook.payload.merge({ workflow_filters: workflow_filters }))
-    end
-  end
-
   # TODO: Move to a query object.
   def workflow_repositories(target_project_name, filters)
     repositories = Project.get_by_name(target_project_name).repositories
@@ -145,5 +119,15 @@ def workflow_architectures(repository, filters)
 
     architectures
   end
+
+  def report_to_scm(workflow_filters)
+    workflow_repositories(target_project_name, workflow_filters).each do |repository|
+      # TODO: Fix n+1 queries
+      workflow_architectures(repository, workflow_filters).each do |architecture|
+        # We cannot report multibuild flavors here... so they will be missing from the initial report
+        SCMStatusReporter.new({ project: target_project_name, package: target_package_name, repository: repository.name, arch: architecture.name },
+                              scm_webhook.payload, @token.scm_token).call
+      end
+    end
+  end
 end
-# rubocop:enable Metrics/ClassLength
diff --git a/src/api/app/models/workflow/step/branch_package_step.rb b/src/api/app/models/workflow/step/branch_package_step.rb
@@ -12,22 +12,12 @@ def call(options = {})
   private
 
   def branch_package(workflow_filters = {})
-    branched_package = find_or_create_branched_package
+    create_branched_package if scm_webhook.new_pull_request? || (scm_webhook.updated_pull_request? && target_package.blank?)
 
-    add_or_update_branch_request_file(package: branched_package)
-
-    create_or_update_subscriptions(branched_package, workflow_filters)
-
-    workflow_repositories(target_project_name, workflow_filters).each do |repository|
-      # TODO: Fix n+1 queries
-      workflow_architectures(repository, workflow_filters).each do |architecture|
-        # We cannot report multibuild flavors here... so they will be missing from the initial report
-        SCMStatusReporter.new({ project: target_project_name, package: target_package_name, repository: repository.name, arch: architecture.name },
-                              scm_webhook.payload, @token.scm_token).call
-      end
-    end
-
-    branched_package
+    create_or_update_subscriptions(target_package, workflow_filters)
+    add_branch_request_file(package: target_package)
+    report_to_scm(workflow_filters)
+    target_package
   end
 
   def check_source_access
@@ -44,9 +34,7 @@ def check_source_access
     Pundit.authorize(@token.user, src_package, :create_branch?)
   end
 
-  def find_or_create_branched_package
-    return target_package if scm_webhook.updated_pull_request? && target_package.present?
-
+  def create_branched_package
     check_source_access
 
     begin

diff --git a/src/api/app/models/workflow/step/link_package_step.rb b/src/api/app/models/workflow/step/link_package_step.rb
@@ -6,22 +6,20 @@ def call(options = {})
     return unless valid?
 
     workflow_filters = options.fetch(:workflow_filters, {})
+    link_package(workflow_filters)
+  end
 
-    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
+  private
 
-    add_or_update_branch_request_file(package: target_package)
+  def link_package(workflow_filters = {})
+    create_target_package if scm_webhook.new_pull_request? || (scm_webhook.updated_pull_request? && target_package.blank?)
+
+    create_or_update_subscriptions(target_package, workflow_filters)
+    add_branch_request_file(package: target_package)
     report_to_scm(workflow_filters)
     target_package
   end
 
-  private
-
   def target_project
     Project.find_by(name: target_project_name)
   end
@@ -83,15 +81,4 @@ def link_xml(opts = {})
     # "<link package=\"foo\" project=\"bar\" />"
     Nokogiri::XML::Builder.new { |x| x.link(opts) }.doc.root.to_s
   end
-
-  def report_to_scm(workflow_filters)
-    workflow_repositories(target_project_name, workflow_filters).each do |repository|
-      # TODO: Fix n+1 queries
-      workflow_architectures(repository, workflow_filters).each do |architecture|
-        # We cannot report multibuild flavors here... so they will be missing from the initial report
-        SCMStatusReporter.new({ project: target_project_name, package: target_package_name, repository: repository.name, arch: architecture.name },
-                              scm_webhook.payload, @token.scm_token).call
-      end
-    end
-  end
 end