From e17a13ce19cfbb64ee730c855e8e742233c593c7 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 12 Nov 2018 13:04:30 +1100 Subject: [PATCH] FEATURE: additional "related messages" section This splits out previous message correspondence from suggeted and instead has a dedicated section called "related messages" --- .../components/related-messages.js.es6 | 17 +++ .../components/suggested-topics.js.es6 | 1 + .../javascripts/discourse/models/topic.js.es6 | 14 +++ .../templates/components/related-messages.hbs | 7 ++ .../templates/components/suggested-topics.hbs | 1 - .../javascripts/discourse/templates/topic.hbs | 3 + .../stylesheets/common/base/topic-post.scss | 2 +- app/assets/stylesheets/common/base/topic.scss | 20 +-- .../stylesheets/common/printer-friendly.scss | 2 +- .../stylesheets/desktop/topic-post.scss | 4 +- app/assets/stylesheets/mobile/topic-post.scss | 2 +- app/serializers/suggested_topics_mixin.rb | 11 ++ config/locales/client.en.yml | 3 + lib/topic_query.rb | 114 ++++++++++-------- lib/topic_view.rb | 10 +- spec/components/topic_query_spec.rb | 17 +-- 16 files changed, 151 insertions(+), 77 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/related-messages.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/related-messages.hbs diff --git a/app/assets/javascripts/discourse/components/related-messages.js.es6 b/app/assets/javascripts/discourse/components/related-messages.js.es6 new file mode 100644 index 00000000000..1094f688095 --- /dev/null +++ b/app/assets/javascripts/discourse/components/related-messages.js.es6 @@ -0,0 +1,17 @@ +import computed from "ember-addons/ember-computed-decorators"; +import { iconHTML } from "discourse-common/lib/icon-library"; + +export default Ember.Component.extend({ + elementId: "related-messages", + classNames: ["suggested-topics"], + + @computed("topic") + relatedTitle(topic) { + const href = this.currentUser && this.currentUser.pmPath(topic); + return href + ? `${iconHTML("envelope", { + class: "private-message-glyph" + })}${I18n.t("related_messages.title")}` + : I18n.t("related_messages.title"); + } +}); diff --git a/app/assets/javascripts/discourse/components/suggested-topics.js.es6 b/app/assets/javascripts/discourse/components/suggested-topics.js.es6 index e95df8fe62e..78d512aae00 100644 --- a/app/assets/javascripts/discourse/components/suggested-topics.js.es6 +++ b/app/assets/javascripts/discourse/components/suggested-topics.js.es6 @@ -4,6 +4,7 @@ import { iconHTML } from "discourse-common/lib/icon-library"; export default Ember.Component.extend({ elementId: "suggested-topics", + classNames: ["suggested-topics"], @computed("topic") suggestedTitle(topic) { diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6 index 4be65438569..66dc5d888b0 100644 --- a/app/assets/javascripts/discourse/models/topic.js.es6 +++ b/app/assets/javascripts/discourse/models/topic.js.es6 @@ -124,6 +124,20 @@ const Topic = RestModel.extend({ return newTags; }, + @computed("related_messages") + relatedMessages(relatedMessages) { + if (relatedMessages) { + const store = this.store; + + return this.set( + "related_messages", + relatedMessages.map(st => { + return store.createRecord("topic", st); + }) + ); + } + }, + @computed("suggested_topics") suggestedTopics(suggestedTopics) { if (suggestedTopics) { diff --git a/app/assets/javascripts/discourse/templates/components/related-messages.hbs b/app/assets/javascripts/discourse/templates/components/related-messages.hbs new file mode 100644 index 00000000000..41c98c7d7c8 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/related-messages.hbs @@ -0,0 +1,7 @@ +

{{{relatedTitle}}}

+
+{{basic-topic-list + hideCategory="true" + showPosters="true" + topics=topic.relatedMessages}} +
diff --git a/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs b/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs index 8d0263c71c4..e70cfec46af 100644 --- a/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs +++ b/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs @@ -1,5 +1,4 @@

{{{suggestedTitle}}}

-
{{#if topic.isPrivateMessage}} {{basic-topic-list diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index 037a563527b..e71c671a0c6 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -284,6 +284,9 @@ {{plugin-outlet name="topic-above-suggested" args=(hash model=model)}} + {{#if model.relatedMessages.length}} + {{related-messages topic=model}} + {{/if}} {{#if model.suggestedTopics.length}} {{suggested-topics topic=model}} {{/if}} diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 2fb3e97c130..65c19659cd8 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -726,7 +726,7 @@ a.mention-group { } } -#suggested-topics { +.suggested-topics { .topics { padding-bottom: 15px; } diff --git a/app/assets/stylesheets/common/base/topic.scss b/app/assets/stylesheets/common/base/topic.scss index 9d3a10b8bd9..b2e6a63eb8b 100644 --- a/app/assets/stylesheets/common/base/topic.scss +++ b/app/assets/stylesheets/common/base/topic.scss @@ -168,31 +168,31 @@ a.badge-category { } // Target the .badge-category text, the bullet icon needs to maintain `display: block` -#suggested-topics h3 .badge-wrapper.bullet span.badge-category, -#suggested-topics h3 .badge-wrapper.box span, -#suggested-topics h3 .badge-wrapper.bar span { +.suggested-topics h3 .badge-wrapper.bullet span.badge-category, +.suggested-topics h3 .badge-wrapper.box span, +.suggested-topics h3 .badge-wrapper.bar span { display: inline; } -#suggested-topics h3 .badge-wrapper.bullet span.badge-category { +.suggested-topics h3 .badge-wrapper.bullet span.badge-category { // Override vertical-align: text-top from `badges.css.scss` vertical-align: baseline; line-height: $line-height-medium; } -#suggested-topics h3 .badge-wrapper.bullet, -#suggested-topics h3 .badge-wrapper.bullet span.badge-category-parent-bg, -#suggested-topics h3 .badge-wrapper.bullet span.badge-category-bg { +.suggested-topics h3 .badge-wrapper.bullet, +.suggested-topics h3 .badge-wrapper.bullet span.badge-category-parent-bg, +.suggested-topics h3 .badge-wrapper.bullet span.badge-category-bg { // Top of bullet aligns with top of line - adjust line height to vertically align bullet. line-height: 0.8; } -#suggested-topics .badge-wrapper.bullet span.badge-category, -#suggested-topics .badge-wrapper.bar span.badge-category { +.suggested-topics .badge-wrapper.bullet span.badge-category, +.suggested-topics .badge-wrapper.bar span.badge-category { max-width: 150px; } -#suggested-topics .suggested-topics-title { +.suggested-topics .suggested-topics-title { display: flex; align-items: center; } diff --git a/app/assets/stylesheets/common/printer-friendly.scss b/app/assets/stylesheets/common/printer-friendly.scss index 7a656553088..e76d132bcf3 100644 --- a/app/assets/stylesheets/common/printer-friendly.scss +++ b/app/assets/stylesheets/common/printer-friendly.scss @@ -8,7 +8,7 @@ .topic-map, .post-menu-area.clearfix, div#topic-footer-buttons, - div#suggested-topics, + div.suggested-topics, div#progress-topic-wrapper, #topic-progress-wrapper, div.nums, diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index c2b7cae95b0..cb9dafa3d1f 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -447,7 +447,7 @@ nav.post-controls { width: 757px; } -#suggested-topics { +.suggested-topics { clear: left; padding: 20px 0 15px 0; table { @@ -459,7 +459,7 @@ nav.post-controls { } } -#suggested-topics .topic-statuses .topic-status { +.suggested-topics .topic-statuses .topic-status { padding: 0; i { font-size: 1em; diff --git a/app/assets/stylesheets/mobile/topic-post.scss b/app/assets/stylesheets/mobile/topic-post.scss index 9f3414436fc..802bc5802d1 100644 --- a/app/assets/stylesheets/mobile/topic-post.scss +++ b/app/assets/stylesheets/mobile/topic-post.scss @@ -259,7 +259,7 @@ a.reply-to-tab { margin: 0 auto; } -#suggested-topics { +.suggested-topics { clear: left; padding: 20px 0 15px 0; th.views, diff --git a/app/serializers/suggested_topics_mixin.rb b/app/serializers/suggested_topics_mixin.rb index 361a7f62a8c..e17a4a773f6 100644 --- a/app/serializers/suggested_topics_mixin.rb +++ b/app/serializers/suggested_topics_mixin.rb @@ -1,12 +1,23 @@ module SuggestedTopicsMixin def self.included(klass) + klass.attributes :related_messages klass.attributes :suggested_topics end + def include_related_messages? + object.next_page.nil? && object.related_messages&.topics.present? + end + def include_suggested_topics? object.next_page.nil? && object.suggested_topics&.topics.present? end + def related_messages + object.related_messages.topics.map do |t| + SuggestedTopicSerializer.new(t, scope: scope, root: false) + end + end + def suggested_topics object.suggested_topics.topics.map do |t| SuggestedTopicSerializer.new(t, scope: scope, root: false) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 609a6359945..5a07acf4974 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -252,6 +252,9 @@ en: one: "{{count}} character" other: "{{count}} characters" + related_messages: + title: "Related Messages" + suggested_topics: title: "Suggested Topics" pm_title: "Suggested Messages" diff --git a/lib/topic_query.rb b/lib/topic_query.rb index aa2ba5f31f0..1b643e400f7 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -135,8 +135,71 @@ class TopicQuery (list || Topic).joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{@user.id.to_i})") end + def get_pm_params(topic) + if topic.private_message? + + my_group_ids = topic.topic_allowed_groups + .joins(" + LEFT JOIN group_users gu + ON topic_allowed_groups.group_id = gu.group_id + AND gu.user_id = #{@user.id.to_i} + ") + .where("gu.group_id IS NOT NULL") + .pluck(:group_id) + + target_group_ids = topic.topic_allowed_groups.pluck(:group_id) + + target_users = topic + .topic_allowed_users + + if my_group_ids.present? + + # strip out users in groups you already belong to + target_users = target_users + .joins("LEFT JOIN group_users gu ON gu.user_id = topic_allowed_users.user_id AND gu.group_id IN (#{sanitize_sql_array(my_group_ids)})") + .where('gu.group_id IS NULL') + end + + target_user_ids = target_users + .where('NOT topic_allowed_users.user_id = ?', @user.id) + .pluck(:user_id) + + { + topic: topic, + my_group_ids: my_group_ids, + target_group_ids: target_group_ids, + target_user_ids: target_user_ids + } + end + end + + def list_related_for(topic, pm_params: nil) + return if !topic.private_message? + return if @user.blank? + return if !SiteSetting.enable_personal_messages? + + builder = SuggestedTopicsBuilder.new(topic) + pm_params = pm_params || get_pm_params(topic) + + if pm_params[:my_group_ids].present? + builder.add_results(related_messages_group( + pm_params.merge(count: [6, builder.results_left].max, + exclude: builder.excluded_topic_ids) + )) + else + builder.add_results(related_messages_user( + pm_params.merge(count: [6, builder.results_left].max, + exclude: builder.excluded_topic_ids) + )) + end + + params = { unordered: true } + params[:preload_posters] = true + create_list(:suggested, params, builder.results) + end + # Return a list of suggested topics for a topic - def list_suggested_for(topic) + def list_suggested_for(topic, pm_params: nil) # Don't suggest messages unless we have a user, and private messages are # enabled. @@ -145,42 +208,7 @@ class TopicQuery builder = SuggestedTopicsBuilder.new(topic) - pm_params = - if topic.private_message? - - my_group_ids = topic.topic_allowed_groups - .joins(" - LEFT JOIN group_users gu - ON topic_allowed_groups.group_id = gu.group_id - AND gu.user_id = #{@user.id.to_i} - ") - .where("gu.group_id IS NOT NULL") - .pluck(:group_id) - - target_group_ids = topic.topic_allowed_groups.pluck(:group_id) - - target_users = topic - .topic_allowed_users - - if my_group_ids.present? - - # strip out users in groups you already belong to - target_users = target_users - .joins("LEFT JOIN group_users gu ON gu.user_id = topic_allowed_users.user_id AND gu.group_id IN (#{sanitize_sql_array(my_group_ids)})") - .where('gu.group_id IS NULL') - end - - target_user_ids = target_users - .where('NOT topic_allowed_users.user_id = ?', @user.id) - .pluck(:user_id) - - { - topic: topic, - my_group_ids: my_group_ids, - target_group_ids: target_group_ids, - target_user_ids: target_user_ids - } - end + pm_params = pm_params || get_pm_params(topic) # When logged in we start with different results if @user @@ -194,18 +222,6 @@ class TopicQuery pm_params.merge(count: builder.results_left) )) unless builder.full? - if pm_params[:my_group_ids].present? - builder.add_results(related_messages_group( - pm_params.merge(count: [3, builder.results_left].max, - exclude: builder.excluded_topic_ids) - ), :ultra_high) - else - builder.add_results(related_messages_user( - pm_params.merge(count: [3, builder.results_left].max, - exclude: builder.excluded_topic_ids) - ), :ultra_high) - end - 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? diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 710ca9ddf1b..139ff65dc41 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -402,8 +402,16 @@ class TopicView @initial_load end + def pm_params + @pm_params ||= TopicQuery.new(@user).get_pm_params(topic) + end + def suggested_topics - @suggested_topics ||= TopicQuery.new(@user).list_suggested_for(topic) + @suggested_topics ||= TopicQuery.new(@user).list_suggested_for(topic, pm_params: pm_params) + end + + def related_messages + @related_messages ||= TopicQuery.new(@user).list_related_for(topic, pm_params: pm_params) end # This is pending a larger refactor, that allows custom orders diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 07e96aee9fa..91b97406879 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -644,7 +644,7 @@ describe TopicQuery do end end - context 'suggested_for message do' do + context 'list_related_for do' do let(:user) do Fabricate(:admin) @@ -679,11 +679,6 @@ describe TopicQuery 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) @@ -693,17 +688,17 @@ describe TopicQuery do 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( + expect(TopicQuery.new(user).list_related_for(pm_to_group).topics.map(&:id)).to( eq([related_by_group_pm.id]) ) - expect(TopicQuery.new(user).list_suggested_for(pm_to_user).topics.map(&:id)).to( - eq([related_by_user_pm.id, new_pm.id, unread_pm.id]) + expect(TopicQuery.new(user).list_related_for(pm_to_user).topics.map(&:id)).to( + eq([related_by_user_pm.id]) ) SiteSetting.enable_personal_messages = false - expect(TopicQuery.new(user).list_suggested_for(pm_to_group)).to be_blank - expect(TopicQuery.new(user).list_suggested_for(pm_to_user)).to be_blank + expect(TopicQuery.new(user).list_related_for(pm_to_group)).to be_blank + expect(TopicQuery.new(user).list_related_for(pm_to_user)).to be_blank end end