Merge pull request #1393 from adrianschroeter/fix_expand_packages

Fix expand packages

src/api/app/controllers/source_controller.rb

@@ -122,12 +122,15 @@ def render_project_packages
     if params.has_key? :expand
       packages = @project.expand_all_packages
     else
-      packages = @project.packages.pluck(:name, :project_id)
+      packages = @project.packages.pluck(:name)
     end
-    packages = @project.map_packages_to_projects(packages)
     output = String.new
     output << "<directory count='#{packages.length}'>\n"
-    output << packages.map { |p| p[1].nil? ? "  <entry name=\"#{p[0]}\"/>\n" : "  <entry name=\"#{p[0]}\" originproject=\"#{p[1]}\"/>\n" }.join
+    if params.has_key? :expand
+      output << packages.map { |p| "  <entry name=\"#{p[0]}\" originproject=\"#{p[1]}\"/>\n" }.join
+    else
+      output << packages.map { |p| "  <entry name=\"#{p}\"/>\n" }.join
+    end
     output << "</directory>\n"
     output
   end

src/api/app/controllers/webui/project_controller.rb

@@ -891,10 +891,10 @@ def set_maintained_project
 
   def load_project_info
     find_maintenance_infos
-    @ipackages = @project.packages_from_linked_projects
     @packages = @project.packages.order_by_name.pluck(:name)
-
+    @ipackages = @project.expand_all_packages.find_all{ |p| not @packages.include?(p[0]) }
     @linking_projects = @project.find_linking_projects.map { |p| p.name }
+
     reqs = @project.request_ids_by_class
     @requests = (reqs['reviews'] + reqs['targets'] + reqs['incidents'] + reqs['maintenance_release']).sort.uniq
 

src/api/app/models/project.rb

@@ -1103,37 +1103,28 @@ def expand_maintained_projects
     return projects
   end
 
-  def packages_from_linked_projects
-    Package.
-      joins('LEFT OUTER JOIN linked_projects ON packages.project_id = linked_projects.linked_db_project_id').
-      where('linked_projects.db_project_id = ? AND packages.name NOT IN (?)', id, packages.pluck(:name)).
-      order('LOWER(packages.name) ASC, linked_projects.position ASC').
-      includes(:project).
-      pluck("packages.name", "projects.name").to_a.uniq(&:first)
-  end
-
   # return array of [:name, :project_id] tuples
-  def expand_all_packages(project_map = {}, package_map = {})
+  def expand_all_packages(packages = [], project_map = {}, package_map = {})
     # check for project link cycle
     return [] if project_map[self]
     project_map[self] = 1
 
-    packages = self.packages.pluck(:name, :project_id)
+    self.packages.joins(:project).pluck(:name, "projects.name").each do |name, prj_name|
+      next if package_map[name]
+      packages << [name, prj_name]
+      package_map[name] = 1
+    end
 
     # second path, all packages from indirect linked projects
     self.linkedprojects.each do |lp|
       if lp.linked_db_project.nil?
         # FIXME: this is a remote project
       else
-        lp.linked_db_project.expand_all_packages(project_map, package_map).each do |name, prj_id|
-          next if package_map[name]
-          packages << [name, prj_id]
-          package_map[name] = 1
-        end
+        lp.linked_db_project.expand_all_packages(packages, project_map, package_map)
       end
     end
 
-    return packages
+    packages.sort!{ |a, b| a.first.downcase <=> b.first.downcase }
   end
 
   # return array of [:name, :package_id] tuples for all products
@@ -1159,26 +1150,6 @@ def expand_all_products
     return products
   end
 
-  # this is needed to displaying package and project names
-  # packages is an array of :name, :db_project_id
-  # return [package_name, project_name] where project_name is nil
-  # if the project is local
-  def map_packages_to_projects(packages)
-    prj_names = Hash.new
-    Project.where(id: packages.map { |a| a[1] }.uniq).pluck(:id, :name).each do |id, name|
-      prj_names[id] = name
-    end
-    ret = []
-    packages.each do |name, prj_id|
-      if prj_id==self.id
-        ret << [name, nil]
-      else
-        ret << [name, prj_names[prj_id]]
-      end
-    end
-    ret
-  end
-
   def add_repository_with_targets(repoName, source_repo, add_target_repos = [])
     return if self.repositories.where(name: repoName).exists?
     trepo = self.repositories.create :name => repoName

src/api/db/migrate/20151030130011_mark_events.rb

@@ -6,6 +6,7 @@ def up
 
     # unless there is a delayed job for it
     Delayed::Job.where("handler like '%ruby/object:SendEventEmails%'").each do |j|
+      next unless j.payload_object.event
       j.payload_object.event.update(mails_sent: false)
     end
 

src/api/script/start_test_backend

@@ -294,7 +294,6 @@ ActiveRecord::Base.transaction(requires_new: true) do
   Package.all.each{ |pkg| pkg.sources_changed }
   UpdatePackageMetaJob.new.perform
 end
-Project.find_by_name('BaseDistro').update_product_autopackages
 
 
 @http_user = nil

src/api/test/fixtures/packages.yml

@@ -140,6 +140,17 @@ BaseDistro_product:
   activity_index: 10.0
   delta: 1
   project: BaseDistro
+BaseDistro_product_fixed_release:
+  name: _product:fixed-release
+  title: Our single product
+  description: ''
+  created_at: 2007-05-16 16:19:18.000000000 Z
+  updated_at: 2007-05-16 16:19:18.000000000 Z
+  url: ''
+  update_counter: 1
+  activity_index: 10.0
+  delta: 1
+  project: BaseDistro
 BaseDistro_patchinfo:
   name: patchinfo
   title: patchinfo container

