From b34b1b6fe384b7ee3a0896079476ef5c77c05804 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 13 Feb 2018 13:28:29 +1100 Subject: [PATCH] FIX: invite to message was not allowing groups Previously we were incorrectly checking mentionable instead of messageable Also fix edge case where multiple groups sharing a name mean that exact match override is not working Also cleans up params sent to user selector --- .../discourse/components/user-selector.js.es6 | 28 +++++++++++------ .../discourse/controllers/invite.js.es6 | 5 ++++ .../discourse/lib/user-search.js.es6 | 9 +++--- .../discourse/templates/modal/invite.hbs | 30 +++++++------------ app/controllers/users_controller.rb | 1 + test/javascripts/lib/user-search-test.js.es6 | 6 +++- 6 files changed, 45 insertions(+), 34 deletions(-) diff --git a/app/assets/javascripts/discourse/components/user-selector.js.es6 b/app/assets/javascripts/discourse/components/user-selector.js.es6 index 830a74d708c..3c9aefe6fb6 100644 --- a/app/assets/javascripts/discourse/components/user-selector.js.es6 +++ b/app/assets/javascripts/discourse/components/user-selector.js.es6 @@ -16,20 +16,30 @@ export default TextField.extend({ didInsertElement(opts) { this._super(); + + const bool = (n => { + const val = this.get(n); + return val === true || val === "true"; + }); + var self = this, selected = [], groups = [], currentUser = this.currentUser, - includeMentionableGroups = this.get('includeMentionableGroups') === 'true', - includeMessageableGroups = this.get('includeMessageableGroups') === 'true', - includeGroups = this.get('includeGroups') === 'true', - allowedUsers = this.get('allowedUsers') === 'true'; + includeMentionableGroups = bool('includeMentionableGroups'), + includeMessageableGroups = bool('includeMessageableGroups'), + includeGroups = bool('includeGroups'), + allowedUsers = bool('allowedUsers'), + excludeCurrentUser = bool('excludeCurrentUser'), + single = bool('single'), + allowAny = bool('allowAny'), + disabled = bool('disabled'); function excludedUsernames() { // hack works around some issues with allowAny eventing - const usernames = self.get('single') ? [] : selected; + const usernames = single ? [] : selected; - if (currentUser && self.get('excludeCurrentUser')) { + if (currentUser && excludeCurrentUser) { return usernames.concat([currentUser.get('username')]); } return usernames; @@ -37,9 +47,9 @@ export default TextField.extend({ this.$().val(this.get('usernames')).autocomplete({ template: findRawTemplate('user-selector-autocomplete'), - disabled: this.get('disabled'), - single: this.get('single'), - allowAny: this.get('allowAny'), + disabled: disabled, + single: single, + allowAny: allowAny, updateData: (opts && opts.updateData) ? opts.updateData : false, dataSource(term) { diff --git a/app/assets/javascripts/discourse/controllers/invite.js.es6 b/app/assets/javascripts/discourse/controllers/invite.js.es6 index 71509d9e137..178bb02234b 100644 --- a/app/assets/javascripts/discourse/controllers/invite.js.es6 +++ b/app/assets/javascripts/discourse/controllers/invite.js.es6 @@ -158,6 +158,11 @@ export default Ember.Controller.extend(ModalFunctionality, { return Group.findAll({ term: term, ignore_automatic: true }); }, + @computed('isPrivateTopic', 'isMessage') + includeMentionableGroups(isPrivateTopic, isMessage) { + return !isPrivateTopic && !isMessage; + }, + @computed('isMessage', 'emailOrUsername', 'invitingExistingUserToTopic') successMessage(isMessage, emailOrUsername, invitingExistingUserToTopic) { if (this.get('hasGroups')) { diff --git a/app/assets/javascripts/discourse/lib/user-search.js.es6 b/app/assets/javascripts/discourse/lib/user-search.js.es6 index 29ad228a050..ceed4404e76 100644 --- a/app/assets/javascripts/discourse/lib/user-search.js.es6 +++ b/app/assets/javascripts/discourse/lib/user-search.js.es6 @@ -69,10 +69,11 @@ function organizeResults(r, options) { if (r.groups) { r.groups.every(function(g) { - if (results.length > limit && options.term.toLowerCase() !== g.name.toLowerCase()) return false; - if (exclude.indexOf(g.name) === -1) { - groups.push(g); - results.push(g); + if (options.term.toLowerCase() === g.name.toLowerCase() || results.length < limit) { + if (exclude.indexOf(g.name) === -1) { + groups.push(g); + results.push(g); + } } return true; }); diff --git a/app/assets/javascripts/discourse/templates/modal/invite.hbs b/app/assets/javascripts/discourse/templates/modal/invite.hbs index 7ed29ae7ddd..4c27a8ee286 100644 --- a/app/assets/javascripts/discourse/templates/modal/invite.hbs +++ b/app/assets/javascripts/discourse/templates/modal/invite.hbs @@ -14,26 +14,16 @@ {{else}} {{#if allowExistingMembers}} - {{#if isPrivateTopic}} - {{user-selector single="true" - allowAny=true - excludeCurrentUser="true" - usernames=emailOrUsername - allowedUsers="true" - topicId=topicId - placeholderKey=placeholderKey - autocomplete="off"}} - {{else}} - {{user-selector - single="true" - allowAny=true - excludeCurrentUser="true" - includeMentionableGroups="true" - hasGroups=hasGroups - usernames=emailOrUsername - placeholderKey=placeholderKey - autocomplete="off"}} - {{/if}} + {{user-selector + single=true + allowAny=true + excludeCurrentUser=true + includeMentionableGroups=includeMentionableGroups + includeMessageableGroups=isMessage + hasGroups=hasGroups + usernames=emailOrUsername + placeholderKey=placeholderKey + autocomplete="off"}} {{else}} {{text-field value=emailOrUsername placeholderKey="topic.invite_reply.email_placeholder"}} {{/if}} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 72e706e86c8..b26f1a5312d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -751,6 +751,7 @@ class UsersController < ApplicationController if include_groups || 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| { name: m.name, full_name: m.full_name } diff --git a/test/javascripts/lib/user-search-test.js.es6 b/test/javascripts/lib/user-search-test.js.es6 index 9cdb8302216..98b59388597 100644 --- a/test/javascripts/lib/user-search-test.js.es6 +++ b/test/javascripts/lib/user-search-test.js.es6 @@ -47,6 +47,10 @@ QUnit.module("lib:user-search", { } ], groups: [ + { + "name": "bob", + "usernames": [] + }, { "name": "team", "usernames": [] @@ -61,4 +65,4 @@ QUnit.test("it places groups unconditionally for exact match", assert => { return userSearch({term: 'Team'}).then((results)=>{ assert.equal(results[results.length-1]["name"], "team"); }); -}); \ No newline at end of file +});