diff --git a/app/assets/javascripts/discourse/components/header-extra-info.js.es6 b/app/assets/javascripts/discourse/components/header-extra-info.js.es6 index e0c481ef1..b71d9b1b1 100644 --- a/app/assets/javascripts/discourse/components/header-extra-info.js.es6 +++ b/app/assets/javascripts/discourse/components/header-extra-info.js.es6 @@ -4,6 +4,9 @@ const TopicCategoryComponent = Ember.Component.extend({ needsSecondRow: Ember.computed.gt('secondRowItems.length', 0), secondRowItems: function() { return []; }.property(), + pmPath: function() { + return this.get('currentUser').pmPath(this.get('topic')); + }.property('topic'), showPrivateMessageGlyph: function() { return !this.get('topic.is_warning') && this.get('topic.isPrivateMessage'); }.property('topic.is_warning', 'topic.isPrivateMessage'), diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index d4e63bcb9..442899731 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -70,6 +70,26 @@ const User = RestModel.extend({ return Discourse.getURL(`/users/${this.get('username_lower')}`); }, + pmPath(topic) { + const userId = this.get('id'); + const username = this.get('username_lower'); + + const details = topic && topic.get('details'); + const allowedUsers = details && details.get('allowed_users'); + const groups = details && details.get('allowed_groups'); + + // directly targetted so go to inbox + if (!groups || (allowedUsers && allowedUsers.findBy("id", userId))) { + return Discourse.getURL(`/users/${username}/messages`); + } else { + if (groups && groups[0]) + { + return Discourse.getURL(`/users/${username}/messages/group/${groups[0].name}`); + } + } + + }, + adminPath: url('username_lower', "/admin/users/%@"), mutedTopicsPath: url('/latest?state=muted'), diff --git a/app/assets/javascripts/discourse/templates/components/header-extra-info.hbs b/app/assets/javascripts/discourse/templates/components/header-extra-info.hbs index 614e48e38..4d5b79875 100644 --- a/app/assets/javascripts/discourse/templates/components/header-extra-info.hbs +++ b/app/assets/javascripts/discourse/templates/components/header-extra-info.hbs @@ -3,7 +3,9 @@

{{#if showPrivateMessageGlyph}} - {{fa-icon "envelope"}} + + {{fa-icon "envelope"}} + {{/if}} {{#if topic.details.loaded}} diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index 1b5b86afe..93358cf1b 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -30,7 +30,9 @@ {{else}}

{{#unless model.is_warning}} - {{fa-icon "envelope"}} + + {{fa-icon "envelope"}} + {{/unless}} {{#if model.details.loaded}} @@ -108,9 +110,17 @@ {{#if model.details.suggested_topics.length}}
-

{{i18n "suggested_topics.title"}}

+

{{unbound view.suggestedTitle}}

+ {{#if model.isPrivateMessage}} + {{basic-topic-list + hideCategory="true" + showPosters="true" + topics=model.details.suggested_topics + postsAction="showTopicEntrance"}} + {{else}} {{basic-topic-list topics=model.details.suggested_topics postsAction="showTopicEntrance"}} + {{/if}}

{{{view.browseMoreMessage}}}

diff --git a/app/assets/javascripts/discourse/views/topic.js.es6 b/app/assets/javascripts/discourse/views/topic.js.es6 index a0520d58b..cc2d6252a 100644 --- a/app/assets/javascripts/discourse/views/topic.js.es6 +++ b/app/assets/javascripts/discourse/views/topic.js.es6 @@ -20,9 +20,7 @@ const TopicView = Ember.View.extend(AddCategoryClass, AddArchetypeClass, Scrolli SHORT_POST: 1200, categoryFullSlug: Em.computed.alias('topic.category.fullSlug'), - - postStream: Em.computed.alias('controller.model.postStream'), - + postStream: Em.computed.alias('topic.postStream'), archetype: Em.computed.alias('topic.archetype'), _composeChanged: function() { @@ -120,9 +118,19 @@ const TopicView = Ember.View.extend(AddCategoryClass, AddArchetypeClass, Scrolli this.appEvents.trigger('topic:scrolled', offset); }, + pmPath: function() { + return this.get('controller.currentUser').pmPath(this.get('topic')); + }.property(), + browseMoreMessage: function() { + + // TODO decide what to show for pms + if (this.get('topic.isPrivateMessage')) { + return; + } + var opts = { latestLink: "" + I18n.t("topic.view_latest_topics") + "" }, - category = this.get('controller.content.category'); + category = this.get('topic.category'); if(category && Em.get(category, 'id') === Discourse.Site.currentProp("uncategorized_category_id")) { category = null; @@ -154,7 +162,13 @@ const TopicView = Ember.View.extend(AddCategoryClass, AddArchetypeClass, Scrolli } else { return I18n.t("topic.read_more", opts); } - }.property('topicTrackingState.messageCount', 'controller.content.category') + }.property('topicTrackingState.messageCount', 'topic'), + + suggestedTitle: function(){ + return this.get('controller.model.isPrivateMessage') ? + I18n.t("suggested_topics.pm_title") : + I18n.t("suggested_topics.title"); + }.property() }); function highlight(postNumber) { diff --git a/app/serializers/suggested_topic_serializer.rb b/app/serializers/suggested_topic_serializer.rb index 7efa31861..8708f5542 100644 --- a/app/serializers/suggested_topic_serializer.rb +++ b/app/serializers/suggested_topic_serializer.rb @@ -1,5 +1,20 @@ class SuggestedTopicSerializer < ListableTopicSerializer - attributes :archetype, :like_count, :views, :category_id + # need to embed so we have users + # front page json gets away without embedding + class SuggestedPosterSerializer < ApplicationSerializer + attributes :extras, :description + has_one :user, serializer: BasicUserSerializer, embed: :objects + end + attributes :archetype, :like_count, :views, :category_id + has_many :posters, serializer: SuggestedPosterSerializer, embed: :objects + + def include_posters? + object.private_message? + end + + def posters + object.posters || [] + end end diff --git a/app/serializers/topic_poster_serializer.rb b/app/serializers/topic_poster_serializer.rb index 540aa0c86..a3c3f2864 100644 --- a/app/serializers/topic_poster_serializer.rb +++ b/app/serializers/topic_poster_serializer.rb @@ -1,6 +1,4 @@ class TopicPosterSerializer < ApplicationSerializer - attributes :extras, :description has_one :user, serializer: BasicUserSerializer - end diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 866baa4d9..bacdb2dcf 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -88,8 +88,8 @@ class TopicViewSerializer < ApplicationSerializer end if object.suggested_topics.try(:topics).present? - result[:suggested_topics] = object.suggested_topics.topics.map do |user| - SuggestedTopicSerializer.new(user, scope: scope, root: false) + result[:suggested_topics] = object.suggested_topics.topics.map do |topic| + SuggestedTopicSerializer.new(topic, scope: scope, root: false) end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index bbb79d83d..4feee950c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -203,6 +203,7 @@ en: suggested_topics: title: "Suggested Topics" + pm_title: "Suggested Messages" about: simple_title: "About" diff --git a/lib/topic_query.rb b/lib/topic_query.rb index c837c429c..598f7ad8f 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -6,6 +6,7 @@ require_dependency 'topic_list' require_dependency 'suggested_topics_builder' require_dependency 'topic_query_sql' +require_dependency 'avatar_lookup' class TopicQuery # Could be rewritten to %i if Ruby 1.9 is no longer supported @@ -59,16 +60,62 @@ class TopicQuery # Return a list of suggested topics for a topic def list_suggested_for(topic) + return if topic.private_message? && !@user + builder = SuggestedTopicsBuilder.new(topic) + pm_params = + if topic.private_message? + + group_ids = topic.topic_allowed_groups + .where('group_id IN (SELECT group_id FROM group_users WHERE user_id = :user_id)', user_id: @user.id) + .pluck(:group_id) + { + topic: topic, + my_group_ids: group_ids, + target_group_ids: topic.topic_allowed_groups.pluck(:group_id), + target_user_ids: topic.topic_allowed_users.pluck(:user_id) - [@user.id] + } + end + # When logged in we start with different results if @user - builder.add_results(unread_results(topic: topic, per_page: builder.results_left), :high) - builder.add_results(new_results(topic: topic, per_page: builder.category_results_left)) unless builder.full? - end - builder.add_results(random_suggested(topic, builder.results_left, builder.excluded_topic_ids)) unless builder.full? + if topic.private_message? - create_list(:suggested, {unordered: true}, builder.results) + builder.add_results(new_messages( + pm_params.merge(count: builder.results_left) + )) unless builder.full? + + builder.add_results(unread_messages( + pm_params.merge(count: builder.results_left) + )) unless builder.full? + + else + builder.add_results(unread_results(topic: topic, per_page: builder.results_left), :high) + builder.add_results(new_results(topic: topic, per_page: builder.category_results_left)) unless builder.full? + end + end + + if topic.private_message? + + builder.add_results(related_messages_group( + pm_params.merge(count: [3, builder.results_left].max, + exclude: builder.excluded_topic_ids) + )) if pm_params[:my_group_ids].present? + + builder.add_results(related_messages_user( + pm_params.merge(count: [3, builder.results_left].max, + exclude: builder.excluded_topic_ids) + )) + else + builder.add_results(random_suggested(topic, builder.results_left, builder.excluded_topic_ids)) unless builder.full? + end + + params = {unordered: true} + if topic.private_message? + params[:preload_posters] = true + end + create_list(:suggested, params, builder.results) end # The latest view of topics @@ -233,11 +280,26 @@ class TopicQuery topics = prioritize_pinned_topics(topics, options) end - topics = topics.to_a.each do |t| + topics = topics.to_a + + if options[:preload_posters] + user_ids = [] + topics.each do |ft| + user_ids << ft.user_id << ft.last_post_user_id << ft.featured_user_ids << ft.allowed_user_ids + end + + avatar_lookup = AvatarLookup.new(user_ids) + topics.each do |t| + t.posters = t.posters_summary(avatar_lookup: avatar_lookup) + end + end + + topics.each do |t| + t.allowed_user_ids = filter == :private_messages ? t.allowed_users.map{|u| u.id} : [] end - list = TopicList.new(filter, @user, topics.to_a, options.merge(@options)) + list = TopicList.new(filter, @user, topics, options.merge(@options)) list.per_page = per_page_setting list end @@ -496,6 +558,82 @@ class TopicQuery list end + def new_messages(params) + + TopicQuery.new_filter(messages_for_groups_or_user(params[:my_group_ids]), 10.years.ago) + .limit(params[:count]) + + end + + def unread_messages(params) + TopicQuery.unread_filter(messages_for_groups_or_user(params[:my_group_ids])) + .limit(params[:count]) + end + + def related_messages_user(params) + messages_for_user + .limit(params[:count]) + .where('topics.id IN ( + SELECT ta.topic_id + FROM topic_allowed_users ta + WHERE ta.user_id IN (:user_ids) + ) OR + topics.id IN ( + SELECT tg.topic_id + FROM topic_allowed_groups tg + WHERE tg.group_id IN (:group_ids) + ) + ', user_ids: (params[:target_user_ids] || []) + [-10], + group_ids: ((params[:target_group_ids] - params[:my_group_ids]) || []) + [-10]) + + end + + def related_messages_group(params) + messages_for_groups_or_user(params[:my_group_ids]) + .limit(params[:count]) + .where('topics.id IN ( + SELECT ta.topic_id + FROM topic_allowed_users ta + WHERE ta.user_id IN (:user_ids) + ) OR + topics.id IN ( + SELECT tg.topic_id + FROM topic_allowed_groups tg + WHERE tg.group_id IN (:group_ids) + ) + ', user_ids: (params[:target_user_ids] || []) + [-10], + group_ids: ((params[:target_group_ids] - params[:my_group_ids]) || []) + [-10]) + + end + + def messages_for_groups_or_user(group_ids) + if group_ids.present? + base_messages + .where('topics.id IN ( + SELECT topic_id + FROM topic_allowed_groups tg + JOIN group_users gu ON gu.user_id = :user_id AND gu.group_id = tg.group_id + WHERE gu.group_id IN (:group_ids) + )', user_id: @user.id, group_ids: group_ids) + else + messages_for_user + end + end + + def messages_for_user + base_messages.where('topics.id IN ( + SELECT topic_id + FROM topic_allowed_users + WHERE user_id = :user_id + )', user_id: @user.id) + end + + def base_messages + Topic + .where('topics.archetype = ?', Archetype.private_message) + .joins("LEFT JOIN topic_users tu ON topics.id = tu.topic_id AND tu.user_id = #{@user.id.to_i}") + .order('topics.bumped_at DESC') + end def random_suggested(topic, count, excluded_topic_ids=[]) result = default_results(unordered: true, per_page: count).where(closed: false, archived: false) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 9072610f5..752c04b46 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -295,7 +295,6 @@ class TopicView end def suggested_topics - return nil if topic.private_message? @suggested_topics ||= TopicQuery.new(@user).list_suggested_for(topic) end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index b0bd19c90..6b5cf4602 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -325,13 +325,13 @@ describe PostCreator do end it "fails for dupe post accross topic" do - first = create_post - second = create_post + first = create_post(raw: "this is a test #{SecureRandom.hex}") + second = create_post(raw: "this is a test #{SecureRandom.hex}") dupe = "hello 123 test #{SecureRandom.hex}" - response_1 = create_post(raw: dupe, user: first.user, topic_id: first.topic_id) - response_2 = create_post(raw: dupe, user: first.user, topic_id: second.topic_id) + response_1 = PostCreator.create(first.user, raw: dupe, topic_id: first.topic_id) + response_2 = PostCreator.create(first.user, raw: dupe, topic_id: second.topic_id) expect(response_1.errors.count).to eq(0) expect(response_2.errors.count).to eq(1) diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 634e4418e..44f219d47 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -446,8 +446,70 @@ describe TopicQuery do end end - context 'suggested_for' do + context 'suggested_for message do' do + let(:user) do + Fabricate(:admin) + end + + let(:sender) do + Fabricate(:admin) + end + + let(:group_with_user) do + group = Fabricate(:group) + group.add(user) + group.save + group + end + + def create_pm(user, opts=nil) + unless opts + opts = user + user = nil + end + + create_post(opts.merge(user: user, archetype: Archetype.private_message)).topic + end + + def read(user,topic,post_number) + TopicUser.update_last_read(user, topic, post_number, 10000) + end + + it 'returns the correct suggestions' do + + + pm_to_group = create_pm(sender, target_group_names: [group_with_user.name]) + pm_to_user = create_pm(sender, target_usernames: [user.username]) + + new_pm = create_pm(target_usernames: [user.username]) + + unread_pm = create_pm(target_usernames: [user.username]) + read(user,unread_pm, 0) + + old_unrelated_pm = create_pm(target_usernames: [user.username]) + read(user, old_unrelated_pm, 1) + + + related_by_user_pm = create_pm(sender, target_usernames: [user.username]) + read(user, related_by_user_pm, 1) + + + related_by_group_pm = create_pm(sender, target_group_names: [group_with_user.name]) + read(user, related_by_group_pm, 1) + + + expect(TopicQuery.new(user).list_suggested_for(pm_to_group).topics.map(&:id)).to( + eq([related_by_group_pm.id, related_by_user_pm.id, pm_to_user.id]) + ) + + expect(TopicQuery.new(user).list_suggested_for(pm_to_user).topics.map(&:id)).to( + eq([new_pm.id, unread_pm.id, related_by_user_pm.id]) + ) + end + end + + context 'suggested_for' do before do RandomTopicSelector.clear_cache! diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index b828d0abc..020a240c5 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -38,7 +38,14 @@ module Helpers args[:topic_id] = args[:topic].id if args[:topic] user = args.delete(:user) || Fabricate(:user) args[:category] = args[:category].name if args[:category].is_a?(Category) - PostCreator.create(user, args) + creator = PostCreator.new(user, args) + post = creator.create + + if creator.errors.present? + raise StandardError.new(creator.errors.full_messages.join(" ")) + end + + post end def generate_username(length=10)