Commit 51455693ad9539de823d3b5caa0c8f581e71787d

Authored by Antonio Terceiro
1 parent 4817e9d4

Database access optimization

  * Rewrite ProfileListBlock logic to become simpler and faster
  * Rewrite membership logic to use named scopes instead of loading the
    entire list and doing filters with Ruby code.
  * Moved Noosfero-specific membership logic from access control plugin to
    Noosfero models.
  * Removed useless tests
  * Fixed tests that assumed the slow implementation
app/controllers/my_profile/profile_members_controller.rb
@@ -10,7 +10,11 @@ class ProfileMembersController < MyProfileController @@ -10,7 +10,11 @@ class ProfileMembersController < MyProfileController
10 def update_roles 10 def update_roles
11 @roles = params[:roles] ? environment.roles.find(params[:roles].select{|r|!r.to_i.zero?}) : [] 11 @roles = params[:roles] ? environment.roles.find(params[:roles].select{|r|!r.to_i.zero?}) : []
12 @roles = @roles.select{|r| r.has_kind?('Profile') } 12 @roles = @roles.select{|r| r.has_kind?('Profile') }
13 - @person = profile.members.find { |m| m.id == params[:person].to_i } 13 + begin
  14 + @person = profile.members.find(params[:person])
  15 + rescue ActiveRecord::RecordNotFound
  16 + @person = nil
  17 + end
14 if @person && @person.define_roles(@roles, profile) 18 if @person && @person.define_roles(@roles, profile)
15 session[:notice] = _('Roles successfuly updated') 19 session[:notice] = _('Roles successfuly updated')
16 else 20 else
@@ -21,7 +25,11 @@ class ProfileMembersController < MyProfileController @@ -21,7 +25,11 @@ class ProfileMembersController < MyProfileController
21 25
22 def change_role 26 def change_role
23 @roles = Profile::Roles.organization_member_roles(environment.id) 27 @roles = Profile::Roles.organization_member_roles(environment.id)
24 - @member = profile.members.find { |m| m.id == params[:id].to_i } 28 + begin
  29 + @member = profile.members.find(params[:id])
  30 + rescue ActiveRecord::RecordNotFound
  31 + @member = nil
  32 + end
25 if @member 33 if @member
26 @associations = @member.find_roles(@profile) 34 @associations = @member.find_roles(@profile)
27 else 35 else
app/models/communities_block.rb
@@ -32,28 +32,8 @@ class CommunitiesBlock < ProfileListBlock @@ -32,28 +32,8 @@ class CommunitiesBlock < ProfileListBlock
32 end 32 end
33 end 33 end
34 34
35 - def profile_count  
36 - if owner.kind_of?(Environment)  
37 - owner.communities.count(:conditions => { :visible => true })  
38 - else  
39 - owner.communities(:visible => true).count  
40 - end  
41 - end  
42 -  
43 - def profile_finder  
44 - @profile_finder ||= CommunitiesBlock::Finder.new(self)  
45 - end  
46 -  
47 - class Finder < ProfileListBlock::Finder  
48 - def ids  
49 - # FIXME when owner is an environment (i.e. listing communities globally  
50 - # this can become SLOW)  
51 - if block.owner.kind_of?(Environment)  
52 - block.owner.communities.all(:conditions => {:visible => true}, :limit => block.limit, :order => Noosfero::SQL.random_function).map(&:id)  
53 - else  
54 - block.owner.communities(:visible => true).map(&:id)  
55 - end  
56 - end 35 + def profiles
  36 + owner.communities
57 end 37 end
58 38
59 end 39 end
app/models/enterprises_block.rb
@@ -28,29 +28,8 @@ class EnterprisesBlock &lt; ProfileListBlock @@ -28,29 +28,8 @@ class EnterprisesBlock &lt; ProfileListBlock
28 end 28 end
29 end 29 end
30 30
31 - def profile_count  
32 - if owner.kind_of?(Environment)  
33 - owner.enterprises.count(:conditions => { :visible => true })  
34 - else  
35 - owner.enterprises(:visible => true).count  
36 - end  
37 -  
38 - end  
39 -  
40 - def profile_finder  
41 - @profile_finder ||= EnterprisesBlock::Finder.new(self)  
42 - end  
43 -  
44 - class Finder < ProfileListBlock::Finder  
45 - def ids  
46 - # FIXME when owner is an environment (i.e. listing enterprises globally  
47 - # this can become SLOW)  
48 - if block.owner.kind_of?(Environment)  
49 - block.owner.enterprises.all(:conditions => {:visible => true}, :limit => block.limit, :order => Noosfero::SQL.random_function).map(&:id)  
50 - else  
51 - block.owner.enterprises.select(&:visible).map(&:id)  
52 - end  
53 - end 31 + def profiles
  32 + owner.enterprises
54 end 33 end
55 34
56 end 35 end
app/models/environment.rb
@@ -70,7 +70,7 @@ class Environment &lt; ActiveRecord::Base @@ -70,7 +70,7 @@ class Environment &lt; ActiveRecord::Base
70 end 70 end
71 71
72 def admins 72 def admins
73 - self.members_by_role(Environment::Roles.admin(self.id)) 73 + Person.members_of(self).all(:conditions => ['role_assignments.role_id = ?', Environment::Roles.admin(self).id])
74 end 74 end
75 75
76 # returns the available features for a Environment, in the form of a 76 # returns the available features for a Environment, in the form of a
app/models/favorite_enterprises_block.rb
@@ -20,18 +20,8 @@ class FavoriteEnterprisesBlock &lt; ProfileListBlock @@ -20,18 +20,8 @@ class FavoriteEnterprisesBlock &lt; ProfileListBlock
20 end 20 end
21 end 21 end
22 22
23 - def profile_count  
24 - owner.favorite_enterprises.count  
25 - end  
26 -  
27 - def profile_finder  
28 - @profile_finder ||= FavoriteEnterprisesBlock::Finder.new(self)  
29 - end  
30 -  
31 - class Finder < ProfileListBlock::Finder  
32 - def ids  
33 - block.owner.favorite_enterprises.map(&:id)  
34 - end 23 + def profiles
  24 + owner.favorite_enterprises
