From fb72e2665f9f69f95f84fe2cfc3f6b0b3841a39a Mon Sep 17 00:00:00 2001 From: riking Date: Fri, 23 Jan 2015 21:03:44 -0800 Subject: [PATCH 1/7] PERF :racehorse: Don't calculate preload data for non-xhr json requests This will help out anyone querying as API instead of through a browser. --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b3158c509f7..ff805687b44 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -155,7 +155,7 @@ class ApplicationController < ActionController::Base # If we are rendering HTML, preload the session data def preload_json # We don't preload JSON on xhr or JSON request - return if request.xhr? + return if request.xhr? || request.format.json? preload_anonymous_data From 1d24d8471ef3cad7df341468a1e5ba14c1761460 Mon Sep 17 00:00:00 2001 From: riking Date: Fri, 23 Jan 2015 21:04:14 -0800 Subject: [PATCH 2/7] FEATURE: Latest posts endpoint at /posts.json --- app/controllers/posts_controller.rb | 23 +++++++++++++++++++- app/serializers/post_serializer.rb | 33 +++++++++++++++++++++++------ config/routes.rb | 1 + 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 2d0b936d012..26c19008731 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -5,7 +5,7 @@ require_dependency 'distributed_memoizer' class PostsController < ApplicationController # Need to be logged in for all actions here - before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :reply_history, :revisions, :latest_revision, :expand_embed, :markdown, :raw, :cooked] + before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :reply_history, :revisions, :latest_revision, :expand_embed, :markdown_id, :markdown_num, :cooked, :latest] skip_before_filter :check_xhr, only: [:markdown_id, :markdown_num, :short_link] @@ -25,6 +25,27 @@ class PostsController < ApplicationController end end + def latest + posts = Post.order(created_at: :desc) + .where('posts.id > ?', Post.last.id - 50) # last 50 post IDs only, to avoid counting deleted posts in security check + .includes(topic: :category) + .includes(:user) + .limit(50) + # Remove posts the user doesn't have permission to see + # This isn't leaking any information we weren't already through the post ID numbers + posts = posts.reject { |post| !guardian.can_see?(post) } + + counts = PostAction.counts_for(posts, current_user) + + render_json_dump(serialize_data(posts, + PostSerializer, + scope: guardian, + root: 'latest_posts', + add_raw: true, + all_post_actions: counts) + ) + end + def cooked post = find_post_from_params render json: {cooked: post.cooked} diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index b6ca0ba9212..bb99bfd85e8 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -1,12 +1,17 @@ class PostSerializer < BasicPostSerializer # To pass in additional information we might need - attr_accessor :topic_view, + INSTANCE_VARS = [:topic_view, :parent_post, :add_raw, :single_post_link_counts, :draft_sequence, - :post_actions + :post_actions, + :all_post_actions] + + INSTANCE_VARS.each do |v| + self.send(:attr_accessor, v) + end attributes :post_number, :post_type, @@ -53,6 +58,15 @@ class PostSerializer < BasicPostSerializer :static_doc, :via_email + def initialize(object, opts) + super(object, opts) + PostSerializer::INSTANCE_VARS.each do |name| + if opts.include? name + self.send("#{name}=", opts[name]) + end + end + end + def topic_slug object.try(:topic).try(:slug) end @@ -154,6 +168,13 @@ class PostSerializer < BasicPostSerializer scope.is_staff? && object.deleted_by.present? end + # Helper function to decide between #post_actions and @all_post_actions + def actions + return post_actions if post_actions.present? + return all_post_actions[object.id] if all_post_actions.present? + nil + end + # Summary of the actions taken on this post def actions_summary result = [] @@ -167,7 +188,7 @@ class PostSerializer < BasicPostSerializer id: id, count: count, hidden: (sym == :vote), - can_act: scope.post_can_act?(object, sym, taken_actions: post_actions) + can_act: scope.post_can_act?(object, sym, taken_actions: actions) } if sym == :notify_user && scope.current_user.present? && scope.current_user == object.user @@ -182,9 +203,9 @@ class PostSerializer < BasicPostSerializer active_flags[id].count > 0 end - if post_actions.present? && post_actions.has_key?(id) + if actions.present? && actions.has_key?(id) action_summary[:acted] = true - action_summary[:can_undo] = scope.can_delete?(post_actions[id]) + action_summary[:can_undo] = scope.can_delete?(actions[id]) end # only show public data @@ -225,7 +246,7 @@ class PostSerializer < BasicPostSerializer end def include_bookmarked? - post_actions.present? && post_actions.keys.include?(PostActionType.types[:bookmark]) + actions.present? && actions.keys.include?(PostActionType.types[:bookmark]) end def include_display_username? diff --git a/config/routes.rb b/config/routes.rb index 3c288b57d08..8a41b8be10c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -269,6 +269,7 @@ Discourse::Application.routes.draw do get "uploads/:site/:sha" => "uploads#show", constraints: { site: /\w+/, sha: /[a-z0-9]{40}/} post "uploads" => "uploads#create" + get "posts" => "posts#latest" get "posts/by_number/:topic_id/:post_number" => "posts#by_number" get "posts/:id/reply-history" => "posts#reply_history" get "posts/:username/deleted" => "posts#deleted_posts", constraints: {username: USERNAME_ROUTE_FORMAT} From 9e9119d1c1970999b252c8333e7349544e67e896 Mon Sep 17 00:00:00 2001 From: riking Date: Fri, 23 Jan 2015 21:22:19 -0800 Subject: [PATCH 3/7] FEATURE: Enable pagination of /posts.json --- app/controllers/posts_controller.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 26c19008731..9505c240dec 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -26,8 +26,14 @@ class PostsController < ApplicationController end def latest + params.permit(:before) + last_post_id = params[:before].to_i + last_post_id = Post.last.id if last_post_id <= 0 + + # last 50 post IDs only, to avoid counting deleted posts in security check posts = Post.order(created_at: :desc) - .where('posts.id > ?', Post.last.id - 50) # last 50 post IDs only, to avoid counting deleted posts in security check + .where('posts.id <= ?', last_post_id) + .where('posts.id > ?', last_post_id - 50) .includes(topic: :category) .includes(:user) .limit(50) From 6c410ed09366bef17ccc109c4c957e0c9fb45ed5 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 30 Jan 2015 01:11:41 +0530 Subject: [PATCH 4/7] FIX: strip whitespaces from user email --- app/models/user.rb | 9 ++++++--- spec/models/user_spec.rb | 10 ++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index b90c5544d22..4a8cd72b099 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -62,7 +62,7 @@ class User < ActiveRecord::Base delegate :last_sent_email_address, :to => :email_logs - before_validation :downcase_email + before_validation :strip_downcase_email validates_presence_of :username validate :username_validator @@ -764,8 +764,11 @@ class User < ActiveRecord::Base self.username_lower = username.downcase end - def downcase_email - self.email = self.email.downcase if self.email + def strip_downcase_email + if self.email + self.email = self.email.strip + self.email = self.email.downcase + end end def username_validator diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a13dc476f32..69bd8cb3a6c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -261,8 +261,14 @@ describe User do it "downcases email addresses" do user = Fabricate.build(:user, email: 'Fancy.Caps.4.U@gmail.com') - user.save - expect(user.reload.email).to eq('fancy.caps.4.u@gmail.com') + user.valid? + expect(user.email).to eq('fancy.caps.4.u@gmail.com') + end + + it "strips whitespace from email addresses" do + user = Fabricate.build(:user, email: ' example@gmail.com ') + user.valid? + expect(user.email).to eq('example@gmail.com') end end From 4a2786dbf23ec0a4054dac2cf99af37a468b1e46 Mon Sep 17 00:00:00 2001 From: Jeff Atwood Date: Thu, 29 Jan 2015 13:42:15 -0800 Subject: [PATCH 5/7] better copy on enable names setting --- config/locales/server.en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b8470eb7864..a75be34e8d1 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1013,7 +1013,7 @@ en: disable_edit_notifications: "Disables edit notifications by the system user when 'download_remote_images_to_local' is active." - enable_names: "Allow showing user full names. Disable to hide full names." + enable_names: "Show the user's full name on their profile, user card, and emails. Disable to hide full name everywhere." display_name_on_posts: "Show a user's full name on their posts in addition to their @username." invites_per_page: "Default invites shown on the user page." short_progress_text_threshold: "After the number of posts in a topic goes above this number, the progress bar will only show the current post number. If you change the progress bar's width, you may need to change this value." From 6a68e8c27284b542e344ea3b1a8d81311b62e39c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 29 Jan 2015 22:53:48 +0100 Subject: [PATCH 6/7] FIX: use CDN for user card/profile background and user avatars (for real this time) --- app/assets/javascripts/discourse.js | 6 ++++++ .../discourse/dialects/quote_dialect.js | 1 + app/assets/javascripts/discourse/models/user.js | 14 +++++--------- .../javascripts/discourse/views/user-card.js.es6 | 3 ++- lib/pretty_text.rb | 12 ++++++++---- spec/components/pretty_text_spec.rb | 8 ++++---- 6 files changed, 26 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/discourse.js b/app/assets/javascripts/discourse.js index 27b12c53abb..feb375d90c0 100644 --- a/app/assets/javascripts/discourse.js +++ b/app/assets/javascripts/discourse.js @@ -24,6 +24,12 @@ window.Discourse = Ember.Application.createWithMixins(Discourse.Ajax, { return u + url; }, + getURLWithCDN: function(url) { + url = this.getURL(url); + if (Discourse.CDN) { url = Discourse.CDN + url; } + return url; + }, + Resolver: DiscourseResolver, _titleChanged: function() { diff --git a/app/assets/javascripts/discourse/dialects/quote_dialect.js b/app/assets/javascripts/discourse/dialects/quote_dialect.js index b0b8f5bbb85..3c50598cfcb 100644 --- a/app/assets/javascripts/discourse/dialects/quote_dialect.js +++ b/app/assets/javascripts/discourse/dialects/quote_dialect.js @@ -1,4 +1,5 @@ var esc = Handlebars.Utils.escapeExpression; + Discourse.BBCode.register('quote', {noWrap: true, singlePara: true}, function(contents, bbParams, options) { var params = {'class': 'quote'}, username = null; diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index 7a5bd5c37c5..fde76922559 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/app/assets/javascripts/discourse/models/user.js @@ -72,10 +72,9 @@ Discourse.User = Discourse.Model.extend({ @type {String} **/ profileBackground: function() { - var background = this.get('profile_background'); - if(Em.isEmpty(background) || !Discourse.SiteSettings.allow_profile_backgrounds) { return; } - - return 'background-image: url(' + background + ')'; + var url = this.get('profile_background'); + if (Em.isEmpty(url) || !Discourse.SiteSettings.allow_profile_backgrounds) { return; } + return 'background-image: url(' + Discourse.getURLWithCDN(url) + ')'; }.property('profile_background'), /** @@ -442,6 +441,7 @@ Discourse.User.reopenClass(Discourse.Singleton, { avatarTemplate: function(username, uploadedAvatarId) { var url; + if (uploadedAvatarId) { url = "/user_avatar/" + Discourse.BaseUrl + @@ -456,11 +456,7 @@ Discourse.User.reopenClass(Discourse.Singleton, { Discourse.LetterAvatarVersion + ".png"; } - url = Discourse.getURL(url); - if (Discourse.CDN) { - url = Discourse.CDN + url; - } - return url; + return Discourse.getURLWithCDN(url); }, /** diff --git a/app/assets/javascripts/discourse/views/user-card.js.es6 b/app/assets/javascripts/discourse/views/user-card.js.es6 index 12394202978..66e62b88d58 100644 --- a/app/assets/javascripts/discourse/views/user-card.js.es6 +++ b/app/assets/javascripts/discourse/views/user-card.js.es6 @@ -11,6 +11,7 @@ export default Discourse.View.extend(CleansUp, { addBackground: function() { var url = this.get('controller.user.card_background'); + if (!this.get('allowBackgrounds')) { return; } var $this = this.$(); @@ -19,7 +20,7 @@ export default Discourse.View.extend(CleansUp, { if (Ember.isEmpty(url)) { $this.css('background-image', '').addClass('no-bg'); } else { - $this.css('background-image', "url(" + url + ")").removeClass('no-bg'); + $this.css('background-image', "url(" + Discourse.getURLWithCDN(url) + ")").removeClass('no-bg'); } }.observes('controller.user.card_background'), diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index ee5aa578331..d095bd0c6a3 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -1,11 +1,13 @@ require 'v8' require 'nokogiri' +require_dependency 'url_helper' require_dependency 'excerpt_parser' require_dependency 'post' module PrettyText class Helpers + include UrlHelper def t(key, opts) key = "js." + key @@ -21,15 +23,15 @@ module PrettyText # function here are available to v8 def avatar_template(username) return "" unless username - user = User.find_by(username_lower: username.downcase) - user.avatar_template if user.present? + return "" unless user.present? + schemaless absolute user.avatar_template end def is_username_valid(username) return false unless username username = username.downcase - return User.exec_sql('SELECT 1 FROM users WHERE username_lower = ?', username).values.length == 1 + User.exec_sql('SELECT 1 FROM users WHERE username_lower = ?', username).values.length == 1 end end @@ -128,7 +130,9 @@ module PrettyText context.eval("Discourse.SiteSettings = #{SiteSetting.client_settings_json};") context.eval("Discourse.CDN = '#{Rails.configuration.action_controller.asset_host}';") context.eval("Discourse.BaseUrl = 'http://#{RailsMultisite::ConnectionManagement.current_hostname}';") - context.eval("Discourse.getURL = function(url) {return '#{Discourse::base_uri}' + url};") + + context.eval("Discourse.getURL = function(url) { return '#{Discourse::base_uri}' + url };") + context.eval("Discourse.getURLWithCDN = function(url) { url = Discourse.getURL(url); if (Discourse.CDN) { url = Discourse.CDN + url; } return url; };") end def self.markdown(text, opts=nil) diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 7dda67ae999..00bb220717e 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -12,20 +12,20 @@ describe PrettyText do before(:each) do eviltrout = User.new - eviltrout.stubs(:avatar_template).returns("http://test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/{size}.png") + eviltrout.stubs(:avatar_template).returns("//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/{size}.png") User.expects(:find_by).with(username_lower: "eviltrout").returns(eviltrout) end it "produces a quote even with new lines in it" do - expect(PrettyText.cook("[quote=\"EvilTrout, post:123, topic:456, full:true\"]ddd\n[/quote]")).to match_html "" + expect(PrettyText.cook("[quote=\"EvilTrout, post:123, topic:456, full:true\"]ddd\n[/quote]")).to match_html "" end it "should produce a quote" do - expect(PrettyText.cook("[quote=\"EvilTrout, post:123, topic:456, full:true\"]ddd[/quote]")).to match_html "" + expect(PrettyText.cook("[quote=\"EvilTrout, post:123, topic:456, full:true\"]ddd[/quote]")).to match_html "" end it "trims spaces on quote params" do - expect(PrettyText.cook("[quote=\"EvilTrout, post:555, topic: 666\"]ddd[/quote]")).to match_html "" + expect(PrettyText.cook("[quote=\"EvilTrout, post:555, topic: 666\"]ddd[/quote]")).to match_html "" end end From d1ec1e2681c4ac1af1b9ecb89c3a43ee8de6fa11 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 29 Jan 2015 17:17:32 -0500 Subject: [PATCH 7/7] Allow code to save custom fields without saving the attached object --- app/models/concerns/has_custom_fields.rb | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/app/models/concerns/has_custom_fields.rb b/app/models/concerns/has_custom_fields.rb index 854dee20ef9..7b687619021 100644 --- a/app/models/concerns/has_custom_fields.rb +++ b/app/models/concerns/has_custom_fields.rb @@ -78,17 +78,6 @@ module HasCustomFields !@custom_fields || @custom_fields_orig == @custom_fields end - protected - - def refresh_custom_fields_from_db - target = Hash.new - _custom_fields.pluck(:name,:value).each do |key, value| - self.class.append_custom_field(target, key, value) - end - @custom_fields_orig = target - @custom_fields = @custom_fields_orig.dup - end - def save_custom_fields if !custom_fields_clean? dup = @custom_fields.dup @@ -134,4 +123,16 @@ module HasCustomFields refresh_custom_fields_from_db end end + + protected + + def refresh_custom_fields_from_db + target = Hash.new + _custom_fields.pluck(:name,:value).each do |key, value| + self.class.append_custom_field(target, key, value) + end + @custom_fields_orig = target + @custom_fields = @custom_fields_orig.dup + end + end