FEATURE: unread pms go in front of notification report

also refactor fat controller

app/controllers/notifications_controller.rb

@@ -3,17 +3,8 @@ class NotificationsController < ApplicationController
   before_filter :ensure_logged_in
 
   def index
-    notifications = current_user.notifications.recent.includes(:topic)
+    notifications = Notification.recent_report(current_user)
 
-    if notifications.present?
-      notifications += current_user.notifications
-        .order('created_at desc')
-        .where(read: false, notification_type: Notification.types[:private_message])
-        .where('id < ?', notifications.last.id)
-        .limit(5)
-    end
-
-    notifications = notifications.to_a
     current_user.saw_notification_id(notifications.first.id) if notifications.present?
     current_user.reload
     current_user.publish_notifications_state

app/models/notification.rb

@@ -8,7 +8,7 @@ class Notification < ActiveRecord::Base
   validates_presence_of :notification_type
 
   scope :unread, lambda { where(read: false) }
-  scope :recent, lambda { order('created_at desc').limit(10) }
+  scope :recent, lambda {|n=nil| n ||= 10; order('created_at desc').limit(n) }
 
   after_save :refresh_notification_count
   after_destroy :refresh_notification_count
@@ -89,6 +89,33 @@ class Notification < ActiveRecord::Base
     Post.where(topic_id: topic_id, post_number: post_number).first
   end
 
+  def self.recent_report(user, count = nil)
+
+    notifications = user.notifications.recent(count).includes(:topic)
+
+    if notifications.present?
+      notifications += user.notifications
+        .order('created_at desc')
+        .where(read: false, notification_type: Notification.types[:private_message])
+        .where('id < ?', notifications.last.id)
+        .limit(count)
+    end
+
+    notifications.sort do |x,y|
+      if x.unread_pm? && !y.unread_pm?
+        -1
+      elsif y.unread_pm? && !x.unread_pm?
+        1
+      else
+        y.created_at <=> x.created_at
+      end
+    end.take(count)
+
+  end
+
+  def unread_pm?
+    Notification.types[:private_message] == self.notification_type && !read
+  end
 
   protected
 

spec/models/notification_spec.rb

@@ -205,6 +205,7 @@ describe Notification do
     end
   end
 
+
   describe 'ensure consistency' do
     it 'deletes notifications if post is missing or deleted' do
 
@@ -229,3 +230,45 @@ describe Notification do
   end
 
 end
+
+# pulling this out cause I don't want an observer
+describe Notification do
+  describe '#recent_report' do
+    let(:user){ Fabricate(:user) }
+    let(:post){ Fabricate(:post) }
+
+    def fab(type, read)
+      @i ||= 0
+      @i += 1
+      Notification.create!(read: read, user_id: user.id, topic_id: post.topic_id, post_number: post.post_number, data: '[]',
+                           notification_type: type, created_at: @i.days.from_now)
+    end
+
+    def unread_pm
+      fab(Notification.types[:private_message], false)
+    end
+
+    def pm
+      fab(Notification.types[:private_message], true)
+    end
+
+    def regular
+      fab(Notification.types[:liked], true)
+    end
+
+
+
+    it 'orders stuff correctly' do
+      a = unread_pm
+          regular
+      c = pm
+      d = regular
+
+      # bumps unread pms to front of list
+
+      notifications = Notification.recent_report(user, 3)
+      notifications.map{|n| n.id}.should == [a.id, d.id, c.id]
+
+    end
+  end
+end