From 3d6d7c8abefd32528916c7137a3b84e0c2cfed32 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 30 Oct 2013 15:45:13 -0400 Subject: [PATCH] SiteSetting to hide regular names from users --- .../discourse/controllers/user_controller.js | 4 +- .../discourse/routes/application_routes.js | 4 +- .../discourse/routes/preferences_routes.js | 5 ++ .../discourse/routes/user_invited_route.js | 6 ++ .../templates/user/user.js.handlebars | 4 +- .../javascripts/discourse/views/post_view.js | 48 +++++++------- .../views/user/activity_filter_view.js | 11 ++-- app/controllers/users_controller.rb | 8 ++- app/models/site_setting.rb | 2 + app/models/user_search.rb | 66 ++++++++++--------- app/serializers/basic_post_serializer.rb | 4 ++ app/serializers/post_serializer.rb | 5 +- app/serializers/user_serializer.rb | 4 ++ config/locales/server.en.yml | 2 + lib/freedom_patches/active_record_base.rb | 4 ++ spec/controllers/users_controller_spec.rb | 24 +++++++ spec/models/user_search_spec.rb | 37 +++++++---- .../serializers/basic_post_serializer_spec.rb | 25 +++++++ spec/serializers/post_serializer_spec.rb | 17 +++++ spec/serializers/user_serializer_spec.rb | 39 +++++++++++ 20 files changed, 236 insertions(+), 83 deletions(-) create mode 100644 spec/serializers/basic_post_serializer_spec.rb create mode 100644 spec/serializers/user_serializer_spec.rb diff --git a/app/assets/javascripts/discourse/controllers/user_controller.js b/app/assets/javascripts/discourse/controllers/user_controller.js index 74f221c60bc..541f116ea32 100644 --- a/app/assets/javascripts/discourse/controllers/user_controller.js +++ b/app/assets/javascripts/discourse/controllers/user_controller.js @@ -12,9 +12,7 @@ Discourse.UserController = Discourse.ObjectController.extend({ return this.get('content.username') === Discourse.User.currentProp('username'); }.property('content.username'), - collapsedInfo: function() { - return !this.get('indexStream'); - }.property('indexStream'), + collapsedInfo: Em.computed.not('indexStream'), canSeePrivateMessages: function() { return this.get('viewingSelf') || Discourse.User.currentProp('staff'); diff --git a/app/assets/javascripts/discourse/routes/application_routes.js b/app/assets/javascripts/discourse/routes/application_routes.js index 139b863591c..bc58a4e1165 100644 --- a/app/assets/javascripts/discourse/routes/application_routes.js +++ b/app/assets/javascripts/discourse/routes/application_routes.js @@ -49,9 +49,9 @@ Discourse.Route.buildRoutes(function() { this.route('index', { path: '/'} ); this.resource('userActivity', { path: '/activity' }, function() { - var resource = this; + var self = this; Object.keys(Discourse.UserAction.TYPES).forEach(function (userAction) { - resource.route(userAction, { path: userAction.replace("_", "-") }); + self.route(userAction, { path: userAction.replace("_", "-") }); }); }); diff --git a/app/assets/javascripts/discourse/routes/preferences_routes.js b/app/assets/javascripts/discourse/routes/preferences_routes.js index ca7d3aadbba..56902421051 100644 --- a/app/assets/javascripts/discourse/routes/preferences_routes.js +++ b/app/assets/javascripts/discourse/routes/preferences_routes.js @@ -15,6 +15,11 @@ Discourse.PreferencesRoute = Discourse.RestrictedUserRoute.extend({ this.render('preferences', { into: 'user', outlet: 'userOutlet', controller: 'preferences' }); }, + setupController: function(controller, model) { + controller.set('model', model); + this.controllerFor('user').set('indexStream', false); + }, + actions: { showAvatarSelector: function() { Discourse.Route.showModal(this, 'avatarSelector'); diff --git a/app/assets/javascripts/discourse/routes/user_invited_route.js b/app/assets/javascripts/discourse/routes/user_invited_route.js index de08b47eb9f..420b730edfc 100644 --- a/app/assets/javascripts/discourse/routes/user_invited_route.js +++ b/app/assets/javascripts/discourse/routes/user_invited_route.js @@ -13,5 +13,11 @@ Discourse.UserInvitedRoute = Discourse.Route.extend({ model: function() { return Discourse.InviteList.findInvitedBy(this.modelFor('user')); + }, + + setupController: function(controller, model) { + controller.set('model', model); + this.controllerFor('user').set('indexStream', false); } + }); \ No newline at end of file diff --git a/app/assets/javascripts/discourse/templates/user/user.js.handlebars b/app/assets/javascripts/discourse/templates/user/user.js.handlebars index bcbbaf307ff..535d7f83383 100644 --- a/app/assets/javascripts/discourse/templates/user/user.js.handlebars +++ b/app/assets/javascripts/discourse/templates/user/user.js.handlebars @@ -4,9 +4,9 @@
diff --git a/app/assets/javascripts/discourse/views/post_view.js b/app/assets/javascripts/discourse/views/post_view.js index 57c3f5e548e..ada28927ba2 100644 --- a/app/assets/javascripts/discourse/views/post_view.js +++ b/app/assets/javascripts/discourse/views/post_view.js @@ -135,36 +135,38 @@ Discourse.PostView = Discourse.GroupedView.extend(Ember.Evented, { } }, - /** - Toggle the replies this post is a reply to + actions: { + /** + Toggle the replies this post is a reply to - @method showReplyHistory - **/ - toggleReplyHistory: function(post) { + @method showReplyHistory + **/ + toggleReplyHistory: function(post) { - var replyHistory = post.get('replyHistory'), - topicController = this.get('controller'), - origScrollTop = $(window).scrollTop(); + var replyHistory = post.get('replyHistory'), + topicController = this.get('controller'), + origScrollTop = $(window).scrollTop(); - if (replyHistory.length > 0) { - var origHeight = this.$('.embedded-posts.top').height(); - - replyHistory.clear(); - Em.run.next(function() { - $(window).scrollTop(origScrollTop - origHeight); - }); - } else { - post.set('loadingReplyHistory', true); - - var self = this; - topicController.get('postStream').findReplyHistory(post).then(function () { - post.set('loadingReplyHistory', false); + if (replyHistory.length > 0) { + var origHeight = this.$('.embedded-posts.top').height(); + replyHistory.clear(); Em.run.next(function() { - $(window).scrollTop(origScrollTop + self.$('.embedded-posts.top').height()); + $(window).scrollTop(origScrollTop - origHeight); }); - }); + } else { + post.set('loadingReplyHistory', true); + + var self = this; + topicController.get('postStream').findReplyHistory(post).then(function () { + post.set('loadingReplyHistory', false); + + Em.run.next(function() { + $(window).scrollTop(origScrollTop + self.$('.embedded-posts.top').height()); + }); + }); + } } }, diff --git a/app/assets/javascripts/discourse/views/user/activity_filter_view.js b/app/assets/javascripts/discourse/views/user/activity_filter_view.js index fa16b72d89d..5f3378569dd 100644 --- a/app/assets/javascripts/discourse/views/user/activity_filter_view.js +++ b/app/assets/javascripts/discourse/views/user/activity_filter_view.js @@ -2,15 +2,14 @@ This view handles rendering of an activity in a user's profile @class ActivityFilterView - @extends Discourse.View + @extends Ember.Component @namespace Discourse @module Discourse **/ -Discourse.ActivityFilterView = Discourse.View.extend({ +Discourse.ActivityFilterView = Ember.Component.extend({ tagName: 'li', classNameBindings: ['active', 'noGlyph'], - userActionType: Em.computed.alias('controller.userActionType'), shouldRerender: Discourse.View.renderIfChanged('count'), noGlyph: Em.computed.empty('icon'), @@ -19,9 +18,9 @@ Discourse.ActivityFilterView = Discourse.View.extend({ if (content) { return parseInt(this.get('userActionType'), 10) === parseInt(Em.get(content, 'action_type'), 10); } else { - return this.blank('userActionType'); + return this.get('indexStream'); } - }.property('userActionType', 'content.action_type'), + }.property('userActionType', 'indexStream'), activityCount: function() { return this.get('content.count') || this.get('count'); @@ -75,4 +74,4 @@ Discourse.ActivityFilterView = Discourse.View.extend({ }); -Discourse.View.registerHelper('activityFilter', Discourse.ActivityFilterView); +Discourse.View.registerHelper('discourse-activity-filter', Discourse.ActivityFilterView); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 711830b08be..b2a696a69af 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -240,10 +240,12 @@ class UsersController < ApplicationController topic_id = params[:topic_id] topic_id = topic_id.to_i if topic_id - results = UserSearch.search term, topic_id + results = UserSearch.new(term, topic_id).search - render json: { users: results.as_json(only: [ :username, :name, :use_uploaded_avatar, :upload_avatar_template, :uploaded_avatar_id], - methods: :avatar_template) } + user_fields = [:username, :use_uploaded_avatar, :upload_avatar_template, :uploaded_avatar_id] + user_fields << :name if SiteSetting.enable_names? + + render json: { users: results.as_json(only: user_fields, methods: :avatar_template) } end # [LEGACY] avatars in quotes/oneboxes might still be pointing to this route diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 8599c6719aa..9a7adf6c224 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -270,6 +270,8 @@ class SiteSetting < ActiveRecord::Base # hidden setting only used by system setting(:uncategorized_category_id, -1, hidden: true) + client_setting(:enable_names, true) + def self.call_discourse_hub? self.enforce_global_nicknames? && self.discourse_org_access_key.present? end diff --git a/app/models/user_search.rb b/app/models/user_search.rb index e6f9078ad01..81050e511e8 100644 --- a/app/models/user_search.rb +++ b/app/models/user_search.rb @@ -1,36 +1,38 @@ +# Searches for a user by username or full text or name (if enabled in SiteSettings) class UserSearch - def self.search term, topic_id = nil - sql = User.sql_builder( -"select id, username, name, email, use_uploaded_avatar, uploaded_avatar_template, uploaded_avatar_id from users u -/*left_join*/ -/*where*/ -/*order_by*/") - - if topic_id - sql.left_join "(select distinct p.user_id from posts p where topic_id = :topic_id) s on s.user_id = u.id", topic_id: topic_id - end - - if term.present? - sql.where("username_lower like :term_like or - to_tsvector('simple', name) @@ - to_tsquery('simple', - regexp_replace( - regexp_replace( - cast(plainto_tsquery(:term) as text) - ,'\''(?: |$)', ':*''', 'g'), - '''', '', 'g') - )", term: term, term_like: "#{term.downcase}%") - - sql.order_by "case when username_lower = :term then 0 else 1 end asc" - end - - if topic_id - sql.order_by "case when s.user_id is null then 0 else 1 end desc" - end - - sql.order_by "case when last_seen_at is null then 0 else 1 end desc, last_seen_at desc, username asc limit(20)" - - sql.exec + def initialize(term, topic_id=nil) + @term = term + @term_like = "#{term.downcase}%" + @topic_id = topic_id end + + def search + users = User.order(User.sql_fragment("CASE WHEN username_lower = ? THEN 0 ELSE 1 END ASC", :term)) + + if @term.present? + if SiteSetting.enable_names? + users = users.where("username_lower LIKE :term_like OR + TO_TSVECTOR('simple', name) @@ + TO_TSQUERY('simple', + REGEXP_REPLACE( + REGEXP_REPLACE( + CAST(PLAINTO_TSQUERY(:term) AS TEXT) + ,'\''(?: |$)', ':*''', 'g'), + '''', '', 'g') + )", term: @term, term_like: @term_like) + else + users = users.where("username_lower LIKE :term_like", term_like: @term_like) + end + end + + if @topic_id + users = users.joins(User.sql_fragment("LEFT JOIN (SELECT DISTINCT p.user_id FROM POSTS p WHERE topic_id = ?) s ON s.user_id = users.id", @topic_id)) + .order("CASE WHEN s.user_id IS NULL THEN 0 ELSE 1 END DESC") + end + + users.order("CASE WHEN last_seen_at IS NULL THEN 0 ELSE 1 END DESC, last_seen_at DESC, username ASC") + .limit(20) + end + end diff --git a/app/serializers/basic_post_serializer.rb b/app/serializers/basic_post_serializer.rb index f200193aa40..8f8727116fc 100644 --- a/app/serializers/basic_post_serializer.rb +++ b/app/serializers/basic_post_serializer.rb @@ -31,4 +31,8 @@ class BasicPostSerializer < ApplicationSerializer end end + def include_name? + SiteSetting.enable_names? + end + end diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 17c931859f7..d65215b0110 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -112,7 +112,6 @@ class PostSerializer < BasicPostSerializer def reply_to_user { username: object.reply_to_user.username, - name: object.reply_to_user.name, avatar_template: object.reply_to_user.avatar_template } end @@ -194,6 +193,10 @@ class PostSerializer < BasicPostSerializer post_actions.present? && post_actions.keys.include?(PostActionType.types[:bookmark]) end + def include_display_username? + SiteSetting.enable_names? + end + private def post_actions diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 54fcf4ca20d..4533d320bc3 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -92,4 +92,8 @@ class UserSerializer < BasicUserSerializer User.gravatar_template(object.email) end + def include_name? + SiteSetting.enable_names? + end + end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a41a342245d..4c67e4cb33b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -721,6 +721,8 @@ en: dominating_topic_minimum_percent: "What percentage of posts a user has to make in a topic before we consider it dominating." + enable_names: "Allow users to show their full names" + notification_types: mentioned: "%{display_username} mentioned you in %{link}" liked: "%{display_username} liked your post in %{link}" diff --git a/lib/freedom_patches/active_record_base.rb b/lib/freedom_patches/active_record_base.rb index d4f8f2467c1..c1ccedec68f 100644 --- a/lib/freedom_patches/active_record_base.rb +++ b/lib/freedom_patches/active_record_base.rb @@ -11,6 +11,10 @@ class ActiveRecord::Base exec_sql(*args).cmd_tuples end + def self.sql_fragment(*sql_array) + ActiveRecord::Base.send(:sanitize_sql_array, sql_array) + end + # exists fine in rails4 unless rails4? # note: update_attributes still spins up a transaction this can cause contention diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 5fc9d91ad4c..609242792c5 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -893,6 +893,30 @@ describe UsersController do json["users"].map { |u| u["username"] }.should include(user.username) end + context "when `enable_names` is true" do + before do + SiteSetting.stubs(:enable_names?).returns(true) + end + + it "returns names" do + xhr :post, :search_users, term: user.name + json = JSON.parse(response.body) + json["users"].map { |u| u["name"] }.should include(user.name) + end + end + + context "when `enable_names` is false" do + before do + SiteSetting.stubs(:enable_names?).returns(false) + end + + it "returns names" do + xhr :post, :search_users, term: user.name + json = JSON.parse(response.body) + json["users"].map { |u| u["name"] }.should_not include(user.name) + end + end + end describe 'send_activation_email' do diff --git a/spec/models/user_search_spec.rb b/spec/models/user_search_spec.rb index e813f915a64..2e73312d5db 100644 --- a/spec/models/user_search_spec.rb +++ b/spec/models/user_search_spec.rb @@ -21,53 +21,68 @@ describe UserSearch do Fabricate :post, user: user6, topic: topic end + def search_for(*args) + UserSearch.new(*args).search + end + # this is a seriously expensive integration test, re-creating this entire test db is too expensive # reuse it "operates correctly" do # normal search - results = UserSearch.search user1.name.split(" ").first + results = search_for(user1.name.split(" ").first) results.size.should == 1 results.first.should == user1 # lower case - results = UserSearch.search user1.name.split(" ").first.downcase + results = search_for(user1.name.split(" ").first.downcase) results.size.should == 1 results.first.should == user1 # username - results = UserSearch.search user4.username + results = search_for(user4.username) results.size.should == 1 results.first.should == user4 # case insensitive - results = UserSearch.search user4.username.upcase + results = search_for(user4.username.upcase) results.size.should == 1 results.first.should == user4 # substrings - results = UserSearch.search "mr" + results = search_for("mr") results.size.should == 6 - results = UserSearch.search "mrb" + results = search_for("mrb") results.size.should == 3 - results = UserSearch.search "MR" + results = search_for("MR") results.size.should == 6 - results = UserSearch.search "MRB" + results = search_for("MRB") results.size.should == 3 # topic priority - results = UserSearch.search "mrb", topic.id + results = search_for("mrb", topic.id) results.first.should == user1 - results = UserSearch.search "mrb", topic2.id + results = search_for("mrb", topic2.id) results.first.should == user2 - results = UserSearch.search "mrb", topic3.id + results = search_for("mrb", topic3.id) results.first.should == user5 + + # When searching by name is enabled, it returns the record + SiteSetting.stubs(:enable_names).returns(true) + results = search_for("Tarantino") + results.size.should == 1 + + # When searching by name is disabled, it will not return the record + SiteSetting.stubs(:enable_names).returns(false) + results = search_for("Tarantino") + results.size.should == 0 + end end diff --git a/spec/serializers/basic_post_serializer_spec.rb b/spec/serializers/basic_post_serializer_spec.rb new file mode 100644 index 00000000000..c7f1388d029 --- /dev/null +++ b/spec/serializers/basic_post_serializer_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' +require_dependency 'post' +require_dependency 'user' + +describe BasicPostSerializer do + + context "name" do + let(:user) { Fabricate.build(:user) } + let(:post) { Fabricate.build(:post, user: user) } + let(:serializer) { BasicPostSerializer.new(post, scope: Guardian.new, root: false) } + let(:json) { serializer.as_json } + + it "returns the name it when `enable_names` is true" do + SiteSetting.stubs(:enable_names?).returns(true) + json[:name].should be_present + end + + it "doesn't return the name it when `enable_names` is false" do + SiteSetting.stubs(:enable_names?).returns(false) + json[:name].should be_blank + end + + end + +end diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index 5ac51470e80..b16d7954f8a 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -58,4 +58,21 @@ describe PostSerializer do end end + context "display_username" do + let(:user) { Fabricate.build(:user) } + let(:post) { Fabricate.build(:post, user: user) } + let(:serializer) { PostSerializer.new(post, scope: Guardian.new, root: false) } + let(:json) { serializer.as_json } + + it "returns the display_username it when `enable_names` is on" do + SiteSetting.stubs(:enable_names).returns(true) + json[:display_username].should be_present + end + + it "doesn't return the display_username it when `enable_names` is off" do + SiteSetting.stubs(:enable_names).returns(false) + json[:display_username].should be_blank + end + end + end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb new file mode 100644 index 00000000000..0c4e558a598 --- /dev/null +++ b/spec/serializers/user_serializer_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' +require_dependency 'user' + +describe UserSerializer do + + context "with a user" do + let(:user) { Fabricate.build(:user) } + let(:serializer) { UserSerializer.new(user, scope: Guardian.new, root: false) } + let(:json) { serializer.as_json } + + it "produces json" do + json.should be_present + end + + context "with `enable_names` true" do + before do + SiteSetting.stubs(:enable_names?).returns(true) + end + + it "has a name" do + json[:name].should == "Bruce Wayne" + end + end + + context "with `enable_names` false" do + before do + SiteSetting.stubs(:enable_names?).returns(false) + end + + it "has a name" do + puts json[:name] + json[:name].should be_blank + end + end + + + end + +end