35 end 25 end
36 26
37 end 27 end
app/models/friends_block.rb
@@ -19,18 +19,8 @@ class FriendsBlock &lt; ProfileListBlock @@ -19,18 +19,8 @@ class FriendsBlock &lt; ProfileListBlock
19 end 19 end
20 end 20 end
21 21
22 - class FriendsBlock::Finder < ProfileListBlock::Finder  
23 - def ids  
24 - self.block.owner.friend_ids  
25 - end  
26 - end  
27 -  
28 - def profile_finder  
29 - @profile_finder ||= FriendsBlock::Finder.new(self)  
30 - end  
31 -  
32 - def profile_count  
33 - owner.friends.visible.count 22 + def profiles
  23 + owner.friends
34 end 24 end
35 25
36 end 26 end
app/models/members_block.rb
@@ -19,20 +19,8 @@ class MembersBlock &lt; ProfileListBlock @@ -19,20 +19,8 @@ class MembersBlock &lt; ProfileListBlock
19 end 19 end
20 end 20 end
21 21
22 - def profile_count  
23 - owner.members.select {|member| member.visible? }.count 22 + def profiles
  23 + owner.members
24 end 24 end
25 25
26 - def profile_finder  
27 - @profile_finder ||= MembersBlock::Finder.new(self)  
28 - end  
29 -  
30 - # Finds random members, up to the limit.  
31 - class Finder < ProfileListBlock::Finder  
32 - def ids  
33 - block.owner.members.select {|member| member.visible? }.map(&:id)  
34 - end  
35 - end  
36 -  
37 -  
38 end 26 end
app/models/people_block.rb
@@ -12,14 +12,8 @@ class PeopleBlock &lt; ProfileListBlock @@ -12,14 +12,8 @@ class PeopleBlock &lt; ProfileListBlock
12 _('Random people') 12 _('Random people')
13 end 13 end
14 14
15 - def profile_finder  
16 - @profile_finder ||= PeopleBlock::Finder.new(self)  
17 - end  
18 -  
19 - class Finder < ProfileListBlock::Finder  
20 - def ids  
21 - block.owner.people.visible.all(:limit => block.limit, :order => Noosfero::SQL.random_function).map(&:id)  
22 - end 15 + def profiles
  16 + owner.people
23 end 17 end
24 18
25 def footer 19 def footer
@@ -28,8 +22,4 @@ class PeopleBlock &lt; ProfileListBlock @@ -28,8 +22,4 @@ class PeopleBlock &lt; ProfileListBlock
28 end 22 end
29 end 23 end
30 24
31 - def profile_count  
32 - owner.people.visible.count  
33 - end  
34 -  
35 end 25 end
app/models/person.rb
@@ -3,6 +3,12 @@ class Person &lt; Profile @@ -3,6 +3,12 @@ class Person &lt; Profile
3 3
4 acts_as_accessor 4 acts_as_accessor
5 5
  6 + named_scope :members_of, lambda { |resource| { :select => 'DISTINCT profiles.*', :joins => :role_assignments, :conditions => ['role_assignments.resource_type = ? AND role_assignments.resource_id = ?', resource.class.base_class.name, resource.id ] } }
  7 +
  8 + def memberships
  9 + Profile.memberships_of(self)
  10 + end
  11 +
6 has_many :friendships, :dependent => :destroy 12 has_many :friendships, :dependent => :destroy
7 has_many :friends, :class_name => 'Person', :through => :friendships 13 has_many :friends, :class_name => 'Person', :through => :friendships
8 14
@@ -132,27 +138,14 @@ class Person &lt; Profile @@ -132,27 +138,14 @@ class Person &lt; Profile
132 new_conditions 138 new_conditions
133 end 139 end
134 140
135 - def memberships(conditions = {})  
136 - # FIXME this should be a proper ActiveRecord relationship!  
137 - Profile.find(  
138 - :all,  
139 - :conditions => self.class.conditions_for_profiles(conditions, self),  
140 - :joins => "LEFT JOIN role_assignments ON profiles.id = role_assignments.resource_id AND role_assignments.resource_type = \'#{Profile.base_class.name}\'",  
141 - :select => 'profiles.*').uniq 141 + def enterprises
  142 + memberships.enterprises
142 end 143 end
143 144
144 - def enterprise_memberships(conditions = {})  
145 - memberships({:type => Enterprise.name}.merge(conditions)) 145 + def communities
  146 + memberships.communities
146 end 147 end
147 148
148 - alias :enterprises :enterprise_memberships  
149 -  
150 - def community_memberships(conditions = {})  
151 - memberships({:type => Community.name}.merge(conditions))  
152 - end  
153 -  
154 - alias :communities :community_memberships  
155 -  
156 validates_presence_of :user_id 149 validates_presence_of :user_id
157 validates_uniqueness_of :user_id 150 validates_uniqueness_of :user_id
158 151
app/models/profile.rb
@@ -51,6 +51,18 @@ class Profile &lt; ActiveRecord::Base @@ -51,6 +51,18 @@ class Profile &lt; ActiveRecord::Base
51 51
52 acts_as_accessible 52 acts_as_accessible
53 53
  54 + named_scope :memberships_of, lambda { |person| { :select => 'DISTINCT profiles.*', :joins => :role_assignments, :conditions => ['role_assignments.accessor_type = ? AND role_assignments.accessor_id = ?', person.class.base_class.name, person.id ] } }
  55 + named_scope :enterprises, :conditions => "profiles.type = 'Enterprise'"
  56 + named_scope :communities, :conditions => "profiles.type = 'Community'"
  57 +
  58 + def members
  59 + Person.members_of(self)
  60 + end
  61 +
  62 + def members_by_role(role)
  63 + Person.members_of(self).all(:conditions => ['role_assignments.role_id = ?', role.id])
  64 + end
  65 +
