diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index fd10255c0ac..598303f05af 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/app/assets/javascripts/discourse/models/user.js @@ -274,34 +274,35 @@ Discourse.User.reopenClass({ }); }, + /** + Find a user by username + + @method find + @param {String} username the username of the user we want to find + **/ find: function(username) { - var promise, - _this = this; - promise = new RSVP.Promise(); - $.ajax({ - url: "/users/" + username + '.json', - success: function(json) { - // todo: decompose to object - var user; - json.user.stats = _this.groupStats(json.user.stats.map(function(s) { - var obj; - obj = Em.Object.create(s); - obj.isPM = obj.action_type === Discourse.UserAction.NEW_PRIVATE_MESSAGE || obj.action_type === Discourse.UserAction.GOT_PRIVATE_MESSAGE; - return obj; + + // Check the preload store first + return PreloadStore.get("user_" + username, function() { + return $.ajax({ url: "/users/" + username + '.json' }); + }).then(function (json) { + + // Create a user from the resulting JSON + json.user.stats = Discourse.User.groupStats(json.user.stats.map(function(s) { + var stat = Em.Object.create(s); + stat.set('isPM', stat.get('action_type') === Discourse.UserAction.NEW_PRIVATE_MESSAGE || + stat.get('action_type') === Discourse.UserAction.GOT_PRIVATE_MESSAGE); + return stat; + })); + + if (json.user.stream) { + json.user.stream = Discourse.UserAction.collapseStream(json.user.stream.map(function(ua) { + return Discourse.UserAction.create(ua); })); - if (json.user.stream) { - json.user.stream = Discourse.UserAction.collapseStream(json.user.stream.map(function(ua) { - return Discourse.UserAction.create(ua); - })); - } - user = Discourse.User.create(json.user); - return promise.resolve(user); - }, - error: function(xhr) { - return promise.reject(xhr); } + + return Discourse.User.create(json.user); }); - return promise; }, createAccount: function(name, email, password, username, passwordConfirm, challenge) { diff --git a/app/assets/javascripts/preload_store.js b/app/assets/javascripts/preload_store.js index 55422a674a2..480cee568bb 100644 --- a/app/assets/javascripts/preload_store.js +++ b/app/assets/javascripts/preload_store.js @@ -1,23 +1,45 @@ -/* We can insert data into the PreloadStore when the document is loaded. - The data can be accessed once by a key, after which it is removed */ + +/** + We can insert data into the PreloadStore when the document is loaded. + The data can be accessed once by a key, after which it is removed + + @class PreloadStore +**/ PreloadStore = { data: {}, + + /** + Store an object in the store + + @method store + @param {String} key the key to store the object with + @param {String} value the object we're inserting into the store + **/ store: function(key, value) { this.data[key] = value; }, - /* To retrieve a key, you provide the key you want, plus a finder to - load it if the key cannot be found. Once the key is used once, it is - removed from the store. So, for example, you can't load a preloaded topic - more than once. */ + + /** + To retrieve a key, you provide the key you want, plus a finder to + load it if the key cannot be found. Once the key is used once, it is + removed from the store. So, for example, you can't load a preloaded topic + more than once. + + @method get + @param {String} key the key to look up the object with + @param {function} finder a function to find the object with + @returns {Promise} a promise that will eventually be the object we want. + **/ get: function(key, finder) { - var promise, result; - promise = new RSVP.Promise(); + var promise = new RSVP.Promise(); + if (this.data[key]) { promise.resolve(this.data[key]); delete this.data[key]; } else { + if (finder) { - result = finder(); + var result = finder(); // If the finder returns a promise, we support that too if (result.then) { @@ -30,22 +52,35 @@ PreloadStore = { promise.resolve(result); } } else { - promise.resolve(void 0); + promise.resolve(null); } } return promise; }, - /* Does the store contain a particular key? Does not delete, just returns - true or false. */ + + /** + Does the store contain a particular key? Does not delete. + + @method contains + @param {String} key the key to look up the object with + @returns {Boolean} whether the object exists + **/ contains: function(key) { return this.data[key] !== void 0; }, - /* If we are sure it's preloaded, we don't have to supply a finder. Just - returns undefined if it's not in the store. */ + + /** + If we are sure it's preloaded, we don't have to supply a finder. Just returns + undefined if it's not in the store. + + @method getStatic + @param {String} key the key to look up the object with + @returns {Object} the object from the store + **/ getStatic: function(key) { - var result; - result = this.data[key]; + var result = this.data[key]; delete this.data[key]; return result; } + }; diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8a471af7b4a..cb7ab5c0d1d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,7 +2,7 @@ require_dependency 'discourse_hub' class UsersController < ApplicationController - skip_before_filter :check_xhr, only: [:password_reset, :update, :activate_account, :avatar, :authorize_email, :user_preferences_redirect] + skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :avatar, :authorize_email, :user_preferences_redirect] skip_before_filter :authorize_mini_profiler, only: [:avatar] skip_before_filter :check_restricted_access, only: [:avatar] @@ -10,8 +10,15 @@ class UsersController < ApplicationController def show @user = fetch_user_from_params - anonymous_etag(@user) do - render_serialized(@user, UserSerializer) + user_serializer = UserSerializer.new(@user, scope: guardian, root: 'user') + respond_to do |format| + format.html do + store_preloaded("user_#{@user.username}", MultiJson.dump(user_serializer)) + end + + format.json do + render_json_dump(user_serializer) + end end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 9e90ef0dede..fce2b4609e1 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -32,23 +32,23 @@ module ApplicationHelper current_user.try(:admin?) end - def crawlable_meta_data(url, title, description) - # Image to supply as meta data - image = "#{Discourse.base_url}#{SiteSetting.logo_url}" + # Creates open graph and twitter card meta data + def crawlable_meta_data(opts=nil) + + opts ||= {} + opts[:image] ||= "#{Discourse.base_url}#{SiteSetting.logo_url}" + opts[:url] ||= "#{Discourse.base_url}#{request.fullpath}" # Add opengraph tags result = tag(:meta, property: 'og:site_name', content: SiteSetting.title) << "\n" - result << tag(:meta, property: 'og:image', content: image) << "\n" - result << tag(:meta, property: 'og:url', content: url) << "\n" - result << tag(:meta, property: 'og:title', content: title) << "\n" - result << tag(:meta, property: 'og:description', content: description) << "\n" - # Add twitter card - result << tag(:meta, property: 'twitter:card', content: "summary") << "\n" - result << tag(:meta, property: 'twitter:url', content: url) << "\n" - result << tag(:meta, property: 'twitter:title', content: title) << "\n" - result << tag(:meta, property: 'twitter:description', content: description) << "\n" - result << tag(:meta, property: 'twitter:image', content: image) << "\n" + result << tag(:meta, property: 'twitter:card', content: "summary") + [:image, :url, :title, :description].each do |property| + if opts[property].present? + result << tag(:meta, property: "og:#{property}", content: opts[property]) << "\n" + result << tag(:meta, property: "twitter:#{property}", content: opts[property]) << "\n" + end + end result end diff --git a/app/models/user.rb b/app/models/user.rb index 33b703a1c55..85ad4bb74ce 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,7 @@ require_dependency 'email_token' require_dependency 'trust_level' require_dependency 'pbkdf2' +require_dependency 'summarize' class User < ActiveRecord::Base attr_accessible :name, :username, :password, :email, :bio_raw, :website @@ -447,6 +448,11 @@ class User < ActiveRecord::Base username end + def bio_summary + return nil unless bio_cooked.present? + Summarize.new(bio_cooked).summary + end + protected def cook diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb index c6cea4fa945..cddbc237f34 100644 --- a/app/views/topics/show.html.erb +++ b/app/views/topics/show.html.erb @@ -24,5 +24,5 @@ <% content_for :head do %> <%= auto_discovery_link_tag(@topic_view, {action: :feed, format: :rss}, title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %> - <%= crawlable_meta_data(@topic_view.absolute_url, @topic_view.title, @topic_view.summary) %> + <%= crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary) %> <% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb new file mode 100644 index 00000000000..169bd2dc393 --- /dev/null +++ b/app/views/users/show.html.erb @@ -0,0 +1,9 @@ +

<%= @user.username %>

+ +

<%= raw @user.bio_cooked %>

+ +

<%= t 'powered_by_html' %>

+ +<% content_for :head do %> + <%= crawlable_meta_data(title: @user.username, description: @user.bio_summary) %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index eb6271a448f..299e2606f58 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,7 +4,7 @@ require_dependency 'admin_constraint' # This used to be User#username_format, but that causes a preload of the User object # and makes Guard not work properly. -USERNAME_ROUTE_FORMAT = /[A-Za-z0-9\._]+/ +USERNAME_ROUTE_FORMAT = /[A-Za-z0-9\_]+/ Discourse::Application.routes.draw do @@ -88,14 +88,14 @@ Discourse::Application.routes.draw do get 'users/hp' => 'users#get_honeypot_value' get 'user_preferences' => 'users#user_preferences_redirect' - get 'users/:username/private-messages' => 'user_actions#private_messages', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} - get 'users/:username' => 'users#show', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} - put 'users/:username' => 'users#update', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} - get 'users/:username/preferences' => 'users#preferences', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}, :as => :email_preferences - get 'users/:username/preferences/email' => 'users#preferences', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} - put 'users/:username/preferences/email' => 'users#change_email', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} - get 'users/:username/preferences/username' => 'users#preferences', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} - put 'users/:username/preferences/username' => 'users#username', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} + get 'users/:username/private-messages' => 'user_actions#private_messages', :constraints => {:username => USERNAME_ROUTE_FORMAT} + get 'users/:username' => 'users#show', :constraints => {:username => USERNAME_ROUTE_FORMAT} + put 'users/:username' => 'users#update', :constraints => {:username => USERNAME_ROUTE_FORMAT} + get 'users/:username/preferences' => 'users#preferences', :constraints => {:username => USERNAME_ROUTE_FORMAT}, :as => :email_preferences + get 'users/:username/preferences/email' => 'users#preferences', :constraints => {:username => USERNAME_ROUTE_FORMAT} + put 'users/:username/preferences/email' => 'users#change_email', :constraints => {:username => USERNAME_ROUTE_FORMAT} + get 'users/:username/preferences/username' => 'users#preferences', :constraints => {:username => USERNAME_ROUTE_FORMAT} + put 'users/:username/preferences/username' => 'users#username', :constraints => {:username => USERNAME_ROUTE_FORMAT} get 'users/:username/avatar(/:size)' => 'users#avatar', :constraints => {:username => USERNAME_ROUTE_FORMAT} get 'users/:username/invited' => 'users#invited', :constraints => {:username => USERNAME_ROUTE_FORMAT} get 'users/:username/send_activation_email' => 'users#send_activation_email', :constraints => {:username => USERNAME_ROUTE_FORMAT} diff --git a/lib/summarize.rb b/lib/summarize.rb new file mode 100644 index 00000000000..f0ff2bb65d3 --- /dev/null +++ b/lib/summarize.rb @@ -0,0 +1,26 @@ +# Summarize a HTML field into regular text. Used currently +# for meta tags + +class Summarize + include ActionView::Helpers + + def initialize(text) + @text = text + end + + def self.max_length + 500 + end + + def summary + return nil if @text.blank? + + result = sanitize(@text, tags: [], attributes: []) + result.gsub!(/\n/, ' ') + result.strip! + + return result if result.length <= Summarize.max_length + "#{result[0..Summarize.max_length]}..." + end + +end diff --git a/lib/topic_view.rb b/lib/topic_view.rb index f25fe43e6bb..8883fe307ce 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -1,8 +1,8 @@ require_dependency 'guardian' require_dependency 'topic_query' +require_dependency 'summarize' class TopicView - include ActionView::Helpers attr_accessor :topic, :min, :max, :draft, :draft_key, :draft_sequence, :posts @@ -76,11 +76,7 @@ class TopicView def summary return nil if posts.blank? - first_post_content = sanitize(posts.first.cooked, tags: [], attributes: []) - first_post_content.gsub!(/\n/, ' ') - - return first_post_content if first_post_content.length <= 500 - "#{first_post_content[0..500]}..." + Summarize.new(posts.first.cooked).summary end def filter_posts(opts = {}) diff --git a/spec/components/summarize_spec.rb b/spec/components/summarize_spec.rb new file mode 100644 index 00000000000..448671efecd --- /dev/null +++ b/spec/components/summarize_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' +require 'summarize' + +describe Summarize do + + it "is blank when the input is nil" do + Summarize.new(nil).summary.should be_blank + end + + it "is blank when the input is an empty string" do + Summarize.new("").summary.should be_blank + end + + it "removes html tags" do + Summarize.new("hello robin").summary.should == "hello robin" + end + + it "strips leading and trailing space" do + Summarize.new("\t \t hello \t ").summary.should == "hello" + end + + it "trims long strings and adds an ellipsis" do + Summarize.stubs(:max_length).returns(11) + Summarize.new("discourse is a cool forum").summary.should == "discourse is..." + end + +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7a6194bf792..2a2bd279811 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -218,6 +218,7 @@ describe User do its(:email_tokens) { should be_present } its(:bio_cooked) { should be_present } + its(:bio_summary) { should be_present } its(:topics_entered) { should == 0 } its(:posts_read_count) { should == 0 } end