[api] move request id logging to model as much as possible

This is also fixing request number loging on project level

src/api/app/helpers/maintenance_helper.rb

@@ -325,7 +325,7 @@ def instantiate_container(project, opackage, opts = {})
     pkg.store
 
     arguments="&noservice=1"
-    arguments << "&requestid=" << opts[:requestid] if opts[:requestid]
+    arguments << "&requestid=" << opts[:request].number.to_s if opts[:request]
     arguments << "&comment=" << CGI.escape(opts[:comment]) if opts[:comment]
     if opts[:makeoriginolder]
       # rubocop:disable Metrics/LineLength

src/api/app/models/bs_request_action.rb

@@ -467,13 +467,13 @@ def source_cleanup
       splits = self.source_project.split(':')
       return nil if splits.count == 2 && splits[0] == 'home'
 
-      source_project.commit_opts = { comment: self.bs_request.description, request: self.bs_request.number }
+      source_project.commit_opts = { comment: self.bs_request.description, request: self.bs_request }
       source_project.destroy
       return "/source/#{self.source_project}"
     end
     # just remove one package
     source_package = source_project.packages.find_by_name!(self.source_package)
-    source_package.commit_opts = { comment: self.bs_request.description, request: self.bs_request.number }
+    source_package.commit_opts = { comment: self.bs_request.description, request: self.bs_request }
     source_package.destroy
     return Package.source_path(self.source_project, self.source_package)
   end

src/api/app/models/bs_request_action_add_role.rb

@@ -39,7 +39,7 @@ def execute_accept(_opts)
       role = Role.find_by_title!(self.role)
       object.add_group( self.group_name, role )
     end
-    object.store(comment: "add_role request #{self.bs_request.number}", requestid: self.bs_request.number)
+    object.store(comment: "add_role request #{self.bs_request.number}", request: self.bs_request)
   end
 
   def render_xml_attributes(node)

src/api/app/models/bs_request_action_change_devel.rb

@@ -24,7 +24,7 @@ def execute_accept(_opts)
     target_package.develpackage = Package.get_by_project_and_name(self.source_project, self.source_package)
 
     target_package.resolve_devel_package
-    target_package.store(comment: "change_devel request #{self.bs_request.number}", requestid: self.bs_request.number)
+    target_package.store(comment: "change_devel request #{self.bs_request.number}", request: self.bs_request)
   end
 
   #### Alias of methods

src/api/app/models/bs_request_action_delete.rb

@@ -31,7 +31,7 @@ def remove_repository(opts)
       raise RepositoryMissing.new "The repository #{self.target_project} / #{self.target_repository} does not exist"
     end
     r.destroy
-    prj.store(lowprio: opts[:lowprio], comment: opts[:comment], requestid: self.bs_request.number)
+    prj.store(lowprio: opts[:lowprio], comment: opts[:comment], request: self.bs_request)
   end
 
   def render_xml_attributes(node)
@@ -63,12 +63,12 @@ def execute_accept(opts)
     if self.target_package
       package = Package.get_by_project_and_name(self.target_project, self.target_package,
                                                 use_source: true, follow_project_links: false)
-      package.commit_opts = { comment: self.bs_request.description, request: self.bs_request.number }
+      package.commit_opts = { comment: self.bs_request.description, request: self.bs_request }
       package.destroy
       return Package.source_path self.target_project, self.target_package
     else
       project = Project.get_by_name(self.target_project)
-      project.commit_opts = { comment: self.bs_request.description, request: self.bs_request.number }
+      project.commit_opts = { comment: self.bs_request.description, request: self.bs_request }
       project.destroy
       return "/source/#{self.target_project}"
     end

src/api/app/models/bs_request_action_maintenance_incident.rb