54 acts_as_having_boxes 66 acts_as_having_boxes
55 67
56 acts_as_searchable :additional_fields => [ :extra_data_for_index ] 68 acts_as_searchable :additional_fields => [ :extra_data_for_index ]
app/models/profile_list_block.rb
@@ -6,48 +6,18 @@ class ProfileListBlock &lt; Block @@ -6,48 +6,18 @@ class ProfileListBlock &lt; Block
6 _('Random profiles') 6 _('Random profiles')
7 end 7 end
8 8
9 - # Override this method to make the block list specific types of profiles  
10 - # instead of anyone.  
11 - #  
12 - # In this class this method just returns <tt>Profile</tt> (the class). In  
13 - # subclasses you could return <tt>Person</tt>, for instance, if you only want  
14 - # to list people, or <tt>Organization</tt>, if you want organizations only.  
15 - #  
16 - # You don't need to return only classes. You can for instance return an  
17 - # association array from a has_many ActiveRecord association, for example.  
18 - # Actually the only requirement for the object returned by this method is to  
19 - # have a <tt>find</tt> method that accepts the same interface as the  
20 - # ActiveRecord::Base's find method .  
21 - def profile_finder  
22 - @profile_finder ||= ProfileListBlock::Finder.new(self) 9 + # override in subclasses!
  10 + def profiles
  11 + owner.profiles
23 end 12 end
24 13
25 - # Default finder. Finds the most recently added profiles.  
26 - class Finder  
27 - def initialize(block)  
28 - @block = block  
29 - end  
30 - attr_reader :block  
31 - def find  
32 - id_list = self.ids  
33 - result = []  
34 - [block.limit, id_list.size].min.times do  
35 - i = pick_random(id_list.size)  
36 - result << Profile.find(id_list[i])  
37 - id_list.delete_at(i)  
38 - end  
39 - result  
40 - end  
41 - def pick_random(top)  
42 - rand(top)  
43 - end  
44 - def ids  
45 - block.owner.profiles.visible.all(:limit => block.limit, :order => Noosfero::SQL.random_function).map(&:id)  
46 - end 14 + def profile_list
  15 + random = Noosfero::SQL.random_function
  16 + profiles.visible.all(:limit => limit, :select => 'DISTINCT profiles.*, ' + random, :order => random)
47 end 17 end
48 18
49 - def profiles  
50 - profile_finder.find 19 + def profile_count
  20 + profiles.visible.count
51 end 21 end
52 22
53 # the title of the block. Probably will be overriden in subclasses. 23 # the title of the block. Probably will be overriden in subclasses.
@@ -60,7 +30,7 @@ class ProfileListBlock &lt; Block @@ -60,7 +30,7 @@ class ProfileListBlock &lt; Block
60 end 30 end
61 31
62 def content 32 def content
63 - profiles = self.profiles 33 + profiles = self.profile_list
64 title = self.view_title 34 title = self.view_title
65 nl = "\n" 35 nl = "\n"
66 link_method = profile_image_link_method 36 link_method = profile_image_link_method
@@ -89,8 +59,4 @@ class ProfileListBlock &lt; Block @@ -89,8 +59,4 @@ class ProfileListBlock &lt; Block
89 title.gsub('{#}', profile_count.to_s) 59 title.gsub('{#}', profile_count.to_s)
90 end 60 end
91 61
92 - def profile_count  
93 - owner.profiles.visible.count  
94 - end  
95 -  
96 end 62 end
app/views/blocks/my_network/person.rhtml
@@ -3,8 +3,8 @@ @@ -3,8 +3,8 @@
3 content_tag('b', owner.articles.count), owner.public_profile_url.merge(:action => 'sitemap') ) %></li> 3 content_tag('b', owner.articles.count), owner.public_profile_url.merge(:action => 'sitemap') ) %></li>
4 <li><%= link_to(n__('One friend', '%s friends', owner.friends.count) % 4 <li><%= link_to(n__('One friend', '%s friends', owner.friends.count) %
5 content_tag('b', owner.friends.count), owner.public_profile_url.merge(:action => 'friends')) %></li> 5 content_tag('b', owner.friends.count), owner.public_profile_url.merge(:action => 'friends')) %></li>
6 - <li><%= link_to(n__('One community', '%{num} communities', owner.communities(:visible => true).size) %  
7 - {:num => content_tag('b', owner.communities.size)}, owner.public_profile_url.merge(:action => 'communities')) %></li> 6 + <li><%= link_to(n__('One community', '%{num} communities', owner.communities.visible.size) %
  7 + {:num => content_tag('b', owner.communities.visible.size)}, owner.public_profile_url.merge(:action => 'communities')) %></li>
8 <li><%= link_to(n_('One tag', '%s tags', owner.article_tags.size) % 8 <li><%= link_to(n_('One tag', '%s tags', owner.article_tags.size) %
9 content_tag('b', owner.article_tags.size), owner.public_profile_url.merge(:action => 'tags')) %></li> 9 content_tag('b', owner.article_tags.size), owner.public_profile_url.merge(:action => 'tags')) %></li>
10 </ul> 10 </ul>
test/unit/add_member_test.rb
@@ -12,10 +12,9 @@ class AddMemberTest &lt; ActiveSupport::TestCase @@ -12,10 +12,9 @@ class AddMemberTest &lt; ActiveSupport::TestCase
12 c.update_attribute(:closed, true) 12 c.update_attribute(:closed, true)
13 TaskMailer.stubs(:deliver_target_notification) 13 TaskMailer.stubs(:deliver_target_notification)
14 task = fast_create(AddMember, :requestor_id => p.id, :target_id => c.id, :target_type => 'Community') 14 task = fast_create(AddMember, :requestor_id => p.id, :target_id => c.id, :target_type => 'Community')
15 - assert_difference c, :members, [p] do  
16 - task.finish  
17 - c.reload  
18 - end 15 + task.finish
  16 +
  17 + assert_equal [p], c.members
