From f1e2923a91022e77b320a8f3ab25ff166802283e Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 26 Mar 2013 14:18:35 -0400 Subject: [PATCH] Display correct post counts, even with a filter active --- .../discourse/controllers/topic_controller.js | 7 +-- .../discourse/templates/topic.js.handlebars | 16 +++--- .../javascripts/discourse/views/topic_view.js | 56 ++++++++----------- app/serializers/topic_view_serializer.rb | 20 ++++++- lib/topic_view.rb | 21 +++++-- spec/components/topic_view_spec.rb | 39 ++++++++++--- 6 files changed, 98 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index aa4a23c9a5d..3f180ae7de9 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -62,11 +62,9 @@ Discourse.TopicController = Discourse.ObjectController.extend({ hideProgress: function() { if (!this.get('content.loaded')) return true; if (!this.get('currentPost')) return true; - if (this.get('content.highest_post_number') < 2) return true; - if (this.get('bestOf')) return true; - if (this.get('userFilters.length')) return true; + if (this.get('content.filtered_posts_count') < 2) return true; return false; - }.property('content.loaded', 'currentPost', 'bestOf', 'userFilters.length'), + }.property('content.loaded', 'currentPost', 'content.filtered_posts_count'), selectPost: function(post) { post.toggleProperty('selected'); @@ -215,6 +213,7 @@ Discourse.TopicController = Discourse.ObjectController.extend({ }); topicController.updateBottomBar(); + topicController.set('filtered_posts_count', result.filtered_posts_count); topicController.set('loadingBelow', false); topicController.set('seenBottom', false); }); diff --git a/app/assets/javascripts/discourse/templates/topic.js.handlebars b/app/assets/javascripts/discourse/templates/topic.js.handlebars index b7ebf207fa3..c432e04d354 100644 --- a/app/assets/javascripts/discourse/templates/topic.js.handlebars +++ b/app/assets/javascripts/discourse/templates/topic.js.handlebars @@ -6,7 +6,7 @@
{{#if view.showFavoriteButton}} - + {{/if}} {{#if view.editingTopic}} @@ -41,15 +41,15 @@ {{view Discourse.SelectedPostsView}}
-
+
-
@@ -128,12 +128,12 @@ {{render share}} {{render quoteButton}} {{#if Discourse.currentUser.admin}} - {{render topicAdminMenu controller.content}} + {{render topicAdminMenu content}} {{/if}} diff --git a/app/assets/javascripts/discourse/views/topic_view.js b/app/assets/javascripts/discourse/views/topic_view.js index 8629acc3f79..c60952af722 100644 --- a/app/assets/javascripts/discourse/views/topic_view.js +++ b/app/assets/javascripts/discourse/views/topic_view.js @@ -18,20 +18,13 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { SHORT_POST: 1200, // Update the progress bar using sweet animations - updateBar: (function() { + updateBar: function() { var $topicProgress, bg, currentWidth, progressWidth, ratio, totalWidth; if (!this.get('topic.loaded')) return; $topicProgress = $('#topic-progress'); if (!$topicProgress.length) return; - // Don't show progress when there is only one post - if (this.get('topic.highest_post_number') === 1) { - $topicProgress.hide(); - } else { - $topicProgress.show(); - } - - ratio = this.get('progressPosition') / this.get('topic.highest_post_number'); + ratio = this.get('progressPosition') / this.get('topic.filtered_posts_count'); totalWidth = $topicProgress.width(); progressWidth = ratio * totalWidth; bg = $topicProgress.find('.bg'); @@ -53,15 +46,15 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { } else { bg.animate({ width: progressWidth }, 400); } - }).observes('progressPosition', 'topic.highest_post_number', 'topic.loaded'), + }.observes('progressPosition', 'topic.filtered_posts_count', 'topic.loaded'), - updateTitle: (function() { + updateTitle: function() { var title; title = this.get('topic.title'); if (title) return Discourse.set('title', title); - }).observes('topic.loaded', 'topic.title'), + }.observes('topic.loaded', 'topic.title'), - currentPostChanged: (function() { + currentPostChanged: function() { var current = this.get('controller.currentPost'); var topic = this.get('topic'); @@ -93,13 +86,13 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { } else { $('#jump-bottom').attr('disabled', false); } - }).observes('controller.currentPost', 'controller.bestOf', 'topic.highest_post_number'), + }.observes('controller.currentPost', 'controller.bestOf', 'topic.highest_post_number'), - composeChanged: (function() { + composeChanged: function() { var composerController = Discourse.get('router.composerController'); composerController.clearState(); - return composerController.set('topic', this.get('topic')); - }).observes('composer'), + composerController.set('topic', this.get('topic')); + }.observes('composer'), // This view is being removed. Shut down operations willDestroyElement: function() { @@ -267,18 +260,18 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { return this.loadMore(this.getPost($post)); }, - postCountChanged: (function() { + postCountChanged: function() { this.set('controller.seenBottom', false); - }).observes('topic.highest_post_number'), + }.observes('topic.highest_post_number'), loadMore: function(post) { - if (!post) { return; } - if (this.get('controller.loading')) { return; } + if (!post) return; + if (this.get('controller.loading')) return; // Don't load if we know we're at the bottom - if (this.get('topic.highest_post_number') === post.get('post_number')) { return; } + if (this.get('topic.highest_post_number') === post.get('post_number')) return; - if (this.get('controller.seenBottom')) { return; } + if (this.get('controller.seenBottom')) return; // Don't double load ever if (this.topic.posts.last().post_number !== post.post_number) return; @@ -298,7 +291,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { if (result.suggested_topics) { var suggested = Em.A(); result.suggested_topics.each(function(st) { - return suggested.pushObject(Discourse.Topic.create(st)); + suggested.pushObject(Discourse.Topic.create(st)); }); topicView.set('topic.suggested_topics', suggested); } @@ -350,11 +343,10 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }, updateDock: function(postView) { - var post; if (!postView) return; - post = postView.get('post'); + var post = postView.get('post'); if (!post) return; - this.set('progressPosition', post.get('post_number')); + this.set('progressPosition', post.get('index')); }, nonUrgentPositionUpdate: Discourse.debounce(function(opts){ @@ -370,7 +362,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }, updatePosition: function(userActive) { - var $lastPost, firstLoaded, lastPostOffset, offset, + var $lastPost, firstLoaded, lastPostOffset, offset, title, info, rows, screenTrack, _this, currentPost; _this = this; @@ -382,7 +374,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { // if we have no rows if(!info) { return; } - // top on screen + // top on screen if(info.top === 0 || info.onScreen[0] === 0 || info.bottom === 0) { this.prevPage($(rows[0])); } @@ -393,7 +385,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { this.nextPage($(rows[info.bottom])); } - // update dock + // update dock this.updateDock(Ember.View.views[rows[info.bottom].id]); // mark everything on screen read @@ -403,10 +395,10 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }); this.nonUrgentPositionUpdate({ - userActive: userActive, + userActive: userActive, currentPost: currentPost || this.getPost($(rows[info.bottom])).get('post_number') }); - + offset = window.pageYOffset || $('html').scrollTop(); firstLoaded = this.get('firstPostLoaded'); if (!this.docAt) { diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 76325cc9d1a..9b4b73b7691 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -44,7 +44,8 @@ class TopicViewSerializer < ApplicationSerializer :posts, :at_bottom, :highest_post_number, - :pinned + :pinned, + :filtered_posts_count has_one :created_by, serializer: BasicUserSerializer, embed: :objects has_one :last_poster, serializer: BasicUserSerializer, embed: :objects @@ -98,6 +99,10 @@ class TopicViewSerializer < ApplicationSerializer object.post_action_visibility.present? end + def filtered_posts_count + object.filtered_posts_count + end + def voted_in_topic object.voted_in_topic? end @@ -204,13 +209,22 @@ class TopicViewSerializer < ApplicationSerializer @posts = [] @highest_number_in_posts = 0 if object.posts.present? - object.posts.each do |p| + object.posts.each_with_index do |p, idx| @highest_number_in_posts = p.post_number if p.post_number > @highest_number_in_posts ps = PostSerializer.new(p, scope: scope, root: false) ps.topic_slug = object.topic.slug ps.topic_view = object p.topic = object.topic - @posts << ps.as_json + + post_json = ps.as_json + + if object.index_reverse + post_json[:index] = object.index_offset - idx + else + post_json[:index] = object.index_offset + idx + 1 + end + + @posts << post_json end end @posts diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 3a74d8d35a9..c918e96c499 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -4,11 +4,8 @@ require_dependency 'summarize' class TopicView - attr_accessor :topic, - :draft, - :draft_key, - :draft_sequence, - :posts + attr_reader :topic, :posts, :index_offset, :index_reverse + attr_accessor :draft, :draft_key, :draft_sequence def initialize(topic_id, user=nil, options={}) @topic = find_topic(topic_id) @@ -34,6 +31,7 @@ class TopicView @user = user @initial_load = true + @index_reverse = false filter_posts(options) @@ -75,6 +73,10 @@ class TopicView @topic.title end + def filtered_posts_count + @filtered_posts_count ||= @filtered_posts.count + end + def summary return nil if posts.blank? Summarize.new(posts.first.cooked).summary @@ -146,8 +148,13 @@ class TopicView sort_order = sort_order_for_post_number(post_number) return nil unless sort_order + + # Find posts before the `sort_order` @posts = @filtered_posts.order('sort_order desc').where("sort_order < ?", sort_order) + @index_offset = @posts.count + @index_reverse = true + @posts = @posts.includes(:reply_to_user).includes(:topic).joins(:user).limit(SiteSetting.posts_per_page) end @@ -158,6 +165,7 @@ class TopicView sort_order = sort_order_for_post_number(post_number) return nil unless sort_order + @index_offset = @filtered_posts.where("sort_order <= ?", sort_order).count @posts = @filtered_posts.order('sort_order').where("sort_order > ?", sort_order) @posts = @posts.includes(:reply_to_user).includes(:topic).joins(:user).limit(SiteSetting.posts_per_page) end @@ -263,6 +271,7 @@ class TopicView private def filter_posts_in_range(min, max) + max_index = (filtered_post_ids.length - 1) # If we're off the charts, return nil @@ -272,6 +281,8 @@ class TopicView max = max_index if max > max_index min = 0 if min < 0 + @index_offset = min + # TODO: Sort might be off @posts = Post.where(id: filtered_post_ids[min..max]) .includes(:user) diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index dc200023adf..cbf6927c499 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -7,7 +7,6 @@ describe TopicView do let(:coding_horror) { Fabricate(:coding_horror) } let(:first_poster) { topic.user } - let(:topic_view) { TopicView.new(topic.id, coding_horror) } it "raises a not found error if the topic doesn't exist" do @@ -192,10 +191,14 @@ describe TopicView do it "returns undeleted posts after a post" do topic_view.filter_posts_after(p1.post_number).should == [p2, p3, p5] topic_view.should_not be_initial_load + topic_view.index_offset.should == 1 + topic_view.index_reverse.should be_false end it "clips to the end boundary" do topic_view.filter_posts_after(p2.post_number).should == [p3, p5] + topic_view.index_offset.should == 2 + topic_view.index_reverse.should be_false end it "returns nothing after the last post" do @@ -209,6 +212,8 @@ describe TopicView do it "returns deleted posts to an admin" do coding_horror.admin = true topic_view.filter_posts_after(p1.post_number).should == [p2, p3, p4] + topic_view.index_offset.should == 1 + topic_view.index_reverse.should be_false end end @@ -216,10 +221,14 @@ describe TopicView do it "returns undeleted posts before a post" do topic_view.filter_posts_before(p5.post_number).should == [p3, p2, p1] topic_view.should_not be_initial_load + topic_view.index_offset.should == 3 + topic_view.index_reverse.should be_true end it "clips to the beginning boundary" do topic_view.filter_posts_before(p3.post_number).should == [p2, p1] + topic_view.index_offset.should == 2 + topic_view.index_reverse.should be_true end it "returns nothing before the first post" do @@ -234,35 +243,47 @@ describe TopicView do it "returns deleted posts to an admin" do coding_horror.admin = true topic_view.filter_posts_before(p5.post_number).should == [p4, p3, p2] + topic_view.index_offset.should == 4 + topic_view.index_reverse.should be_true end end describe "filter_posts_near" do - def topic_view_posts_near(post) - TopicView.new(topic.id, coding_horror, post_number: post.post_number).posts + def topic_view_near(post) + TopicView.new(topic.id, coding_horror, post_number: post.post_number) end it "snaps to the lower boundary" do - topic_view_posts_near(p1).should == [p1, p2, p3] + near_view = topic_view_near(p1) + near_view.posts.should == [p1, p2, p3] + near_view.index_offset.should == 0 + near_view.index_reverse.should be_false end it "snaps to the upper boundary" do - topic_view_posts_near(p5).should == [p2, p3, p5] + near_view = topic_view_near(p5) + near_view.posts.should == [p2, p3, p5] + near_view.index_offset.should == 1 + near_view.index_reverse.should be_false end it "returns the posts in the middle" do - topic_view_posts_near(p2).should == [p1, p2, p3] + near_view = topic_view_near(p2) + near_view.posts.should == [p1, p2, p3] + near_view.index_offset.should == 0 + near_view.index_reverse.should be_false end it "returns deleted posts to an admin" do coding_horror.admin = true - topic_view_posts_near(p3).should == [p2, p3, p4] + near_view = topic_view_near(p3) + near_view.posts.should == [p2, p3, p4] + near_view.index_offset.should == 1 + near_view.index_reverse.should be_false end - end - end end