remove story downvoting, add story hiding

stories should either be reported for spam (coming later), upvoted,
or left alone rather than being downvoted for being uninteresting.
since users don't like leaving uninteresting things alone, they can
now hide stories from their view without affecting the story's
score.

hiding is implemented as a Vote with its vote set to 0 and the
reason set to "H"

add a /hidden url which shows all of a user's hidden stories

while i'm here, simplify Vote guts and add some tests to make sure
all the flip-flopping stuff works right
This commit is contained in:
joshua stein 2014-03-03 17:14:33 -06:00
parent 99c551cbfe
commit 9535b05490
10 changed files with 207 additions and 135 deletions

View file

@ -7,35 +7,40 @@
var _Lobsters = Class.extend({ var _Lobsters = Class.extend({
curUser: null, curUser: null,
storyDownvoteReasons: { <%= Vote::STORY_REASONS.map{|k,v|
"#{k.inspect}: #{v.inspect}" }.join(", ") %> },
commentDownvoteReasons: { <%= Vote::COMMENT_REASONS.map{|k,v| commentDownvoteReasons: { <%= Vote::COMMENT_REASONS.map{|k,v|
"#{k.inspect}: #{v.inspect}" }.join(", ") %> }, "#{k.inspect}: #{v.inspect}" }.join(", ") %> },
upvote: function(voterEl) { upvoteStory: function(voterEl) {
Lobsters.vote("story", voterEl, 1); Lobsters.vote("story", voterEl, 1);
}, },
downvote: function(voterEl) { hideStory: function(hiderEl) {
Lobsters._showDownvoteWhyAt("story", voterEl, function(k) { if (!Lobsters.curUser)
Lobsters.vote("story", voterEl, -1, k); }); return Lobsters.bounceToLogin();
var li = $(hiderEl).closest(".story, .comment");
var act;
if (li.hasClass("hidden")) {
act = "unhide";
li.removeClass("hidden");
hiderEl.innerHTML = "hide";
} else {
act = "hide";
li.addClass("hidden");
hiderEl.innerHTML = "unhide";
}
$.post("/stories/" + li.attr("data-shortid") + "/" + act);
}, },
upvoteComment: function(voterEl) { upvoteComment: function(voterEl) {
Lobsters.vote("comment", voterEl, 1); Lobsters.vote("comment", voterEl, 1);
}, },
downvoteComment: function(voterEl) { downvoteComment: function(voterEl) {
Lobsters._showDownvoteWhyAt("comment", voterEl, function(k) {
Lobsters.vote("comment", voterEl, -1, k); });
},
_showDownvoteWhyAt: function(thingType, voterEl, onChooseWhy) {
if (!Lobsters.curUser)
return Lobsters.bounceToLogin();
var li = $(voterEl).closest(".story, .comment"); var li = $(voterEl).closest(".story, .comment");
if (li.hasClass("downvoted")) { if (li.hasClass("downvoted")) {
/* already upvoted, neutralize */ /* already upvoted, neutralize */
Lobsters.vote(thingType, voterEl, -1, null); Lobsters.vote("comment", voterEl, -1, null);
return; return;
} }
@ -53,13 +58,7 @@ var _Lobsters = Class.extend({
var d = $("<div id=\"downvote_why\"></div>"); var d = $("<div id=\"downvote_why\"></div>");
var reasons; $.each(Lobsters.commentDownvoteReasons, function(k, v) {
if (thingType == "comment")
reasons = Lobsters.commentDownvoteReasons;
else
reasons = Lobsters.storyDownvoteReasons;
$.each(reasons, function(k, v) {
var a = $("<a href=\"#\"" + (k == "" ? " class=\"cancelreason\"" : "") + var a = $("<a href=\"#\"" + (k == "" ? " class=\"cancelreason\"" : "") +
">" + v + "</a>"); ">" + v + "</a>");
@ -68,7 +67,7 @@ var _Lobsters = Class.extend({
$("#downvote_why_shadow").remove(); $("#downvote_why_shadow").remove();
if (k != "") if (k != "")
onChooseWhy(k); Lobsters.vote("comment", voterEl, -1, k);
return false; return false;
}); });
@ -205,8 +204,7 @@ var _Lobsters = Class.extend({
}) })
.success(function(data) { .success(function(data) {
if (data && data.title) if (data && data.title)
title_field.val(data.title.substr(0, title_field.val(data.title.substr(0, title_field.maxLength));
title_field.maxLength));
button.val(old_value); button.val(old_value);
button.prop("disabled", false); button.prop("disabled", false);
@ -220,6 +218,7 @@ var _Lobsters = Class.extend({
bounceToLogin: function() { bounceToLogin: function() {
document.location = "/login?return=" + document.location = "/login?return=" +
encodeURIComponent(document.location); encodeURIComponent(document.location);
return false;
}, },
}); });
@ -235,12 +234,12 @@ $(document).ready(function() {
return false; return false;
}); });
$("li.story a.downvoter").click(function() { $("li.story a.upvoter").click(function() {
Lobsters.downvote(this); Lobsters.upvoteStory(this);
return false; return false;
}); });
$("li.story a.upvoter").click(function() { $("li.story a.hider").click(function() {
Lobsters.upvote(this); Lobsters.hideStory(this);
return false; return false;
}); });

View file

@ -509,6 +509,12 @@ li .byline a.inactive_user {
text-decoration: line-through; text-decoration: line-through;
} }
li.story.hidden {
opacity: 0.25;
}
ol.show_hidden li.story.hidden {
opacity: 1.0 !important;
}
li.story.expired { li.story.expired {
opacity: 0.6; opacity: 0.6;

View file

@ -10,6 +10,15 @@ class HomeController < ApplicationController
# for rss feeds, load the user's tag filters if a token is passed # 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 :find_user_from_rss_token, :only => [ :index, :newest ]
def hidden
@stories = find_stories({ :hidden => true })
@heading = @title = "Hidden Stories"
@cur_url = "/hidden"
render :action => "index"
end
def index def index
@stories = find_stories @stories = find_stories
@ -38,7 +47,6 @@ class HomeController < ApplicationController
@heading = @title = "Newest Stories" @heading = @title = "Newest Stories"
@cur_url = "/newest" @cur_url = "/newest"
@newest = true
@rss_link = "<link rel=\"alternate\" type=\"application/rss+xml\" " << @rss_link = "<link rel=\"alternate\" type=\"application/rss+xml\" " <<
"title=\"RSS 2.0 - Newest Items\" href=\"/newest.rss" << "title=\"RSS 2.0 - Newest Items\" href=\"/newest.rss" <<
@ -76,7 +84,6 @@ class HomeController < ApplicationController
@heading = @title = "Recent Stories" @heading = @title = "Recent Stories"
@cur_url = "/recent" @cur_url = "/recent"
@recent = true
render :action => "index" render :action => "index"
end end
@ -145,14 +152,28 @@ private
def _find_stories(how) def _find_stories(how)
stories = Story.where(:is_expired => false) stories = Story.where(:is_expired => false)
if @user && !(how[:newest] || how[:by_user]) if @user && !how[:by_user] && !how[:hidden]
# exclude downvoted items # exclude downvoted and hidden items
stories = stories.where( stories = stories.where(
Story.arel_table[:id].not_in( Story.arel_table[:id].not_in(
Vote.arel_table.where( Vote.arel_table.where(
Vote.arel_table[:user_id].eq(@user.id) Vote.arel_table[:user_id].eq(@user.id)
).where( ).where(
Vote.arel_table[:vote].lt(0) Vote.arel_table[:vote].lteq(0)
).where(
Vote.arel_table[:comment_id].eq(nil)
).project(
Vote.arel_table[:story_id]
)
)
)
elsif @user && how[:hidden]
stories = stories.where(
Story.arel_table[:id].in(
Vote.arel_table.where(
Vote.arel_table[:user_id].eq(@user.id)
).where(
Vote.arel_table[:vote].eq(0)
).where( ).where(
Vote.arel_table[:comment_id].eq(nil) Vote.arel_table[:comment_id].eq(nil)
).project( ).project(

View file

@ -1,6 +1,6 @@
class StoriesController < ApplicationController class StoriesController < ApplicationController
before_filter :require_logged_in_user_or_400, before_filter :require_logged_in_user_or_400,
:only => [ :upvote, :downvote, :unvote, :preview ] :only => [ :upvote, :unvote, :hide, :unhide, :preview ]
before_filter :require_logged_in_user, :only => [ :destroy, :create, :edit, before_filter :require_logged_in_user, :only => [ :destroy, :create, :edit,
:fetch_url_title, :new ] :fetch_url_title, :new ]
@ -201,21 +201,24 @@ class StoriesController < ApplicationController
render :text => "ok" render :text => "ok"
end end
def downvote def hide
if !(story = find_story) if !(story = find_story)
return render :text => "can't find story", :status => 400 return render :text => "can't find story", :status => 400
end end
if !Vote::STORY_REASONS[params[:reason]] Vote.vote_thusly_on_story_or_comment_for_user_because(0, story.id,
return render :text => "invalid reason", :status => 400 nil, @user.id, "H")
render :text => "ok"
end
def unhide
if !(story = find_story)
return render :text => "can't find story", :status => 400
end end
if !@user.can_downvote?(story) Vote.vote_thusly_on_story_or_comment_for_user_because(0, story.id,
return render :text => "not permitted to downvote", :status => 400 nil, @user.id, nil)
end
Vote.vote_thusly_on_story_or_comment_for_user_because(-1, story.id,
nil, @user.id, params[:reason])
render :text => "ok" render :text => "ok"
end end

View file

@ -10,8 +10,6 @@ class Story < ActiveRecord::Base
validates_length_of :description, :maximum => (64 * 1024) validates_length_of :description, :maximum => (64 * 1024)
validates_presence_of :user_id validates_presence_of :user_id
DOWNVOTABLE_DAYS = 14
# after this many minutes old, a story cannot be edited # after this many minutes old, a story cannot be edited
MAX_EDIT_MINS = 30 MAX_EDIT_MINS = 30
@ -119,8 +117,7 @@ class Story < ActiveRecord::Base
end end
def calculated_hotness def calculated_hotness
# don't immediately kill stories at 0 by bumping up score by one order = Math.log([ score.abs, 1 ].max, 10)
order = Math.log([ (score + 1).abs, 1 ].max, 10)
if score > 0 if score > 0
sign = 1 sign = 1
elsif score < 0 elsif score < 0
@ -226,14 +223,6 @@ class Story < ActiveRecord::Base
"hotness = '#{self.calculated_hotness}' WHERE id = #{self.id.to_i}") "hotness = '#{self.calculated_hotness}' WHERE id = #{self.id.to_i}")
end end
def is_downvotable?
if self.created_at
Time.now - self.created_at <= DOWNVOTABLE_DAYS.days
else
false
end
end
def is_editable_by_user?(user) def is_editable_by_user?(user)
if user && user.is_moderator? if user && user.is_moderator?
return true return true
@ -406,26 +395,4 @@ class Story < ActiveRecord::Base
def url_or_comments_url def url_or_comments_url
self.url.blank? ? self.comments_url : self.url self.url.blank? ? self.comments_url : self.url
end end
def vote_summary_for(user)
r_counts = {}
r_whos = {}
Vote.where(:story_id => self.id, :comment_id => nil).each do |v|
r_counts[v.reason.to_s] ||= 0
r_counts[v.reason.to_s] += v.vote
if user && user.is_moderator?
r_whos[v.reason.to_s] ||= []
r_whos[v.reason.to_s].push v.user.username
end
end
r_counts.keys.sort.map{|k|
if k == ""
"+#{r_counts[k]}"
else
"#{r_counts[k]} #{Vote::STORY_REASONS[k]}" +
(user && user.is_moderator?? " (#{r_whos[k].join(", ")})" : "")
end
}.join(", ")
end
end end

View file

@ -2,16 +2,6 @@ class Vote < ActiveRecord::Base
belongs_to :user belongs_to :user
belongs_to :story belongs_to :story
STORY_REASONS = {
"O" => "Off-topic",
"Q" => "Low Quality",
"A" => "Already Posted",
"T" => "Poorly Tagged",
"L" => "Poorly Titled",
"S" => "Spam",
"" => "Cancel",
}
COMMENT_REASONS = { COMMENT_REASONS = {
"O" => "Off-topic", "O" => "Off-topic",
"I" => "Incorrect", "I" => "Incorrect",
@ -82,7 +72,7 @@ class Vote < ActiveRecord::Base
v = Vote.where(:user_id => user_id, :story_id => story_id, v = Vote.where(:user_id => user_id, :story_id => story_id,
:comment_id => comment_id).first_or_initialize :comment_id => comment_id).first_or_initialize
if !v.new_record? && v.vote == vote if !v.new_record? && v.vote == vote && vote != 0
return return
end end
@ -91,35 +81,22 @@ class Vote < ActiveRecord::Base
Vote.transaction do Vote.transaction do
# unvote # unvote
if vote == 0 if vote == 0 && reason == nil
if !v.new_record? # neutralize previous vote
if v.vote == -1 upvote = (v.vote == 1 ? -1 : 0)
downvote = -1 downvote = (v.vote == -1 ? -1 : 0)
else v.destroy!
upvote = -1
end
end
v.destroy
# new vote or change vote # new vote or change vote
else else
if v.new_record? if !v.new_record?
if vote == -1 upvote = (v.vote == 1 ? -1 : 0)
downvote = 1 downvote = (v.vote == -1 ? -1 : 0)
else
upvote = 1
end
elsif v.vote == -1
# changing downvote to upvote
downvote = -1
upvote = 1
elsif v.vote == 1
# changing upvote to downvote
upvote = -1
downvote = 1
end end
upvote += (vote == 1 ? 1 : 0)
downvote += (vote == -1 ? 1 : 0)
v.vote = vote v.vote = vote
v.reason = reason v.reason = reason
v.save! v.save!

View file

@ -1,21 +1,20 @@
<ol class="stories list"> <ol class="stories list <%= @cur_url == "/hidden" ? "show_hidden" : "" %>">
<%= render :partial => "stories/listdetail", :collection => @stories, <%= render :partial => "stories/listdetail", :collection => @stories,
:as => :story %> :as => :story %>
</ol> </ol>
<div class="morelink"> <div class="morelink">
<% if @page && @page > 1 %> <% if @page && @page > 1 %>
<a href="<%= @tag ? "/t/#{@tag.tag}" : (@newest ? "/newest" + <a href="<%= @cur_url %><%= @cur_url == "/" ? "" : "/" %><%=
(@for_user ? "/#{@for_user}" : "") : "") %><%= @page == 2 ? "/" : @page == 2 ? "" : "page/#{@page - 1}" %>">&lt;&lt; Page
"/page/#{@page - 1}" %>">&lt;&lt; Page <%= @page - 1 %></a> <%= @page - 1 %></a>
<% end %> <% end %>
<% if @show_more %> <% if @show_more %>
<% if @page && @page > 1 %> <% if @page && @page > 1 %>
| |
<% end %> <% end %>
<a href="<%= @tag ? "/t/#{@tag.tag}" : (@newest ? "/newest" + <a href="<%= @cur_url %><%= @cur_url == "/" ? "" : "/" %>page/<%=
(@for_user ? "/#{@for_user}" : "") : "") %>/page/<%= @page + 1 %>">Page @page + 1 %>">Page <%= @page + 1 %> &gt;&gt;</a>
<%= @page + 1 %> &gt;&gt;</a>
<% end %> <% end %>
</div> </div>

View file

@ -1,6 +1,7 @@
<li id="story_<%= story.short_id %>" data-shortid="<%= story.short_id %>" <li id="story_<%= story.short_id %>" data-shortid="<%= story.short_id %>"
class="story <%= story.vote == 1 ? "upvoted" : (story.vote == -1 ? class="story <%= story.vote == 1 ? "upvoted" : "" %> <%= story.vote == -1 ?
"downvoted" : "") %> <%= story.is_expired? ? "expired" : "" %>"> "downvoted" : "" %> <%= story.vote == 0 ? "hidden" : "" %> <%= story.is_expired? ?
"expired" : "" %>">
<div class="story_liner"> <div class="story_liner">
<div class="voters"> <div class="voters">
<% if @user %> <% if @user %>
@ -8,12 +9,7 @@ class="story <%= story.vote == 1 ? "upvoted" : (story.vote == -1 ?
<% else %> <% else %>
<%= link_to "", login_url, :class => "upvoter" %> <%= link_to "", login_url, :class => "upvoter" %>
<% end %> <% end %>
<div class="score"><%= story.upvotes - story.downvotes %></div> <div class="score"><%= story.score %></div>
<% if @user && @user.can_downvote?(story) %>
<a class="downvoter"></a>
<% else %>
<span class="downvoter downvoter_stub"></span>
<% end %>
</div> </div>
<div class="details"> <div class="details">
<span class="link"> <span class="link">
@ -67,6 +63,15 @@ class="story <%= story.vote == 1 ? "upvoted" : (story.vote == -1 ?
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% if !story.is_gone? && @user %>
<% if story.vote == 0 %>
| <%= link_to "unhide", story_unhide_url(story.short_id),
:class => "hider" %>
<% else %>
| <%= link_to "hide", story_hide_url(story.short_id),
:class => "hider" %>
<% end %>
<% end %>
<% if !story.is_gone? && (@user || story.comments_count > 0) %> <% if !story.is_gone? && (@user || story.comments_count > 0) %>
<span class="comments_label"> <span class="comments_label">
| |
@ -75,12 +80,6 @@ class="story <%= story.vote == 1 ? "upvoted" : (story.vote == -1 ?
(story.comments_count == 1 ? "" : "s") %></a> (story.comments_count == 1 ? "" : "s") %></a>
</span> </span>
<% end %> <% end %>
<% if defined?(single_story) && single_story %>
<% if story.downvotes > 0 %>
| <%= story.vote_summary_for(@user).downcase %>
<% end %>
<% end %>
<% end %> <% end %>
</div> </div>
</div> </div>

View file

@ -14,6 +14,8 @@ Lobsters::Application.routes.draw do
get "/newest/:user/page/:page" => "home#newest_by_user" get "/newest/:user/page/:page" => "home#newest_by_user"
get "/recent" => "home#recent" get "/recent" => "home#recent"
get "/recent/page/:page" => "home#recent" get "/recent/page/:page" => "home#recent"
get "/hidden" => "home#hidden"
get "/hidden/page/:page" => "home#hidden"
get "/threads" => "comments#threads" get "/threads" => "comments#threads"
get "/threads/:user" => "comments#threads" get "/threads/:user" => "comments#threads"
@ -40,9 +42,10 @@ Lobsters::Application.routes.draw do
resources :stories do resources :stories do
post "upvote" post "upvote"
post "downvote"
post "unvote" post "unvote"
post "undelete" post "undelete"
post "hide"
post "unhide"
end end
post "/stories/fetch_url_title", :format => "json" post "/stories/fetch_url_title", :format => "json"
post "/stories/preview" => "stories#preview" post "/stories/preview" => "stories#preview"

98
spec/models/vote_spec.rb Normal file
View file

@ -0,0 +1,98 @@
require "spec_helper"
describe Vote do
it "applies a story upvote and karma properly" do
s = Story.make!
s.upvotes.should == 1
s.downvotes.should == 0
s.user.karma.should == 0
u = User.make!
Vote.vote_thusly_on_story_or_comment_for_user_because(1, s.id,
nil, u.id, nil)
s.reload
s.upvotes.should == 2
s.user.karma.should == 1
end
it "does nothing when upvoting an existing upvote" do
s = Story.make!
u = User.make!
2.times do
Vote.vote_thusly_on_story_or_comment_for_user_because(1, s.id,
nil, u.id, nil)
s.reload
s.upvotes.should == 2
s.user.karma.should == 1
end
end
it "has no effect on a story score when casting a hide vote" do
s = Story.make!
s.upvotes.should == 1
u = User.make!
Vote.vote_thusly_on_story_or_comment_for_user_because(0, s.id,
nil, u.id, "H")
s.reload
s.user.karma.should == 0
s.upvotes.should == 1
s.downvotes.should == 0
end
it "removes karma and upvote when downvoting an upvote" do
s = Story.make!
c = Comment.make!(:story_id => s.id)
c.user.karma.should == 0
u = User.make!
Vote.vote_thusly_on_story_or_comment_for_user_because(1, s.id,
c.id, u.id, nil)
c.reload
c.user.karma.should == 1
# initial poster upvote plus new user's vote
c.upvotes.should == 2
c.downvotes.should == 0
# flip vote
Vote.vote_thusly_on_story_or_comment_for_user_because(-1, s.id,
c.id, u.id, Vote::COMMENT_REASONS.keys.first)
c.reload
c.user.karma.should == -1
c.upvotes.should == 1
c.downvotes.should == 1
end
it "neutralizes karma and upvote when unvoting an upvote" do
s = Story.make!
c = Comment.make!(:story_id => s.id)
u = User.make!
Vote.vote_thusly_on_story_or_comment_for_user_because(1, s.id,
c.id, u.id, nil)
c.reload
c.user.karma.should == 1
c.upvotes.should == 2
c.downvotes.should == 0
Vote.vote_thusly_on_story_or_comment_for_user_because(0, s.id,
c.id, u.id, nil)
c.reload
c.user.karma.should == 0
c.upvotes.should == 1
c.downvotes.should == 0
end
end