product-category: fetch only necessary categories

Instead of fetching every category to display the icons, fetch only 3 of them through the correct sql query. This also fixes a big memory leak. (ActionItem3084)

product-category: fetch only necessary categories
Instead of fetching every category to display the icons, fetch only 3 of them through the correct sql query. This also fixes a big memory leak. (ActionItem3084)
Rodrigo Souto
1 parent b0e5e0c9
Showing 7 changed files with 55 additions and 39 deletions Show diff stats
app/helpers/application_helper.rb
app/models/enterprise.rb
app/models/product.rb
app/models/product_category.rb
test/unit/enterprise_test.rb
test/unit/product_category_test.rb
test/unit/product_test.rb
@@ -514,24 +514,25 @@ module ApplicationHelper
   def profile_cat_icons( profile )
     if profile.class == Enterprise
-      icons = profile.product_categories.map{ |c| c.size > 1 ? c[1] : nil }.
-        compact.uniq.map do |c|
-          cat_name = c.gsub( /[-_\s,.;'"]+/, '_' )
-          cat_icon = "/images/icons-cat/#{cat_name}.png"
-          if ! File.exists? RAILS_ROOT.to_s() + '/public/' + cat_icon
-            cat_icon = '/images/icons-cat/undefined.png'
-          end
-          content_tag('span',
-            content_tag( 'span', c ),
-            :title => c,
-            :class => 'product-cat-icon cat_icon_' + cat_name,
-            :style => "background-image:url(#{cat_icon})"
-          )
-        end.join("\n").html_safe
-        content_tag('div',
-          content_tag( 'span', _('Principal Product Categories'), :class => 'header' ) +"\n"+ icons,
-          :class => 'product-category-icons'
+      icons = profile.product_categories.unique_by_level(2).limit(3).map do |c|
+        filtered_category = c.filtered_category.blank? ? c.path.split('/').last : c.filtered_category
+        category_title = filtered_category.split(/[-_\s,.;'"]+/).map(&:capitalize).join(' ')
+        category_name = category_title.gsub(' ', '_' )
+        category_icon = "/images/icons-cat/#{category_name}.png"
+        if ! File.exists? RAILS_ROOT.to_s() + '/public/' + category_icon
+          category_icon = '/images/icons-cat/undefined.png'
+        end
+        content_tag('span',
+          content_tag( 'span', category_title ),
+          :title => category_title,
+          :class => 'product-cat-icon cat_icon_' + category_name,
+          :style => "background-image:url(#{category_icon})"
         )
+      end.join("\n").html_safe
+      content_tag('div',
+        content_tag( 'span', _('Principal Product Categories'), :class => 'header' ) +"\n"+ icons,
+        :class => 'product-category-icons'
+      )
     else
       ''
     end
@@ -17,7 +17,7 @@ class Enterprise &lt; Organization
   has_and_belongs_to_many :fans, :class_name => 'Person', :join_table => 'favorite_enteprises_people'
   def product_categories
-    products.includes(:product_category).map{|p| p.category_full_name}.compact
+    ProductCategory.by_enterprise(self)
   end
   N_('Organization website'); N_('Historic and current context'); N_('Activities short description'); N_('City'); N_('State'); N_('Country'); N_('ZIP code')
@@ -88,10 +88,6 @@ class Product &lt; ActiveRecord::Base
     image ? image.public_filename(size) : '/images/icons-app/product-default-pic-%s.png' % size
   end
-  def category_full_name
-    product_category ? product_category.full_name.split('/') : nil
-  end
-
   acts_as_having_image
   def save_image
@@ -3,6 +3,15 @@ class ProductCategory &lt; Category
   has_many :products
   has_many :inputs
+  named_scope :unique, :select => 'DISTINCT ON (path) categories.*'
+  named_scope :by_enterprise, lambda { |enterprise| {
+    :joins => :products,
+    :conditions => ['products.profile_id = ?', enterprise.id]
+  }}
+  named_scope :unique_by_level, lambda { |level| {
+    :select => "DISTINCT ON (filtered_category) split_part(path, '/', #{level}) AS filtered_category, categories.*"
+  }}
+
   def all_products
     Product.find(:all, :conditions => { :product_category_id => (all_children << self).map(&:id) })
   end
@@ -234,12 +234,12 @@ class EnterpriseTest &lt; ActiveSupport::TestCase
     assert enterprise.enable(person)
   end
-  should 'list product categories full name' do
+  should 'list product categories' do
     subcategory = fast_create(ProductCategory, :name => 'Products subcategory', :parent_id => @product_category.id)
     ent = fast_create(Enterprise, :name => 'test ent', :identifier => 'test_ent')
     p = ent.products.create!(:name => 'test prod', :product_category => subcategory)
-    assert_equal [p.category_full_name], ent.product_categories
+    assert_equivalent [subcategory], ent.product_categories
   end
   should 'not create a products block for enterprise if environment do not let' do
@@ -36,4 +36,29 @@ class ProductCategoryTest &lt; ActiveSupport::TestCase
     assert_equal [c11], ProductCategory.menu_categories(c1, nil)
   end
+  should 'provide a scope based on the enterprise' do
+    enterprise = fast_create(Enterprise)
+    c1 = ProductCategory.create!(:name => 'test cat 1', :environment => Environment.default)
+    c2 = ProductCategory.create!(:name => 'test cat 2', :environment => Environment.default)
+    c3 = ProductCategory.create!(:name => 'test cat 3', :environment => Environment.default)
+    p1 = Product.create(:name => 'product1', :product_category => c1, :profile_id => enterprise.id)
+    p2 = Product.create(:name => 'product2', :product_category => c1, :profile_id => enterprise.id)
+    p3 = Product.create(:name => 'product3', :product_category => c2, :profile_id => enterprise.id)
+
+    scope = ProductCategory.by_enterprise(enterprise)
+
+    assert_equal ActiveRecord::NamedScope::Scope, scope.class
+    assert_equivalent [c1,c2], scope
+  end
+
+  should 'fetch unique categories by level' do
+    c1 = ProductCategory.create!(:name => 'test cat 1', :environment => Environment.default)
+    c11 = ProductCategory.create!(:name => 'test cat 11', :environment => Environment.default, :parent => c1)
+    c12 = ProductCategory.create!(:name => 'test cat 12', :environment => Environment.default, :parent => c1)
+    c111 = ProductCategory.create!(:name => 'test cat 111', :environment => Environment.default, :parent => c11)
+    c112 = ProductCategory.create!(:name => 'test cat 112', :environment => Environment.default, :parent => c11)
+
+    assert_equivalent ['', 'test-cat-11', 'test-cat-12'], ProductCategory.unique_by_level(2).map(&:filtered_category)
+  end
+
 end
@@ -89,21 +89,6 @@ class ProductTest &lt; ActiveSupport::TestCase
     end
   end
-  should 'calculate category full name' do
-    cat = mock
-    cat.expects(:full_name).returns('A/B/C')
-
-    p = Product.new
-    p.stubs(:product_category).returns(cat)
-    assert_equal ['A','B','C'], p.category_full_name
-  end
-
-  should 'return a nil cateory full name when not categorized' do
-    p = Product.new
-    p.stubs(:product_category).returns(nil)
-    assert_equal nil, p.category_full_name
-  end
-
   should 'have same lat and lng of its enterprise' do
     ent = fast_create(Enterprise, :name => 'test enterprise', :identifier => 'test_enterprise', :lat => 30.0, :lng => 30.0)
     prod = ent.products.create!(:name => 'test product', :product_category => @product_category)
	@@ -17,7 +17,7 @@ class Enterprise < Organization		@@ -17,7 +17,7 @@ class Enterprise < Organization
17	has_and_belongs_to_many :fans, :class_name => 'Person', :join_table => 'favorite_enteprises_people'	17	has_and_belongs_to_many :fans, :class_name => 'Person', :join_table => 'favorite_enteprises_people'
18		18
19	def product_categories	19	def product_categories
20	- products.includes(:product_category).map{\|p\| p.category_full_name}.compact	20	+ ProductCategory.by_enterprise(self)
21	end	21	end
22		22
23	N_('Organization website'); N_('Historic and current context'); N_('Activities short description'); N_('City'); N_('State'); N_('Country'); N_('ZIP code')	23	N_('Organization website'); N_('Historic and current context'); N_('Activities short description'); N_('City'); N_('State'); N_('Country'); N_('ZIP code')
	@@ -3,6 +3,15 @@ class ProductCategory < Category		@@ -3,6 +3,15 @@ class ProductCategory < Category
3	has_many :products	3	has_many :products
4	has_many :inputs	4	has_many :inputs
5		5
		6	+ named_scope :unique, :select => 'DISTINCT ON (path) categories.*'
		7	+ named_scope :by_enterprise, lambda { \|enterprise\| {
		8	+ :joins => :products,
		9	+ :conditions => ['products.profile_id = ?', enterprise.id]
		10	+ }}
		11	+ named_scope :unique_by_level, lambda { \|level\| {
		12	+ :select => "DISTINCT ON (filtered_category) split_part(path, '/', #{level}) AS filtered_category, categories.*"
		13	+ }}
		14	+
6	def all_products	15	def all_products
7	Product.find(:all, :conditions => { :product_category_id => (all_children << self).map(&:id) })	16	Product.find(:all, :conditions => { :product_category_id => (all_children << self).map(&:id) })
8	end	17	end
	@@ -234,12 +234,12 @@ class EnterpriseTest < ActiveSupport::TestCase		@@ -234,12 +234,12 @@ class EnterpriseTest < ActiveSupport::TestCase
234	assert enterprise.enable(person)	234	assert enterprise.enable(person)
235	end	235	end
236		236
237	- should 'list product categories full name' do	237	+ should 'list product categories' do
238	subcategory = fast_create(ProductCategory, :name => 'Products subcategory', :parent_id => @product_category.id)	238	subcategory = fast_create(ProductCategory, :name => 'Products subcategory', :parent_id => @product_category.id)
239	ent = fast_create(Enterprise, :name => 'test ent', :identifier => 'test_ent')	239	ent = fast_create(Enterprise, :name => 'test ent', :identifier => 'test_ent')
240	p = ent.products.create!(:name => 'test prod', :product_category => subcategory)	240	p = ent.products.create!(:name => 'test prod', :product_category => subcategory)
241		241
242	- assert_equal [p.category_full_name], ent.product_categories	242	+ assert_equivalent [subcategory], ent.product_categories
243	end	243	end
244		244
245	should 'not create a products block for enterprise if environment do not let' do	245	should 'not create a products block for enterprise if environment do not let' do
	@@ -36,4 +36,29 @@ class ProductCategoryTest < ActiveSupport::TestCase		@@ -36,4 +36,29 @@ class ProductCategoryTest < ActiveSupport::TestCase
36	assert_equal [c11], ProductCategory.menu_categories(c1, nil)	36	assert_equal [c11], ProductCategory.menu_categories(c1, nil)
37	end	37	end
38		38
		39	+ should 'provide a scope based on the enterprise' do
		40	+ enterprise = fast_create(Enterprise)
		41	+ c1 = ProductCategory.create!(:name => 'test cat 1', :environment => Environment.default)
		42	+ c2 = ProductCategory.create!(:name => 'test cat 2', :environment => Environment.default)
		43	+ c3 = ProductCategory.create!(:name => 'test cat 3', :environment => Environment.default)
		44	+ p1 = Product.create(:name => 'product1', :product_category => c1, :profile_id => enterprise.id)
		45	+ p2 = Product.create(:name => 'product2', :product_category => c1, :profile_id => enterprise.id)
		46	+ p3 = Product.create(:name => 'product3', :product_category => c2, :profile_id => enterprise.id)
		47	+
		48	+ scope = ProductCategory.by_enterprise(enterprise)
		49	+
		50	+ assert_equal ActiveRecord::NamedScope::Scope, scope.class
		51	+ assert_equivalent [c1,c2], scope
		52	+ end
		53	+
		54	+ should 'fetch unique categories by level' do
		55	+ c1 = ProductCategory.create!(:name => 'test cat 1', :environment => Environment.default)
		56	+ c11 = ProductCategory.create!(:name => 'test cat 11', :environment => Environment.default, :parent => c1)
		57	+ c12 = ProductCategory.create!(:name => 'test cat 12', :environment => Environment.default, :parent => c1)
		58	+ c111 = ProductCategory.create!(:name => 'test cat 111', :environment => Environment.default, :parent => c11)
		59	+ c112 = ProductCategory.create!(:name => 'test cat 112', :environment => Environment.default, :parent => c11)
		60	+
		61	+ assert_equivalent ['', 'test-cat-11', 'test-cat-12'], ProductCategory.unique_by_level(2).map(&:filtered_category)
		62	+ end
		63	+
39	end	64	end
	@@ -89,21 +89,6 @@ class ProductTest < ActiveSupport::TestCase		@@ -89,21 +89,6 @@ class ProductTest < ActiveSupport::TestCase
89	end	89	end
90	end	90	end
91		91
92	- should 'calculate category full name' do
93	- cat = mock
94	- cat.expects(:full_name).returns('A/B/C')
95	-
96	- p = Product.new
97	- p.stubs(:product_category).returns(cat)
98	- assert_equal ['A','B','C'], p.category_full_name
99	- end
100	-
101	- should 'return a nil cateory full name when not categorized' do
102	- p = Product.new
103	- p.stubs(:product_category).returns(nil)
104	- assert_equal nil, p.category_full_name
105	- end
106	-
107	should 'have same lat and lng of its enterprise' do	92	should 'have same lat and lng of its enterprise' do
108	ent = fast_create(Enterprise, :name => 'test enterprise', :identifier => 'test_enterprise', :lat => 30.0, :lng => 30.0)	93	ent = fast_create(Enterprise, :name => 'test enterprise', :identifier => 'test_enterprise', :lat => 30.0, :lng => 30.0)
109	prod = ent.products.create!(:name => 'test product', :product_category => @product_category)	94	prod = ent.products.create!(:name => 'test product', :product_category => @product_category)