src/api/test/functional/webui/project_controller_test.rb

@@ -33,16 +33,19 @@ def test_project_show
     page.must_have_selector '#project_title'
   end
 
+  uses_transaction :test_project_show_inherited_packages
   def test_project_show_inherited_packages
     use_js
     visit project_show_path(project: 'BaseDistro:Update')
     page.must_have_selector '#project_title'
     click_link("Inherited Packages")
     within "table#ipackages_wrapper_table" do
       assert_equal "_product", find(:xpath, '(.//td/a)[1]').text
-      assert_equal "pack1", find(:xpath, '(.//td/a)[2]').text
-      assert_equal "Pack3", find(:xpath, '(.//td/a)[3]').text
-      assert_equal "patchinfo", find(:xpath, '(.//td/a)[4]').text
+      assert_equal "_product:fixed-release", find(:xpath, '(.//td/a)[2]').text
+      assert_equal "pack1", find(:xpath, '(.//td/a)[3]').text
+      # "pack2" is filtered since it exists in :Update project
+      assert_equal "Pack3", find(:xpath, '(.//td/a)[4]').text
+      assert_equal "patchinfo", find(:xpath, '(.//td/a)[5]').text
     end
   end
 

src/api/test/unit/code_quality_test.rb

@@ -102,9 +102,6 @@ def setup
       'Webui::PatchinfoController#save'                                         => 240.1,
       'Webui::ProjectController#check_devel_package_status'                     => 81.95,
       'Webui::SearchController#set_parameters'                                  => 98.04,
-      'Webui::WebuiHelper#flag_status'                                          => 93.0,
-      'Webui::WebuiController#do_backend_login'                                 => 96.31,
-      'Webui::WebuiController#check_user'                                       => 88.41,
       'WizardController#package_wizard'                                         => 97.46
   }
 

src/api/test/unit/project_test.rb

@@ -855,15 +855,21 @@ def test_cycle_handling
   test 'linked_packages returns all packages from projects inherited by one level' do
     child = projects('BaseDistro2.0_LinkedUpdateProject')
 
-    assert_equal child.packages_from_linked_projects, [['pack2', 'BaseDistro2.0'], ['pack2.linked', 'BaseDistro2.0']]
+    assert_equal [["pack2", "BaseDistro2.0"], ["pack2.linked", "BaseDistro2.0"],
+                  ["pack_local", "BaseDistro2.0:LinkedUpdateProject"]],
+                 child.expand_all_packages
   end
 
-  test 'linked_packages returns all packages from projects inherited by two levels' do
+  def test_all_packages_from_projects_inherited_by_two_levels_and_two_links_in_project
     CONFIG['global_write_through'] = false
     parent2 = projects('BaseDistro2.0')
     parent1 = projects('BaseDistro2.0_LinkedUpdateProject')
     child = projects('Apache')
 
+    parent2.linkedprojects.create(project: parent2,
+                               linked_db_project_id: projects('home_Iggy').id,
+                               position: 1)
+
     child.linkedprojects.create(project: child,
                                linked_db_project_id: parent1.id,
                                position: 1)
@@ -872,10 +878,10 @@ def test_cycle_handling
                                 linked_db_project_id: parent2.id,
                                 position: 2)
 
-    result =  parent1.packages + parent2.packages
+    result = projects('home_Iggy').packages + child.packages + parent1.packages + parent2.packages
     result.sort! { |a, b| a.name.downcase <=> b.name.downcase }.map! { |package| [package.name, package.project.name] }
 
-    assert_equal child.packages_from_linked_projects, result
+    assert_equal result, child.expand_all_packages
     CONFIG['global_write_through'] = true
   end
 
@@ -887,7 +893,10 @@ def test_cycle_handling
     pack2 = parent.packages.where(name: 'pack2').first
     child.packages << pack2.dup
 
-    assert_equal child.packages_from_linked_projects, [['pack2.linked', 'BaseDistro2.0']]
+    assert_equal [["pack2", "BaseDistro2.0:LinkedUpdateProject"],
+                  ["pack2.linked", "BaseDistro2.0"],
+                  ["pack_local", "BaseDistro2.0:LinkedUpdateProject"]],
+                 child.expand_all_packages
     CONFIG['global_write_through'] = true
   end
 
@@ -908,10 +917,10 @@ def test_cycle_handling
     pack2 = parent2.packages.where(name: 'pack2').first
     child.packages << pack2.dup
 
-    result = parent1.packages + parent2.packages.where(name: 'pack2.linked')
+    result = child.packages + parent1.packages + parent2.packages.where(name: 'pack2.linked')
     result.sort! { |a, b| a.name.downcase <=> b.name.downcase }.map! { |package| [package.name, package.project.name] }
 
-    assert_equal child.packages_from_linked_projects, result
+    assert_equal result, child.expand_all_packages
     CONFIG['global_write_through'] = true
   end
 
@@ -933,10 +942,10 @@ def test_cycle_handling
     pack2 = base_distro.packages.where(name: 'pack2').first
     base_distro_update.packages << pack2.dup
 
-    result = base_distro_update.packages + base_distro.packages.where(name: 'pack2.linked')
+    result = child.packages + base_distro_update.packages + base_distro.packages.where(name: 'pack2.linked')
     result.sort! { |a, b| a.name.downcase <=> b.name.downcase }.map! { |package| [package.name, package.project.name] }
 
-    assert_equal child.packages_from_linked_projects, result
+    assert_equal result, child.expand_all_packages
     CONFIG['global_write_through'] = true
   end