Commit 451c0a9f61029d19738ae9a3f8add9ef57283aed
1 parent
3aa65efa
Exists in
master
and in
22 other branches
ActionItem616: promoting public_profile to column
To be able to make some database optimizations in the blocks of profile listing and still filter out the private profiles the public_profile attribute had to be promotted from setting to column git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@2394 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
10 changed files
with
70 additions
and
14 deletions
Show diff stats
app/models/communities_block.rb
| @@ -36,7 +36,11 @@ class CommunitiesBlock < ProfileListBlock | @@ -36,7 +36,11 @@ class CommunitiesBlock < ProfileListBlock | ||
| 36 | def ids | 36 | def ids |
| 37 | # FIXME when owner is an environment (i.e. listing communities globally | 37 | # FIXME when owner is an environment (i.e. listing communities globally |
| 38 | # this can become SLOW) | 38 | # this can become SLOW) |
| 39 | - block.owner.communities.select(&:public_profile).map(&:id) | 39 | + if block.owner.kind_of?(Environment) |
| 40 | + Community.find(:all, :conditions => {:environment_id => block.owner.id, :public_profile => true}, :limit => block.limit, :order => 'random()').map(&:id) | ||
| 41 | + else | ||
| 42 | + block.owner.communities.select(&:public_profile).map(&:id) | ||
| 43 | + end | ||
| 40 | end | 44 | end |
| 41 | end | 45 | end |
| 42 | 46 |
app/models/enterprises_block.rb
| @@ -37,7 +37,11 @@ class EnterprisesBlock < ProfileListBlock | @@ -37,7 +37,11 @@ class EnterprisesBlock < ProfileListBlock | ||
| 37 | def ids | 37 | def ids |
| 38 | # FIXME when owner is an environment (i.e. listing enterprises globally | 38 | # FIXME when owner is an environment (i.e. listing enterprises globally |
| 39 | # this can become SLOW) | 39 | # this can become SLOW) |
| 40 | - block.owner.enterprises.map(&:id) | 40 | + if block.owner.kind_of?(Environment) |
| 41 | + Enterprise.find(:all, :conditions => {:environment_id => block.owner.id, :public_profile => true}, :limit => block.limit, :order => 'random()').map(&:id) | ||
| 42 | + else | ||
| 43 | + block.owner.enterprises.select(&:public_profile).map(&:id) | ||
| 44 | + end | ||
| 41 | end | 45 | end |
| 42 | end | 46 | end |
| 43 | 47 |
app/models/people_block.rb
| @@ -18,7 +18,7 @@ class PeopleBlock < ProfileListBlock | @@ -18,7 +18,7 @@ class PeopleBlock < ProfileListBlock | ||
| 18 | 18 | ||
| 19 | class Finder < ProfileListBlock::Finder | 19 | class Finder < ProfileListBlock::Finder |
| 20 | def ids | 20 | def ids |
| 21 | - Person.find(:all, :select => 'id', :conditions => { :environment_id => block.owner.id}) | 21 | + Person.find(:all, :select => 'id', :conditions => { :environment_id => block.owner.id, :public_profile => true}, :limit => block.limit, :order => 'random()') |
| 22 | end | 22 | end |
| 23 | end | 23 | end |
| 24 | 24 |
app/models/profile.rb
| @@ -57,7 +57,7 @@ class Profile < ActiveRecord::Base | @@ -57,7 +57,7 @@ class Profile < ActiveRecord::Base | ||
| 57 | 57 | ||
| 58 | acts_as_having_settings :field => :data | 58 | acts_as_having_settings :field => :data |
| 59 | 59 | ||
| 60 | - settings_items :public_profile, :public_content, :type => :boolean, :default => true | 60 | + settings_items :public_content, :type => :boolean, :default => true |
| 61 | 61 | ||
| 62 | acts_as_mappable :default_units => :kms | 62 | acts_as_mappable :default_units => :kms |
| 63 | 63 |
app/models/profile_list_block.rb
| @@ -42,7 +42,7 @@ class ProfileListBlock < Block | @@ -42,7 +42,7 @@ class ProfileListBlock < Block | ||
| 42 | rand(top) | 42 | rand(top) |
| 43 | end | 43 | end |
| 44 | def ids | 44 | def ids |
| 45 | - Profile.connection.select_all('select id from profiles').map { |entry| entry['id'].to_i } | 45 | + Profile.find(:all, :limit => block.limit, :order => 'random()', :conditions => {:environment_id => block.owner.id, :public_profile => true}).map(&:id) |
| 46 | end | 46 | end |
| 47 | end | 47 | end |
| 48 | 48 |
test/unit/communities_block_test.rb
| @@ -25,7 +25,7 @@ class CommunitiesBlockTest < Test::Unit::TestCase | @@ -25,7 +25,7 @@ class CommunitiesBlockTest < Test::Unit::TestCase | ||
| 25 | block.limit = 2 | 25 | block.limit = 2 |
| 26 | 26 | ||
| 27 | owner = mock | 27 | owner = mock |
| 28 | - block.expects(:owner).returns(owner) | 28 | + block.expects(:owner).at_least_once.returns(owner) |
| 29 | 29 | ||
| 30 | member1 = mock; member1.stubs(:id).returns(1); member1.stubs(:public_profile).returns(true) | 30 | member1 = mock; member1.stubs(:id).returns(1); member1.stubs(:public_profile).returns(true) |
| 31 | member2 = mock; member2.stubs(:id).returns(2); member2.stubs(:public_profile).returns(true) | 31 | member2 = mock; member2.stubs(:id).returns(2); member2.stubs(:public_profile).returns(true) |
| @@ -81,7 +81,7 @@ class CommunitiesBlockTest < Test::Unit::TestCase | @@ -81,7 +81,7 @@ class CommunitiesBlockTest < Test::Unit::TestCase | ||
| 81 | private_community.add_member(user) | 81 | private_community.add_member(user) |
| 82 | 82 | ||
| 83 | block = CommunitiesBlock.new | 83 | block = CommunitiesBlock.new |
| 84 | - block.expects(:owner).returns(user) | 84 | + block.expects(:owner).at_least_once.returns(user) |
| 85 | 85 | ||
| 86 | assert_equal [public_community], block.profiles | 86 | assert_equal [public_community], block.profiles |
| 87 | end | 87 | end |
test/unit/enterprises_block_test.rb
| @@ -19,17 +19,16 @@ class EnterprisesBlockTest < Test::Unit::TestCase | @@ -19,17 +19,16 @@ class EnterprisesBlockTest < Test::Unit::TestCase | ||
| 19 | assert_kind_of EnterprisesBlock::Finder, EnterprisesBlock.new.profile_finder | 19 | assert_kind_of EnterprisesBlock::Finder, EnterprisesBlock.new.profile_finder |
| 20 | end | 20 | end |
| 21 | 21 | ||
| 22 | - should 'list owner communities' do | ||
| 23 | - | 22 | + should 'list owner enterprises' do |
| 24 | block = EnterprisesBlock.new | 23 | block = EnterprisesBlock.new |
| 25 | block.limit = 2 | 24 | block.limit = 2 |
| 26 | 25 | ||
| 27 | owner = mock | 26 | owner = mock |
| 28 | - block.expects(:owner).returns(owner) | 27 | + block.expects(:owner).at_least_once.returns(owner) |
| 29 | 28 | ||
| 30 | - member1 = mock; member1.stubs(:id).returns(1) | ||
| 31 | - member2 = mock; member2.stubs(:id).returns(2) | ||
| 32 | - member3 = mock; member3.stubs(:id).returns(3) | 29 | + member1 = stub(:id => 1, :public_profile => true ) |
| 30 | + member2 = stub(:id => 2, :public_profile => true ) | ||
| 31 | + member3 = stub(:id => 3, :public_profile => true ) | ||
| 33 | 32 | ||
| 34 | owner.expects(:enterprises).returns([member1, member2, member3]) | 33 | owner.expects(:enterprises).returns([member1, member2, member3]) |
| 35 | 34 | ||
| @@ -42,6 +41,33 @@ class EnterprisesBlockTest < Test::Unit::TestCase | @@ -42,6 +41,33 @@ class EnterprisesBlockTest < Test::Unit::TestCase | ||
| 42 | assert_equal [member3, member1], block.profiles | 41 | assert_equal [member3, member1], block.profiles |
| 43 | end | 42 | end |
| 44 | 43 | ||
| 44 | + should 'not list private enterprises in environment' do | ||
| 45 | + env = Environment.create!(:name => 'test env') | ||
| 46 | + p1 = Enterprise.create!(:name => 'test1', :identifier => 'test1', :environment_id => env.id, :public_profile => true) | ||
| 47 | + p2 = Enterprise.create!(:name => 'test2', :identifier => 'test2', :environment_id => env.id, :public_profile => false) #private profile | ||
| 48 | + block = EnterprisesBlock.new | ||
| 49 | + env.boxes.first.blocks << block | ||
| 50 | + block.save! | ||
| 51 | + ids = block.profile_finder.ids | ||
| 52 | + assert_includes ids, p1.id | ||
| 53 | + assert_not_includes ids, p2.id | ||
| 54 | + end | ||
| 55 | + | ||
| 56 | + should 'not list private enterprises in profile' do | ||
| 57 | + person = create_user('test_user').person | ||
| 58 | + role = Profile::Roles.member | ||
| 59 | + e1 = Enterprise.create!(:name => 'test1', :identifier => 'test1', :public_profile => true) | ||
| 60 | + e1.affiliate(person, role) | ||
| 61 | + e2 = Enterprise.create!(:name => 'test2', :identifier => 'test2', :public_profile => false) #private profile | ||
| 62 | + e2.affiliate(person, role) | ||
| 63 | + block = EnterprisesBlock.new | ||
| 64 | + person.boxes.first.blocks << block | ||
| 65 | + block.save! | ||
| 66 | + ids = block.profile_finder.ids | ||
| 67 | + assert_includes ids, e1.id | ||
| 68 | + assert_not_includes ids, e2.id | ||
| 69 | + end | ||
| 70 | + | ||
| 45 | should 'link to all enterprises for profile' do | 71 | should 'link to all enterprises for profile' do |
| 46 | profile = Profile.new | 72 | profile = Profile.new |
| 47 | profile.expects(:identifier).returns('theprofile') | 73 | profile.expects(:identifier).returns('theprofile') |
test/unit/people_block_test.rb
| @@ -26,7 +26,7 @@ class PeopleBlockTest < ActiveSupport::TestCase | @@ -26,7 +26,7 @@ class PeopleBlockTest < ActiveSupport::TestCase | ||
| 26 | should 'list people' do | 26 | should 'list people' do |
| 27 | owner = mock | 27 | owner = mock |
| 28 | owner.expects(:id).returns(99) | 28 | owner.expects(:id).returns(99) |
| 29 | - Person.expects(:find).with(:all, :select => 'id', :conditions => { :environment_id => 99}).returns([]) | 29 | + Person.expects(:find).with(:all, :select => 'id', :conditions => { :environment_id => 99, :public_profile => true}, :limit => 6, :order => 'random()').returns([]) |
| 30 | block = PeopleBlock.new | 30 | block = PeopleBlock.new |
| 31 | block.expects(:owner).returns(owner).at_least_once | 31 | block.expects(:owner).returns(owner).at_least_once |
| 32 | block.content | 32 | block.content |
test/unit/profile_list_block_test.rb
| @@ -42,6 +42,19 @@ class ProfileListBlockTest < Test::Unit::TestCase | @@ -42,6 +42,19 @@ class ProfileListBlockTest < Test::Unit::TestCase | ||
| 42 | assert_kind_of String, instance_eval(&block.content) | 42 | assert_kind_of String, instance_eval(&block.content) |
| 43 | end | 43 | end |
| 44 | 44 | ||
| 45 | + should 'not list private profiles' do | ||
| 46 | + env = Environment.create!(:name => 'test env') | ||
| 47 | + p1 = Profile.create!(:name => 'test1', :identifier => 'test1', :environment => env) | ||
| 48 | + p2 = Profile.create!(:name => 'test2', :identifier => 'test2', :environment => env, :public_profile => false) # private profile | ||
| 49 | + block = ProfileListBlock.new | ||
| 50 | + env.boxes.first.blocks << block | ||
| 51 | + block.save! | ||
| 52 | + | ||
| 53 | + ids = block.profile_finder.ids | ||
| 54 | + assert_includes ids, p1.id | ||
| 55 | + assert_not_includes ids, p2.id | ||
| 56 | + end | ||
| 57 | + | ||
| 45 | should 'use finders to find profiles to be listed' do | 58 | should 'use finders to find profiles to be listed' do |
| 46 | block = ProfileListBlock.new | 59 | block = ProfileListBlock.new |
| 47 | finder = mock | 60 | finder = mock |
test/unit/profile_test.rb
| @@ -480,6 +480,15 @@ class ProfileTest < Test::Unit::TestCase | @@ -480,6 +480,15 @@ class ProfileTest < Test::Unit::TestCase | ||
| 480 | assert_equal false, p.public_profile | 480 | assert_equal false, p.public_profile |
| 481 | end | 481 | end |
| 482 | 482 | ||
| 483 | + should 'be able to find the public profiles but not private ones' do | ||
| 484 | + p1 = Profile.create!(:name => 'test1', :identifier => 'test1', :public_profile => true) | ||
| 485 | + p2 = Profile.create!(:name => 'test2', :identifier => 'test2', :public_profile => false) | ||
| 486 | + | ||
| 487 | + result = Profile.find(:all, :conditions => {:public_profile => true}) | ||
| 488 | + assert_includes result, p1 | ||
| 489 | + assert_not_includes result, p2 | ||
| 490 | + end | ||
| 491 | + | ||
| 483 | should 'have public content by default' do | 492 | should 'have public content by default' do |
| 484 | assert_equal true, Profile.new.public_content | 493 | assert_equal true, Profile.new.public_content |
| 485 | end | 494 | end |