19 end 18 end
20 19
21 should 'require requestor' do 20 should 'require requestor' do
test/unit/communities_block_test.rb
@@ -15,32 +15,16 @@ class CommunitiesBlockTest &lt; Test::Unit::TestCase @@ -15,32 +15,16 @@ class CommunitiesBlockTest &lt; Test::Unit::TestCase
15 assert_not_equal ProfileListBlock.description, CommunitiesBlock.description 15 assert_not_equal ProfileListBlock.description, CommunitiesBlock.description
16 end 16 end
17 17
18 - should 'use its own finder' do  
19 - assert_not_equal CommunitiesBlock::Finder, ProfileListBlock::Finder  
20 - assert_kind_of CommunitiesBlock::Finder, CommunitiesBlock.new.profile_finder  
21 - end  
22 -  
23 should 'list owner communities' do 18 should 'list owner communities' do
24 -  
25 block = CommunitiesBlock.new 19 block = CommunitiesBlock.new
26 - block.limit = 2  
27 20
28 owner = mock 21 owner = mock
29 - block.expects(:owner).at_least_once.returns(owner)  
30 -  
31 - community1 = mock; community1.stubs(:id).returns(1); community1.stubs(:visible).returns(true)  
32 - community2 = mock; community2.stubs(:id).returns(2); community2.stubs(:visible).returns(true)  
33 - community3 = mock; community3.stubs(:id).returns(3); community3.stubs(:visible).returns(true) 22 + block.stubs(:owner).returns(owner)
34 23
35 - owner.expects(:communities).returns([community1, community2, community3])  
36 -  
37 - block.profile_finder.expects(:pick_random).with(3).returns(2)  
38 - block.profile_finder.expects(:pick_random).with(2).returns(0) 24 + list = []
  25 + owner.stubs(:communities).returns(list)
39 26
40 - Profile.expects(:find).with(3).returns(community3)  
41 - Profile.expects(:find).with(1).returns(community1)  
42 -  
43 - assert_equal [community3, community1], block.profiles 27 + assert_same list, block.profiles
44 end 28 end
45 29
46 should 'link to all communities of profile' do 30 should 'link to all communities of profile' do
@@ -85,86 +69,4 @@ class CommunitiesBlockTest &lt; Test::Unit::TestCase @@ -85,86 +69,4 @@ class CommunitiesBlockTest &lt; Test::Unit::TestCase
85 assert_equivalent [public_community, private_community], block.profiles 69 assert_equivalent [public_community, private_community], block.profiles
86 end 70 end
87 71
88 - should 'not list non-visible communities' do  
89 - user = create_user('testuser').person  
90 -  
91 - visible_community = fast_create(Community, :environment_id => Environment.default.id)  
92 - visible_community.add_member(user)  
93 -  
94 - not_visible_community = fast_create(Community, :environment_id => Environment.default.id, :visible => false)  
95 - not_visible_community.add_member(user)  
96 -  
97 - block = CommunitiesBlock.new  
98 - block.expects(:owner).at_least_once.returns(user)  
99 -  
100 - assert_equal [visible_community], block.profiles  
101 - end  
102 -  
103 - should 'count number of owner communities' do  
104 - user = create_user('testuser').person  
105 -  
106 - community1 = fast_create(Community, :environment_id => Environment.default.id, :visible => true)  
107 - community1.add_member(user)  
108 -  
109 - community2 = fast_create(Community, :environment_id => Environment.default.id, :visible => true)  
110 - community2.add_member(user)  
111 -  
112 - block = CommunitiesBlock.new  
113 - block.expects(:owner).at_least_once.returns(user)  
114 -  
115 - assert_equal 2, block.profile_count  
116 - end  
117 -  
118 - should 'count non-public profile communities' do  
119 - user = create_user('testuser').person  
120 -  
121 - community_public = fast_create(Community, :environment_id => Environment.default.id, :public_profile => true)  
122 - community_public.add_member(user)  
123 -  
124 - community_private = fast_create(Community, :public_profile => false)  
125 - community_private.add_member(user)  
126 -  
127 - block = CommunitiesBlock.new  
128 - block.expects(:owner).at_least_once.returns(user)  
129 -  
130 - assert_equal 2, block.profile_count  
131 - end  
132 -  
133 - should 'not count non-visible profile communities' do  
134 - user = create_user('testuser').person  
135 -  
136 - visible_community = fast_create(Community, :name => 'tcommunity 1', :identifier => 'comm1', :visible => true)  
137 - visible_community.add_member(user)  
138 -  
139 - not_visible_community = fast_create(Community, :name => ' community 2', :identifier => 'comm2', :visible => false)  
140 - not_visible_community.add_member(user)  
141 -  
142 - block = CommunitiesBlock.new  
143 - block.expects(:owner).at_least_once.returns(user)  
144 -  
145 - assert_equal 1, block.profile_count  
146 - end  
147 -  
148 - should 'count non-public environment communities' do  
149 - community_public = fast_create(Community, :name => 'tcommunity 1', :identifier => 'comm1', :public_profile => true)  
150 -  
151 - community_private = fast_create(Community, :name => ' community 2', :identifier => 'comm2', :public_profile => false)  
152 -  
153 - block = CommunitiesBlock.new  
154 - block.expects(:owner).at_least_once.returns(Environment.default)  
155 -  
156 - assert_equal 2, block.profile_count  
157 - end  
158 -  
159 - should 'not count non-visible environment communities' do  
160 - visible_community = fast_create(Community, :name => 'tcommunity 1', :identifier => 'comm1', :visible => true)  
161 -  
162 - not_visible_community = fast_create(Community, :name => ' community 2', :identifier => 'comm2', :visible => false)  
163 -  
164 - block = CommunitiesBlock.new  
165 - block.expects(:owner).at_least_once.returns(Environment.default)  
166 -  
167 - assert_equal 1, block.profile_count  
168 - end  
169 -  
170 end 72 end
test/unit/create_enterprise_test.rb
@@ -99,6 +99,7 @@ class CreateEnterpriseTest &lt; Test::Unit::TestCase @@ -99,6 +99,7 @@ class CreateEnterpriseTest &lt; Test::Unit::TestCase
99 should 'actually create an enterprise when finishing the task and associate the task requestor as its owner through the "user" association' do 99 should 'actually create an enterprise when finishing the task and associate the task requestor as its owner through the "user" association' do
100 100
101 environment = fast_create(Environment) 101 environment = fast_create(Environment)
  102 + environment.create_roles
102 region = fast_create(Region, :name => 'My region', :environment_id => environment.id) 103 region = fast_create(Region, :name => 'My region', :environment_id => environment.id)
103 validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) 104 validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id)
104 region.validators << validator 105 region.validators << validator
@@ -135,6 +136,7 @@ class CreateEnterpriseTest &lt; Test::Unit::TestCase @@ -135,6 +136,7 @@ class CreateEnterpriseTest &lt; Test::Unit::TestCase
135 should 'actually create an enterprise when finishing the task and associate the task requestor as its owner through the "user" association even when environment is not default' do 136 should 'actually create an enterprise when finishing the task and associate the task requestor as its owner through the "user" association even when environment is not default' do
136 137
137 environment = fast_create(Environment) 138 environment = fast_create(Environment)
  139 + environment.create_roles
