From d491d4f997e00454ef7f9d7b56ce9fbe4099125e Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Mon, 13 Apr 2015 20:33:13 +0530 Subject: [PATCH] FEATURE: invite existing users to private topic --- .../discourse/components/user-selector.js.es6 | 6 ++- .../discourse/controllers/invite.js.es6 | 37 +++++++++++++------ .../discourse/lib/user-search.js.es6 | 8 ++-- .../discourse/templates/modal/invite.hbs | 6 ++- app/controllers/users_controller.rb | 3 +- app/models/user_search.rb | 13 +++++++ config/locales/client.en.yml | 1 + lib/guardian.rb | 2 - spec/controllers/users_controller_spec.rb | 19 ++++++++++ 9 files changed, 74 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/discourse/components/user-selector.js.es6 b/app/assets/javascripts/discourse/components/user-selector.js.es6 index d0d210a68..d3bed034e 100644 --- a/app/assets/javascripts/discourse/components/user-selector.js.es6 +++ b/app/assets/javascripts/discourse/components/user-selector.js.es6 @@ -7,7 +7,8 @@ export default TextField.extend({ var self = this, selected = [], currentUser = this.currentUser, - includeGroups = this.get('includeGroups') === 'true'; + includeGroups = this.get('includeGroups') === 'true', + allowedUsers = this.get('allowedUsers') === 'true'; function excludedUsernames() { if (currentUser && self.get('excludeCurrentUser')) { @@ -27,7 +28,8 @@ export default TextField.extend({ term: term.replace(/[^a-zA-Z0-9_]/, ''), topicId: self.get('topicId'), exclude: excludedUsernames(), - includeGroups: includeGroups + includeGroups, + allowedUsers }); }, diff --git a/app/assets/javascripts/discourse/controllers/invite.js.es6 b/app/assets/javascripts/discourse/controllers/invite.js.es6 index 0dcf67c69..2201c3eb5 100644 --- a/app/assets/javascripts/discourse/controllers/invite.js.es6 +++ b/app/assets/javascripts/discourse/controllers/invite.js.es6 @@ -15,11 +15,15 @@ export default ObjectController.extend(ModalFunctionality, { disabled: function() { if (this.get('saving')) return true; if (this.blank('emailOrUsername')) return true; + // when inviting to forum, email must be valid if (!this.get('invitingToTopic') && !Discourse.Utilities.emailValid(this.get('emailOrUsername'))) return true; + // normal users (not admin) can't invite users to private topic via email + if (!this.get('isAdmin') && this.get('isPrivateTopic') && Discourse.Utilities.emailValid(this.get('emailOrUsername'))) return true; + // when invting to private topic via email, group name must be specified + if (this.get('isPrivateTopic') && this.blank('groupNames') && Discourse.Utilities.emailValid(this.get('emailOrUsername'))) return true; if (this.get('model.details.can_invite_to')) return false; - if (this.get('isPrivateTopic') && this.blank('groupNames')) return true; return false; - }.property('emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'groupNames', 'saving'), + }.property('isAdmin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'groupNames', 'saving'), buttonTitle: function() { return this.get('saving') ? I18n.t('topic.inviting') : I18n.t('topic.invite_reply.action'); @@ -31,20 +35,23 @@ export default ObjectController.extend(ModalFunctionality, { return this.get('model') !== Discourse.User.current(); }.property('model'), + topicId: Ember.computed.alias('model.id'), + // Is Private Topic? (i.e. visible only to specific group members) isPrivateTopic: Em.computed.and('invitingToTopic', 'model.category.read_restricted'), + // Is Private Message? isMessage: Em.computed.equal('model.archetype', 'private_message'), // Allow Existing Members? (username autocomplete) allowExistingMembers: function() { - return this.get('invitingToTopic') && !this.get('isPrivateTopic'); - }.property('invitingToTopic', 'isPrivateTopic'), + return this.get('invitingToTopic'); + }.property('invitingToTopic'), // Show Groups? (add invited user to private group) showGroups: function() { - return this.get('isAdmin') && (Discourse.Utilities.emailValid(this.get('emailOrUsername')) || this.get('isPrivateTopic') || !this.get('invitingToTopic')) && !Discourse.SiteSettings.enable_sso; - }.property('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'invitingToTopic'), + return this.get('isAdmin') && (Discourse.Utilities.emailValid(this.get('emailOrUsername')) || this.get('isPrivateTopic') || !this.get('invitingToTopic')) && !Discourse.SiteSettings.enable_sso && !this.get('isMessage'); + }.property('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic'), // Instructional text for the modal. inviteInstructions: function() { @@ -55,13 +62,19 @@ export default ObjectController.extend(ModalFunctionality, { // inviting to a message return I18n.t('topic.invite_private.email_or_username'); } else if (this.get('invitingToTopic')) { - // when inviting to topic, display instructions based on provided entity - if (this.blank('emailOrUsername')) { - return I18n.t('topic.invite_reply.to_topic_blank'); - } else if (Discourse.Utilities.emailValid(this.get('emailOrUsername'))) { - return I18n.t('topic.invite_reply.to_topic_email'); + // inviting to a private/public topic + if (this.get('isPrivateTopic') && !this.get('isAdmin')) { + // inviting to a private topic and is not admin + return I18n.t('topic.invite_reply.to_username'); } else { - return I18n.t('topic.invite_reply.to_topic_username'); + // when inviting to a topic, display instructions based on provided entity + if (this.blank('emailOrUsername')) { + return I18n.t('topic.invite_reply.to_topic_blank'); + } else if (Discourse.Utilities.emailValid(this.get('emailOrUsername'))) { + return I18n.t('topic.invite_reply.to_topic_email'); + } else { + return I18n.t('topic.invite_reply.to_topic_username'); + } } } else { // inviting to forum diff --git a/app/assets/javascripts/discourse/lib/user-search.js.es6 b/app/assets/javascripts/discourse/lib/user-search.js.es6 index 3311f518d..790a00ff0 100644 --- a/app/assets/javascripts/discourse/lib/user-search.js.es6 +++ b/app/assets/javascripts/discourse/lib/user-search.js.es6 @@ -6,7 +6,7 @@ var cache = {}, currentTerm, oldSearch; -function performSearch(term, topicId, includeGroups, resultsFn) { +function performSearch(term, topicId, includeGroups, allowedUsers, resultsFn) { var cached = cache[term]; if (cached) { resultsFn(cached); @@ -17,7 +17,8 @@ function performSearch(term, topicId, includeGroups, resultsFn) { oldSearch = $.ajax(Discourse.getURL('/users/search/users'), { data: { term: term, topic_id: topicId, - include_groups: includeGroups } + include_groups: includeGroups, + topic_allowed_users: allowedUsers } }); var returnVal = CANCELLED_STATUS; @@ -75,6 +76,7 @@ function organizeResults(r, options) { export default function userSearch(options) { var term = options.term || "", includeGroups = options.includeGroups, + allowedUsers = options.allowedUsers, topicId = options.topicId; @@ -101,7 +103,7 @@ export default function userSearch(options) { resolve(CANCELLED_STATUS); }, 5000); - debouncedSearch(term, topicId, includeGroups, function(r) { + debouncedSearch(term, topicId, includeGroups, allowedUsers, function(r) { clearTimeout(clearPromise); resolve(organizeResults(r, options)); }); diff --git a/app/assets/javascripts/discourse/templates/modal/invite.hbs b/app/assets/javascripts/discourse/templates/modal/invite.hbs index e5e17e32e..5984aa86a 100644 --- a/app/assets/javascripts/discourse/templates/modal/invite.hbs +++ b/app/assets/javascripts/discourse/templates/modal/invite.hbs @@ -10,7 +10,11 @@ {{else}} {{#if allowExistingMembers}} - {{user-selector single="true" allowAny=true excludeCurrentUser="true" usernames=emailOrUsername includeGroups="true" placeholderKey=placeholderKey}} + {{#if isPrivateTopic}} + {{user-selector single="true" allowAny=true excludeCurrentUser="true" usernames=emailOrUsername allowedUsers="true" topicId=topicId placeholderKey=placeholderKey}} + {{else}} + {{user-selector single="true" allowAny=true excludeCurrentUser="true" usernames=emailOrUsername placeholderKey=placeholderKey}} + {{/if}} {{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 23dfb4870..28c4ea7c9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -445,8 +445,9 @@ class UsersController < ApplicationController term = params[:term].to_s.strip topic_id = params[:topic_id] topic_id = topic_id.to_i if topic_id + topic_allowed_users = params[:topic_allowed_users] || false - results = UserSearch.new(term, topic_id: topic_id, searching_user: current_user).search + results = UserSearch.new(term, topic_id: topic_id, topic_allowed_users: topic_allowed_users, searching_user: current_user).search user_fields = [:username, :upload_avatar_template, :uploaded_avatar_id] user_fields << :name if SiteSetting.enable_names? diff --git a/app/models/user_search.rb b/app/models/user_search.rb index 535d1c29f..cbb18312f 100644 --- a/app/models/user_search.rb +++ b/app/models/user_search.rb @@ -5,6 +5,7 @@ class UserSearch @term = term @term_like = "#{term.downcase}%" @topic_id = opts[:topic_id] + @topic_allowed_users = opts[:topic_allowed_users] @searching_user = opts[:searching_user] @limit = opts[:limit] || 20 end @@ -36,6 +37,18 @@ class UserSearch users = users.not_suspended end + # Only show users who have access to private topic + if @topic_id && @topic_allowed_users == "true" + allowed_user_ids = [] + topic = Topic.find_by(id: @topic_id) + + if topic.category && topic.category.read_restricted + users = users.includes(:secure_categories) + .where("users.admin = TRUE OR categories.id = ?", topic.category.id) + .references(:categories) + end + end + users.order("CASE WHEN last_seen_at IS NULL THEN 0 ELSE 1 END DESC, last_seen_at DESC, username ASC") .limit(@limit) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c2d95b258..9dcca7b4e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1089,6 +1089,7 @@ en: to_topic_blank: "Enter the username or email address of the person you'd like to invite to this topic." to_topic_email: "You've entered an email address. We'll email an invitation that allows your friend to immediately reply to this topic." to_topic_username: "You've entered a username. We'll send a notification to that user with a link inviting them to this topic." + to_username: "Enter the username of the person you'd like to invite. We'll send a notification to that user with a link inviting them to this topic." email_placeholder: 'name@example.com' success_email: "We mailed out an invitation to {{emailOrUsername}}. We'll notify you when the invitation is redeemed. Check the invitations tab on your user page to keep track of your invites." diff --git a/lib/guardian.rb b/lib/guardian.rb index 51cedb1e9..69f2db0b7 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -214,14 +214,12 @@ class Guardian return false unless ( SiteSetting.enable_local_logins && (!SiteSetting.must_approve_users? || is_staff?) ) return true if is_admin? return false if ! can_see?(object) - return false if group_ids.present? if object.is_a?(Topic) && object.category if object.category.groups.any? return true if object.category.groups.all? { |g| can_edit_group?(g) } end - return false if object.category.read_restricted end user.has_trust_level?(TrustLevel[2]) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 159e007c7..f4a62a80b 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1164,6 +1164,25 @@ describe UsersController do expect(json["users"].map { |u| u["username"] }).to include(user.username) end + it "searches only for users who have access to private topic" do + privileged_user = Fabricate(:user, trust_level: 4, username: "joecabit", name: "Lawrence Tierney") + privileged_group = Fabricate(:group) + privileged_group.add(privileged_user) + privileged_group.save + + category = Fabricate(:category) + category.set_permissions(privileged_group => :readonly) + category.save + + private_topic = Fabricate(:topic, category: category) + + xhr :post, :search_users, term: user.name.split(" ").last, topic_id: private_topic.id, topic_allowed_users: "true" + expect(response).to be_success + json = JSON.parse(response.body) + expect(json["users"].map { |u| u["username"] }).to_not include(user.username) + expect(json["users"].map { |u| u["username"] }).to include(privileged_user.username) + end + context "when `enable_names` is true" do before do SiteSetting.enable_names = true