@@ -82,7 +82,7 @@ def _merge_pkg_into_maintenance_incident(incidentProject, source_project, source
         new_pkg = incidentProject.packages.create(:name => source_package, :title => pkg_title, :description => pkg_description)
         new_pkg.flags.create(:status => 'enable', :flag => 'build')
         new_pkg.flags.create(:status => 'enable', :flag => 'publish') unless incidentProject.flags.find_by_flag_and_status('access', 'disable')
-        new_pkg.store(comment: "maintenance_incident request #{self.bs_request.number}", requestid: self.bs_request.number)
+        new_pkg.store(comment: "maintenance_incident request #{self.bs_request.number}", request: self.bs_request)
       end
 
       # use specified release project if defined
@@ -134,7 +134,7 @@ def _merge_pkg_into_maintenance_incident(incidentProject, source_project, source
         else
           new_pkg = Package.new(:name => source_package, :title => pkg.title, :description => pkg.description)
           incidentProject.packages << new_pkg
-          new_pkg.store(comment: "maintenance_incident request #{self.bs_request.number}", requestid: self.bs_request.number)
+          new_pkg.store(comment: "maintenance_incident request #{self.bs_request.number}", request: self.bs_request)
         end
       else
         # no link and not a patchinfo
@@ -172,7 +172,7 @@ def merge_into_maintenance_incident(incidentProject, source_project, source_pack
     pkg = _merge_pkg_into_maintenance_incident(incidentProject, source_project, source_package, releaseproject, request)
 
     incidentProject.save!
-    incidentProject.store(comment: "maintenance_incident request #{self.bs_request.number}", requestid: self.bs_request.number)
+    incidentProject.store(comment: "maintenance_incident request #{self.bs_request.number}", request: self.bs_request)
     pkg
   end
 

src/api/app/models/bs_request_action_set_bugowner.rb

@@ -36,7 +36,7 @@ def execute_accept(_opts)
     end
     object.add_user( self.person_name, bugowner, true ) if self.person_name # runs with ignoreLock
     object.add_group( self.group_name, bugowner, true ) if self.group_name  # runs with ignoreLock
-    object.store(comment: "set_bugowner request #{self.bs_request.number}", requestid: self.bs_request.number)
+    object.store(comment: "set_bugowner request #{self.bs_request.number}", request: self.bs_request)
   end
 
   def render_xml_attributes(node)

src/api/app/models/bs_request_action_submit.rb

@@ -60,7 +60,7 @@ def execute_accept(opts)
       linked_package = target_project.find_package(self.target_package)
       if linked_package
         # exists via project links
-        opts = { requestid: self.bs_request.number.to_s }
+        opts = { request: self.bs_request }
         opts[:makeoriginolder] = true if self.makeoriginolder
         instantiate_container(target_project, linked_package.update_instance, opts)
         target_package = target_project.packages.find_by_name(linked_package.name)
@@ -79,7 +79,7 @@ def execute_accept(opts)
           target_package.develpackage = Package.find_by_project_and_name( self.source_project, self.source_package )
           relinkSource=true
         end
-        target_package.store(comment: "submit request #{self.bs_request.number}", requestid: self.bs_request.number)
+        target_package.store(comment: "submit request #{self.bs_request.number}", request: self.bs_request)
       end
     end
 

src/api/app/models/package.rb

@@ -720,7 +720,8 @@ def write_to_backend
     if CONFIG['global_write_through'] && !@commit_opts[:no_backend_write]
       query = { user: User.current ? User.current.login : User.nobody_login }
       query[:comment] = @commit_opts[:comment] unless @commit_opts[:comment].blank?
-      query[:requestid] = @commit_opts[:requestid] unless @commit_opts[:requestid].blank?
+      # the request number is the requestid parameter in the backend api
+      query[:requestid] = @commit_opts[:request].number if @commit_opts[:request]
       Suse::Backend.put_source(self.source_path('_meta', query), to_axml)
       logger.tagged('backend_sync') { logger.debug "Saved Package #{self.project.name}/#{self.name}" }
     else
@@ -741,8 +742,10 @@ def delete_on_backend
 
     if CONFIG['global_write_through'] && !@commit_opts[:no_backend_write]
       path = source_path
-      path << Suse::Backend.build_query_from_hash({ user: User.current.login, comment: commit_opts[:comment], requestid: commit_opts[:request]},
-                                                  [:user, :comment, :requestid])
+
+      h = { user: User.current.login, comment: commit_opts[:comment] }
+      h[:requestid] = commit_opts[:request].number if commit_opts[:request]
+      path << Suse::Backend.build_query_from_hash(h, [:user, :comment, :requestid])
       begin
         Suse::Backend.delete path
       rescue ActiveXML::Transport::NotFoundError
@@ -1108,7 +1111,7 @@ def close_requests
     self.open_requests_with_package_as_source_or_target.each do |request|
       logger.debug "#{self.class} #{self.name} doing close_requests on request #{request.id} with #{@commit_opts.inspect}"
       # Don't alter the request that is the trigger of this close_requests run
-      next if request.number == @commit_opts[:request]
+      next if request == @commit_opts[:request]
 
       request.bs_request_actions.each do |action|
         if action.source_project == self.project.name && action.source_package == self.name

src/api/app/models/project.rb

@@ -192,7 +192,7 @@ def revoke_requests
     self.open_requests_with_project_as_source_or_target.each do |request|
       logger.debug "#{self.class} #{self.name} doing revoke_requests with #{@commit_opts.inspect}"
       # Don't alter the request that is the trigger of this revoke_requests run
-      next if request.number == @commit_opts[:request]
+      next if request == @commit_opts[:request]
 
       request.bs_request_actions.each do |action|
         if action.source_project == self.name
@@ -218,7 +218,7 @@ def revoke_requests
     # but leave the requests otherwise untouched.
     self.open_requests_with_by_project_review.each do |request|
       # Don't alter the request that is the trigger of this revoke_requests run
-      next if request.number == @commit_opts[:request]
+      next if request == @commit_opts[:request]
 
       request.obsolete_reviews(:by_project => self.name)
     end
@@ -839,7 +839,8 @@ def write_to_backend
       login = @commit_opts[:login] || User.current.login
       query = { user: login }
       query[:comment] = @commit_opts[:comment] unless @commit_opts[:comment].blank?
-      query[:requestid] = @commit_opts[:requestid] unless @commit_opts[:requestid].blank?
+      # api request number is requestid in backend
+      query[:requestid] = @commit_opts[:request].number if @commit_opts[:request]
       query[:lowprio] = '1' if @commit_opts[:lowprio]
       logger.debug "Writing #{self.name} to backend"
       Suse::Backend.put_source(self.source_path('_meta', query), to_axml)
@@ -858,7 +859,9 @@ def write_to_backend
   def delete_on_backend
     if CONFIG['global_write_through'] && !@commit_opts[:no_backend_write]
       path = source_path
-      path << Suse::Backend.build_query_from_hash({user: User.current.login, comment: @commit_opts[:comment]}, [:user, :comment])
+      h = {user: User.current.login, comment: @commit_opts[:comment]}
+      h[:requestid] = @commit_opts[:request].number if @commit_opts[:request]
+      path << Suse::Backend.build_query_from_hash(h, [:user, :comment, :requestid])
       begin
         Suse::Backend.delete path
       rescue ActiveXML::Transport::NotFoundError
@@ -1513,7 +1516,7 @@ def unlock_by_request(request)
     f = self.flags.find_by_flag_and_status('lock', 'enable')
     if f
       self.flags.delete(f)
-      self.store(comment: "Request got revoked", request: request.number, lowprio: 1)
+      self.store(comment: "Request got revoked", request: request, lowprio: 1)
     end
   end
 

src/api/test/functional/request_controller_test.rb

@@ -3076,6 +3076,12 @@ def test_project_delete_request_with_pending
     post "/request/#{iddelete}?cmd=changestate&newstate=accepted"
     assert_response :success
 
+    # verify request number loging
+    login_king
+    get '/source/home:Iggy:todo/_project/_history?meta=1&deleted=1'
+    assert_xml_tag(:parent => { :tag => 'revision' }, :tag => 'requestid', :content => iddelete)
+    assert_response :success
+
     # cleanup
     delete '/source/home:Iggy:todo'
     assert_response 404 # already removed

src/api/test/unit/code_quality_test.rb

@@ -74,16 +74,16 @@ def setup
       'BsRequestAction#create_expand_package'                                   => 443.16,
       'BsRequestAction#default_reviewers'                                       => 141.02,
       'BsRequestAction#store_from_xml'                                          => 88.01,
-      'BsRequestActionMaintenanceIncident#_merge_pkg_into_maintenance_incident' => 134.86,
+      'BsRequestActionMaintenanceIncident#_merge_pkg_into_maintenance_incident' => 130.81,
       'BsRequestActionMaintenanceRelease#sanity_check!'                         => 81.82,
-      'BsRequestActionSubmit#execute_accept'                                    => 132.43,
+      'BsRequestActionSubmit#execute_accept'                                    => 126.42,
       'BsRequestPermissionCheck#cmd_changestate_permissions'                    => 117.09,
       'RequestSourceDiff::ActionSourceDiffer#diff_for_source'                   => 94.62,
       'BuildController#file'                                                    => 127.42,
       'BuildController#project_index'                                           => 129.0,
       'ConfigurationsController#update'                                         => 85.63,
       'IssueTrackersController#update'                                          => 100.78,
-      'MaintenanceHelper#instantiate_container'                                 => 160.21,
+      'MaintenanceHelper#instantiate_container'                                 => 163.57,
       'Owner::_extract_from_container'                                          => 80.11,
       'Owner::search'                                                           => 80.51,
       'PersonController#internal_register'                                      => 112.01,