138 region = fast_create(Region, :name => 'My region', :environment_id => environment.id) 140 region = fast_create(Region, :name => 'My region', :environment_id => environment.id)
139 validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) 141 validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id)
140 region.validators << validator 142 region.validators << validator
test/unit/enterprises_block_test.rb
@@ -15,85 +15,16 @@ class EnterprisesBlockTest &lt; Test::Unit::TestCase @@ -15,85 +15,16 @@ class EnterprisesBlockTest &lt; Test::Unit::TestCase
15 assert_not_equal ProfileListBlock.description, EnterprisesBlock.description 15 assert_not_equal ProfileListBlock.description, EnterprisesBlock.description
16 end 16 end
17 17
18 - should 'use its own finder' do  
19 - assert_not_equal EnterprisesBlock::Finder, ProfileListBlock::Finder  
20 - assert_kind_of EnterprisesBlock::Finder, EnterprisesBlock.new.profile_finder  
21 - end  
22 -  
23 should 'list owner enterprises' do 18 should 'list owner enterprises' do
24 block = EnterprisesBlock.new 19 block = EnterprisesBlock.new
25 - block.limit = 2  
26 20
27 owner = mock 21 owner = mock
28 block.expects(:owner).at_least_once.returns(owner) 22 block.expects(:owner).at_least_once.returns(owner)
29 23
30 - member1 = stub(:id => 1, :visible => true )  
31 - member2 = stub(:id => 2, :visible => true )  
32 - member3 = stub(:id => 3, :visible => true )  
33 -  
34 - owner.expects(:enterprises).returns([member1, member2, member3])  
35 -  
36 - block.profile_finder.expects(:pick_random).with(3).returns(2)  
37 - block.profile_finder.expects(:pick_random).with(2).returns(0)  
38 -  
39 - Profile.expects(:find).with(3).returns(member3)  
40 - Profile.expects(:find).with(1).returns(member1)  
41 -  
42 - assert_equal [member3, member1], block.profiles  
43 - end  
44 -  
45 - should 'list private enterprises in environment' do  
46 - env = Environment.create!(:name => 'test_env')  
47 - enterprise1 = fast_create(Enterprise, :environment_id => env.id, :public_profile => true)  
48 - enterprise2 = fast_create(Enterprise, :environment_id => env.id, :public_profile => false) #private profile  
49 - block = EnterprisesBlock.new  
50 - env.boxes.first.blocks << block  
51 - block.save!  
52 - ids = block.profile_finder.ids  
53 - assert_includes ids, enterprise1.id  
54 - assert_includes ids, enterprise2.id  
55 - end 24 + list = []
  25 + owner.expects(:enterprises).returns(list)
