Commit 8169e3ca4de771d494eafd1403cc9809516bd547
1 parent
bd787d26
Exists in
master
and in
29 other branches
ActionItem43: reformulating ProfileListBlock logic. Finder now is responsible for everything.
git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@1367 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
4 changed files
with
57 additions
and
37 deletions
Show diff stats
app/models/members_block.rb
@@ -15,20 +15,33 @@ class MembersBlock < ProfileListBlock | @@ -15,20 +15,33 @@ class MembersBlock < ProfileListBlock | ||
15 | end | 15 | end |
16 | end | 16 | end |
17 | 17 | ||
18 | - class Finder | 18 | + def profile_finder |
19 | + @profile_finder ||= MembersBlock::Finder.new(self) | ||
20 | + end | ||
19 | 21 | ||
20 | - def initialize(members) | ||
21 | - @members = members | 22 | + # Finds random members, up to the limit. |
23 | + class Finder | ||
24 | + def initialize(block) | ||
25 | + @block = block | ||
26 | + end | ||
27 | + attr_reader :block | ||
28 | + | ||
29 | + def find | ||
30 | + ids = block.owner.members.map(&:id) | ||
31 | + result = [] | ||
32 | + [block.limit, ids.size].min.times do | ||
33 | + i = pick_random(ids.size) | ||
34 | + result << Profile.find(ids[i]) | ||
35 | + ids.delete_at(i) | ||
36 | + end | ||
37 | + result | ||
22 | end | 38 | end |
23 | 39 | ||
24 | - # FIXME optimize this !!! | ||
25 | - def find(options) | ||
26 | - Profile.find(:all, options.merge(:conditions => { :id => @members.map(&:id) })) | 40 | + def pick_random(top) |
41 | + rand(top) | ||
27 | end | 42 | end |
28 | - end | ||
29 | 43 | ||
30 | - def profile_finder | ||
31 | - @profile_finder ||= Finder.new(owner.members) | ||
32 | end | 44 | end |
33 | 45 | ||
46 | + | ||
34 | end | 47 | end |
app/models/profile_list_block.rb
@@ -19,22 +19,22 @@ class ProfileListBlock < Block | @@ -19,22 +19,22 @@ class ProfileListBlock < Block | ||
19 | # have a <tt>find</tt> method that accepts the same interface as the | 19 | # have a <tt>find</tt> method that accepts the same interface as the |
20 | # ActiveRecord::Base's find method . | 20 | # ActiveRecord::Base's find method . |
21 | def profile_finder | 21 | def profile_finder |
22 | - Profile | 22 | + @profile_finder ||= ProfileListBlock::Finder.new(self) |
23 | end | 23 | end |
24 | 24 | ||
25 | - def profiles | ||
26 | - # FIXME pick random people instead | ||
27 | - finder = profile_finder | ||
28 | - options = { :limit => self.limit, :order => 'created_at desc' } | ||
29 | - if finder.is_a?(Class) | ||
30 | - finder.find(:all, options) | ||
31 | - else | ||
32 | - finder.find(options) | 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 | + Profile.find(:all, :limit => block.limit, :order => 'created_at desc') | ||
33 | end | 33 | end |
34 | end | 34 | end |
35 | 35 | ||
36 | - def random(top) | ||
37 | - Kernel.rand(top) | 36 | + def profiles |
37 | + profile_finder.find | ||
38 | end | 38 | end |
39 | 39 | ||
40 | # the title of the block. Probably will be overriden in subclasses. | 40 | # the title of the block. Probably will be overriden in subclasses. |
test/unit/members_block_test.rb
@@ -22,13 +22,30 @@ class MembersBlockTest < Test::Unit::TestCase | @@ -22,13 +22,30 @@ class MembersBlockTest < Test::Unit::TestCase | ||
22 | assert_equal 'link-to-members', instance_eval(&block.footer) | 22 | assert_equal 'link-to-members', instance_eval(&block.footer) |
23 | end | 23 | end |
24 | 24 | ||
25 | - should 'pick only members' do | 25 | + should 'pick random members' do |
26 | + | ||
26 | profile = create_user('mytestuser').person | 27 | profile = create_user('mytestuser').person |
27 | block = MembersBlock.new | 28 | block = MembersBlock.new |
28 | block.box = profile.boxes.first | 29 | block.box = profile.boxes.first |
30 | + block.limit = 2 | ||
29 | block.save! | 31 | block.save! |
30 | 32 | ||
31 | - assert_equal profile.members, block.profiles | 33 | + owner = mock |
34 | + block.expects(:owner).returns(owner) | ||
35 | + | ||
36 | + member1 = mock; member1.stubs(:id).returns(1) | ||
37 | + member2 = mock; member2.stubs(:id).returns(2) | ||
38 | + member3 = mock; member3.stubs(:id).returns(3) | ||
39 | + | ||
40 | + owner.expects(:members).returns([member1, member2, member3]) | ||
41 | + | ||
42 | + block.profile_finder.expects(:pick_random).with(3).returns(2) | ||
43 | + block.profile_finder.expects(:pick_random).with(2).returns(0) | ||
44 | + | ||
45 | + Profile.expects(:find).with(3).returns(member3) | ||
46 | + Profile.expects(:find).with(1).returns(member1) | ||
47 | + | ||
48 | + assert_equal [member3, member1], block.profiles | ||
32 | end | 49 | end |
33 | 50 | ||
34 | end | 51 | end |
test/unit/profile_list_block_test.rb
@@ -39,31 +39,21 @@ class ProfileListBlockTest < Test::Unit::TestCase | @@ -39,31 +39,21 @@ class ProfileListBlockTest < Test::Unit::TestCase | ||
39 | assert_kind_of String, instance_eval(&block.content) | 39 | assert_kind_of String, instance_eval(&block.content) |
40 | end | 40 | end |
41 | 41 | ||
42 | - should 'find in Profile by default' do | ||
43 | - assert_equal Profile, ProfileListBlock.new.profile_finder | ||
44 | - end | 42 | + should 'pick most recently-added profiles by default' do |
43 | + Profile.expects(:find).with(:all, { :limit => 10, :order => 'created_at desc'}) | ||
45 | 44 | ||
46 | - should 'ask profile finder for profiles' do | ||
47 | block = ProfileListBlock.new | 45 | block = ProfileListBlock.new |
48 | - block.expects(:profile_finder).returns(Profile).once | ||
49 | - Profile.expects(:find).with(:all, is_a(Hash)).returns([]) | 46 | + block.limit = 10 |
50 | block.profiles | 47 | block.profiles |
51 | end | 48 | end |
52 | 49 | ||
53 | - should 'support non-class finders' do | 50 | + should 'use finders to find profiles to be listed' do |
54 | block = ProfileListBlock.new | 51 | block = ProfileListBlock.new |
55 | finder = mock | 52 | finder = mock |
56 | block.expects(:profile_finder).returns(finder).once | 53 | block.expects(:profile_finder).returns(finder).once |
57 | - finder.expects(:find).with(is_a(Hash)).once | 54 | + finder.expects(:find) |
58 | block.profiles | 55 | block.profiles |
59 | end | 56 | end |
60 | 57 | ||
61 | - should 'pick random people' | ||
62 | - | ||
63 | - should 'use Kernel.rand to generate random numbers' do | ||
64 | - Kernel.expects(:rand).with(77).once | ||
65 | - ProfileListBlock.new.random(77) | ||
66 | - end | ||
67 | - | ||
68 | 58 | ||
69 | end | 59 | end |