From 2d6b15a34dc8eaf859a357febf3454365fbb5333 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 12 Dec 2014 11:47:20 -0500 Subject: [PATCH] Load fewer posts when the android platform is detected --- .../javascripts/admin/models/site_setting.js | 8 -- .../discourse/models/post_stream.js | 73 +------------------ .../javascripts/discourse/models/site.js | 8 -- app/controllers/topics_controller.rb | 3 +- app/serializers/topic_view_serializer.rb | 7 +- config/locales/server.en.yml | 1 + config/site_settings.yml | 4 +- lib/topic_view.rb | 5 +- spec/components/topic_view_spec.rb | 10 +++ .../models/post-stream-test.js.es6 | 5 +- 10 files changed, 28 insertions(+), 96 deletions(-) diff --git a/app/assets/javascripts/admin/models/site_setting.js b/app/assets/javascripts/admin/models/site_setting.js index 2990d908cc0..aa43219b495 100644 --- a/app/assets/javascripts/admin/models/site_setting.js +++ b/app/assets/javascripts/admin/models/site_setting.js @@ -1,11 +1,3 @@ -/** - Our data model for interacting with site settings. - - @class SiteSetting - @extends Discourse.Model - @namespace Discourse - @module Discourse -**/ Discourse.SiteSetting = Discourse.Model.extend({ validationMessage: null, diff --git a/app/assets/javascripts/discourse/models/post_stream.js b/app/assets/javascripts/discourse/models/post_stream.js index 583a7939c81..cfdfa67d6b4 100644 --- a/app/assets/javascripts/discourse/models/post_stream.js +++ b/app/assets/javascripts/discourse/models/post_stream.js @@ -1,59 +1,17 @@ -/** - We use this class to keep on top of streaming and filtering posts within a topic. - - @class PostStream - @extends Ember.Object - @namespace Discourse - @module Discourse -**/ Discourse.PostStream = Em.Object.extend({ - /** - Are we currently loading posts in any way? - - @property loading - **/ loading: Em.computed.or('loadingAbove', 'loadingBelow', 'loadingFilter', 'stagingPost'), - notLoading: Em.computed.not('loading'), - filteredPostsCount: Em.computed.alias("stream.length"), - /** - Have we loaded any posts? - - @property hasPosts - **/ - hasPosts: function(){ + hasPosts: function() { return this.get('posts.length') > 0; }.property("posts.@each"), - /** - Do we have a stream list of post ids? - - @property hasStream - **/ hasStream: Em.computed.gt('filteredPostsCount', 0), - - /** - Can we append more posts to our current stream? - - @property canAppendMore - **/ canAppendMore: Em.computed.and('notLoading', 'hasPosts', 'lastPostNotLoaded'), - - /** - Can we prepend more posts to our current stream? - - @property canPrependMore - **/ canPrependMore: Em.computed.and('notLoading', 'hasPosts', 'firstPostNotLoaded'), - /** - Have we loaded the first post in the stream? - - @property firstPostPresent - **/ firstPostPresent: function() { if (!this.get('hasLoadedData')) { return false; } return !!this.get('posts').findProperty('id', this.get('firstPostId')); @@ -61,47 +19,22 @@ Discourse.PostStream = Em.Object.extend({ firstPostNotLoaded: Em.computed.not('firstPostPresent'), - /** - The first post that we have loaded. Useful for checking to see if we should scroll upwards - - @property firstLoadedPost - **/ firstLoadedPost: function() { return _.first(this.get('posts')); }.property('posts.@each'), - /** - The last post we have loaded. Useful for checking to see if we should load more - - @property lastLoadedPost - **/ lastLoadedPost: function() { return _.last(this.get('posts')); }.property('posts.@each'), - /** - Returns the id of the first post in the set - - @property firstPostId - **/ firstPostId: function() { return this.get('stream')[0]; }.property('stream.@each'), - /** - Returns the id of the last post in the set - - @property lastPostId - **/ lastPostId: function() { return _.last(this.get('stream')); }.property('stream.@each'), - /** - Have we loaded the last post in the stream? - - @property loadedAllPosts - **/ loadedAllPosts: function() { if (!this.get('hasLoadedData')) { return false; } return !!this.get('posts').findProperty('id', this.get('lastPostId')); @@ -149,7 +82,7 @@ Discourse.PostStream = Em.Object.extend({ var firstIndex = this.indexOf(firstPost); if (firstIndex === -1) { return []; } - var startIndex = firstIndex - Discourse.SiteSettings.posts_chunksize; + var startIndex = firstIndex - this.get('topic.chunk_size'); if (startIndex < 0) { startIndex = 0; } return stream.slice(startIndex, firstIndex); @@ -173,7 +106,7 @@ Discourse.PostStream = Em.Object.extend({ if ((lastIndex + 1) >= this.get('highest_post_number')) { return []; } // find our window of posts - return stream.slice(lastIndex+1, lastIndex+Discourse.SiteSettings.posts_chunksize+1); + return stream.slice(lastIndex+1, lastIndex + this.get('topic.chunk_size') + 1); }.property('lastLoadedPost', 'stream.@each'), diff --git a/app/assets/javascripts/discourse/models/site.js b/app/assets/javascripts/discourse/models/site.js index c8e09a9dce1..e81c16002b3 100644 --- a/app/assets/javascripts/discourse/models/site.js +++ b/app/assets/javascripts/discourse/models/site.js @@ -1,11 +1,3 @@ -/** - A data model representing the site (instance of Discourse) - - @class Site - @extends Discourse.Model - @namespace Discourse - @module Discourse -**/ Discourse.Site = Discourse.Model.extend({ isReadOnly: Em.computed.alias('is_readonly'), diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 9e976f21247..55038625d0d 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -47,6 +47,7 @@ class TopicsController < ApplicationController opts = params.slice(:username_filters, :filter, :page, :post_number, :show_deleted) username_filters = opts[:username_filters] + opts[:slow_platform] = true if request.user_agent =~ /Android/ opts[:username_filters] = username_filters.split(',') if username_filters.is_a?(String) begin @@ -58,7 +59,7 @@ class TopicsController < ApplicationController end page = params[:page].to_i - if (page < 0) || ((page - 1) * SiteSetting.posts_chunksize > @topic_view.topic.highest_post_number) + if (page < 0) || ((page - 1) * @topic_view.chunk_size > @topic_view.topic.highest_post_number) raise Discourse::NotFound end diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 030a1e648dc..92c08c34620 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -42,8 +42,8 @@ class TopicViewSerializer < ApplicationSerializer :has_deleted, :actions_summary, :expandable_first_post, - :is_warning - + :is_warning, + :chunk_size # Define a delegator for each attribute of the topic we want attributes(*topic_attributes) @@ -113,6 +113,9 @@ class TopicViewSerializer < ApplicationSerializer result end + def chunk_size + object.chunk_size + end def is_warning object.topic.private_message? && object.topic.subtype == TopicSubtype.moderator_warning diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9066964a928..64c6e6ab4d5 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -739,6 +739,7 @@ en: track_external_right_clicks: "Track external links that are right clicked (eg: open in new tab) disabled by default because it rewrites URLs" topics_per_page: "How many topics are loaded by default on the topic list, and when scrolling down to load more topics" posts_chunksize: "How many posts are loaded by default on a topic, and when scrolling down to load more posts" + posts_slow_chunksize: "Like `posts_chunksize` but for slow platforms (such as Android)" site_contact_username: "All automated private messages will be from this user; if left blank the default System account will be used." send_welcome_message: "Send all new users a welcome private message with a quick start guide." suppress_reply_directly_below: "Don't show the expandable reply count on a post when there is only a single reply directly below this post." diff --git a/config/site_settings.yml b/config/site_settings.yml index b7276e60ef1..90b031808ce 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -653,9 +653,11 @@ developer: default: false client: true posts_chunksize: - client: true default: 20 min: 1 + posts_slow_chunksize: + default: 10 + min: 1 embedding: embeddable_host: '' diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 526a8831d33..ed9f2ea3065 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -5,7 +5,7 @@ require_dependency 'gaps' class TopicView - attr_reader :topic, :posts, :guardian, :filtered_posts + attr_reader :topic, :posts, :guardian, :filtered_posts, :chunk_size attr_accessor :draft, :draft_key, :draft_sequence, :user_custom_fields def initialize(topic_id, user=nil, options={}) @@ -20,7 +20,8 @@ class TopicView @page = @page.to_i @page = 1 if @page.zero? - @limit ||= SiteSetting.posts_chunksize + @chunk_size = options[:slow_platform] ? SiteSetting.posts_slow_chunksize : SiteSetting.posts_chunksize + @limit ||= @chunk_size setup_filtered_posts diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index b25263bd9dc..675b2a40bae 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -25,6 +25,16 @@ describe TopicView do lambda { TopicView.new(topic.id, admin) }.should_not raise_error end + context "chunk_size" do + it "returns `posts_chunksize` by default" do + TopicView.new(topic.id, coding_horror).chunk_size.should == SiteSetting.posts_chunksize + end + + it "returns `posts_slow_chunksize` when slow_platform is true" do + tv = TopicView.new(topic.id, coding_horror, slow_platform: true) + tv.chunk_size.should == SiteSetting.posts_slow_chunksize + end + end context "with a few sample posts" do let!(:p1) { Fabricate(:post, topic: topic, user: first_poster, percent_rank: 1 )} diff --git a/test/javascripts/models/post-stream-test.js.es6 b/test/javascripts/models/post-stream-test.js.es6 index 50b86de67e1..fccbbd24165 100644 --- a/test/javascripts/models/post-stream-test.js.es6 +++ b/test/javascripts/models/post-stream-test.js.es6 @@ -1,7 +1,7 @@ module("Discourse.PostStream"); var buildStream = function(id, stream) { - var topic = Discourse.Topic.create({id: id}); + var topic = Discourse.Topic.create({id: id, chunk_size: 5}); var ps = topic.get('postStream'); if (stream) { ps.set('stream', stream); @@ -20,7 +20,6 @@ test('defaults', function() { blank(postStream.get('posts'), "there are no posts in a stream by default"); ok(!postStream.get('loaded'), "it has never loaded"); present(postStream.get('topic')); - }); test('appending posts', function() { @@ -182,7 +181,6 @@ test("loading", function() { }); test("nextWindow", function() { - Discourse.SiteSettings.posts_chunksize = 5; var postStream = buildStream(1234, [1,2,3,5,8,9,10,11,13,14,15,16]); blank(postStream.get('nextWindow'), 'With no posts loaded, the window is blank'); @@ -199,7 +197,6 @@ test("nextWindow", function() { }); test("previousWindow", function() { - Discourse.SiteSettings.posts_chunksize = 5; var postStream = buildStream(1234, [1,2,3,5,8,9,10,11,13,14,15,16]); blank(postStream.get('previousWindow'), 'With no posts loaded, the window is blank');