From 3f5048118802116679ef28c32728f697cbfb8404 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 12 Feb 2020 10:11:10 +0200 Subject: [PATCH] Improvements to group mentions (#8927) * FIX: Avoid highlight mention to groups that are not public * UX: Composer autocomplete will suggest all visible group names --- .../discourse/components/composer-editor.js.es6 | 2 +- app/controllers/users_controller.rb | 13 ++++++------- spec/requests/users_controller_spec.rb | 11 ++++++++++- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index e1bef560707..5dfa3b039a3 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -180,7 +180,7 @@ export default Component.extend({ term, topicId, categoryId, - includeMentionableGroups: true + includeGroups: true }); }, diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2e1dd305925..ac6d16f13c8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -308,7 +308,7 @@ class UsersController < ApplicationController groups = Group.where(name: usernames).pluck(:name) mentionable_groups = if current_user - Group.mentionable(current_user) + Group.mentionable .where(name: usernames) .pluck(:name, :user_count) .map do |name, user_count| @@ -901,22 +901,21 @@ class UsersController < ApplicationController groups = if current_user - if params[:include_mentionable_groups] == 'true' + if params[:include_groups] == 'true' + Group.visible_groups(current_user) + elsif params[:include_mentionable_groups] == 'true' Group.mentionable(current_user) elsif params[:include_messageable_groups] == 'true' Group.messageable(current_user) end end - include_groups = params[:include_groups] == "true" - # blank term is only handy for in-topic search of users after @ # we do not want group results ever if term is blank - include_groups = groups = nil if term.blank? + groups = nil if term.blank? - if include_groups || groups + if groups groups = Group.search_groups(term, groups: groups) - groups = groups.where(visibility_level: Group.visibility_levels[:public]) if include_groups groups = groups.order('groups.name asc') to_render[:groups] = groups.map do |m| diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 2a692fa159e..9869ce538fe 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -3176,6 +3176,15 @@ describe UsersController do ) end + let!(:private_group) do + Fabricate(:group, + mentionable_level: Group::ALIAS_LEVELS[:members_mods_and_admins], + messageable_level: Group::ALIAS_LEVELS[:members_mods_and_admins], + visibility_level: Group.visibility_levels[:members], + name: 'aaa4' + ) + end + describe 'when signed in' do before do sign_in(user) @@ -3198,7 +3207,7 @@ describe UsersController do groups = JSON.parse(response.body)["groups"] expect(groups.map { |group| group['name'] }) - .to_not include(mentionable_group_2.name) + .to_not include(private_group.name) end it "doesn't search for groups" do