From 611b5f996e7a2b4a59ee820da0d1cf61d1552354 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Wed, 10 Jun 2015 14:36:47 -0400
Subject: [PATCH] FIX: unpinned topics shouldn't remain pinned on categories
 page

---
 app/models/category_list.rb                 | 27 +++++++++++++++++++++
 lib/topic_query.rb                          |  3 ++-
 spec/components/category_list_spec.rb       | 21 ++++++++++++++++
 spec/models/category_featured_topic_spec.rb |  6 +++--
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/app/models/category_list.rb b/app/models/category_list.rb
index b7868cdb0f8..b5cae822b87 100644
--- a/app/models/category_list.rb
+++ b/app/models/category_list.rb
@@ -1,3 +1,5 @@
+require_dependency 'pinned_check'
+
 class CategoryList
   include ActiveModel::Serialization
 
@@ -17,6 +19,8 @@ class CategoryList
 
     prune_empty
     find_user_data
+    sort_unpinned
+    trim_results
   end
 
   private
@@ -151,4 +155,27 @@ class CategoryList
         @all_topics.each { |ft| ft.user_data = topic_lookup[ft.id] }
       end
     end
+
+    def sort_unpinned
+      if @guardian.current_user && @all_topics.present?
+        # Put unpinned topics at the end of the list
+        @categories.each do |c|
+          next if c.displayable_topics.blank? || c.displayable_topics.size <= latest_posts_count
+          unpinned = []
+          c.displayable_topics.each do |t|
+            unpinned << t if t.pinned_at && PinnedCheck.unpinned?(t, t.user_data)
+          end
+          unless unpinned.empty?
+            c.displayable_topics = (c.displayable_topics - unpinned) + unpinned
+          end
+        end
+      end
+    end
+
+    def trim_results
+      @categories.each do |c|
+        next if c.displayable_topics.blank?
+        c.displayable_topics = c.displayable_topics[0,latest_posts_count]
+      end
+    end
 end
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index 15af3dd5157..13c3f9bb4b6 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -135,9 +135,10 @@ class TopicQuery
   def list_category_topic_ids(category)
     query = default_results(category: category.id)
     pinned_ids = query.where('pinned_at IS NOT NULL AND category_id = ?', category.id)
+                      .limit(nil)
                       .order('pinned_at DESC').pluck(:id)
     non_pinned_ids = query.where('pinned_at IS NULL OR category_id <> ?', category.id).pluck(:id)
-    (pinned_ids + non_pinned_ids)[0...@options[:per_page]]
+    (pinned_ids + non_pinned_ids)
   end
 
   def list_new_in_category(category)
diff --git a/spec/components/category_list_spec.rb b/spec/components/category_list_spec.rb
index 104173d3b91..808abcd5d81 100644
--- a/spec/components/category_list_spec.rb
+++ b/spec/components/category_list_spec.rb
@@ -102,6 +102,27 @@ describe CategoryList do
       end
     end
 
+    context "with pinned topics in a category" do
+      let!(:topic1) { Fabricate(:topic, category: topic_category, bumped_at: 8.minutes.ago) }
+      let!(:topic2) { Fabricate(:topic, category: topic_category, bumped_at: 5.minutes.ago) }
+      let!(:topic3) { Fabricate(:topic, category: topic_category, bumped_at: 2.minutes.ago) }
+      let!(:pinned) { Fabricate(:topic, category: topic_category, pinned_at: 10.minutes.ago, bumped_at: 10.minutes.ago) }
+      let(:category) { category_list.categories.first }
+
+      before do
+        SiteSetting.stubs(:category_featured_topics).returns(2)
+      end
+
+      it "returns pinned topic first" do
+        expect(category.displayable_topics.map(&:id)).to eq([pinned.id, topic3.id])
+      end
+
+      it "returns topics in bumped_at order if pinned was unpinned" do
+        PinnedCheck.stubs(:unpinned?).returns(true)
+        expect(category.displayable_topics.map(&:id)).to eq([topic3.id, topic2.id])
+      end
+    end
+
   end
 
   describe 'category order' do
diff --git a/spec/models/category_featured_topic_spec.rb b/spec/models/category_featured_topic_spec.rb
index 4859ea7cde3..38c3383a458 100644
--- a/spec/models/category_featured_topic_spec.rb
+++ b/spec/models/category_featured_topic_spec.rb
@@ -34,9 +34,11 @@ describe CategoryFeaturedTopic do
 
 
     it 'should feature stuff in the correct order' do
+      SiteSetting.stubs(:category_featured_topics).returns(3)
 
       category = Fabricate(:category)
-      _t3 = Fabricate(:topic, category_id: category.id, bumped_at: 7.minutes.ago)
+      t4 = Fabricate(:topic, category_id: category.id, bumped_at: 10.minutes.ago)
+      t3 = Fabricate(:topic, category_id: category.id, bumped_at: 7.minutes.ago)
       t2 = Fabricate(:topic, category_id: category.id, bumped_at: 4.minutes.ago)
       t1 = Fabricate(:topic, category_id: category.id, bumped_at: 5.minutes.ago)
       pinned = Fabricate(:topic, category_id: category.id, pinned_at: 10.minutes.ago, bumped_at: 10.minutes.ago)
@@ -45,7 +47,7 @@ describe CategoryFeaturedTopic do
 
       expect(
         CategoryFeaturedTopic.where(category_id: category.id).pluck(:topic_id)
-      ).to eq([pinned.id, t2.id, t1.id])
+      ).to eq([pinned.id, t2.id, t1.id, t3.id])
 
     end
   end