56 26
57 - should 'not list invisible enterprises in environment' do  
58 - env = Environment.create!(:name => 'test_env')  
59 - enterprise1 = fast_create(Enterprise, :environment_id => env.id, :visible => true)  
60 - enterprise2 = fast_create(Enterprise, :environment_id => env.id, :visible => false) #invisible profile  
61 - block = EnterprisesBlock.new  
62 - env.boxes.first.blocks << block  
63 - block.save!  
64 - ids = block.profile_finder.ids  
65 - assert_includes ids, enterprise1.id  
66 - assert_not_includes ids, enterprise2.id  
67 - end  
68 -  
69 - should 'list private enterprises in profile' do  
70 - person = create_user('testuser').person  
71 - enterprise1 = fast_create(Enterprise, :public_profile => true)  
72 - role = Profile::Roles.member(enterprise1.environment.id)  
73 - enterprise1.affiliate(person, role)  
74 - enterprise2 = fast_create(Enterprise, :public_profile => false)  
75 - enterprise2.affiliate(person, role)  
76 - block = EnterprisesBlock.new  
77 - person.boxes.first.blocks << block  
78 - block.save!  
79 - ids = block.profile_finder.ids  
80 - assert_includes ids, enterprise1.id  
81 - assert_includes ids, enterprise2.id  
82 - end  
83 -  
84 - should 'not list invisible enterprises in profile' do  
85 - person = create_user('testuser').person  
86 - enterprise1 = fast_create(Enterprise, :visible => true)  
87 - role = Profile::Roles.member(enterprise1.environment.id)  
88 - enterprise1.affiliate(person, role)  
89 - enterprise2 = fast_create(Enterprise, :visible => false)  
90 - enterprise2.affiliate(person, role)  
91 - block = EnterprisesBlock.new  
92 - person.boxes.first.blocks << block  
93 - block.save!  
94 - ids = block.profile_finder.ids  
95 - assert_includes ids, enterprise1.id  
96 - assert_not_includes ids, enterprise2.id 27 + assert_same list, block.profiles
97 end 28 end
98 29
99 should 'link to all enterprises for profile' do 30 should 'link to all enterprises for profile' do
@@ -139,61 +70,4 @@ class EnterprisesBlockTest &lt; Test::Unit::TestCase @@ -139,61 +70,4 @@ class EnterprisesBlockTest &lt; Test::Unit::TestCase
139 assert_equal 2, block.profile_count 70 assert_equal 2, block.profile_count
140 end 71 end
141 72
142 - should 'count non-public person enterprises' do  
143 - user = fast_create(Person)  
144 -  
145 - ent1 = fast_create(Enterprise, :public_profile => true)  
146 - ent1.expects(:closed?).returns(false)  
147 - ent1.add_member(user)  
148 -  
149 - ent2 = fast_create(Enterprise, :public_profile => false)  
150 - ent2.expects(:closed?).returns(false)  
151 - ent2.add_member(user)  
152 -  
153 - block = EnterprisesBlock.new  
154 - block.expects(:owner).at_least_once.returns(user)  
155 -  
156 - assert_equal 2, block.profile_count  
157 - end  
158 -  
159 - should 'not count non-visible person enterprises' do  
160 - user = fast_create(Person)  
161 -  
162 - ent1 = fast_create(Enterprise, :visible => true)  
163 - ent1.expects(:closed?).returns(false)  
164 - ent1.add_member(user)  
165 -  
166 - ent2 = fast_create(Enterprise, :visible => false)  
167 - ent2.expects(:closed?).returns(false)  
168 - ent2.add_member(user)  
169 -  
170 - block = EnterprisesBlock.new  
171 - block.expects(:owner).at_least_once.returns(user)  
172 -  
173 - assert_equal 1, block.profile_count  
174 - end  
175 -  
176 -  
177 - should 'count non-public environment enterprises' do  
178 - env = fast_create(Environment)  
179 - ent1 = fast_create(Enterprise, :environment_id => env.id, :public_profile => true)  
180 - ent2 = fast_create(Enterprise, :environment_id => env.id, :public_profile => false)  
181 -  
182 - block = EnterprisesBlock.new  
183 - block.expects(:owner).at_least_once.returns(env)  
184 -  
185 - assert_equal 2, block.profile_count  
186 - end  
187 -  
188 - should 'not count non-visible environment enterprises' do  
189 - env = fast_create(Environment)  
190 - ent1 = fast_create(Enterprise, :name => 'test enterprise 1', :identifier => 'ent1', :environment_id => env.id, :visible => true)  
191 - ent2 = fast_create(Enterprise, :name => 'test enterprise 2', :identifier => 'ent2', :environment_id => env.id, :visible => false)  
192 -  
193 - block = EnterprisesBlock.new  
194 - block.expects(:owner).at_least_once.returns(env)  
195 -  
196 - assert_equal 1, block.profile_count  
197 - end  
198 -  
199 end 73 end
test/unit/favorite_enterprises_block_test.rb
@@ -14,65 +14,17 @@ class FavoriteEnterprisesBlockTest &lt; ActiveSupport::TestCase @@ -14,65 +14,17 @@ class FavoriteEnterprisesBlockTest &lt; ActiveSupport::TestCase
14 assert_not_equal ProfileListBlock.description, FavoriteEnterprisesBlock.description 14 assert_not_equal ProfileListBlock.description, FavoriteEnterprisesBlock.description
15 end 15 end
16 16
17 - should 'use its own finder' do  
18 - assert_not_equal FavoriteEnterprisesBlock::Finder, ProfileListBlock::Finder  
19 - assert_kind_of FavoriteEnterprisesBlock::Finder, FavoriteEnterprisesBlock.new.profile_finder  
20 - end  
21 -  
22 should 'list owner favorite enterprises' do 17 should 'list owner favorite enterprises' do
23 18
24 block = FavoriteEnterprisesBlock.new 19 block = FavoriteEnterprisesBlock.new
25 - block.limit = 2  
26 20
27 owner = mock 21 owner = mock
28 block.expects(:owner).returns(owner) 22 block.expects(:owner).returns(owner)
29 23
30 - member1 = mock; member1.stubs(:id).returns(1)  
31 - member2 = mock; member2.stubs(:id).returns(2)  
32 - member3 = mock; member3.stubs(:id).returns(3)  
33 -  
34 - owner.expects(:favorite_enterprises).returns([member1, member2, member3]) 24 + list = []
  25 + owner.expects(:favorite_enterprises).returns(list)
35 26
36 - block.profile_finder.expects(:pick_random).with(3).returns(2)  
37 - block.profile_finder.expects(:pick_random).with(2).returns(0)  
38 -  
39 - Profile.expects(:find).with(3).returns(member3)  
40 - Profile.expects(:find).with(1).returns(member1)  
41 -  
42 - assert_equal [member3, member1], block.profiles  
43 - end  
44 -  
45 - should 'link to all enterprises for person' do  
46 - person = Person.new  
47 - person.expects(:identifier).returns('theprofile')  
48 - block = FavoriteEnterprisesBlock.new  
49 - block.expects(:owner).returns(person)  
50 -  
51 - expects(:__).with('View all').returns('View all enterprises')  
52 - expects(:link_to).with('View all enterprises', :controller => 'profile', :profile => 'theprofile', :action => 'favorite_enterprises')  
53 -  
54 - instance_eval(&block.footer)  
55 - end  
56 -  
57 - should 'give empty footer for unsupported owner type' do  
58 - block = FavoriteEnterprisesBlock.new  
59 - block.expects(:owner).returns(1)  
60 - assert_equal '', block.footer  
61 - end  
62 -  
63 - should 'count number of owner favorite enterprises' do  
64 - user = create_user('testuser').person  
65 -  
66 - ent1 = fast_create(Enterprise, :name => 'test enterprise 1', :identifier => 'ent1')  
67 -  
68 - ent2 = fast_create(Enterprise, :name => 'test enterprise 2', :identifier => 'ent2')  
69 -  
70 - user.favorite_enterprises << [ent1, ent2]  
71 -  
72 - block = FavoriteEnterprisesBlock.new  
73 - block.expects(:owner).at_least_once.returns(user)  
74 -  
75 - assert_equal 2, block.profile_count 27 + assert_same list, block.profiles
76 end 28 end
77 29
78 end 30 end
test/unit/friends_block_test.rb
@@ -11,11 +11,6 @@ class FriendsBlockTest &lt; ActiveSupport::TestCase @@ -11,11 +11,6 @@ class FriendsBlockTest &lt; ActiveSupport::TestCase
11 assert_not_equal ProfileListBlock.new.default_title, FriendsBlock.new.default_title 11 assert_not_equal ProfileListBlock.new.default_title, FriendsBlock.new.default_title
12 end 12 end
13 13
14 - should 'use its own finder' do  
15 - assert_not_equal ProfileListBlock::Finder, FriendsBlock::Finder  
16 - assert_kind_of FriendsBlock::Finder, FriendsBlock.new.profile_finder  
17 - end  
18 -  
19 should 'list owner friends' do 14 should 'list owner friends' do
20 p1 = create_user('testuser1').person 15 p1 = create_user('testuser1').person
21 p2 = create_user('testuser2').person 16 p2 = create_user('testuser2').person
test/unit/members_block_test.rb
@@ -28,74 +28,14 @@ class MembersBlockTest &lt; Test::Unit::TestCase @@ -28,74 +28,14 @@ class MembersBlockTest &lt; Test::Unit::TestCase
28 28
29 should 'pick random members' do 29 should 'pick random members' do
30 block = MembersBlock.new 30 block = MembersBlock.new
31 - block.limit = 2  
32 - block.save!  
33 31
34 owner = mock 32 owner = mock
35 block.expects(:owner).returns(owner) 33 block.expects(:owner).returns(owner)
36 34
37 - member1 = stub(:id => 1, :visible? => true)  
38 - member2 = stub(:id => 2, :visible? => true)  
39 - member3 = stub(:id => 3, :visible? => true)  
40 -  
41 - owner.expects(:members).returns([member1, member2, member3])  
42 -  
43 - block.profile_finder.expects(:pick_random).with(3).returns(2)  
44 - block.profile_finder.expects(:pick_random).with(2).returns(0)  
45 -  
46 - Profile.expects(:find).with(3).returns(member3)  
47 - Profile.expects(:find).with(1).returns(member1)  
48 -  
49 - assert_equal [member3, member1], block.profiles  
50 - end  
51 -  
52 - should 'count number of owner members' do  
53 - profile = create_user('mytestuser').person  
54 - owner = mock  
55 -  
56 - member1 = mock  
57 - member2 = mock  
58 - member3 = mock  
59 -  
60 - member1.stubs(:visible?).returns(true)  
61 - member2.stubs(:visible?).returns(true)  
62 - member3.stubs(:visible?).returns(true)  
63 -  
64 - owner.expects(:members).returns([member1, member2, member3]) 35 + list = []
  36 + owner.expects(:members).returns(list)
