Refactor HomeController

Este commit está contenido en:
Andrey Chernih 2014-07-07 20:05:53 +04:00
padre 9e849de0f7
commit fc52db5424
Se han modificado 8 ficheros con 583 adiciones y 162 borrados

1
.rspec
Ver fichero

@ -1 +1,2 @@
--no-colour
--require spec_helper

Ver fichero

@ -1,14 +1,7 @@
class HomeController < ApplicationController
STORIES_PER_PAGE = 25
# how many points a story has to have to probably get on the front page
HOT_STORY_POINTS = 5
# how many days old a story can be to get on the bottom half of /recent
RECENT_DAYS_OLD = 3
# for rss feeds, load the user's tag filters if a token is passed
before_filter :find_user_from_rss_token, :only => [ :index, :newest ]
before_filter { @page = page }
def about
begin
@ -20,8 +13,18 @@ class HomeController < ApplicationController
end
end
def privacy
begin
render :action => "privacy"
rescue
render :text => "<div class=\"box wide\">" <<
"You apparently have no privacy." <<
"</div>", :layout => "application"
end
end
def hidden
@stories = find_stories({ :hidden => true })
@stories, @show_more = get_from_cache(hidden: true) { paginate stories.hidden }
@heading = @title = "Hidden Stories"
@cur_url = "/hidden"
@ -30,7 +33,7 @@ class HomeController < ApplicationController
end
def index
@stories = find_stories({ :hottest => true })
@stories, @show_more = get_from_cache(hottest: true) { paginate stories.hottest }
@rss_link ||= { :title => "RSS 2.0",
:href => "/rss#{@user ? "?token=#{@user.rss_token}" : ""}" }
@ -52,7 +55,7 @@ class HomeController < ApplicationController
end
def newest
@stories = find_stories({ :newest => true })
@stories, @show_more = get_from_cache(newest: true) { paginate stories.newest }
@heading = @title = "Newest Stories"
@cur_url = "/newest"
@ -76,7 +79,7 @@ class HomeController < ApplicationController
def newest_by_user
by_user = User.where(:username => params[:user]).first!
@stories = find_stories({ :by_user => by_user })
@stories, @show_more = get_from_cache(by_user: by_user) { paginate stories.by_user(by_user) }
@heading = @title = "Newest Stories by #{by_user.username}"
@cur_url = "/newest/#{by_user.username}"
@ -87,18 +90,15 @@ class HomeController < ApplicationController
render :action => "index"
end
def privacy
begin
render :action => "privacy"
rescue
render :text => "<div class=\"box wide\">" <<
"You apparently have no privacy." <<
"</div>", :layout => "application"
end
end
def recent
@stories = find_stories({ :recent => true })
@stories, @show_more = get_from_cache(recent: true) do
scope = if page == 1
stories.recent
else
stories.newest
end
paginate scope
end
@heading = @title = "Recent Stories"
@cur_url = "/recent"
@ -113,7 +113,7 @@ class HomeController < ApplicationController
def tagged
@tag = Tag.where(:tag => params[:tag]).first!
@stories = find_stories({ :tag => @tag })
@stories, @show_more = get_from_cache(tag: @tag) { paginate stories.tagged(@tag) }
@heading = @title = @tag.description.blank?? @tag.tag : @tag.description
@cur_url = tag_url(@tag.tag)
@ -139,7 +139,7 @@ class HomeController < ApplicationController
@cur_url << "/#{params[:length]}"
end
@stories = find_stories({ :top => true, :length => length })
@stories, @show_more = get_from_cache(top: true, length: length) { paginate stories.top(length) }
if length[:dur] > 1
@heading = @title = "Top Stories of the Past #{length[:dur]} " <<
@ -152,152 +152,33 @@ class HomeController < ApplicationController
end
private
def find_stories(how = {})
@page = how[:page] = 1
if params[:page].to_i > 0
@page = how[:page] = params[:page].to_i
end
# guest views have caching, but don't bother for logged-in users or dev or
# when the user has tag filters
if Rails.env.development? || @user || tags_filtered_by_cookie.any?
stories, @show_more = _find_stories(how)
def filtered_tag_ids
if @user
@user.tag_filters.map{|tf| tf.tag_id }
else
stories, @show_more = Rails.cache.fetch("stories " <<
how.sort.map{|k,v| "#{k}=#{v.to_param}" }.join(" "),
:expires_in => 45) do
_find_stories(how)
end
tags_filtered_by_cookie.map{|t| t.id }
end
stories
end
def _find_stories(how)
stories = Story.unmerged.where(:is_expired => false)
def stories
StoryRepository.new(@user, exclude_tags: filtered_tag_ids)
end
if @user
hidden_arel = Vote.arel_table.where(
Vote.arel_table[:user_id].eq(@user.id)
).where(
Vote.arel_table[:vote].lteq(0)
).where(
Vote.arel_table[:comment_id].eq(nil)
).project(
Vote.arel_table[:story_id]
)
def page
params[:page].to_i > 0 ? params[:page].to_i : 1
end
if how[:hidden]
stories = stories.where(Story.arel_table[:id].in(hidden_arel))
elsif !how[:by_user]
stories = stories.where(Story.arel_table[:id].not_in(hidden_arel))
end
end
def paginate(scope)
StoriesPaginator.new(scope, page, @user).get
end
if how[:tag] || how[:hottest]
stories = stories.where("(CAST(upvotes AS integer) - " <<
"CAST(downvotes AS integer)) >= -2")
end
if how[:tag]
stories = stories.where(
Story.arel_table[:id].in(
Tagging.arel_table.where(
Tagging.arel_table[:tag_id].eq(how[:tag].id)
).project(
Tagging.arel_table[:story_id]
)
)
)
elsif how[:by_user]
stories = stories.where(:user_id => how[:by_user].id)
def get_from_cache(opts={}, &block)
if Rails.env.development? || @user || tags_filtered_by_cookie.any?
yield
else
filtered_tag_ids = []
if @user
filtered_tag_ids = @user.tag_filters.map{|tf| tf.tag_id }
else
filtered_tag_ids = tags_filtered_by_cookie.map{|t| t.id }
end
if filtered_tag_ids.any?
stories = stories.where(
Story.arel_table[:id].not_in(
Tagging.arel_table.where(
Tagging.arel_table[:tag_id].in(filtered_tag_ids)
).project(
Tagging.arel_table[:story_id]
)
)
)
end
key = opts.merge(page: page).sort.map{|k,v| "#{k}=#{v.to_param}" }.join(" ")
Rails.cache.fetch("stories #{key}", :expires_in => 45, &block)
end
if how[:recent] && how[:page] == 1
# try to help recently-submitted stories that didn't gain traction
story_ids = []
10.times do |x|
# grab the list of stories from the past n days, shifting out popular
# stories that did gain traction
story_ids = stories.select(:id, :upvotes, :downvotes).
where(Story.arel_table[:created_at].gt((RECENT_DAYS_OLD + x).days.ago)).
order("stories.created_at DESC").
reject{|s| s.score > HOT_STORY_POINTS }
if story_ids.length > STORIES_PER_PAGE + 1
# keep the top half (newest stories)
keep_ids = story_ids[0 .. ((STORIES_PER_PAGE + 1) * 0.5)]
story_ids = story_ids[keep_ids.length - 1 ... story_ids.length]
# make the bottom half a random selection of older stories
while keep_ids.length <= STORIES_PER_PAGE + 1
story_ids.shuffle!
keep_ids.push story_ids.shift
end
stories = Story.where(:id => keep_ids)
break
end
end
elsif how[:top] && how[:length]
stories = stories.where("created_at >= (NOW() - INTERVAL " <<
"#{how[:length][:dur]} #{how[:length][:intv].upcase})")
end
order = "hotness"
if how[:newest] || how[:recent] || how[:tag]
order = "stories.created_at DESC"
elsif how[:top]
order = "(CAST(upvotes AS integer) - CAST(downvotes AS integer)) DESC"
end
stories = stories.includes(
:user, :taggings => :tag
).limit(
STORIES_PER_PAGE + 1
).offset(
(how[:page] - 1) * STORIES_PER_PAGE
).order(
order
).to_a
show_more = false
if stories.count > STORIES_PER_PAGE
show_more = true
stories.pop
end
if @user
votes = Vote.votes_by_user_for_stories_hash(@user.id, stories.map(&:id))
stories.each do |s|
if votes[s.id]
s.vote = votes[s.id]
end
end
end
[ stories, show_more ]
end
end

Ver fichero

@ -0,0 +1,38 @@
class StoriesPaginator
STORIES_PER_PAGE = 25
def initialize(scope, page, user)
@scope = scope
@page = page
@user = user
end
def get
with_pagination_info @scope.limit(STORIES_PER_PAGE + 1)
.offset((@page - 1) * STORIES_PER_PAGE)
.includes(:user, :taggings => :tag)
end
private
def with_pagination_info(scope)
scope = scope.to_a
show_more = scope.count > STORIES_PER_PAGE
scope.pop if show_more
[cache_votes(scope), show_more]
end
def cache_votes(scope)
if @user
votes = Vote.votes_by_user_for_stories_hash(@user.id, scope.map(&:id))
scope.each do |s|
if votes[s.id]
s.vote = votes[s.id]
end
end
end
scope
end
end

131
app/models/story_repository.rb Archivo normal
Ver fichero

@ -0,0 +1,131 @@
class StoryRepository
# how many days old a story can be to get on the bottom half of /recent
RECENT_DAYS_OLD = 3
# how many points a story has to have to probably get on the front page
HOT_STORY_POINTS = 5
def initialize(user, params = {})
@user = user
@params = params
end
def hottest
hottest = positive_ranked base_scope
hottest = filter_downvoted_and_tags hottest
hottest.order('hotness')
end
def hidden
hidden = base_scope
hidden = hidden.where(Story.arel_table[:id].in(hidden_arel)) if @user
hidden = filter_tags hidden, @params[:exclude_tags] if @params[:exclude_tags].try(:any?)
hidden.order('hotness')
end
def newest
newest = filter_downvoted_and_tags base_scope
newest.order('stories.created_at DESC')
end
def by_user(user)
base_scope.where(user_id: user.id)
end
def recent
stories = newest
# try to help recently-submitted stories that didn't gain traction
story_ids = []
10.times do |x|
# grab the list of stories from the past n days, shifting out popular
# stories that did gain traction
story_ids = stories.select(:id, :upvotes, :downvotes, :user_id).
where(Story.arel_table[:created_at].gt((RECENT_DAYS_OLD + x).days.ago)).
order("stories.created_at DESC").
reject{|s| s.score > HOT_STORY_POINTS }
if story_ids.length > StoriesPaginator::STORIES_PER_PAGE + 1
# keep the top half (newest stories)
keep_ids = story_ids[0 .. ((StoriesPaginator::STORIES_PER_PAGE + 1) * 0.5)]
story_ids = story_ids[keep_ids.length - 1 ... story_ids.length]
# make the bottom half a random selection of older stories
while keep_ids.length <= StoriesPaginator::STORIES_PER_PAGE + 1
story_ids.shuffle!
keep_ids.push story_ids.shift
end
stories = Story.where(:id => keep_ids)
break
end
end
stories.order('stories.created_at DESC')
end
def tagged(tag)
tagged = positive_ranked base_scope
tagged = tagged.where(
Story.arel_table[:id].in(
Tagging.arel_table.where(
Tagging.arel_table[:tag_id].eq(tag.id)
).project(
Tagging.arel_table[:story_id]
)
)
)
tagged.order('stories.created_at DESC')
end
def top(length)
top = base_scope.where("created_at >= (NOW() - INTERVAL #{length[:dur]} #{length[:intv].upcase})")
top.order '(CAST(upvotes AS integer) - CAST(downvotes AS integer)) DESC'
end
private
def base_scope
Story.unmerged.where(is_expired: false)
end
def filter_downvoted_and_tags(scope)
scope = filter_downvoted scope if @user
scope = filter_tags scope, @params[:exclude_tags] if @params[:exclude_tags].try(:any?)
scope
end
def filter_downvoted(scope)
scope.where(Story.arel_table[:id].not_in(hidden_arel))
end
def hidden_arel
if @user
hidden_arel = Vote.arel_table.where(
Vote.arel_table[:user_id].eq(@user.id)
).where(
Vote.arel_table[:vote].lteq(0)
).where(
Vote.arel_table[:comment_id].eq(nil)
).project(
Vote.arel_table[:story_id]
)
end
end
def positive_ranked(scope)
scope.where("(CAST(upvotes AS integer) - CAST(downvotes AS integer)) >= -2")
end
def filter_tags(scope, tags)
scope.where(
Story.arel_table[:id].not_in(
Tagging.arel_table.where(
Tagging.arel_table[:tag_id].in(tags)
).project(
Tagging.arel_table[:story_id]
)
)
)
end
end

Ver fichero

@ -0,0 +1,130 @@
describe HomeController do
before { Rails.cache.clear }
before { StoriesPaginator.any_instance.should_receive(:get).and_return [scope, true] }
describe 'GET index' do
let(:scope) { double 'Hottest Scope' }
before { StoryRepository.any_instance.should_receive(:hottest) }
before { get :index }
context 'assigns' do
describe 'rss_link' do
subject { assigns(:rss_link) }
its([:title]) { should eq 'RSS 2.0' }
its([:href]) { should include '/rss' }
end
describe 'page' do
subject { assigns(:page) }
it { should eq 1 }
end
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
describe 'show_more' do
subject { assigns(:show_more) }
it { should eq true }
end
end
end
describe 'GET index' do
let(:scope) { double 'Hidden Scope' }
before { StoryRepository.any_instance.should_receive(:hidden) }
before { get :hidden }
context 'assigns' do
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
end
end
describe 'GET newest' do
let(:scope) { double 'Newest Scope' }
before { StoryRepository.any_instance.should_receive(:newest) }
before { get :newest }
context 'assigns' do
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
end
end
describe 'GET newest_by_user' do
let(:scope) { double 'Newest By User Scope' }
let(:user) { User.make! }
before { StoryRepository.any_instance.should_receive(:by_user).with(user) }
before { get 'newest_by_user', user: user.username }
context 'assigns' do
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
end
end
describe 'GET recent' do
let(:scope) { double 'Recent Scope' }
before { StoryRepository.any_instance.should_receive(:recent) }
before { get 'recent' }
context 'assigns' do
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
end
end
describe 'GET tagged' do
let(:scope) { double 'Tagged Scope' }
let(:tag) { Tag.make! tag: 'tag' }
before { StoryRepository.any_instance.should_receive(:tagged).with(tag) }
before { get 'tagged', tag: tag.tag }
context 'assigns' do
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
end
end
describe 'GET top' do
let(:scope) { double 'Top Scope' }
before { StoryRepository.any_instance.should_receive(:top) }
before { get 'top' }
context 'assigns' do
describe 'stories' do
subject { assigns(:stories) }
it { should eq scope }
end
end
end
end

Ver fichero

@ -0,0 +1,54 @@
describe StoriesPaginator do
let(:current_user) { User.make! }
let(:paginator) { described_class.new(scope, 1, current_user) }
describe '.page' do
context 'fake scope' do
let(:scope) { double('Stories Scope').as_null_object }
before do
allow(scope).to receive(:to_a) { scope }
expect(scope).to receive(:limit).with(26) { scope }
expect(scope).to receive(:offset).with(0) { scope }
Vote.stub :votes_by_user_for_stories_hash
end
it 'paginates given scope' do
paginator.stub :result
paginator.get
end
describe 'show more' do
subject { stories.hottest[1] }
it 'is true if scope.count > 25' do
allow(scope).to receive(:count).and_return 26
expect(paginator.get[1]).to eq true
end
it 'is false if scope.count <= 25' do
allow(scope).to receive(:count).and_return 10
expect(paginator.get[1]).to eq false
end
end
end
context 'integration' do
let!(:s1) { Story.make! }
let!(:s2) { Story.make! }
let!(:s3) { Story.make! }
let!(:v1) { Vote.make! story: s1, user: current_user }
let!(:v2) { Vote.make! story: s2 }
let(:scope) { Story.all }
it 'saves if user have voted for the post' do
result = paginator.get[0]
expect(result.find { |s| s.id == s1.id }.vote).to eq 1
expect(result.find { |s| s.id == s2.id }.vote).to be_nil
expect(result.find { |s| s.id == s3.id }.vote).to be_nil
end
end
end
end

Ver fichero

@ -0,0 +1,180 @@
describe StoryRepository do
let(:current_user) { nil }
let(:stories) { described_class.new current_user }
describe '#hottest' do
subject { stories.hottest }
it 'excludes merged stories' do
merged = Story.make! merged_story_id: 10
expect(subject).not_to include merged
end
it 'excludes expired stories' do
expired = Story.make! is_expired: true
expect(subject).not_to include expired
end
it 'excludes stories with filtered tags' do
filtered_tag = Tag.create! tag: 'not_interesting_stuff'
story = Story.make!
tagged_story = Story.make!
tagged_story.taggings.create! tag: filtered_tag
hottest = described_class.new(current_user, exclude_tags: [filtered_tag.id]).hottest
expect(hottest).to include story
expect(hottest).not_to include tagged_story
end
it 'includes story with 3 downvote and 1 upvotes' do
story = Story.make! downvotes: 3
expect(subject).to include story
end
it 'excludes story with 4 downvotes and 1 upvotes' do
story = Story.make! downvotes: 4
expect(subject).not_to include story
end
it 'orders by hotness asc' do
meh = Story.make! hotness: 50
hot = Story.make! hotness: 100
cool = Story.make! hotness: 25
expect(subject).to eq [cool, meh, hot]
end
context 'logged in' do
let(:current_user) { User.make! }
it 'excludes downvoted stories' do
downvoted = Story.make!
Vote.create! story: downvoted, user: current_user, vote: -1
expect(subject).not_to include downvoted
end
end
end
describe '#hidden' do
subject { stories.hidden }
context 'logged in' do
let(:current_user) { User.make! }
it 'includes downvoted story' do
downvoted = Story.make!
Vote.create! story: downvoted, user: current_user, vote: -1
expect(subject).to include downvoted
end
it 'excludes visible story' do
visible = Story.make!
expect(subject).not_to include visible
end
end
end
describe '#newest' do
subject { stories.newest }
it 'orders by created_at' do
story1 = Story.make! created_at: 1.hour.ago
story2 = Story.make! created_at: 5.hours.ago
story3 = Story.make! created_at: 5.minutes.ago
expect(subject).to eq [story3, story1, story2]
end
end
describe '#by_user' do
let(:another_user) { User.make! }
subject { stories.by_user(another_user) }
it 'orders by created_at' do
story1 = Story.make! user_id: another_user.id
story2 = Story.make!
story3 = Story.make! user_id: another_user.id
expect(subject).to eq [story1, story3]
end
end
describe '#recent' do
subject { stories.recent }
it 'orders by created_at' do
story1 = Story.make! created_at: 1.hour.ago
story2 = Story.make! created_at: 5.hours.ago
story3 = Story.make! created_at: 5.minutes.ago
expect(subject).to eq [story3, story1, story2]
end
end
describe '#tagged' do
let(:tag) { Tag.make! }
let(:tagged) do
story = Story.make!
story.taggings.create! tag: tag
story
end
subject { stories.tagged(tag) }
it 'excludes stories not tagged' do
story = Story.make!
expect(subject).not_to include story
end
it 'includes stories tagged' do
expect(subject).to include tagged
end
it 'includes story with 3 downvote and 1 upvotes' do
tagged.update_attribute :downvotes, 3
expect(subject).to include tagged
end
it 'excludes story with 4 downvotes and 1 upvotes' do
tagged.update_attribute :downvotes, 4
expect(subject).not_to include tagged
end
end
describe '#top' do
context 'filtering' do
subject { stories.top(length) }
let!(:month_ago) { Story.make! created_at: 1.month.ago }
let!(:ten_month_ago) { Story.make! created_at: 10.month.ago }
context '2 weeks' do
let(:length) { { dur: 2, intv: 'Week' } }
it { should_not include month_ago }
end
context '2 month' do
let(:length) { { dur: 2, intv: 'Month' } }
it { should include month_ago }
it { should_not include ten_month_ago }
end
end
context 'ordering' do
it 'orders by votes difference' do
scope = stories.top dur: 12, intv: 'Month'
s1 = Story.make! downvotes: 10
s2 = Story.make! downvotes: 5
s3 = Story.make! downvotes: 3
s4 = Story.make! downvotes: 8
expect(scope).to eq [s3, s2, s4, s1]
end
end
end
end

Ver fichero

@ -44,3 +44,9 @@ Message.blueprint do
subject { "message subject #{sn}" }
body { "message body #{sn}" }
end
Vote.blueprint do
story
user
vote { 1 }
end