From 8169e3ca4de771d494eafd1403cc9809516bd547 Mon Sep 17 00:00:00 2001
From: AntonioTerceiro <AntonioTerceiro@3f533792-8f58-4932-b0fe-aaf55b0a4547>
Date: Wed, 13 Feb 2008 23:26:02 +0000
Subject: [PATCH] ActionItem43: reformulating ProfileListBlock logic. Finder now is responsible for everything.

---
 app/models/members_block.rb          | 31 ++++++++++++++++++++++---------
 app/models/profile_list_block.rb     | 22 +++++++++++-----------
 test/unit/members_block_test.rb      | 21 +++++++++++++++++++--
 test/unit/profile_list_block_test.rb | 20 +++++---------------
 4 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/app/models/members_block.rb b/app/models/members_block.rb
index 8544493..483e6e7 100644
--- a/app/models/members_block.rb
+++ b/app/models/members_block.rb
@@ -15,20 +15,33 @@ class MembersBlock < ProfileListBlock
     end
   end
 
-  class Finder
+  def profile_finder
+    @profile_finder ||= MembersBlock::Finder.new(self)
+  end
 
-    def initialize(members)
-      @members = members
+  # Finds random members, up to the limit.
+  class Finder
+    def initialize(block)
+      @block = block
+    end
+    attr_reader :block
+
+    def find
+      ids = block.owner.members.map(&:id)
+      result = []
+      [block.limit, ids.size].min.times do
+        i = pick_random(ids.size)
+        result << Profile.find(ids[i])
+        ids.delete_at(i)
+      end
+      result
     end
 
-    # FIXME optimize this !!!
-    def find(options)
-      Profile.find(:all, options.merge(:conditions => { :id => @members.map(&:id) }))
+    def pick_random(top)
+      rand(top)
     end
-  end
 
-  def profile_finder
-    @profile_finder ||= Finder.new(owner.members)
   end
 
+
 end
diff --git a/app/models/profile_list_block.rb b/app/models/profile_list_block.rb
index af35a2a..7394d9d 100644
--- a/app/models/profile_list_block.rb
+++ b/app/models/profile_list_block.rb
@@ -19,22 +19,22 @@ class ProfileListBlock < Block
   # have a <tt>find</tt> method that accepts the same interface as the
   # ActiveRecord::Base's find method .
   def profile_finder
-    Profile
+    @profile_finder ||= ProfileListBlock::Finder.new(self)
   end
 
-  def profiles
-    # FIXME pick random people instead
-    finder = profile_finder
-    options = { :limit => self.limit, :order => 'created_at desc' } 
-    if finder.is_a?(Class)
-      finder.find(:all, options)
-    else
-      finder.find(options)
+  # Default finder. Finds the most recently added profiles.
+  class Finder
+    def initialize(block)
+      @block = block
+    end
+    attr_reader :block
+    def find
+      Profile.find(:all, :limit => block.limit, :order => 'created_at desc')
     end
   end
 
-  def random(top)
-    Kernel.rand(top)
+  def profiles
+    profile_finder.find
   end
 
   # the title of the block. Probably will be overriden in subclasses.
diff --git a/test/unit/members_block_test.rb b/test/unit/members_block_test.rb
index 4ecbc73..57e02c5 100644
--- a/test/unit/members_block_test.rb
+++ b/test/unit/members_block_test.rb
@@ -22,13 +22,30 @@ class MembersBlockTest < Test::Unit::TestCase
     assert_equal 'link-to-members', instance_eval(&block.footer)
   end
 
-  should 'pick only members' do
+  should 'pick random members' do
+
     profile = create_user('mytestuser').person
     block = MembersBlock.new
     block.box = profile.boxes.first
+    block.limit = 2
     block.save!
 
-    assert_equal profile.members, block.profiles
+    owner = mock
+    block.expects(:owner).returns(owner)
+
+    member1 = mock; member1.stubs(:id).returns(1)
+    member2 = mock; member2.stubs(:id).returns(2)
+    member3 = mock; member3.stubs(:id).returns(3)
+
+    owner.expects(:members).returns([member1, member2, member3])
+    
+    block.profile_finder.expects(:pick_random).with(3).returns(2)
+    block.profile_finder.expects(:pick_random).with(2).returns(0)
+
+    Profile.expects(:find).with(3).returns(member3)
+    Profile.expects(:find).with(1).returns(member1)
+
+    assert_equal [member3, member1], block.profiles
   end
 
 end
diff --git a/test/unit/profile_list_block_test.rb b/test/unit/profile_list_block_test.rb
index 16fb381..b2580b1 100644
--- a/test/unit/profile_list_block_test.rb
+++ b/test/unit/profile_list_block_test.rb
@@ -39,31 +39,21 @@ class ProfileListBlockTest < Test::Unit::TestCase
     assert_kind_of String, instance_eval(&block.content)
   end
 
-  should 'find in Profile by default' do
-    assert_equal Profile, ProfileListBlock.new.profile_finder
-  end
+  should 'pick most recently-added profiles by default' do
+    Profile.expects(:find).with(:all, { :limit => 10, :order => 'created_at desc'})
 
-  should 'ask profile finder for profiles' do
     block = ProfileListBlock.new
-    block.expects(:profile_finder).returns(Profile).once
-    Profile.expects(:find).with(:all, is_a(Hash)).returns([])
+    block.limit = 10
     block.profiles
   end
 
-  should 'support non-class finders' do
+  should 'use finders to find profiles to be listed' do
     block = ProfileListBlock.new
     finder = mock
     block.expects(:profile_finder).returns(finder).once
-    finder.expects(:find).with(is_a(Hash)).once
+    finder.expects(:find)
     block.profiles
   end
 
-  should 'pick random people'
-
-  should 'use Kernel.rand to generate random numbers' do
-    Kernel.expects(:rand).with(77).once
-    ProfileListBlock.new.random(77)
-  end
-
 
 end
--
libgit2 0.21.2