From a6f0c2e8fdfe136054edfd12965bd53a523ff2f6 Mon Sep 17 00:00:00 2001 From: Tim Craft Date: Fri, 30 Nov 2012 12:10:31 +0000 Subject: [PATCH 1/5] Fix comments that describe usage of UserCollectionProxy --- lib/intercom/user.rb | 4 ++-- lib/intercom/user_collection_proxy.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/intercom/user.rb b/lib/intercom/user.rb index 85bbbbe4..d25c46ab 100644 --- a/lib/intercom/user.rb +++ b/lib/intercom/user.rb @@ -70,11 +70,11 @@ def self.create(params) # Examples: # Intercom::User.all.count # > 5346 - # Intercom::User.each do |user| + # Intercom::User.all.each do |user| # puts user.inspect # end # > ["user1@example.com" ,"user2@example.com" ,....] - # Intercom::User.map(&:email) + # Intercom::User.all.map(&:email) # > ["user1@example.com" ,"user2@example.com" ,....] # # @return [UserCollectionProxy] diff --git a/lib/intercom/user_collection_proxy.rb b/lib/intercom/user_collection_proxy.rb index 91274b93..4c065d1b 100644 --- a/lib/intercom/user_collection_proxy.rb +++ b/lib/intercom/user_collection_proxy.rb @@ -12,7 +12,7 @@ module Intercom # Intercom::User.all.count # # Iterating over each user - # Intercom::User.each do |user| + # Intercom::User.all.each do |user| # puts user.inspect # end # From 4746b59aa79cd74f0b1471eda3c67475828f30bb Mon Sep 17 00:00:00 2001 From: Tim Craft Date: Fri, 30 Nov 2012 12:14:28 +0000 Subject: [PATCH 2/5] Include Enumerable module into UserCollectionProxy instead of defining map --- lib/intercom/user_collection_proxy.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/intercom/user_collection_proxy.rb b/lib/intercom/user_collection_proxy.rb index 4c065d1b..5e378332 100644 --- a/lib/intercom/user_collection_proxy.rb +++ b/lib/intercom/user_collection_proxy.rb @@ -38,14 +38,6 @@ def each(&block) end end - # yields each {User} to the block provided and collects the output in the same way as Enumerable#map - # @return [Array] - def map - out = [] - each { |e| out << yield(e) } - out - end - - alias :collect :map + include Enumerable end end From cca51e898e36312f2357fb0c38a81c7029c92e88 Mon Sep 17 00:00:00 2001 From: Tim Craft Date: Fri, 30 Nov 2012 12:19:12 +0000 Subject: [PATCH 3/5] Fix typo/whitespace in User class comment --- lib/intercom/user.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/intercom/user.rb b/lib/intercom/user.rb index d25c46ab..a715dec3 100644 --- a/lib/intercom/user.rb +++ b/lib/intercom/user.rb @@ -8,13 +8,13 @@ module Intercom # # == Example usage # * Fetching a user - # Intercom::User.find_by_email("bob@example.") + # Intercom::User.find_by_email("bob@example.com") # # * Getting the count of all users # Intercom::User.all.count # # * Fetching all users - # Intercom::User.all.each {|user| puts user.email } + # Intercom::User.all.each { |user| puts user.email } # # * Updating custom data on a user # user = Intercom::User.find_by_email("bob@example.com") From 0756b957c3612f62cfc759a542355f8d4f93bcd3 Mon Sep 17 00:00:00 2001 From: Darragh Curran Date: Fri, 30 Nov 2012 13:08:46 +0000 Subject: [PATCH 4/5] improve test that users.all.count uses api response total_count rather than paging all and counting --- spec/unit/intercom/user_collection_proxy_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/unit/intercom/user_collection_proxy_spec.rb b/spec/unit/intercom/user_collection_proxy_spec.rb index 61c1c1b0..7405c529 100644 --- a/spec/unit/intercom/user_collection_proxy_spec.rb +++ b/spec/unit/intercom/user_collection_proxy_spec.rb @@ -18,8 +18,10 @@ Intercom::User.all.collect { |user| user.email }.must_equal %W(user1@example.com user2@example.com user3@example.com) end - it "yields each user to the block" do - Intercom.expects(:get).with("users", {:per_page => 1}).returns(page_of_users(1,1)) + it "counts users based on total_count in paged_response" do + paged_response = page_of_users(1, 1) + Intercom.expects(:get).with("users", {:per_page => 1}).returns(paged_response) + paged_response.expects(:[]).with("total_count").returns(3) Intercom::User.all.count.must_equal 3 end From f1778306fdce21c83e41a79aa337aa8775bddd98 Mon Sep 17 00:00:00 2001 From: Darragh Curran Date: Fri, 30 Nov 2012 13:55:37 +0000 Subject: [PATCH 5/5] explicitly fail if someone attempts to use count with block (as supported by Enumerable) on UserCollectionProxy --- lib/intercom/user_collection_proxy.rb | 5 +++-- spec/unit/intercom/user_collection_proxy_spec.rb | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/intercom/user_collection_proxy.rb b/lib/intercom/user_collection_proxy.rb index 5e378332..02ea092a 100644 --- a/lib/intercom/user_collection_proxy.rb +++ b/lib/intercom/user_collection_proxy.rb @@ -17,8 +17,11 @@ module Intercom # end # class UserCollectionProxy + include Enumerable + # @return [Integer] number of users tracked on Intercom for this application def count + raise ArgumentError.new("count doesn't support block argument") if block_given? response = Intercom.get("users", {:per_page => 1}) response["total_count"] end @@ -37,7 +40,5 @@ def each(&block) fetch_another_page = !current_page["next_page"].nil? end end - - include Enumerable end end diff --git a/spec/unit/intercom/user_collection_proxy_spec.rb b/spec/unit/intercom/user_collection_proxy_spec.rb index 7405c529..b3020cd4 100644 --- a/spec/unit/intercom/user_collection_proxy_spec.rb +++ b/spec/unit/intercom/user_collection_proxy_spec.rb @@ -25,6 +25,12 @@ Intercom::User.all.count.must_equal 3 end + it "explicitly doesn't support block argument for count" do + assert_raises ArgumentError do + Intercom::User.all.count {|_| true } + end + end + it "loads multiple pages" do Intercom.expects(:get).with("users", {:page => 1}).returns(page_of_users(1, 1)) Intercom.expects(:get).with("users", {:page => 2}).returns(page_of_users(2, 1))