65 37
66 - block = MembersBlock.new  
67 - block.expects(:owner).returns(owner)  
68 - assert_equal 3, block.profile_count  
69 - end  
70 -  
71 - should 'count non-public community members' do  
72 - community = fast_create(Community)  
73 -  
74 - private_p = fast_create(Person, :public_profile => false)  
75 - public_p = fast_create(Person, :public_profile => true)  
76 -  
77 - community.add_member(private_p)  
78 - community.add_member(public_p)  
79 -  
80 - block = MembersBlock.new  
81 - block.expects(:owner).at_least_once.returns(community)  
82 -  
83 - assert_equal 2, block.profile_count  
84 - end  
85 -  
86 - should 'not count non-visible community members' do  
87 - community = fast_create(Community)  
88 -  
89 - private_p = fast_create(Person, :visible => false)  
90 - public_p = fast_create(Person, :visible => true)  
91 -  
92 - community.add_member(private_p)  
93 - community.add_member(public_p)  
94 -  
95 - block = MembersBlock.new  
96 - block.expects(:owner).at_least_once.returns(community)  
97 -  
98 - assert_equal 1, block.profile_count 38 + assert_same list, block.profiles
99 end 39 end
100 40
101 end 41 end
test/unit/people_block_test.rb
@@ -18,11 +18,6 @@ class PeopleBlockTest &lt; ActiveSupport::TestCase @@ -18,11 +18,6 @@ class PeopleBlockTest &lt; ActiveSupport::TestCase
18 assert_not_equal ProfileListBlock.new.help, PeopleBlock.new.help 18 assert_not_equal ProfileListBlock.new.help, PeopleBlock.new.help
19 end 19 end
20 20
21 - should 'use its own finder' do  
22 - assert_not_equal ProfileListBlock::Finder, PeopleBlock::Finder  
23 - assert_kind_of PeopleBlock::Finder, PeopleBlock.new.profile_finder  
24 - end  
25 -  
26 should 'list people' do 21 should 'list people' do
27 owner = fast_create(Environment) 22 owner = fast_create(Environment)
28 block = PeopleBlock.new 23 block = PeopleBlock.new
@@ -49,24 +44,6 @@ class PeopleBlockTest &lt; ActiveSupport::TestCase @@ -49,24 +44,6 @@ class PeopleBlockTest &lt; ActiveSupport::TestCase
49 instance_eval(&block.footer) 44 instance_eval(&block.footer)
50 end 45 end
51 46
52 - should 'count number of public and private people' do  
53 - env = Environment.create!(:name => 'test environment')  
54 - private_p = fast_create(Person, :environment_id => env.id, :public_profile => false)  
55 - public_p = fast_create(Person, :environment_id => env.id, :public_profile => true)  
56 -  
57 - env.boxes.first.blocks << block = PeopleBlock.new  
58 - assert_equal 2, block.profile_count  
59 - end  
60 -  
61 - should 'count number of visible people' do  
62 - env = Environment.create!(:name => 'test environment')  
63 - invisible_p = fast_create(Person, :environment_id => env.id, :visible => false)  
64 - visible_p = fast_create(Person, :environment_id => env.id, :visible => true)  
65 -  
66 - env.boxes.first.blocks << block = PeopleBlock.new  
67 - assert_equal 1, block.profile_count  
68 - end  
69 -  
70 protected 47 protected
71 48
72 def content_tag(tag, text, options = {}) 49 def content_tag(tag, text, options = {})
test/unit/person_test.rb
@@ -32,7 +32,7 @@ class PersonTest &lt; Test::Unit::TestCase @@ -32,7 +32,7 @@ class PersonTest &lt; Test::Unit::TestCase
32 e.affiliate(p, member_role) 32 e.affiliate(p, member_role)
33 33
34 assert p.memberships.include?(e) 34 assert p.memberships.include?(e)
35 - assert p.enterprise_memberships.include?(e) 35 + assert p.enterprises.include?(e)
36 end 36 end
37 37
38 should 'belong to communities' do 38 should 'belong to communities' do
@@ -41,7 +41,7 @@ class PersonTest &lt; Test::Unit::TestCase @@ -41,7 +41,7 @@ class PersonTest &lt; Test::Unit::TestCase
41 41
42 c.add_member(p) 42 c.add_member(p)
43 43
44 - assert p.community_memberships.include?(c), "Community should add a new member" 44 + assert p.communities.include?(c), "Community should add a new member"
45 end 45 end
46 46
47 should 'be associated with a user' do 47 should 'be associated with a user' do
test/unit/profile_list_block_test.rb
@@ -19,22 +19,17 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase @@ -19,22 +19,17 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase
19 end 19 end
20 20
21 should 'list people' do 21 should 'list people' do
22 - User.destroy_all  
23 - person1 = create_user('testperson1').person  
24 - person2 = create_user('testperson2').person  
25 - person3 = create_user('testperson3').person 22 + env = fast_create(Environment)
26 23
27 - owner = fast_create(Environment)  
28 - owner.boxes << Box.new  
29 - block = ProfileListBlock.new  
30 - owner.boxes.first.blocks << block  
31 - block.save! 24 + person1 = create_user('testperson1', :environment => env).person
  25 + person2 = create_user('testperson2', :environment => env).person
  26 + person3 = create_user('testperson3', :environment => env).person
