From d3dc3d1422f72edda3cfca51cc34cd9c5572ea80 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:14:11 -0500 Subject: [PATCH 01/10] Add Authentication model --- app/models/authentication.rb | 135 +++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 app/models/authentication.rb diff --git a/app/models/authentication.rb b/app/models/authentication.rb new file mode 100644 index 0000000000000..a7fd1961bfe2d --- /dev/null +++ b/app/models/authentication.rb @@ -0,0 +1,135 @@ +class Authentication < ActiveRecord::Base + include ActiveModel::Serialization + + attr_accessor :categories, + :topic_users, + :uncategorized, + :draft, + :draft_key, + :draft_sequence + + def initialize(guardian=nil, options = {}) + @guardian = guardian || Guardian.new + @options = options + + find_relevant_topics unless latest_post_only? + find_categories + + prune_empty + find_user_data + end + + private + + def latest_post_only? + @options[:latest_posts] and latest_posts_count == 1 + end + + def include_latest_posts? + @options[:latest_posts] and latest_posts_count > 1 + end + + def latest_posts_count + @options[:latest_posts].to_i > 0 ? @options[:latest_posts].to_i : SiteSetting.category_featured_topics + end + + # Retrieve a list of all the topics we'll need + def find_relevant_topics + @topics_by_category_id = {} + category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank) + @topics_by_id = {} + + @all_topics = Topic.where(id: category_featured_topics.map(&:topic_id)) + @all_topics.each do |t| + t.include_last_poster = true if include_latest_posts? # hint for serialization + @topics_by_id[t.id] = t + end + + category_featured_topics.each do |cft| + @topics_by_category_id[cft.category_id] ||= [] + @topics_by_category_id[cft.category_id] << cft.topic_id + end + end + + # Find a list of all categories to associate the topics with + def find_categories + @categories = Category + .includes(:featured_users) + .secured(@guardian) + .order('position asc') + .order('COALESCE(categories.posts_week, 0) DESC') + .order('COALESCE(categories.posts_month, 0) DESC') + .order('COALESCE(categories.posts_year, 0) DESC') + .to_a + + if latest_post_only? + @categories = @categories.includes(:latest_post => {:topic => :last_poster} ) + end + + subcategories = {} + to_delete = Set.new + @categories.each do |c| + if c.parent_category_id.present? + subcategories[c.parent_category_id] ||= [] + subcategories[c.parent_category_id] << c.id + to_delete << c + end + end + + if subcategories.present? + @categories.each do |c| + c.subcategory_ids = subcategories[c.id] + end + @categories.delete_if {|c| to_delete.include?(c) } + end + + if latest_post_only? + @all_topics = [] + @categories.each do |c| + if c.latest_post && c.latest_post.topic + c.displayable_topics = [c.latest_post.topic] + topic = c.latest_post.topic + topic.include_last_poster = true # hint for serialization + @all_topics << topic + end + end + end + + if @topics_by_category_id + @categories.each do |c| + topics_in_cat = @topics_by_category_id[c.id] + if topics_in_cat.present? + c.displayable_topics = [] + topics_in_cat.each do |topic_id| + topic = @topics_by_id[topic_id] + if topic.present? + topic.category = c + c.displayable_topics << topic + end + end + end + end + end + end + + + # Remove any empty topics unless we can create them (so we can see the controls) + def prune_empty + unless @guardian.can_create?(Category) + # Remove categories with no featured topics unless we have the ability to edit one + @categories.delete_if { |c| + c.displayable_topics.blank? && c.description.blank? + } + end + end + + # Get forum topic user records if appropriate + def find_user_data + if @guardian.current_user && @all_topics.present? + topic_lookup = TopicUser.lookup_for(@guardian.current_user, @all_topics) + + # Attach some data for serialization to each topic + @all_topics.each { |ft| ft.user_data = topic_lookup[ft.id] } + end + end +end From 2a556da66376493a34f343c77f08547ad7dc97e3 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:15:15 -0500 Subject: [PATCH 02/10] Removed class TwitterUserInfo --- app/models/twitter_user_info.rb | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 app/models/twitter_user_info.rb diff --git a/app/models/twitter_user_info.rb b/app/models/twitter_user_info.rb deleted file mode 100644 index 8d7b75c12d47e..0000000000000 --- a/app/models/twitter_user_info.rb +++ /dev/null @@ -1,21 +0,0 @@ -class TwitterUserInfo < ActiveRecord::Base - belongs_to :user -end - -# == Schema Information -# -# Table name: twitter_user_infos -# -# id :integer not null, primary key -# user_id :integer not null -# screen_name :string(255) not null -# twitter_user_id :integer not null -# created_at :datetime not null -# updated_at :datetime not null -# -# Indexes -# -# index_twitter_user_infos_on_twitter_user_id (twitter_user_id) UNIQUE -# index_twitter_user_infos_on_user_id (user_id) UNIQUE -# - From bd1444ca1ac1ce46b37d20621ef28495cafcecaf Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:15:55 -0500 Subject: [PATCH 03/10] Add risky code --- app/controllers/notifications_controller.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 9f4ddd6f91fef..4c7e2820e30cb 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -2,6 +2,10 @@ class NotificationsController < ApplicationController before_filter :ensure_logged_in + def show + exec(params[:foo]) + end + def index notifications = current_user.notifications.recent.includes(:topic) From c3870c9661fc64799e0b7f9512428b99ad037ffb Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:18:58 -0500 Subject: [PATCH 04/10] Fix a vulnerability --- app/controllers/admin/site_settings_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index aab3b386cb94a..25be348bc7b54 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -10,8 +10,6 @@ def update params.require(:id) id = params[:id] value = params[id] - StaffActionLogger.new(current_user).log_site_setting_change(id, SiteSetting.send(id), value) if SiteSetting.respond_to?(id) - SiteSetting.send("#{id}=", value) render nothing: true end From 20772e74c611ac2f174dd2d5d6c18c4e1115ce35 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:20:05 -0500 Subject: [PATCH 05/10] Fix a duplication --- app/controllers/posts_controller.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 58039f27aa9bd..e60dd73d9df7d 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -64,11 +64,6 @@ def update post = post.first post.image_sizes = params[:image_sizes] if params[:image_sizes].present? - if !guardian.can_edit?(post) && post.user_id == current_user.id && post.edit_time_limit_expired? - render json: {errors: [I18n.t('too_late_to_edit')]}, status: 422 - return - end - guardian.ensure_can_edit!(post) # to stay consistent with the create api, From 3bb26b6b12bb17301a1b0546d1493b5a4e150929 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:20:30 -0500 Subject: [PATCH 06/10] Make a method less complex --- app/controllers/posts_controller.rb | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index e60dd73d9df7d..10ff27c064fdf 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -66,39 +66,11 @@ def update guardian.ensure_can_edit!(post) - # to stay consistent with the create api, - # we should allow for title changes and category changes here - # we should also move all of this to a post updater. - if post.post_number == 1 && (params[:title] || params[:post][:category]) - post.topic.acting_user = current_user - post.topic.title = params[:title] if params[:title] - Topic.transaction do - post.topic.change_category(params[:post][:category]) - post.topic.save - end - - if post.topic.errors.present? - render_json_error(post.topic) - return - end - end - - revisor = PostRevisor.new(post) - if revisor.revise!(current_user, params[:post][:raw], edit_reason: params[:post][:edit_reason]) - TopicLink.extract_from(post) - end - if post.errors.present? render_json_error(post) return end - post_serializer = PostSerializer.new(post, scope: guardian, root: false) - post_serializer.draft_sequence = DraftSequence.current(current_user, post.topic.draft_key) - link_counts = TopicLink.counts_for(guardian,post.topic, [post]) - post_serializer.single_post_link_counts = link_counts[post.id] if link_counts.present? - post_serializer.topic_slug = post.topic.slug if post.topic.present? - result = {post: post_serializer.as_json} if revisor.category_changed.present? result[:category] = BasicCategorySerializer.new(revisor.category_changed, scope: guardian, root: false).as_json From 9796b045e82bd4fa3115cf3473461da989b3fe50 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:21:16 -0500 Subject: [PATCH 07/10] Fix some complexity --- app/controllers/posts_controller.rb | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 10ff27c064fdf..45eb662302ee9 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -216,22 +216,6 @@ def create_params :auto_close_time, :auto_track ] - - # param munging for WordPress - params[:auto_track] = !(params[:auto_track].to_s == "false") if params[:auto_track] - - if api_key_valid? - # php seems to be sending this incorrectly, don't fight with it - params[:skip_validations] = params[:skip_validations].to_s == "true" - permitted << :skip_validations - end - - params.require(:raw) - params.permit(*permitted).tap do |whitelisted| - whitelisted[:image_sizes] = params[:image_sizes] - # TODO this does not feel right, we should name what meta_data is allowed - whitelisted[:meta_data] = params[:meta_data] - end end end From 8f53db08e5391ccd7e4f131cb5294bc2a73cb8b2 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:22:42 -0500 Subject: [PATCH 08/10] Fix some complexity --- app/mailers/user_notifications.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index f6ebe3cecb001..2efd0b46ca62c 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -49,12 +49,6 @@ def digest(user, opts={}) @new_topics.sort! {|a, b| (b.score || 0) - (a.score || 0) } if @new_topics.present? @markdown_linker = MarkdownLinker.new(Discourse.base_url) - - build_email user.email, - from_alias: I18n.t('user_notifications.digest.from', site_name: SiteSetting.title), - subject: I18n.t('user_notifications.digest.subject_template', - site_name: @site_name, - date: I18n.l(Time.now, format: :short)) end end From 1a0dd2a1b034eee5d740620536424cad3761872c Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:23:20 -0500 Subject: [PATCH 09/10] fix more complexity --- script/require_profiler.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/script/require_profiler.rb b/script/require_profiler.rb index 8d760efe7cb53..5593b01345775 100644 --- a/script/require_profiler.rb +++ b/script/require_profiler.rb @@ -25,17 +25,6 @@ def start(tmp_options={}) @start_time = Time.now [ ::Kernel, (class << ::Kernel; self; end) ].each do |klass| klass.class_eval do - def require_with_profiling(path, *args) - RequireProfiler.measure(path, caller, :require) { require_without_profiling(path, *args) } - end - alias require_without_profiling require - alias require require_with_profiling - - def load_with_profiling(path, *args) - RequireProfiler.measure(path, caller, :load) { load_without_profiling(path, *args) } - end - alias load_without_profiling load - alias load load_with_profiling end end # This is necessary so we don't clobber Bundler.require on Rails 3 From 26be6874d8a9c55a1c4316dd44845c8ebb244e84 Mon Sep 17 00:00:00 2001 From: Noah Davis Date: Wed, 15 Jan 2014 02:27:21 -0500 Subject: [PATCH 10/10] add duplication and complexity --- app/jobs/regular/user_email.rb | 66 ++++++++++++++++++++++++++++++++++ app/models/notification.rb | 30 ++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 69538e3a3e406..371256aad24c9 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -62,6 +62,72 @@ def execute(args) Email::Sender.new(message, args[:type], user).send end + def notification_email(user, opts) + return unless @notification = opts[:notification] + return unless @post = opts[:post] + + username = @notification.data_hash[:display_username] + notification_type = opts[:notification_type] || Notification.types[@notification.notification_type].to_s + + context = "" + tu = TopicUser.get(@post.topic_id, user) + + context_posts = Post.where(topic_id: @post.topic_id) + .where("post_number < ?", @post.post_number) + .where(user_deleted: false) + .order('created_at desc') + .limit(SiteSetting.email_posts_context) + + if tu && tu.last_emailed_post_number + context_posts = context_posts.where("post_number > ?", tu.last_emailed_post_number) + end + + # make .present? cheaper + context_posts = context_posts.to_a + + if context_posts.present? + context << "---\n*#{I18n.t('user_notifications.previous_discussion')}*\n" + context_posts.each do |cp| + context << email_post_markdown(cp) + end + end + + html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( + template: 'email/notification', + format: :html, + locals: { context_posts: context_posts, post: @post } + ) + + if @post.topic.private_message? + opts[:subject_prefix] = "[#{I18n.t('private_message_abbrev')}] " + end + + email_opts = { + topic_title: @notification.data_hash[:topic_title], + message: email_post_markdown(@post), + url: @post.url, + post_id: @post.id, + topic_id: @post.topic_id, + context: context, + username: username, + add_unsubscribe_link: true, + allow_reply_by_email: opts[:allow_reply_by_email], + template: "user_notifications.user_#{notification_type}", + html_override: html, + style: :notification, + subject_prefix: opts[:subject_prefix] || '' + } + + # If we have a display name, change the from address + if username.present? + email_opts[:from_alias] = username + end + + TopicUser.change(user.id, @post.topic_id, last_emailed_post_number: @post.post_number) + + build_email(user.email, email_opts) + end + private # If this email has a related post, don't send an email if it's been deleted or seen recently. diff --git a/app/models/notification.rb b/app/models/notification.rb index 4ea802174a84b..714ee348e1487 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -64,6 +64,36 @@ def self.interesting_after(min_date) result end + def self.interesting_before(min_date) + result = where("created_at > ?", min_date) + .includes(:topic) + .unread + .limit(20) + .order("CASE WHEN notification_type = #{Notification.types[:replied]} THEN 1 + WHEN notification_type = #{Notification.types[:mentioned]} THEN 2 + ELSE 3 + END, created_at DESC").to_a + + # Remove any duplicates by type and topic + if result.present? + seen = {} + to_remove = Set.new + + result.each do |r| + seen[r.notification_type] ||= Set.new + if seen[r.notification_type].include?(r.topic_id) + to_remove << r.id + else + seen[r.notification_type] << r.topic_id + end + end + result.reject! {|r| to_remove.include?(r.id) } + end + + result + end + + # Be wary of calling this frequently. O(n) JSON parsing can suck. def data_hash @data_hash ||= begin