Commit c124eb8e7709f777e7f6e48cba3f74c8e9fdb055
1 parent
82dae706
Exists in
master
and in
29 other branches
organization: register and use members_count to fetch most_popular organizations
(ActionItem3039)
Showing
7 changed files
with
95 additions
and
12 deletions
Show diff stats
app/models/organization.rb
| @@ -26,11 +26,7 @@ class Organization < Profile | @@ -26,11 +26,7 @@ class Organization < Profile | ||
| 26 | 26 | ||
| 27 | has_many :mailings, :class_name => 'OrganizationMailing', :foreign_key => :source_id, :as => 'source' | 27 | has_many :mailings, :class_name => 'OrganizationMailing', :foreign_key => :source_id, :as => 'source' |
| 28 | 28 | ||
| 29 | - named_scope :more_popular, | ||
| 30 | - :select => "#{Profile.qualified_column_names}, count(resource_id) as total", | ||
| 31 | - :group => Profile.qualified_column_names, | ||
| 32 | - :joins => "LEFT OUTER JOIN role_assignments ON profiles.id = role_assignments.resource_id", | ||
| 33 | - :order => "total DESC" | 29 | + named_scope :more_popular, :order => 'members_count DESC' |
| 34 | 30 | ||
| 35 | def validation_methodology | 31 | def validation_methodology |
| 36 | self.validation_info ? self.validation_info.validation_methodology : nil | 32 | self.validation_info ? self.validation_info.validation_methodology : nil |
app/models/profile.rb
| @@ -87,10 +87,6 @@ class Profile < ActiveRecord::Base | @@ -87,10 +87,6 @@ class Profile < ActiveRecord::Base | ||
| 87 | scopes.size == 1 ? scopes.first : Person.or_scope(scopes) | 87 | scopes.size == 1 ? scopes.first : Person.or_scope(scopes) |
| 88 | end | 88 | end |
| 89 | 89 | ||
| 90 | - def members_count | ||
| 91 | - members.count | ||
| 92 | - end | ||
| 93 | - | ||
| 94 | class << self | 90 | class << self |
| 95 | def count_with_distinct(*args) | 91 | def count_with_distinct(*args) |
| 96 | options = args.last || {} | 92 | options = args.last || {} |
| @@ -0,0 +1 @@ | @@ -0,0 +1 @@ | ||
| 1 | +require 'noosfero/role_assignment_ext' |
db/migrate/20140313213142_define_initial_value_for_profiles_members_count.rb
0 → 100644
| @@ -0,0 +1,12 @@ | @@ -0,0 +1,12 @@ | ||
| 1 | +class DefineInitialValueForProfilesMembersCount < ActiveRecord::Migration | ||
| 2 | + def self.up | ||
| 3 | + members_counts = execute("SELECT profiles.id, count(profiles.id) FROM profiles LEFT OUTER JOIN role_assignments ON profiles.id = role_assignments.resource_id WHERE (profiles.type = 'Organization' OR profiles.type = 'Community' OR profiles.type = 'Enterprise') GROUP BY profiles.id;") | ||
| 4 | + members_counts.each do |count| | ||
| 5 | + execute("UPDATE profiles SET members_count=#{count['count'].to_i} WHERE profiles.id=#{count['id']};") | ||
| 6 | + end | ||
| 7 | + end | ||
| 8 | + | ||
| 9 | + def self.down | ||
| 10 | + execute("UPDATE profiles SET members_count=0;") | ||
| 11 | + end | ||
| 12 | +end |
| @@ -0,0 +1,29 @@ | @@ -0,0 +1,29 @@ | ||
| 1 | +Rails.configuration.to_prepare do | ||
| 2 | + RoleAssignment.module_eval do | ||
| 3 | + extend CacheCounterHelper | ||
| 4 | + | ||
| 5 | + after_create do |role_assignment| | ||
| 6 | + accessor = role_assignment.accessor | ||
| 7 | + resource = role_assignment.resource | ||
| 8 | + if resource.kind_of?(Organization) | ||
| 9 | + #FIXME This will only work as long as the role_assignment associations | ||
| 10 | + #happen only between profiles, due to the polymorphic column type. | ||
| 11 | + if resource.role_assignments.where(:accessor_id => accessor.id).count == 1 | ||
| 12 | + update_cache_counter(:members_count, resource, 1) | ||
| 13 | + end | ||
| 14 | + end | ||
| 15 | + end | ||
| 16 | + | ||
| 17 | + after_destroy do |role_assignment| | ||
| 18 | + accessor = role_assignment.accessor | ||
| 19 | + resource = role_assignment.resource | ||
| 20 | + if resource.kind_of?(Organization) | ||
| 21 | + #FIXME This will only work as long as the role_assignment associations | ||
| 22 | + #happen only between profiles, due to the polymorphic column type. | ||
| 23 | + if resource.role_assignments.where(:accessor_id => accessor.id).count == 0 | ||
| 24 | + update_cache_counter(:members_count, resource, -1) | ||
| 25 | + end | ||
| 26 | + end | ||
| 27 | + end | ||
| 28 | + end | ||
| 29 | +end |
test/unit/organization_test.rb
| @@ -302,18 +302,17 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -302,18 +302,17 @@ class OrganizationTest < ActiveSupport::TestCase | ||
| 302 | end | 302 | end |
| 303 | 303 | ||
| 304 | should 'find more popular organizations' do | 304 | should 'find more popular organizations' do |
| 305 | - Organization.delete_all | ||
| 306 | o1 = fast_create(Organization) | 305 | o1 = fast_create(Organization) |
| 307 | o2 = fast_create(Organization) | 306 | o2 = fast_create(Organization) |
| 308 | 307 | ||
| 309 | p1 = fast_create(Person) | 308 | p1 = fast_create(Person) |
| 310 | p2 = fast_create(Person) | 309 | p2 = fast_create(Person) |
| 311 | o1.add_member(p1) | 310 | o1.add_member(p1) |
| 312 | - assert_equal [o1,o2] , Organization.more_popular | 311 | + assert_order [o1,o2] , Organization.more_popular |
| 313 | 312 | ||
| 314 | o2.add_member(p1) | 313 | o2.add_member(p1) |
| 315 | o2.add_member(p2) | 314 | o2.add_member(p2) |
| 316 | - assert_equal [o2,o1] , Organization.more_popular | 315 | + assert_order [o2,o1] , Organization.more_popular |
| 317 | end | 316 | end |
| 318 | 317 | ||
| 319 | should 'list organizations that have no members in more popular list' do | 318 | should 'list organizations that have no members in more popular list' do |
| @@ -331,6 +330,7 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -331,6 +330,7 @@ class OrganizationTest < ActiveSupport::TestCase | ||
| 331 | person = fast_create(Person) | 330 | person = fast_create(Person) |
| 332 | organization = fast_create(Organization) | 331 | organization = fast_create(Organization) |
| 333 | organization.add_member(person) | 332 | organization.add_member(person) |
| 333 | + organization.reload | ||
| 334 | 334 | ||
| 335 | assert_equal "one member", organization.more_popular_label | 335 | assert_equal "one member", organization.more_popular_label |
| 336 | end | 336 | end |
| @@ -342,10 +342,12 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -342,10 +342,12 @@ class OrganizationTest < ActiveSupport::TestCase | ||
| 342 | 342 | ||
| 343 | organization.add_member(person1) | 343 | organization.add_member(person1) |
| 344 | organization.add_member(person2) | 344 | organization.add_member(person2) |
| 345 | + organization.reload | ||
| 345 | assert_equal "2 members", organization.more_popular_label | 346 | assert_equal "2 members", organization.more_popular_label |
| 346 | 347 | ||
| 347 | person3 = fast_create(Person) | 348 | person3 = fast_create(Person) |
| 348 | organization.add_member(person3) | 349 | organization.add_member(person3) |
| 350 | + organization.reload | ||
| 349 | assert_equal "3 members", organization.more_popular_label | 351 | assert_equal "3 members", organization.more_popular_label |
| 350 | end | 352 | end |
| 351 | 353 | ||
| @@ -433,5 +435,24 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -433,5 +435,24 @@ class OrganizationTest < ActiveSupport::TestCase | ||
| 433 | end | 435 | end |
| 434 | end | 436 | end |
| 435 | 437 | ||
| 438 | + should 'increase members_count on new membership' do | ||
| 439 | + member = fast_create(Person) | ||
| 440 | + organization = fast_create(Organization) | ||
| 441 | + assert_difference organization, :members_count, 1 do | ||
| 442 | + organization.add_member(member) | ||
| 443 | + organization.reload | ||
| 444 | + end | ||
| 445 | + end | ||
| 446 | + | ||
| 447 | + should 'decrease members_count on membership removal' do | ||
| 448 | + member = fast_create(Person) | ||
| 449 | + organization = fast_create(Organization) | ||
| 450 | + organization.add_member(member) | ||
| 451 | + organization.reload | ||
| 452 | + assert_difference organization, :members_count, -1 do | ||
| 453 | + organization.remove_member(member) | ||
| 454 | + organization.reload | ||
| 455 | + end | ||
| 456 | + end | ||
| 436 | 457 | ||
| 437 | end | 458 | end |
| @@ -0,0 +1,28 @@ | @@ -0,0 +1,28 @@ | ||
| 1 | +require File.dirname(__FILE__) + '/../test_helper' | ||
| 2 | + | ||
| 3 | +class RoleAssignmentExtTest < ActiveSupport::TestCase | ||
| 4 | + should 'increase organization members_count only on the first role_assignment' do | ||
| 5 | + role1 = Role.create!(:name => 'role1') | ||
| 6 | + role2 = Role.create!(:name => 'role2') | ||
| 7 | + member = create_user('person').person | ||
| 8 | + organization = Organization.create!(:name => 'Organization', :identifier => 'organization') | ||
| 9 | + assert_difference organization, :members_count, 1 do | ||
| 10 | + RoleAssignment.create!(:accessor => member, :resource => organization, :role => role1) | ||
| 11 | + RoleAssignment.create!(:accessor => member, :resource => organization, :role => role2) | ||
| 12 | + organization.reload | ||
| 13 | + end | ||
| 14 | + end | ||
| 15 | + | ||
| 16 | + should 'decrease organization members_count only on the last role_assignment' do | ||
| 17 | + role1 = Role.create!(:name => 'role1') | ||
| 18 | + role2 = Role.create!(:name => 'role2') | ||
| 19 | + member = create_user('person').person | ||
| 20 | + organization = Organization.create!(:name => 'Organization', :identifier => 'organization') | ||
| 21 | + RoleAssignment.create!(:accessor => member, :resource => organization, :role => role1) | ||
| 22 | + RoleAssignment.create!(:accessor => member, :resource => organization, :role => role2) | ||
| 23 | + assert_difference organization, :members_count, -1 do | ||
| 24 | + organization.role_assignments.destroy_all | ||
| 25 | + organization.reload | ||
| 26 | + end | ||
| 27 | + end | ||
| 28 | +end |