32 27
33 - profiles = [person1, person3]  
34 - block.expects(:profiles).returns(profiles) 28 + block = ProfileListBlock.new
  29 + block.stubs(:owner).returns(env)
35 30
36 self.expects(:profile_image_link).with(person1).once 31 self.expects(:profile_image_link).with(person1).once
37 - self.expects(:profile_image_link).with(person2).never 32 + self.expects(:profile_image_link).with(person2).once
38 self.expects(:profile_image_link).with(person3).once 33 self.expects(:profile_image_link).with(person3).once
39 34
40 self.expects(:content_tag).returns('<div></div>').at_least_once 35 self.expects(:content_tag).returns('<div></div>').at_least_once
@@ -52,9 +47,9 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase @@ -52,9 +47,9 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase
52 env.boxes.first.blocks << block 47 env.boxes.first.blocks << block
53 block.save! 48 block.save!
54 49
55 - ids = block.profile_finder.ids  
56 - assert_includes ids, profile1.id  
57 - assert_includes ids, profile2.id 50 + profiles = block.profiles
  51 + assert_includes profiles, profile1
  52 + assert_includes profiles, profile2
58 end 53 end
59 54
60 should 'not list invisible profiles' do 55 should 'not list invisible profiles' do
@@ -66,21 +61,9 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase @@ -66,21 +61,9 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase
66 env.boxes.first.blocks << block 61 env.boxes.first.blocks << block
67 block.save! 62 block.save!
68 63
69 - ids = block.profile_finder.ids  
70 - assert_includes ids, profile1.id  
71 - assert_not_includes ids, profile2.id  
72 - end  
73 -  
74 - should 'use finders to find profiles to be listed' do  
75 - block = ProfileListBlock.new  
76 - finder = mock  
77 - block.expects(:profile_finder).returns(finder).once  
78 - finder.expects(:find)  
79 - block.profiles  
80 - end  
81 -  
82 - should 'provide random numbers' do  
83 - assert_respond_to ProfileListBlock::Finder.new(nil), :pick_random 64 + profiles = block.profile_list
  65 + assert_includes profiles, profile1
  66 + assert_not_includes profiles, profile2
84 end 67 end
85 68
86 should 'provide view_title' do 69 should 'provide view_title' do
@@ -139,4 +122,32 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase @@ -139,4 +122,32 @@ class ProfileListBlockTest &lt; Test::Unit::TestCase
139 assert_equal 3, block.profile_count 122 assert_equal 3, block.profile_count
140 end 123 end
141 124
  125 + should 'respect limit when listing profiles' do
  126 + env = fast_create(Environment)
  127 + p1 = fast_create(Person, :environment_id => env.id)
  128 + p2 = fast_create(Person, :environment_id => env.id)
  129 + p3 = fast_create(Person, :environment_id => env.id)
  130 + p4 = fast_create(Person, :environment_id => env.id)
  131 +
  132 + block = ProfileListBlock.new(:limit => 3)
  133 + block.stubs(:owner).returns(env)
  134 +
  135 + assert_equal 3, block.profile_list.size
  136 + end
  137 +
  138 + should 'list random profiles' do
  139 + env = fast_create(Environment)
  140 + p1 = fast_create(Person, :environment_id => env.id)
  141 + p2 = fast_create(Person, :environment_id => env.id)
  142 + p3 = fast_create(Person, :environment_id => env.id)
  143 +
  144 + # force the "random" function to be something we know
  145 + Noosfero::SQL.stubs(:random_function).returns('-id')
  146 +
  147 + block = ProfileListBlock.new
  148 + block.stubs(:owner).returns(env)
  149 +
  150 + assert_equal [p3.id, p2.id, p1.id], block.profile_list.map(&:id)
  151 + end
  152 +
142 end 153 end
test/unit/profile_test.rb
@@ -1771,7 +1771,7 @@ class ProfileTest &lt; Test::Unit::TestCase @@ -1771,7 +1771,7 @@ class ProfileTest &lt; Test::Unit::TestCase
1771 end 1771 end
1772 1772
1773 should "return the number of members on label if the profile has more than one member" do 1773 should "return the number of members on label if the profile has more than one member" do
1774 - p1 = fast_create(Profile) 1774 + p1 = fast_create(Person)
1775 p2 = fast_create(Person) 1775 p2 = fast_create(Person)
1776 c = fast_create(Community) 1776 c = fast_create(Community)
1777 c.add_member(p1) 1777 c.add_member(p1)
vendor/plugins/access_control/lib/acts_as_accessible.rb
@@ -24,13 +24,6 @@ class ActiveRecord::Base @@ -24,13 +24,6 @@ class ActiveRecord::Base
24 role_assignments.map{|ra|ra.destroy if roles.include?(ra.role) && ra.accessor == accessor} 24 role_assignments.map{|ra|ra.destroy if roles.include?(ra.role) && ra.accessor == accessor}
25 end 25 end
26 26
27 - def members  
28 - role_assignments.map(&:accessor).uniq  
29 - end  
30 - def members_by_role(role)  
31 - role_assignments.select{|i| i.role.key == role.key}.map(&:accessor).uniq  
32 - end  
33 -  
34 def roles 27 def roles
35 Role.find_all_by_environment_id(environment.id).select do |r| 28 Role.find_all_by_environment_id(environment.id).select do |r|
36 r.permissions.any?{ |p| PERMISSIONS[self.class.base_class.name].include?(p) } 29 r.permissions.any?{ |p| PERMISSIONS[self.class.base_class.name].include?(p) }