From d2b71681958bc3be844e287e24442feeba9fa007 Mon Sep 17 00:00:00 2001
From: Michael Sofaer <pair+michael@joindiaspora.com>
Date: Sun, 3 Jul 2011 21:16:58 -0700
Subject: [PATCH] Use an explicit per_page param instead of switching on format

---
 app/controllers/notifications_controller.rb   |  5 +-
 .../widgets/notifications-badge.js            |  6 +--
 .../notifications_controller_spec.rb          | 51 +++++++------------
 3 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index 52c5bd3f7..62d2c4d67 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -19,12 +19,13 @@ class NotificationsController < VannaController
     @aspect = :notification
     conditions = {:recipient_id => current_user.id}
     page = opts[:page] || 1
-    notifications = WillPaginate::Collection.create(page, 25, Notification.where(conditions).count ) do |pager|
+    per_page = opts[:per_page] || 25
+    notifications = WillPaginate::Collection.create(page, per_page, Notification.where(conditions).count ) do |pager|
       result = Notification.find(:all,
                                  :conditions => conditions,
                                  :order => 'created_at desc',
                                  :include => [:target, {:actors => :profile}],
-                                 :limit => request.format == :json ? 5 : pager.per_page,
+                                 :limit => pager.per_page,
                                  :offset => pager.offset
                                 )
 
diff --git a/public/javascripts/widgets/notifications-badge.js b/public/javascripts/widgets/notifications-badge.js
index 085923ef4..e8dfd95d0 100644
--- a/public/javascripts/widgets/notifications-badge.js
+++ b/public/javascripts/widgets/notifications-badge.js
@@ -13,12 +13,12 @@
       this.badgeLink.toggle(function(evt) {
           evt.preventDefault();
           evt.stopPropagation();
-          
+
           self.ajaxLoader.show();
           self.badge.addClass("active");
           self.dropdown.css("display", "block");
 
-          self.getNotifications(function() { 
+          self.getNotifications(function() {
             self.renderNotifications();
           });
         },  function(evt) {
@@ -46,7 +46,7 @@
     };
 
     this.getNotifications = function(callback) {
-      $.getJSON("/notifications", function(notifications) {
+      $.getJSON("/notifications?per_page=5", function(notifications) {
         self.notifications = notifications;
         callback.apply(self, []);
       });
diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb
index c9627adc8..71e53e9db 100644
--- a/spec/controllers/notifications_controller_spec.rb
+++ b/spec/controllers/notifications_controller_spec.rb
@@ -48,45 +48,30 @@ describe NotificationsController do
     before do
       @post = Factory(:status_message)
       Factory(:notification, :recipient => @user, :target => @post)
-
-      @fake_request = ActionDispatch::Request.new({})
-      @controller.stub!(:request).and_return(@fake_request)
     end
 
-    context "html request" do
-      before do
-        @fake_request.stub!(:format).and_return(:html)
-      end
+    it 'paginates the notifications' do
+      25.times { Factory(:notification, :recipient => @user, :target => @post) }
 
-      it 'paginates the notifications' do
-        25.times { Factory(:notification, :recipient => @user, :target => @post) }
-
-        @controller.index({})[:notifications].count.should == 25
-        @controller.index(:page => 2)[:notifications].count.should == 1
-      end
-
-      it "includes the actors" do
-        Factory(:notification, :recipient => @user, :target => @post)
-        response = @controller.index({})
-        response[:notifications].first[:actors].first.should be_a(Person)
-      end
-
-      it 'eager loads the target' do
-        response = @controller.index({})
-        response[:notifications].each { |note| note[:target].should be }
-      end
+      @controller.index({})[:notifications].count.should == 25
+      @controller.index(:page => 2)[:notifications].count.should == 1
     end
 
-    context "json request" do
-      before do
-        @fake_request.stub!(:format).and_return(:json)
-      end
+    it "includes the actors" do
+      Factory(:notification, :recipient => @user, :target => @post)
+      response = @controller.index({})
+      response[:notifications].first[:actors].first.should be_a(Person)
+    end
 
-      it "returns just the first 5 notifications" do
-        5.times { Factory(:notification, :recipient => @user, :target => @post) }
-        response = @controller.index({})
-        response[:notifications].length.should == 5
-      end
+    it 'eager loads the target' do
+      response = @controller.index({})
+      response[:notifications].each { |note| note[:target].should be }
+    end
+
+    it "supports a limit per_page parameter" do
+      5.times { Factory(:notification, :recipient => @user, :target => @post) }
+      response = @controller.index({:per_page => 5})
+      response[:notifications].length.should == 5
     end
   end
 end