Revert "rewrite activity stream summary for performance"

This reverts commit 584eb9f.

app/models/stream_item.rb

@@ -37,18 +37,6 @@ class StreamItem < ActiveRecord::Base
   after_destroy :destroy_stream_item_instances
   attr_accessor :unread, :participant, :invalidate_immediately
 
-  before_save :ensure_notification_category
-
-  def ensure_notification_category
-    if self.asset_type == 'Message'
-      self.notification_category ||= get_notification_category
-    end
-  end
-
-  def get_notification_category
-    self.read_attribute(:data)['notification_category'] || self.data.notification_category
-  end
-
   def self.reconstitute_ar_object(type, data)
     return nil unless data
     data = data.instance_variable_get(:@table) if data.is_a?(OpenObject)

lib/api/v1/stream_item.rb

@@ -124,41 +124,20 @@ def api_render_stream(opts)
   end
 
   def api_render_stream_summary(contexts = nil)
-    items = []
-    @current_user.shard.activate do
-
-      base_scope = @current_user.visible_stream_item_instances(:contexts => contexts).joins(:stream_item)
-
-      count_scope = base_scope.except(:order).group('stream_items.asset_type', 'stream_items.notification_category')
-        # as far as I can tell, the 'type' column previously extracted by stream_item_json is identical to asset_type
-
-      total_counts = count_scope.count
-      unread_counts = count_scope.where(:workflow_state => 'unread').count
-
-      # TODO: can remove after DataFixup::PopulateStreamItemNotificationCategory is run
-      if total_counts.delete(['Message', nil]) # i.e. there are Message stream items without notification_category
-        unread_counts.delete(['Message', nil])
-
-        base_scope.where(:stream_items => {:asset_type => "Message", :notification_category => nil}).
-          eager_load(:stream_item).find_each do |i|
-
-          category = i.stream_item.get_notification_category
-          key = ['Message', category]
-          total_counts[key] ||= 0
-          total_counts[key] += 1
-          unless i.read?
-            unread_counts[key] ||= 0
-            unread_counts[key] += 1
-          end
-        end
-      end
-
-      total_counts.each do |key, count|
-        type, category = key
-        items << {:type => type, :notification_category => category,
-          :count => count, :unread_count => unread_counts[key] || 0}
-      end
-      items.sort_by!{|i| i[:type]}
+    opts = {}
+    opts[:contexts] = contexts
+    items = @current_user.shard.activate do
+      # not ideal, but 1. we can't aggregate in the db (boo yml) and
+      # 2. stream_item_json is where categorizing logic lives :(
+      @current_user.visible_stream_item_instances(opts).includes(:stream_item).map { |i|
+        stream_item_json(i, i.stream_item, @current_user, session)
+      }.inject({}) { |result, i|
+        key = [i['type'], i['notification_category']]
+        result[key] ||= {type: i['type'], count: 0, unread_count: 0, notification_category: i['notification_category']}
+        result[key][:count] += 1
+        result[key][:unread_count] += 1 if !i['read_state']
+        result
+      }.values.sort_by{ |i| i[:type] }
     end
     render :json => items
   end

spec/apis/v1/stream_items_api_spec.rb

@@ -61,31 +61,12 @@
     @assignment.update_attribute(:due_at, 1.week.from_now)
     json = api_call(:get, "/api/v1/users/self/activity_stream/summary.json",
                     { :controller => "users", :action => "activity_stream_summary", :format => 'json' })
-
     expect(json).to eq [{"type" => "Conversation", "count" => 1, "unread_count" => 0, "notification_category" => nil}, # conversations don't currently set the unread state on stream items
                     {"type" => "DiscussionTopic", "count" => 2, "unread_count" => 1, "notification_category" => nil},
                     {"type" => "Message", "count" => 1, "unread_count" => 0, "notification_category" => "TestImmediately"} # check a broadcast-policy-based one
                    ]
   end
 
-  it "should still return notification_category in the the activity stream summary if not set (yet)" do
-    # TODO: can remove this spec as well as the code in lib/api/v1/stream_item once the datafixup has been run
-    @context = @course
-    Notification.create(:name => 'Assignment Due Date Changed', :category => "TestImmediately")
-    Assignment.any_instance.stubs(:created_at).returns(4.hours.ago)
-    assignment_model(:course => @course)
-    @assignment.update_attribute(:due_at, 1.week.from_now)
-    @assignment.update_attribute(:due_at, 2.weeks.from_now)
-    # manually set the pre-datafixup state for one of them
-    StreamItem.where(:id => @user.visible_stream_item_instances.first.stream_item).update_all(:notification_category => nil)
-    json = api_call(:get, "/api/v1/users/self/activity_stream/summary.json",
-      { :controller => "users", :action => "activity_stream_summary", :format => 'json' })
-
-    expect(json).to eq [
-          {"type" => "Message", "count" => 2, "unread_count" => 0, "notification_category" => "TestImmediately"} # check a broadcast-policy-based one
-        ]
-  end
-
   it "should format DiscussionTopic" do
     @context = @course
     discussion_topic_model