From 5fc5a7f5ae8c3cf1a89c1a685258e9e942de1508 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Mon, 22 Jul 2019 17:55:49 +0300 Subject: [PATCH] FEATURE: Add search operator to see all direct messages from a user (#7913) * FEATURE: Add search operator to see all direct messages from a user * Only show message if related messages >= 5 * Make "all messages" the hyperlink * Review --- .../components/related-messages.js.es6 | 24 ++++++++ .../components/search-advanced-options.js.es6 | 14 ++--- .../controllers/full-page-search.js.es6 | 4 +- .../templates/components/related-messages.hbs | 4 ++ .../components/search-advanced-options.hbs | 2 +- .../discourse/widgets/search-menu.js.es6 | 2 +- config/locales/client.en.yml | 1 + lib/search.rb | 31 +++++++++- spec/components/search_spec.rb | 59 +++++++++++++++++++ spec/fabricators/post_fabricator.rb | 15 +++++ .../acceptance/search-full-test.js.es6 | 6 +- 11 files changed, 145 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/discourse/components/related-messages.js.es6 b/app/assets/javascripts/discourse/components/related-messages.js.es6 index 807359d924..e7d65a4591 100644 --- a/app/assets/javascripts/discourse/components/related-messages.js.es6 +++ b/app/assets/javascripts/discourse/components/related-messages.js.es6 @@ -5,6 +5,30 @@ export default Ember.Component.extend({ elementId: "related-messages", classNames: ["suggested-topics"], + @computed("topic") + targetUser(topic) { + if (!topic || !topic.isPrivateMessage) { + return; + } + const allowedUsers = topic.details.allowed_users; + if ( + topic.relatedMessages && + topic.relatedMessages.length >= 5 && + allowedUsers.length === 2 && + topic.details.allowed_groups.length === 0 && + allowedUsers.find(u => u.username === this.currentUser.username) + ) { + return allowedUsers.find(u => u.username !== this.currentUser.username); + } + }, + + @computed + searchLink() { + return Discourse.getURL( + `/search?expanded=true&q=%40${this.targetUser.username}%20in%3Apersonal-direct` + ); + }, + @computed("topic") relatedTitle(topic) { const href = this.currentUser && this.currentUser.pmPath(topic); diff --git a/app/assets/javascripts/discourse/components/search-advanced-options.js.es6 b/app/assets/javascripts/discourse/components/search-advanced-options.js.es6 index 75b2e5460a..50da93e6c2 100644 --- a/app/assets/javascripts/discourse/components/search-advanced-options.js.es6 +++ b/app/assets/javascripts/discourse/components/search-advanced-options.js.es6 @@ -19,7 +19,7 @@ const REGEXP_TAGS_REPLACE = /(^(tags?:|#(?=[a-z0-9\-]+::tag))|::tag\s?$)/gi; const REGEXP_IN_MATCH = /^(in|with):(posted|watching|tracking|bookmarks|first|pinned|unpinned|wiki|unseen|image)/gi; const REGEXP_SPECIAL_IN_LIKES_MATCH = /^in:likes/gi; const REGEXP_SPECIAL_IN_TITLE_MATCH = /^in:title/gi; -const REGEXP_SPECIAL_IN_PRIVATE_MATCH = /^in:private/gi; +const REGEXP_SPECIAL_IN_PERSONAL_MATCH = /^in:personal/gi; const REGEXP_SPECIAL_IN_SEEN_MATCH = /^in:seen/gi; const REGEXP_CATEGORY_SLUG = /^(\#[a-zA-Z0-9\-:]+)/gi; @@ -93,7 +93,7 @@ export default Ember.Component.extend({ in: { title: false, likes: false, - private: false, + personal: false, seen: false }, all_tags: false @@ -140,8 +140,8 @@ export default Ember.Component.extend({ ); this.setSearchedTermSpecialInValue( - "searchedTerms.special.in.private", - REGEXP_SPECIAL_IN_PRIVATE_MATCH + "searchedTerms.special.in.personal", + REGEXP_SPECIAL_IN_PERSONAL_MATCH ); this.setSearchedTermSpecialInValue( @@ -512,9 +512,9 @@ export default Ember.Component.extend({ this.updateInRegex(REGEXP_SPECIAL_IN_LIKES_MATCH, "likes"); }, - @observes("searchedTerms.special.in.private") - updateSearchTermForSpecialInPrivate() { - this.updateInRegex(REGEXP_SPECIAL_IN_PRIVATE_MATCH, "private"); + @observes("searchedTerms.special.in.personal") + updateSearchTermForSpecialInPersonal() { + this.updateInRegex(REGEXP_SPECIAL_IN_PERSONAL_MATCH, "personal"); }, @observes("searchedTerms.special.in.seen") diff --git a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 index af832d4fe2..c7175a47fd 100644 --- a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 +++ b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 @@ -161,9 +161,9 @@ export default Ember.Controller.extend({ return ( q && this.currentUser && - (q.indexOf("in:private") > -1 || + (q.indexOf("in:personal") > -1 || q.indexOf( - `private_messages:${this.currentUser.get("username_lower")}` + `personal_messages:${this.currentUser.get("username_lower")}` ) > -1) ); }, diff --git a/app/assets/javascripts/discourse/templates/components/related-messages.hbs b/app/assets/javascripts/discourse/templates/components/related-messages.hbs index 41c98c7d7c..dde8deaea1 100644 --- a/app/assets/javascripts/discourse/templates/components/related-messages.hbs +++ b/app/assets/javascripts/discourse/templates/components/related-messages.hbs @@ -5,3 +5,7 @@ showPosters="true" topics=topic.relatedMessages}} + +{{#if targetUser}} +

{{{i18n "related_messages.see_all" path=searchLink username=targetUser.username}}}

+{{/if}} diff --git a/app/assets/javascripts/discourse/templates/components/search-advanced-options.hbs b/app/assets/javascripts/discourse/templates/components/search-advanced-options.hbs index 371a1c3849..b1d80198cb 100644 --- a/app/assets/javascripts/discourse/templates/components/search-advanced-options.hbs +++ b/app/assets/javascripts/discourse/templates/components/search-advanced-options.hbs @@ -62,7 +62,7 @@
- +
{{/if}} diff --git a/app/assets/javascripts/discourse/widgets/search-menu.js.es6 b/app/assets/javascripts/discourse/widgets/search-menu.js.es6 index 4792f02b6f..808f3282c3 100644 --- a/app/assets/javascripts/discourse/widgets/search-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/search-menu.js.es6 @@ -114,7 +114,7 @@ export default createWidget("search-menu", { this.currentUser.get("username_lower") && type === "private_messages" ) { - query += " in:private"; + query += " in:personal"; } else { query += encodeURIComponent(" " + type + ":" + ctx.id); } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 3b3370d161..23c624d85e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -262,6 +262,7 @@ en: related_messages: title: "Related Messages" + see_all: "See all messages from @%{username}..." suggested_topics: title: "Suggested Topics" diff --git a/lib/search.rb b/lib/search.rb index 08512e3c0d..a109f48a02 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -265,6 +265,27 @@ class Search @advanced_filters end + advanced_filter(/^in:personal-direct$/) do |posts| + if @guardian.user + posts + .joins("LEFT JOIN topic_allowed_groups tg ON posts.topic_id = tg.topic_id") + .where(<<~SQL, user_id: @guardian.user.id) + tg.id IS NULL + AND posts.topic_id IN ( + SELECT tau.topic_id + FROM topic_allowed_users tau + JOIN topic_allowed_users tau2 + ON tau2.topic_id = tau.topic_id + AND tau2.id != tau.id + WHERE tau.user_id = :user_id + AND tau.topic_id = posts.topic_id + GROUP BY tau.topic_id + HAVING COUNT(*) = 1 + ) + SQL + end + end + advanced_filter(/^in:tagged$/) do |posts| posts .where('EXISTS (SELECT 1 FROM topic_tags WHERE topic_tags.topic_id = posts.topic_id)') @@ -631,10 +652,14 @@ class Search elsif word == 'order:likes' @order = :likes nil - elsif word == 'in:private' + elsif %w{in:private in:personal}.include?(word) # remove private after 2.4 release @search_pms = true nil - elsif word =~ /^private_messages:(.+)$/ + elsif word == "in:personal-direct" + @search_pms = true + @direct_pms_only = true + nil + elsif word =~ /^personal_messages:(.+)$/ @search_pms = true nil else @@ -826,7 +851,7 @@ class Search if @search_context.present? if @search_context.is_a?(User) if opts[:private_messages] - posts.private_posts_for_user(@search_context) + @direct_pms_only ? posts : posts.private_posts_for_user(@search_context) else posts.where("posts.user_id = #{@search_context.id}") end diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index d1d7bde413..748f08888c 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -236,6 +236,65 @@ describe Search do end + context 'personal-direct flag' do + let(:current) { Fabricate(:user, admin: true, username: "current_user") } + let(:participant) { Fabricate(:user, username: "participant_1") } + let(:participant_2) { Fabricate(:user, username: "participant_2") } + + let(:group) do + group = Fabricate(:group, has_messages: true) + group.add(current) + group.add(participant) + group + end + + def create_pm(users:, group: nil) + pm = Fabricate(:private_message_post_one_user, user: users.first).topic + users[1..-1].each do |u| + pm.invite(users.first, u.username) + Fabricate(:post, user: u, topic: pm) + end + if group + pm.invite_group(users.first, group) + group.users.each do |u| + Fabricate(:post, user: u, topic: pm) + end + end + pm.reload + end + + it 'can find all direct PMs of the current user' do + pm = create_pm(users: [current, participant]) + pm_2 = create_pm(users: [participant_2, participant]) + pm_3 = create_pm(users: [participant, current]) + pm_4 = create_pm(users: [participant_2, current]) + results = Search.execute("in:personal-direct", guardian: Guardian.new(current)) + expect(results.posts.size).to eq(3) + expect(results.posts.map(&:topic_id)).to contain_exactly(pm.id, pm_3.id, pm_4.id) + end + + it 'can filter direct PMs by @username' do + pm = create_pm(users: [current, participant]) + pm_2 = create_pm(users: [participant, current]) + pm_3 = create_pm(users: [participant_2, current]) + results = Search.execute("@#{participant.username} in:personal-direct", guardian: Guardian.new(current)) + expect(results.posts.size).to eq(2) + expect(results.posts.map(&:topic_id)).to contain_exactly(pm.id, pm_2.id) + expect(results.posts.map(&:user_id).uniq).to contain_exactly(participant.id) + end + + it "doesn't include PMs that have more than 2 participants" do + pm = create_pm(users: [current, participant, participant_2]) + results = Search.execute("@#{participant.username} in:personal-direct", guardian: Guardian.new(current)) + expect(results.posts.size).to eq(0) + end + + it "doesn't include PMs that have groups" do + pm = create_pm(users: [current, participant], group: group) + results = Search.execute("@#{participant.username} in:personal-direct", guardian: Guardian.new(current)) + expect(results.posts.size).to eq(0) + end + end end context 'topics' do diff --git a/spec/fabricators/post_fabricator.rb b/spec/fabricators/post_fabricator.rb index 219a165528..8f0fef9259 100644 --- a/spec/fabricators/post_fabricator.rb +++ b/spec/fabricators/post_fabricator.rb @@ -137,6 +137,21 @@ Fabricator(:private_message_post, from: :post) do raw "Ssshh! This is our secret conversation!" end +Fabricator(:private_message_post_one_user, from: :post) do + user + topic do |attrs| + Fabricate(:private_message_topic, + user: attrs[:user], + created_at: attrs[:created_at], + subtype: TopicSubtype.user_to_user, + topic_allowed_users: [ + Fabricate.build(:topic_allowed_user, user: attrs[:user]), + ] + ) + end + raw "Ssshh! This is our secret conversation!" +end + Fabricator(:post_via_email, from: :post) do incoming_email via_email true diff --git a/test/javascripts/acceptance/search-full-test.js.es6 b/test/javascripts/acceptance/search-full-test.js.es6 index ebebff6d23..d2ec23231c 100644 --- a/test/javascripts/acceptance/search-full-test.js.es6 +++ b/test/javascripts/acceptance/search-full-test.js.es6 @@ -278,7 +278,7 @@ QUnit.test( ); QUnit.test( - "update in:private filter through advanced search ui", + "update in:personal filter through advanced search ui", async assert => { await visit("/search"); await fillIn(".search-query", "none"); @@ -290,8 +290,8 @@ QUnit.test( ); assert.equal( find(".search-query").val(), - "none in:private", - 'has updated search term to "none in:private"' + "none in:personal", + 'has updated search term to "none in:personal"' ); } );