diff --git a/.rspec b/.rspec index 38dcdb5..3bbc6f4 100644 --- a/.rspec +++ b/.rspec @@ -1 +1,2 @@ --no-colour +--require spec_helper diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 3b63979..6374d77 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -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 => "
" << + "You apparently have no privacy." << + "
", :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 => "
" << - "You apparently have no privacy." << - "
", :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 diff --git a/app/models/stories_paginator.rb b/app/models/stories_paginator.rb new file mode 100644 index 0000000..31e340b --- /dev/null +++ b/app/models/stories_paginator.rb @@ -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 diff --git a/app/models/story_repository.rb b/app/models/story_repository.rb new file mode 100644 index 0000000..eb14783 --- /dev/null +++ b/app/models/story_repository.rb @@ -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 diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb new file mode 100644 index 0000000..a50d9cf --- /dev/null +++ b/spec/controllers/home_controller_spec.rb @@ -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 diff --git a/spec/models/stories_paginator_spec.rb b/spec/models/stories_paginator_spec.rb new file mode 100644 index 0000000..da65f83 --- /dev/null +++ b/spec/models/stories_paginator_spec.rb @@ -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 diff --git a/spec/models/story_repository_spec.rb b/spec/models/story_repository_spec.rb new file mode 100644 index 0000000..8967234 --- /dev/null +++ b/spec/models/story_repository_spec.rb @@ -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 diff --git a/spec/support/blueprints.rb b/spec/support/blueprints.rb index 27ffa1b..a58f687 100644 --- a/spec/support/blueprints.rb +++ b/spec/support/blueprints.rb @@ -44,3 +44,9 @@ Message.blueprint do subject { "message subject #{sn}" } body { "message body #{sn}" } end + +Vote.blueprint do + story + user + vote { 1 } +end