diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6 index 1cc221a9a..0571ddb9c 100644 --- a/app/assets/javascripts/discourse/controllers/composer.js.es6 +++ b/app/assets/javascripts/discourse/controllers/composer.js.es6 @@ -332,7 +332,7 @@ export default Ember.ObjectController.extend(Presence, { this.set('similarTopicsMessage', message); } - this.store.find('topic', {similar: {title, raw: body}}).then(function(newTopics) { + this.store.find('similar-topic', {title, raw: body}).then(function(newTopics) { similarTopics.clear(); similarTopics.pushObjects(newTopics.get('content')); diff --git a/app/assets/javascripts/discourse/lib/url.js b/app/assets/javascripts/discourse/lib/url.js index ca1597039..3742887ab 100644 --- a/app/assets/javascripts/discourse/lib/url.js +++ b/app/assets/javascripts/discourse/lib/url.js @@ -109,7 +109,6 @@ Discourse.URL = Ember.Object.createWithMixins({ @param {String} path The path we are routing to. **/ routeTo: function(path) { - if (Em.isEmpty(path)) { return; } if (Discourse.get('requiresRefresh')) { diff --git a/app/assets/javascripts/discourse/models/store.js.es6 b/app/assets/javascripts/discourse/models/store.js.es6 index 9ded087ab..be669b84d 100644 --- a/app/assets/javascripts/discourse/models/store.js.es6 +++ b/app/assets/javascripts/discourse/models/store.js.es6 @@ -143,10 +143,13 @@ export default Ember.Object.extend({ return this.container.lookup('adapter:' + type) || this.container.lookup('adapter:rest'); }, - _lookupSubType(subType, id, root) { + _lookupSubType(subType, type, id, root) { // cheat: we know we already have categories in memory - if (subType === 'category') { + // TODO: topics do their own resolving of `category_id` + // to category. That should either respect this or be + // removed. + if (subType === 'category' && type !== 'topic') { return Discourse.Category.findById(id); } @@ -172,13 +175,13 @@ export default Ember.Object.extend({ } }, - _hydrateEmbedded(obj, root) { + _hydrateEmbedded(type, obj, root) { const self = this; Object.keys(obj).forEach(function(k) { const m = /(.+)\_id$/.exec(k); if (m) { const subType = m[1]; - const hydrated = self._lookupSubType(subType, obj[k], root); + const hydrated = self._lookupSubType(subType, type, obj[k], root); if (hydrated) { obj[subType] = hydrated; delete obj[k]; @@ -196,7 +199,7 @@ export default Ember.Object.extend({ // Experimental: If serialized with a certain option we'll wire up embedded objects // automatically. if (root.__rest_serializer === "1") { - this._hydrateEmbedded(obj, root); + this._hydrateEmbedded(type, obj, root); } const existing = fromMap(type, obj.id); diff --git a/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 b/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 index fefa7a64f..ac1f8a8bb 100644 --- a/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 @@ -16,6 +16,7 @@ function findTopicList(store, filter, filterParams, extras) { extras = extras || {}; return new Ember.RSVP.Promise(function(resolve) { + const session = Discourse.Session.current(); if (extras.cached) { @@ -80,7 +81,6 @@ export default function(filter, extras) { }, model(data, transition) { - // attempt to stop early cause we need this to be called before .sync Discourse.ScreenTrack.current().stop(); diff --git a/app/assets/javascripts/discourse/templates/composer/similar_topics.hbs b/app/assets/javascripts/discourse/templates/composer/similar_topics.hbs index b8f183dd9..ed326e488 100644 --- a/app/assets/javascripts/discourse/templates/composer/similar_topics.hbs +++ b/app/assets/javascripts/discourse/templates/composer/similar_topics.hbs @@ -2,11 +2,5 @@

{{i18n 'composer.similar_topics'}}

diff --git a/app/assets/stylesheets/desktop/compose.scss b/app/assets/stylesheets/desktop/compose.scss index 4c3057265..423266850 100644 --- a/app/assets/stylesheets/desktop/compose.scss +++ b/app/assets/stylesheets/desktop/compose.scss @@ -74,6 +74,17 @@ .posts-count { background-color: scale-color($tertiary, $lightness: -40%); } + + ul { + list-style: none; + margin: 0; + padding: 0; + } + .search-link { + .fa, .blurb { + color: scale-color($tertiary, $lightness: -40%); + } + } } .composer-popup:nth-of-type(2) { diff --git a/app/controllers/similar_topics_controller.rb b/app/controllers/similar_topics_controller.rb new file mode 100644 index 000000000..31bfe2726 --- /dev/null +++ b/app/controllers/similar_topics_controller.rb @@ -0,0 +1,39 @@ +require_dependency 'similar_topic_serializer' +require_dependency 'search/grouped_search_results' + +class SimilarTopicsController < ApplicationController + + class SimilarTopic + def initialize(topic) + @topic = topic + end + + attr_reader :topic + + def blurb + Search::GroupedSearchResults.blurb_for(@topic.try(:blurb)) + end + end + + def index + params.require(:title) + params.require(:raw) + title, raw = params[:title], params[:raw] + [:title, :raw].each { |key| check_length_of(key, params[key]) } + + # Only suggest similar topics if the site has a minimum amount of topics present. + return render json: [] unless Topic.count_exceeds_minimum? + + topics = Topic.similar_to(title, raw, current_user).to_a + topics.map! {|t| SimilarTopic.new(t) } + render_serialized(topics, SimilarTopicSerializer, root: :similar_topics, rest_serializer: true) + end + + protected + + def check_length_of(key, attr) + str = (key == :raw) ? "body" : key.to_s + raise Discourse::InvalidParameters.new(key) if attr.length < SiteSetting.send("min_#{str}_similar_length") + end + +end diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 495d535fd..b422aa993 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -149,19 +149,6 @@ class TopicsController < ApplicationController success ? render_serialized(topic, BasicTopicSerializer) : render_json_error(topic) end - def similar_to - params.require(:title) - params.require(:raw) - title, raw = params[:title], params[:raw] - [:title, :raw].each { |key| check_length_of(key, params[key]) } - - # Only suggest similar topics if the site has a minimum amount of topics present. - return render json: [] unless Topic.count_exceeds_minimum? - - topics = Topic.similar_to(title, raw, current_user).to_a - render_serialized(topics, TopicListItemSerializer, root: :topics) - end - def feature_stats params.require(:category_id) category_id = params[:category_id].to_i @@ -510,11 +497,6 @@ class TopicsController < ApplicationController topic.move_posts(current_user, post_ids_including_replies, args) end - def check_length_of(key, attr) - str = (key == :raw) ? "body" : key.to_s - invalid_param(key) if attr.length < SiteSetting.send("min_#{str}_similar_length") - end - def check_for_status_presence(key, attr) invalid_param(key) unless %w(pinned pinned_globally visible closed archived).include?(attr) end diff --git a/app/models/topic.rb b/app/models/topic.rb index 1ee7926e7..988894dfd 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -397,7 +397,7 @@ class Topic < ActiveRecord::Base return [] unless candidate_ids.present? - similar = Topic.select(sanitize_sql_array(["topics.*, similarity(topics.title, :title) + similarity(topics.title, :raw) AS similarity", title: title, raw: raw])) + similar = Topic.select(sanitize_sql_array(["topics.*, similarity(topics.title, :title) + similarity(topics.title, :raw) AS similarity, p.cooked as blurb", title: title, raw: raw])) .joins("JOIN posts AS p ON p.topic_id = topics.id AND p.post_number = 1") .limit(SiteSetting.max_similar_results) .where("topics.id IN (?)", candidate_ids) diff --git a/app/serializers/similar_topic_serializer.rb b/app/serializers/similar_topic_serializer.rb new file mode 100644 index 000000000..15a4c2dc9 --- /dev/null +++ b/app/serializers/similar_topic_serializer.rb @@ -0,0 +1,17 @@ +class SimilarTopicSerializer < ApplicationSerializer + + has_one :topic, serializer: TopicListItemSerializer, embed: :ids + attributes :id, :blurb, :created_at + + def id + object.topic.id + end + + def blurb + object.blurb + end + + def created_at + object.topic.created_at + end +end diff --git a/config/routes.rb b/config/routes.rb index 98b2f9df8..45fb97610 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -410,7 +410,10 @@ Discourse::Application.routes.draw do put "topics/bulk" put "topics/reset-new" => 'topics#reset_new' post "topics/timings" - get "topics/similar_to" + + get 'topics/similar_to' => 'similar_topics#index' + resources :similar_topics + get "topics/feature_stats" get "topics/created-by/:username" => "list#topics_by", as: "topics_by", constraints: {username: USERNAME_ROUTE_FORMAT} get "topics/private-messages/:username" => "list#private_messages", as: "topics_private_messages", constraints: {username: USERNAME_ROUTE_FORMAT} diff --git a/lib/search/grouped_search_results.rb b/lib/search/grouped_search_results.rb index 742b1493b..c3f661313 100644 --- a/lib/search/grouped_search_results.rb +++ b/lib/search/grouped_search_results.rb @@ -3,7 +3,6 @@ require 'sanitize' class Search class GroupedSearchResults - include ActiveModel::Serialization class TextHelper @@ -26,11 +25,7 @@ class Search end def blurb(post) - cooked = SearchObserver::HtmlScrubber.scrub(post.cooked).squish - terms = @term.split(/\s+/) - blurb = TextHelper.excerpt(cooked, terms.first, radius: 100) - blurb = TextHelper.truncate(cooked, length: 200) if blurb.blank? - Sanitize.clean(blurb) + GroupedSearchResults.blurb_for(post.cooked, @term) end def add(object) @@ -43,6 +38,18 @@ class Search end end + + def self.blurb_for(cooked, term=nil) + cooked = SearchObserver::HtmlScrubber.scrub(cooked).squish + + blurb = nil + if term + terms = term.split(/\s+/) + blurb = TextHelper.excerpt(cooked, terms.first, radius: 100) + end + blurb = TextHelper.truncate(cooked, length: 200) if blurb.blank? + Sanitize.clean(blurb) + end end end diff --git a/spec/controllers/similar_topics_controller_spec.rb b/spec/controllers/similar_topics_controller_spec.rb new file mode 100644 index 000000000..5d5bc09ff --- /dev/null +++ b/spec/controllers/similar_topics_controller_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe SimilarTopicsController do + context 'similar_to' do + + let(:title) { 'this title is long enough to search for' } + let(:raw) { 'this body is long enough to search for' } + + it "requires a title" do + expect { xhr :get, :index, raw: raw }.to raise_error(ActionController::ParameterMissing) + end + + it "requires a raw body" do + expect { xhr :get, :index, title: title }.to raise_error(ActionController::ParameterMissing) + end + + it "raises an error if the title length is below the minimum" do + SiteSetting.stubs(:min_title_similar_length).returns(100) + expect { xhr :get, :index, title: title, raw: raw }.to raise_error(Discourse::InvalidParameters) + end + + it "raises an error if the body length is below the minimum" do + SiteSetting.stubs(:min_body_similar_length).returns(100) + expect { xhr :get, :index, title: title, raw: raw }.to raise_error(Discourse::InvalidParameters) + end + + describe "minimum_topics_similar" do + + before do + SiteSetting.stubs(:minimum_topics_similar).returns(30) + end + + after do + xhr :get, :index, title: title, raw: raw + end + + describe "With enough topics" do + before do + Topic.stubs(:count).returns(50) + end + + it "deletes to Topic.similar_to if there are more topics than `minimum_topics_similar`" do + Topic.expects(:similar_to).with(title, raw, nil).returns([Fabricate(:topic)]) + end + + describe "with a logged in user" do + let(:user) { log_in } + + it "passes a user through if logged in" do + Topic.expects(:similar_to).with(title, raw, user).returns([Fabricate(:topic)]) + end + end + + end + + it "does not call Topic.similar_to if there are fewer topics than `minimum_topics_similar`" do + Topic.stubs(:count).returns(10) + Topic.expects(:similar_to).never + end + + end + + end + +end + diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 72ae0e2eb..7516ce32f 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -245,68 +245,6 @@ describe TopicsController do end end - context 'similar_to' do - - let(:title) { 'this title is long enough to search for' } - let(:raw) { 'this body is long enough to search for' } - - it "requires a title" do - expect { xhr :get, :similar_to, raw: raw }.to raise_error(ActionController::ParameterMissing) - end - - it "requires a raw body" do - expect { xhr :get, :similar_to, title: title }.to raise_error(ActionController::ParameterMissing) - end - - it "raises an error if the title length is below the minimum" do - SiteSetting.stubs(:min_title_similar_length).returns(100) - expect { xhr :get, :similar_to, title: title, raw: raw }.to raise_error(Discourse::InvalidParameters) - end - - it "raises an error if the body length is below the minimum" do - SiteSetting.stubs(:min_body_similar_length).returns(100) - expect { xhr :get, :similar_to, title: title, raw: raw }.to raise_error(Discourse::InvalidParameters) - end - - describe "minimum_topics_similar" do - - before do - SiteSetting.stubs(:minimum_topics_similar).returns(30) - end - - after do - xhr :get, :similar_to, title: title, raw: raw - end - - describe "With enough topics" do - before do - Topic.stubs(:count).returns(50) - end - - it "deletes to Topic.similar_to if there are more topics than `minimum_topics_similar`" do - Topic.expects(:similar_to).with(title, raw, nil).returns([Fabricate(:topic)]) - end - - describe "with a logged in user" do - let(:user) { log_in } - - it "passes a user through if logged in" do - Topic.expects(:similar_to).with(title, raw, user).returns([Fabricate(:topic)]) - end - end - - end - - it "does not call Topic.similar_to if there are fewer topics than `minimum_topics_similar`" do - Topic.stubs(:count).returns(10) - Topic.expects(:similar_to).never - end - - end - - end - - context 'clear_pin' do it 'needs you to be logged in' do expect { xhr :put, :clear_pin, topic_id: 1 }.to raise_error(Discourse::NotLoggedIn)