Skip to content

Commit

Permalink
remove story downvoting, add story hiding
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jcs committed Mar 3, 2014
1 parent 99c551c commit 9535b05
Show file tree
Hide file tree
Showing 10 changed files with 206 additions and 134 deletions.
57 changes: 28 additions & 29 deletions app/assets/javascripts/application.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,35 +7,40 @@
var _Lobsters = Class.extend({
curUser: null,

storyDownvoteReasons: { <%= Vote::STORY_REASONS.map{|k,v|
"#{k.inspect}: #{v.inspect}" }.join(", ") %> },
commentDownvoteReasons: { <%= Vote::COMMENT_REASONS.map{|k,v|
"#{k.inspect}: #{v.inspect}" }.join(", ") %> },

upvote: function(voterEl) {
upvoteStory: function(voterEl) {
Lobsters.vote("story", voterEl, 1);
},
downvote: function(voterEl) {
Lobsters._showDownvoteWhyAt("story", voterEl, function(k) {
Lobsters.vote("story", voterEl, -1, k); });
hideStory: function(hiderEl) {
if (!Lobsters.curUser)
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) {
Lobsters.vote("comment", voterEl, 1);
},
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");
if (li.hasClass("downvoted")) {
/* already upvoted, neutralize */
Lobsters.vote(thingType, voterEl, -1, null);
Lobsters.vote("comment", voterEl, -1, null);
return;
}

Expand All @@ -53,13 +58,7 @@ var _Lobsters = Class.extend({

var d = $("<div id=\"downvote_why\"></div>");

var reasons;
if (thingType == "comment")
reasons = Lobsters.commentDownvoteReasons;
else
reasons = Lobsters.storyDownvoteReasons;

$.each(reasons, function(k, v) {
$.each(Lobsters.commentDownvoteReasons, function(k, v) {
var a = $("<a href=\"#\"" + (k == "" ? " class=\"cancelreason\"" : "") +
">" + v + "</a>");

Expand All @@ -68,7 +67,7 @@ var _Lobsters = Class.extend({
$("#downvote_why_shadow").remove();

if (k != "")
onChooseWhy(k);
Lobsters.vote("comment", voterEl, -1, k);

return false;
});
Expand Down Expand Up @@ -205,8 +204,7 @@ var _Lobsters = Class.extend({
})
.success(function(data) {
if (data && data.title)
title_field.val(data.title.substr(0,
title_field.maxLength));
title_field.val(data.title.substr(0, title_field.maxLength));

button.val(old_value);
button.prop("disabled", false);
Expand All @@ -220,6 +218,7 @@ var _Lobsters = Class.extend({
bounceToLogin: function() {
document.location = "/login?return=" +
encodeURIComponent(document.location);
return false;
},
});

Expand All @@ -235,12 +234,12 @@ $(document).ready(function() {
return false;
});

$("li.story a.downvoter").click(function() {
Lobsters.downvote(this);
$("li.story a.upvoter").click(function() {
Lobsters.upvoteStory(this);
return false;
});
$("li.story a.upvoter").click(function() {
Lobsters.upvote(this);
$("li.story a.hider").click(function() {
Lobsters.hideStory(this);
return false;
});

Expand Down
6 changes: 6 additions & 0 deletions app/assets/stylesheets/application.css
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,12 @@ li .byline a.inactive_user {
text-decoration: line-through;
}

li.story.hidden {
opacity: 0.25;
}
ol.show_hidden li.story.hidden {
opacity: 1.0 !important;
}

li.story.expired {
opacity: 0.6;
Expand Down
31 changes: 26 additions & 5 deletions app/controllers/home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ class HomeController < ApplicationController
# for rss feeds, load the user's tag filters if a token is passed
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
@stories = find_stories

Expand Down Expand Up @@ -38,7 +47,6 @@ def newest

@heading = @title = "Newest Stories"
@cur_url = "/newest"
@newest = true

@rss_link = "<link rel=\"alternate\" type=\"application/rss+xml\" " <<
"title=\"RSS 2.0 - Newest Items\" href=\"/newest.rss" <<
Expand Down Expand Up @@ -76,7 +84,6 @@ def recent

@heading = @title = "Recent Stories"
@cur_url = "/recent"
@recent = true

render :action => "index"
end
Expand Down Expand Up @@ -145,14 +152,28 @@ def find_stories(how = {})
def _find_stories(how)
stories = Story.where(:is_expired => false)

if @user && !(how[:newest] || how[:by_user])
# exclude downvoted items
if @user && !how[:by_user] && !how[:hidden]
# exclude downvoted and hidden items
stories = stories.where(
Story.arel_table[:id].not_in(
Vote.arel_table.where(
Vote.arel_table[:user_id].eq(@user.id)
).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(
Vote.arel_table[:comment_id].eq(nil)
).project(
Expand Down
21 changes: 12 additions & 9 deletions app/controllers/stories_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class StoriesController < ApplicationController
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,
:fetch_url_title, :new ]
Expand Down Expand Up @@ -201,21 +201,24 @@ def upvote
render :text => "ok"
end

def downvote
def hide
if !(story = find_story)
return render :text => "can't find story", :status => 400
end

if !Vote::STORY_REASONS[params[:reason]]
return render :text => "invalid reason", :status => 400
end
Vote.vote_thusly_on_story_or_comment_for_user_because(0, story.id,
nil, @user.id, "H")

render :text => "ok"
end

if !@user.can_downvote?(story)
return render :text => "not permitted to downvote", :status => 400
def unhide
if !(story = find_story)
return render :text => "can't find story", :status => 400
end

Vote.vote_thusly_on_story_or_comment_for_user_because(-1, story.id,
nil, @user.id, params[:reason])
Vote.vote_thusly_on_story_or_comment_for_user_because(0, story.id,
nil, @user.id, nil)

render :text => "ok"
end
Expand Down
35 changes: 1 addition & 34 deletions app/models/story.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ class Story < ActiveRecord::Base
validates_length_of :description, :maximum => (64 * 1024)
validates_presence_of :user_id

DOWNVOTABLE_DAYS = 14

# after this many minutes old, a story cannot be edited
MAX_EDIT_MINS = 30

Expand Down Expand Up @@ -119,8 +117,7 @@ def assign_short_id_and_upvote
end

def calculated_hotness
# don't immediately kill stories at 0 by bumping up score by one
order = Math.log([ (score + 1).abs, 1 ].max, 10)
order = Math.log([ score.abs, 1 ].max, 10)
if score > 0
sign = 1
elsif score < 0
Expand Down Expand Up @@ -226,14 +223,6 @@ def give_upvote_or_downvote_and_recalculate_hotness!(upvote, downvote)
"hotness = '#{self.calculated_hotness}' WHERE id = #{self.id.to_i}")
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)
if user && user.is_moderator?
return true
Expand Down Expand Up @@ -406,26 +395,4 @@ def url_is_editable_by_user?(user)
def url_or_comments_url
self.url.blank? ? self.comments_url : self.url
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
47 changes: 12 additions & 35 deletions app/models/vote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,6 @@ class Vote < ActiveRecord::Base
belongs_to :user
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 = {
"O" => "Off-topic",
"I" => "Incorrect",
Expand Down Expand Up @@ -82,7 +72,7 @@ def self.vote_thusly_on_story_or_comment_for_user_because(vote, story_id,
v = Vote.where(:user_id => user_id, :story_id => story_id,
:comment_id => comment_id).first_or_initialize

if !v.new_record? && v.vote == vote
if !v.new_record? && v.vote == vote && vote != 0
return
end

Expand All @@ -91,35 +81,22 @@ def self.vote_thusly_on_story_or_comment_for_user_because(vote, story_id,

Vote.transaction do
# unvote
if vote == 0
if !v.new_record?
if v.vote == -1
downvote = -1
else
upvote = -1
end
end

v.destroy
if vote == 0 && reason == nil
# neutralize previous vote
upvote = (v.vote == 1 ? -1 : 0)
downvote = (v.vote == -1 ? -1 : 0)
v.destroy!

# new vote or change vote
else
if v.new_record?
if vote == -1
downvote = 1
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
if !v.new_record?
upvote = (v.vote == 1 ? -1 : 0)
downvote = (v.vote == -1 ? -1 : 0)
end

upvote += (vote == 1 ? 1 : 0)
downvote += (vote == -1 ? 1 : 0)

v.vote = vote
v.reason = reason
v.save!
Expand Down
13 changes: 6 additions & 7 deletions app/views/home/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
<ol class="stories list">
<ol class="stories list <%= @cur_url == "/hidden" ? "show_hidden" : "" %>">
<%= render :partial => "stories/listdetail", :collection => @stories,
:as => :story %>
</ol>

<div class="morelink">
<% if @page && @page > 1 %>
<a href="<%= @tag ? "/t/#{@tag.tag}" : (@newest ? "/newest" +
(@for_user ? "/#{@for_user}" : "") : "") %><%= @page == 2 ? "/" :
"/page/#{@page - 1}" %>">&lt;&lt; Page <%= @page - 1 %></a>
<a href="<%= @cur_url %><%= @cur_url == "/" ? "" : "/" %><%=
@page == 2 ? "" : "page/#{@page - 1}" %>">&lt;&lt; Page
<%= @page - 1 %></a>
<% end %>
<% if @show_more %>
<% if @page && @page > 1 %>
|
<% end %>
<a href="<%= @tag ? "/t/#{@tag.tag}" : (@newest ? "/newest" +
(@for_user ? "/#{@for_user}" : "") : "") %>/page/<%= @page + 1 %>">Page
<%= @page + 1 %> &gt;&gt;</a>
<a href="<%= @cur_url %><%= @cur_url == "/" ? "" : "/" %>page/<%=
@page + 1 %>">Page <%= @page + 1 %> &gt;&gt;</a>
<% end %>
</div>
Loading

0 comments on commit 9535b05

Please sign in to comment.