Refactor BranchPackageStep and LinkPackageStep

- Unify variable names
- Use create_or_update_subscriptions always
- Rename method to add_branch_request_file
- Refactor method find_or_create_branched_package to become
create_branched_package.
- Isolate LinkPackage main code in its own method
- Simplify the main code

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
@@ -157,4 +131,3 @@ def report_to_scm(workflow_filters)
     end
   end
 end
-# rubocop:enable Metrics/ClassLength

src/api/app/models/workflow/step/branch_package_step.rb

@@ -12,15 +12,12 @@ def call(options = {})
   private
 
   def branch_package(workflow_filters = {})
-    branched_package = find_or_create_branched_package
-
-    add_or_update_branch_request_file(package: branched_package)
-
-    create_or_update_subscriptions(branched_package, workflow_filters)
+    create_branched_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)
-
-    branched_package
+    target_package
   end
 
   def check_source_access
@@ -37,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

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