From 711371e8c8b60f3bc2e4397fd0bb679241d6c0a8 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 28 Jun 2018 14:54:54 +0800 Subject: [PATCH] FIX: Select+below will ask server for post ids on megatopics. --- .../discourse/controllers/topic.js.es6 | 40 ++++++++++-- app/controllers/topics_controller.rb | 35 ++++++++-- config/routes.rb | 1 + lib/topic_view.rb | 4 +- spec/requests/topics_controller_spec.rb | 64 +++++++++++++++++++ test/javascripts/acceptance/topic-test.js.es6 | 23 +++++++ 6 files changed, 155 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 50fdf050ad5..adae4a7786e 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -152,6 +152,35 @@ export default Ember.Controller.extend(BufferedContent, { this.appEvents.trigger("post-stream:refresh", { force: true }); }, + _updateSelectedPostIds(postIds) { + this.get("selectedPostIds").pushObjects(postIds); + this.set("selectedPostIds", [...new Set(this.get("selectedPostIds"))]); + this._forceRefreshPostStream(); + }, + + _loadPostIds(post) { + if (this.get("loadingPostIds")) return; + + const postStream = this.get("model.postStream"); + const url = `/t/${this.get("model.id")}/post_ids.json`; + + this.set("loadingPostIds", true); + + return ajax(url, { + data: _.merge( + { post_number: post.get("post_number") }, + postStream.get("streamFilters") + ) + }) + .then(result => { + result.post_ids.pushObject(post.get("id")); + this._updateSelectedPostIds(result.post_ids); + }) + .finally(() => { + this.set("loadingPostIds", false); + }); + }, + actions: { showPostFlags(post) { return this.send("showFlags", post); @@ -627,10 +656,13 @@ export default Ember.Controller.extend(BufferedContent, { }, selectBelow(post) { - const stream = [...this.get("model.postStream.stream")]; - const below = stream.slice(stream.indexOf(post.id)); - this.get("selectedPostIds").pushObjects(below); - this._forceRefreshPostStream(); + if (this.get("model.postStream.isMegaTopic")) { + this._loadPostIds(post); + } else { + const stream = [...this.get("model.postStream.stream")]; + const below = stream.slice(stream.indexOf(post.id)); + this._updateSelectedPostIds(below); + } }, deleteSelected() { diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 76059440cdc..68257a3f08a 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -175,22 +175,34 @@ class TopicsController < ApplicationController render_json_dump(wordpress_serializer) end + def post_ids + params.require(:topic_id) + params.permit(:post_number, :username_filters, :filter) + + options = { + filter_post_number: params[:post_number], + filter: params[:filter], + skip_limit: true, + asc: true, + skip_custom_fields: true + } + + fetch_topic_view(options) + render_json_dump(post_ids: @topic_view.posts.pluck(:id)) + end + def posts params.require(:topic_id) params.permit(:post_ids, :post_number, :username_filters, :filter) - default_options = { + options = { filter_post_number: params[:post_number], post_ids: params[:post_ids], asc: ActiveRecord::Type::Boolean.new.deserialize(params[:asc]), filter: params[:filter] } - if (username_filters = params[:username_filters]).present? - default_options[:username_filters] = username_filters.split(',') - end - - @topic_view = TopicView.new(params[:topic_id], current_user, default_options) + fetch_topic_view(options) render_json_dump(TopicViewPostsSerializer.new(@topic_view, scope: guardian, @@ -714,6 +726,17 @@ class TopicsController < ApplicationController ) end + def fetch_topic_view(options) + check_username_filters { |usernames| options[:username_filters] = usernames } + @topic_view = TopicView.new(params[:topic_id], current_user, options) + end + + def check_username_filters + if (username_filters = params[:username_filters]).present? + yield(username_filters.split(',')) if block_given? + end + end + def toggle_mute @topic = Topic.find_by(id: params[:topic_id].to_i) guardian.ensure_can_see!(@topic) diff --git a/config/routes.rb b/config/routes.rb index bf5199f450f..3793b7baed5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -680,6 +680,7 @@ Discourse::Application.routes.draw do get "t/:slug/:topic_id/:post_number" => "topics#show", constraints: { topic_id: /\d+/, post_number: /\d+/ } get "t/:slug/:topic_id/last" => "topics#show", post_number: 99999999, constraints: { topic_id: /\d+/ } get "t/:topic_id/posts" => "topics#posts", constraints: { topic_id: /\d+/ }, format: :json + get "t/:topic_id/post_ids" => "topics#post_ids", constraints: { topic_id: /\d+/ }, format: :json get "t/:topic_id/excerpts" => "topics#excerpts", constraints: { topic_id: /\d+/ }, format: :json post "t/:topic_id/timings" => "topics#timings", constraints: { topic_id: /\d+/ } post "t/:topic_id/invite" => "topics#invite", constraints: { topic_id: /\d+/ } diff --git a/lib/topic_view.rb b/lib/topic_view.rb index f2c5e51894d..f794e9477c9 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -71,7 +71,7 @@ class TopicView filter_posts(options) - if @posts + if @posts && !@skip_custom_fields if (added_fields = User.whitelisted_user_custom_fields(@guardian)).present? @user_custom_fields = User.custom_fields_for_ids(@posts.pluck(:user_id), added_fields) end @@ -534,7 +534,7 @@ class TopicView .order(sort_order: :desc) end - posts = posts.limit(@limit) + posts = posts.limit(@limit) if !@skip_limit filter_posts_by_ids(posts.pluck(:id)) @posts = @posts.unscope(:order).order(sort_order: :desc) if !asc diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index fb9421e7505..e98a2ab94ae 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1451,6 +1451,70 @@ RSpec.describe TopicsController do end end + describe '#post_ids' do + let(:post) { Fabricate(:post) } + let(:topic) { post.topic } + + before do + TopicView.stubs(:chunk_size).returns(1) + end + + it 'returns the right post ids' do + post2 = Fabricate(:post, topic: topic) + post3 = Fabricate(:post, topic: topic) + + get "/t/#{topic.id}/post_ids.json", params: { + post_number: post.post_number + } + + expect(response.status).to eq(200) + + body = JSON.parse(response.body) + + expect(body["post_ids"]).to eq([post2.id, post3.id]) + end + + describe 'filtering by post number with filters' do + describe 'username filters' do + let(:user) { Fabricate(:user) } + let(:post) { Fabricate(:post, user: user) } + let!(:post2) { Fabricate(:post, topic: topic, user: user) } + let!(:post3) { Fabricate(:post, topic: topic) } + + it 'should return the right posts' do + get "/t/#{topic.id}/post_ids.json", params: { + post_number: post.post_number, + username_filters: post2.user.username + } + + expect(response.status).to eq(200) + + body = JSON.parse(response.body) + + expect(body["post_ids"]).to eq([post2.id]) + end + end + + describe 'summary filter' do + let!(:post2) { Fabricate(:post, topic: topic, percent_rank: 0.2) } + let!(:post3) { Fabricate(:post, topic: topic) } + + it 'should return the right posts' do + get "/t/#{topic.id}/post_ids.json", params: { + post_number: post.post_number, + filter: 'summary' + } + + expect(response.status).to eq(200) + + body = JSON.parse(response.body) + + expect(body["post_ids"]).to eq([post2.id]) + end + end + end + end + describe '#posts' do let(:post) { Fabricate(:post) } let(:topic) { post.topic } diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index 1ac08cc3199..77710c79fcc 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -264,3 +264,26 @@ QUnit.test("selecting posts", async assert => { "it should hide the multi select menu" ); }); + +QUnit.test("select below", async assert => { + await visit("/t/internationalization-localization/280"); + await click(".toggle-admin-menu"); + await click(".topic-admin-multi-select .btn"); + await click("#post_3 .select-below"); + + assert.ok( + find(".selected-posts") + .html() + .includes(I18n.t("topic.multi_select.description", { count: 18 })), + "it should select the right number of posts" + ); + + await click("#post_2 .select-below"); + + assert.ok( + find(".selected-posts") + .html() + .includes(I18n.t("topic.multi_select.description", { count: 19 })), + "it should select the right number of posts" + ); +});