defer view creation on so updates are not performed when people navigate to topics

This commit is contained in:
Sam 2013-10-04 17:00:23 +10:00
parent 5bf26ec34e
commit e18b93026a
6 changed files with 49 additions and 20 deletions

View File

@ -47,8 +47,12 @@ class TopicsController < ApplicationController
# (see http://meta.discourse.org/t/noscript-tag-and-some-search-engines/8078) # (see http://meta.discourse.org/t/noscript-tag-and-some-search-engines/8078)
return render 'topics/plain', layout: false if (SiteSetting.enable_escaped_fragments && params.has_key?('_escaped_fragment_')) return render 'topics/plain', layout: false if (SiteSetting.enable_escaped_fragments && params.has_key?('_escaped_fragment_'))
View.create_for(@topic_view.topic, request.remote_ip, current_user)
track_visit_to_topic track_visit_to_topic
if should_track_visit_to_topic?
@topic_view.draft = Draft.get(current_user, @topic_view.draft_key, @topic_view.draft_sequence)
end
perform_show_response perform_show_response
end end
@ -301,9 +305,13 @@ class TopicsController < ApplicationController
end end
def track_visit_to_topic def track_visit_to_topic
return unless should_track_visit_to_topic? Jobs.enqueue(:view_tracker,
TopicUser.track_visit! @topic_view.topic, current_user topic_id: @topic_view.topic.id,
@topic_view.draft = Draft.get(current_user, @topic_view.draft_key, @topic_view.draft_sequence) ip: request.remote_ip,
user_id: (current_user.id if current_user),
track_visit: should_track_visit_to_topic?
)
end end
def should_track_visit_to_topic? def should_track_visit_to_topic?

View File

@ -0,0 +1,17 @@
module Jobs
# Asynchronously send an email to a user
class ViewTracker < Jobs::Base
def execute(args)
topic_id = args[:topic_id]
user_id = args[:user_id]
ip = args[:ip]
track_visit = args[:track_visit]
View.create_for_parent(Topic, topic_id, ip, user_id)
if track_visit
TopicUser.track_visit! topic_id, user_id
end
end
end
end

View File

@ -108,12 +108,15 @@ class TopicUser < ActiveRecord::Base
end end
def track_visit!(topic,user) def track_visit!(topic,user)
topic_id = Topic === topic ? topic.id : topic
user_id = User === user ? user.id : topic
now = DateTime.now now = DateTime.now
rows = TopicUser.where({topic_id: topic.id, user_id: user.id}).update_all({last_visited_at: now}) rows = TopicUser.where({topic_id: topic_id, user_id: user_id}).update_all({last_visited_at: now})
if rows == 0 if rows == 0
TopicUser.create(topic_id: topic.id, user_id: user.id, last_visited_at: now, first_visited_at: now) TopicUser.create(topic_id: topic_id, user_id: user_id, last_visited_at: now, first_visited_at: now)
else else
observe_after_save_callbacks_for topic.id, user.id observe_after_save_callbacks_for topic_id, user_id
end end
end end

View File

@ -5,13 +5,11 @@ class View < ActiveRecord::Base
belongs_to :user belongs_to :user
validates_presence_of :parent_type, :parent_id, :ip_address, :viewed_at validates_presence_of :parent_type, :parent_id, :ip_address, :viewed_at
# TODO: This could happen asyncronously def self.create_for_parent(parent_class, parent_id, ip, user_id)
def self.create_for(parent, ip, user=nil)
# Only store a view once per day per thing per user per ip # Only store a view once per day per thing per user per ip
redis_key = "view:#{parent.class.name}:#{parent.id}:#{Date.today.to_s}" redis_key = "view:#{parent_class.name}:#{parent_id}:#{Date.today.to_s}"
if user.present? if user_id
redis_key << ":user-#{user.id}" redis_key << ":user-#{user_id}"
else else
redis_key << ":ip-#{ip}" redis_key << ":ip-#{ip}"
end end
@ -20,15 +18,20 @@ class View < ActiveRecord::Base
$redis.expire(redis_key, 1.day.to_i) $redis.expire(redis_key, 1.day.to_i)
View.transaction do View.transaction do
View.create(parent: parent, ip_address: ip, viewed_at: Date.today, user: user) View.create!(parent_id: parent_id, parent_type: parent_class.to_s, ip_address: ip, viewed_at: Date.today, user_id: user_id)
# Update the views count in the parent, if it exists. # Update the views count in the parent, if it exists.
if parent.respond_to?(:views) if parent_class.columns_hash["views"]
parent.class.where(id: parent.id).update_all 'views = views + 1' parent_class.where(id: parent_id).update_all 'views = views + 1'
end end
end end
end end
end end
def self.create_for(parent, ip, user=nil)
user_id = user.id if user
create_for_parent(parent.class, parent.id, ip, user_id)
end
end end
# == Schema Information # == Schema Information

View File

@ -427,7 +427,7 @@ class Guardian
def is_my_own?(obj) def is_my_own?(obj)
unless anonymous? unless anonymous?
return obj.user_id == @user.id if obj.respond_to?(:user_id) return obj.user_id == @user.id if obj.respond_to?(:user_id) && obj.user_id && @user.id
return obj.user == @user if obj.respond_to?(:user) return obj.user == @user if obj.respond_to?(:user)
end end

View File

@ -2,8 +2,6 @@ require 'spec_helper'
describe TopicsController do describe TopicsController do
context 'wordpress' do context 'wordpress' do
let!(:user) { log_in(:moderator) } let!(:user) { log_in(:moderator) }
let(:p1) { Fabricate(:post, user: user) } let(:p1) { Fabricate(:post, user: user) }
@ -527,7 +525,7 @@ describe TopicsController do
it 'tracks a visit for all html requests' do it 'tracks a visit for all html requests' do
current_user = log_in(:coding_horror) current_user = log_in(:coding_horror)
TopicUser.expects(:track_visit!).with(topic, current_user) TopicUser.expects(:track_visit!).with(topic.id, current_user.id)
get :show, topic_id: topic.id, slug: topic.slug get :show, topic_id: topic.id, slug: topic.slug
end end