Extract Topic#posters_summary

app/models/topic.rb

@@ -560,60 +560,8 @@ class Topic < ActiveRecord::Base
     save
   end
 
-  # Create the summary of the interesting posters in a topic. Cheats to avoid
-  # many queries.
-  def posters_summary(topic_user = nil, current_user = nil, opts={})
-    return @posters_summary if @posters_summary.present?
-    descriptions = {}
-
-    # Use an avatar lookup object if we have it, otherwise create one just for this forum topic
-    al = opts[:avatar_lookup]
-    if al.blank?
-      al = AvatarLookup.new([user_id, last_post_user_id, featured_user1_id, featured_user2_id, featured_user3_id])
-    end
-
-    # Helps us add a description to a poster
-    add_description = lambda do |u, desc|
-      if u.present?
-        descriptions[u.id] ||= []
-        descriptions[u.id] << I18n.t(desc)
-      end
-    end
-
-    add_description.call(al[user_id], :original_poster)
-    add_description.call(al[featured_user1_id], :most_posts)
-    add_description.call(al[featured_user2_id], :frequent_poster)
-    add_description.call(al[featured_user3_id], :frequent_poster)
-    add_description.call(al[featured_user4_id], :frequent_poster)
-    add_description.call(al[last_post_user_id], :most_recent_poster)
-
-
-    @posters_summary = [al[user_id],
-                        al[last_post_user_id],
-                        al[featured_user1_id],
-                        al[featured_user2_id],
-                        al[featured_user3_id],
-                        al[featured_user4_id]
-                        ].compact.uniq[0..4]
-
-    unless @posters_summary[0] == al[last_post_user_id]
-      # shuffle last_poster to back
-      @posters_summary.reject!{|u| u == al[last_post_user_id]}
-      @posters_summary << al[last_post_user_id]
-    end
-    @posters_summary.map! do |p|
-      if p
-        result = TopicPoster.new
-        result.user = p
-        result.description = descriptions[p.id].join(', ')
-        result.extras = "latest" if al[last_post_user_id] == p
-        result
-      else
-        nil
-      end
-    end.compact!
-
-    @posters_summary
+  def posters_summary(options = {})
+    @posters_summary ||= TopicPostersSummary.new(self, options).summary
   end
 
   # Enable/disable the star on the topic

app/models/topic_posters_summary.rb

@@ -0,0 +1,79 @@
+class TopicPostersSummary
+  attr_reader :topic, :options
+
+  def initialize(topic, options = {})
+    @topic = topic
+    @options = options
+  end
+
+  def summary
+    sorted_top_posters.map { |user| new_topic_poster_for user }
+  end
+
+  private
+
+  def new_topic_poster_for(user)
+    TopicPoster.new.tap do |topic_poster|
+      topic_poster.user = user
+      topic_poster.description = descriptions_for(user)
+      topic_poster.extras = 'latest' if is_latest_poster?(user)
+    end
+  end
+
+  def descriptions_by_id
+    @descriptions_by_id ||= begin
+      user_ids_with_descriptions.inject({}) do |descriptions, (id, description)|
+        descriptions[id] ||= []
+        descriptions[id] << description
+        descriptions
+      end
+    end
+  end
+
+  def descriptions_for(user)
+    descriptions_by_id[user.id].join ', '
+  end
+
+  def shuffle_last_poster_to_back_in(summary)
+    unless last_poster_is_topic_creator?
+      summary.reject!{ |u| u.id == topic.last_post_user_id }
+      summary << avatar_lookup[topic.last_post_user_id]
+    end
+    summary
+  end
+
+  def user_ids_with_descriptions
+    user_ids.zip([
+      :original_poster,
+      :most_recent_poster,
+      :most_posts,
+      :frequent_poster,
+      :frequent_poster,
+      :frequent_poster
+    ].map { |description| I18n.t(description) })
+  end
+
+  def last_poster_is_topic_creator?
+    topic.user_id == topic.last_post_user_id
+  end
+
+  def is_latest_poster?(user)
+    topic.last_post_user_id == user.id
+  end
+
+  def sorted_top_posters
+    shuffle_last_poster_to_back_in top_posters
+  end
+
+  def top_posters
+    user_ids.map { |id| avatar_lookup[id] }.compact.uniq.take(5)
+  end
+
+  def user_ids
+    [ topic.user_id, topic.last_post_user_id, *topic.featured_user_ids ]
+  end
+
+  def avatar_lookup
+    @avatar_lookup ||= options[:avatar_lookup] || AvatarLookup.new(user_ids)
+  end
+end

spec/models/topic_posters_summary_spec.rb

@@ -0,0 +1,71 @@
+require 'spec_helper'
+
+describe TopicPostersSummary do
+  describe '#summary' do
+    let(:summary) { described_class.new(topic).summary }
+
+    let(:topic) do
+      Fabricate(:topic,
+        user: topic_creator,
+        last_poster: last_poster,
+        featured_user1: featured_user1,
+        featured_user2: featured_user2,
+        featured_user3: featured_user3,
+        featured_user4: featured_user4
+      )
+    end
+
+    let(:topic_creator)  { Fabricate(:user) }
+    let(:last_poster)    { nil }
+    let(:featured_user1) { nil }
+    let(:featured_user2) { nil }
+    let(:featured_user3) { nil }
+    let(:featured_user4) { nil }
+
+    it 'contains only the topic creator when there are no other posters' do
+      expect(summary.count).to eq 1
+
+      summary.first.tap do |topic_poster|
+        expect(topic_poster.user).to eq topic_creator
+        expect(topic_poster.extras).to eq 'latest'
+        expect(topic_poster.description).to eq(
+          "#{I18n.t(:original_poster)}, #{I18n.t(:most_recent_poster)}"
+        )
+      end
+    end
+
+    context 'when the lastest poster is also the topic creator' do
+      let(:last_poster)    { topic_creator }
+      let(:featured_user1) { Fabricate(:user) }
+
+      before do
+        topic.last_poster = topic_creator
+        topic.featured_user1 = featured_user1
+        topic.save!
+      end
+
+      it 'keeps the topic creator at the front of the summary' do
+        expect(summary.map(&:user)).to eq([
+          topic_creator,
+          featured_user1
+        ])
+      end
+    end
+
+    context 'when the topic has many posters' do
+      let(:last_poster)    { Fabricate(:user) }
+      let(:featured_user1) { Fabricate(:user) }
+      let(:featured_user2) { Fabricate(:user) }
+      let(:featured_user3) { Fabricate(:user) }
+      let(:featured_user4) { Fabricate(:user) }
+
+      it 'contains only five posters with latest poster at the end' do
+        expect(summary.map(&:user)).to eq([
+          topic_creator,
+          featured_user1, featured_user2, featured_user3,
+          last_poster
+        ])
+      end
+    end
+  end
+end