Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

secure the links on the topic pages, eliminated deleted topics as well.

This commit is contained in:
Sam 2013-06-05 16:10:26 +10:00
parent 913a607528
commit 870e59883b
9 changed files with 137 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/posts_controller.rb
View File

@ -90,7 +90,7 @@ class PostsController < ApplicationController
post_serializer = PostSerializer.new(post, scope: guardian, root: false)
post_serializer.draft_sequence = DraftSequence.current(current_user, post.topic.draft_key)
link_counts = TopicLinkClick.counts_for(post.topic, [post])
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?

4
app/models/category.rb
View File

@ -42,6 +42,8 @@ class Category < ActiveRecord::Base
delegate :post_template, to: 'self.class'
attr_accessor :displayable_topics
# Internal: Update category stats: # of topics in past year, month, week for
# all categories.
def self.update_stats
@ -62,8 +64,6 @@ class Category < ActiveRecord::Base
topics_week = (#{topics_week})")
end
attr_accessor :displayable_topics
# Internal: Generate the text of post prompting to enter category
# description.
def self.post_template

18
app/models/topic.rb
View File

@ -209,8 +209,6 @@ class Topic < ActiveRecord::Base
meta_data[key.to_s]
end
def self.listable_count_per_day(sinceDaysAgo=30)
listable_topics.where('created_at > ?', sinceDaysAgo.days.ago).group('date(created_at)').order('date(created_at)').count
end
@ -219,22 +217,6 @@ class Topic < ActiveRecord::Base
archetype == Archetype.private_message
end
def links_grouped
exec_sql("SELECT ftl.url,
ft.title,
ftl.link_topic_id,
ftl.reflection,
ftl.internal,
MIN(ftl.user_id) AS user_id,
SUM(clicks) AS clicks
FROM topic_links AS ftl
LEFT OUTER JOIN topics AS ft ON ftl.link_topic_id = ft.id
WHERE ftl.topic_id = ?
GROUP BY ftl.url, ft.title, ftl.link_topic_id, ftl.reflection, ftl.internal
ORDER BY clicks DESC",
id).to_a
end
# Search for similar topics
def self.similar_to(title, raw)
return [] unless title.present?

60
app/models/topic_link.rb
View File

@ -22,6 +22,66 @@ class TopicLink < ActiveRecord::Base
errors.add(:base, "can't link to the same topic") if (topic_id == link_topic_id)
end
def self.topic_summary(guardian, topic_id)
# Sam: complicated reports are really hard in AR
builder = SqlBuilder.new("SELECT ftl.url,
ft.title,
ftl.link_topic_id,
ftl.reflection,
ftl.internal,
MIN(ftl.user_id) AS user_id,
SUM(clicks) AS clicks
FROM topic_links AS ftl
LEFT JOIN topics AS ft ON ftl.link_topic_id = ft.id
LEFT JOIN categories AS c ON c.id = ft.category_id
/*where*/
GROUP BY ftl.url, ft.title, ftl.link_topic_id, ftl.reflection, ftl.internal
ORDER BY clicks DESC")
builder.where('ftl.topic_id = :topic_id', topic_id: topic_id)
builder.where('ft.deleted_at IS NULL')
builder.secure_category(guardian.secure_category_ids)
builder.exec.to_a
end
def self.counts_for(guardian,topic, posts)
return {} if posts.blank?
# Sam: I don't know how to write this cleanly in AR,
# in particular the securing logic is tricky and would fallback to SQL anyway
builder = SqlBuilder.new("SELECT
l.post_id,
l.url,
l.clicks,
t.title,
l.internal,
l.reflection
FROM topic_links l
LEFT JOIN topics t ON t.id = l.link_topic_id
LEFT JOIN categories AS c ON c.id = t.category_id
/*where*/
ORDER BY reflection ASC, clicks DESC")
builder.where('t.deleted_at IS NULL')
# not certain if pluck is right, cause it may interfere with caching
builder.where('l.post_id IN (:post_ids)', post_ids: posts.map(&:id))
builder.secure_category(guardian.secure_category_ids)
builder.map_exec(OpenStruct).each_with_object({}) do |l,result|
result[l.post_id] ||= []
result[l.post_id] << {url: l.url,
clicks: l.clicks,
title: l.title,
internal: l.internal,
reflection: l.reflection}
end
end
# Extract any urls in body
def self.extract_from(post)
return unless post.present?

19
app/models/topic_link_click.rb
View File

@ -34,25 +34,6 @@ class TopicLinkClick < ActiveRecord::Base
args[:url]
end
def self.counts_for(topic, posts)
return {} if posts.blank?
links = TopicLink
.includes(:link_topic)
.where(topic_id: topic.id, post_id: posts.map(&:id))
.order('reflection asc, clicks desc')
result = {}
links.each do |l|
result[l.post_id] ||= []
result[l.post_id] << {url: l.url,
clicks: l.clicks,
title: l.link_topic.try(:title),
internal: l.internal,
reflection: l.reflection}
end
result
end
end
# == Schema Information

9
lib/sql_builder.rb
View File

@ -16,6 +16,15 @@ class SqlBuilder
end
end
def secure_category(secure_category_ids, category_alias = 'c')
if secure_category_ids.present?
where("NOT COALESCE(" << category_alias << ".secure, false) OR " << category_alias << ".id IN (:secure_category_ids)", secure_category_ids: secure_category_ids)
else
where("NOT COALESCE(" << category_alias << ".secure, false)")
end
self
end
def to_sql
sql = @sql.dup

12
lib/topic_view.rb
View File

@ -4,20 +4,22 @@ require_dependency 'summarize'
class TopicView
attr_reader :topic, :posts, :index_offset, :index_reverse
attr_reader :topic, :posts, :index_offset, :index_reverse, :guardian
attr_accessor :draft, :draft_key, :draft_sequence
def initialize(topic_id, user=nil, options={})
@topic = find_topic(topic_id)
raise Discourse::NotFound if @topic.blank?
@guardian = Guardian.new(user)
# Special case: If the topic is private and the user isn't logged in, ask them
# to log in!
if @topic.present? && @topic.private_message? && user.blank?
raise Discourse::NotLoggedIn.new
end
Guardian.new(user).ensure_can_see!(@topic)
guardian.ensure_can_see!(@topic)
@post_number, @page = options[:post_number], options[:page]
@limit = options[:limit] || SiteSetting.posts_per_page;
@ -225,18 +227,18 @@ class TopicView
@post_action_visibility ||= begin
result = []
PostActionType.types.each do |k, v|
result << v if Guardian.new(@user).can_see_post_actors?(@topic, v)
result << v if guardian.can_see_post_actors?(@topic, v)
end
result
end
end
def links
@links ||= @topic.links_grouped
@links ||= TopicLink.topic_summary(guardian, @topic.id)
end
def link_counts
@link_counts ||= TopicLinkClick.counts_for(@topic, posts)
@link_counts ||= TopicLink.counts_for(guardian,@topic, posts)
end
# Are we the initial page load? If so, we can return extra information like

25
spec/models/topic_link_click_spec.rb
View File

@ -12,10 +12,6 @@ describe TopicLinkClick do
URI.parse('http://test.host')
end
it 'returns blank from counts_for without posts' do
TopicLinkClick.counts_for(nil, nil).should be_blank
end
context 'topic_links' do
before do
@topic = Fabricate(:topic)
@ -46,25 +42,6 @@ describe TopicLinkClick do
TopicLinkClick.first.ip.to_s.should == '192.168.1.1'
end
context 'counts for' do
before do
@counts_for = TopicLinkClick.counts_for(@topic, [@post])
end
it 'has a counts_for result' do
@counts_for[@post.id].should be_present
end
it 'contains the click we made' do
@counts_for[@post.id].first[:clicks].should == 1
end
it 'has no clicks on another url in the post' do
@counts_for[@post.id].find {|l| l[:url] == 'http://google.com'}[:clicks].should == 0
end
end
end
context 'create_from' do
@ -82,10 +59,8 @@ describe TopicLinkClick do
}.should_not change(TopicLinkClick, :count)
end
end
context 'with a valid url and post_id' do
before do
TopicLinkClick.create_from(url: @topic_link.url, post_id: @post.id, ip: '127.0.0.1')

58
spec/models/topic_link_spec.rb
View File

@ -216,4 +216,62 @@ describe TopicLink do
end
end
describe 'counts_for and topic_summary' do
it 'returns blank without posts' do
TopicLink.counts_for(Guardian.new, nil, nil).should be_blank
end
context 'with data' do
let(:post) do
topic = Fabricate(:topic)
Fabricate(:post_with_external_links, user: topic.user, topic: topic)
end
let(:counts_for) do
TopicLink.counts_for(Guardian.new, post.topic, [post])
end
it 'has the correct results' do
TopicLink.extract_from(post)
topic_link = post.topic.topic_links.first
TopicLinkClick.create(topic_link: topic_link, ip: '192.168.1.1')
counts_for[post.id].should be_present
counts_for[post.id].find {|l| l[:url] == 'http://google.com'}[:clicks].should == 0
counts_for[post.id].first[:clicks].should == 1
array = TopicLink.topic_summary(Guardian.new, post.topic_id)
array.length.should == 4
array[0]["clicks"].should == "1"
end
it 'secures internal links correctly' do
category = Fabricate(:category)
secret_topic = Fabricate(:topic, category: category)
url = "http://#{test_uri.host}/t/topic-slug/#{secret_topic.id}"
post = Fabricate(:post, raw: "hello test topic #{url}")
TopicLink.extract_from(post)
TopicLink.topic_summary(Guardian.new, post.topic_id).count.should == 1
TopicLink.counts_for(Guardian.new, post.topic, [post]).length.should == 1
category.deny(:all)
category.allow(Group[:staff])
category.save
admin = Fabricate(:admin)
TopicLink.topic_summary(Guardian.new, post.topic_id).count.should == 0
TopicLink.topic_summary(Guardian.new(admin), post.topic_id).count.should == 1
TopicLink.counts_for(Guardian.new, post.topic, [post]).length.should == 0
TopicLink.counts_for(Guardian.new(admin), post.topic, [post]).length.should == 1
end
end
end
end