Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix server error on user profile vote summary tab if there are orphaned votes #1491

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
15 changes: 9 additions & 6 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -574,18 +574,21 @@ def my_vote_summary
end

def vote_summary
@votes = Vote.where(recv_user: @user) \
.includes(:post).group(:date_of, :post_id, :vote_type)
@votes = @votes.select(:post_id, :vote_type) \
.select('count(*) as vote_count') \
.select('date(created_at) as date_of')
@votes = Vote.where(recv_user: @user)
.joins(:post)
.group(:date_of, :post_id, :vote_type)

@votes = @votes.select(:post_id, :vote_type)
.select('count(*) as vote_count')
.select('date(votes.created_at) as date_of')

@votes = @votes.order(date_of: :desc, post_id: :desc).all \
.group_by(&:date_of).map do |k, vl|
[k, vl.group_by(&:post), vl.sum { |v| v.vote_type * v.vote_count }]
end \
.paginate(page: params[:page], per_page: 15)

render layout: 'without_sidebar'
@votes
end

def avatar
Expand Down
32 changes: 32 additions & 0 deletions app/jobs/cleanup_votes_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
class CleanupVotesJob < ApplicationJob
queue_as :default

def perform
Community.all.each do |c|
RequestContext.community = c
orphan_votes = Vote.all.reject { |v| v.post.present? }

puts "[#{c.name}] destroying #{orphan_votes.length} #{'orphan vote'.pluralize(orphan_votes.length)}"
Copy link
Member

@cellio cellio Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a safety net, in case we ever accidentally destroy a valid vote (maybe I'm being paranoid): can we record the rows we're nuking somewhere (output, log file, whatever)? This seems very unlikely, but also, a user saying "wait where'd all my votes go???" would be bad if we can't recover the data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can add a log to the successful case of destroy as well, won't hurt (even though we definitely should have backups to restore from even if something extra gets dropped).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have backups, yes. I was thinking vaguely that it might be helpful to be able to assess the scale of the problem (like how many users were affected).


system_user = User.find(-1)

orphan_votes.each do |v|
result = v.destroy

if result
AuditLog.admin_audit(
comment: "Deleted orphaned vote for user ##{v.recv_user_id} " \
"on post ##{v.post_id} " \
"in community ##{c.id} (#{c.name})",
event_type: 'vote_delete',
related: v,
user: system_user
)
else
puts "[#{c.name}] failed to destroy vote \"#{v.id}\""
v.errors.each { |e| puts e.full_message }
end
end
end
end
end
12 changes: 10 additions & 2 deletions app/models/vote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,37 +34,45 @@ def reverse_rep_change
end

def rep_change(direction)
return unless post.present?

change = CategoryPostType.rep_changes[[post.category_id, post.post_type_id]][vote_type] || 0
recv_user.update!(reputation: recv_user.reputation + (direction * change))
end

def post_not_deleted
if post.deleted?
if post&.deleted?
errors.add(:base, 'Votes are locked on deleted posts')
end
end

def check_valid
throw :abort unless valid?
throw :abort unless valid? || post.blank?
end

def add_counter
return unless post.present?

case vote_type
when 1
post.update(upvote_count: post.upvote_count + 1)
when -1
post.update(downvote_count: post.downvote_count + 1)
end

post.recalc_score
end

def remove_counter
return unless post.present?

case vote_type
when 1
post.update(upvote_count: [post.upvote_count - 1, 0].max)
when -1
post.update(downvote_count: [post.downvote_count - 1, 0].max)
end

post.recalc_score
end
end
24 changes: 15 additions & 9 deletions app/views/users/vote_summary.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,21 @@
</div>
<div class="item-list--text-value is-primary grid">
<div class="grid grid--cell is-12">
<div class="grid--cell is-flexible">
<%= link_to generic_share_link(post) do %>
<%= post.post_type.is_top_level ? post.title : post.parent.title %>
<% end %>
</div>
<div class="grid--cell">
<span class="badge is-tag is-muted is-filled"><%= post.post_type.name %></span>
<a href="<%= category_path(post.category) %>" class="badge is-tag is-muted"><%= post.category.name %></a>
</div>
<% if post.present? %>
<div class="grid--cell is-flexible">
<%= link_to generic_share_link(post) do %>
<%= post.post_type.is_top_level ? post.title : post.parent.title %>
<% end %>
</div>
<div class="grid--cell">
<span class="badge is-tag is-muted is-filled"><%= post.post_type.name %></span>
<a href="<%= category_path(post.category) %>" class="badge is-tag is-muted"><%= post.category.name %></a>
</div>
<% else %>
<div class="grid--cell">
<%= I18n.t('votes.summary.post_missing') %>
</div>
<% end %>
</div>
</div>
</div>
Expand Down
4 changes: 4 additions & 0 deletions config/locales/strings/en.votes.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
en:
votes:
summary:
post_missing: 'Post not found'
4 changes: 4 additions & 0 deletions config/schedule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
runner 'scripts/cleanup_drafts.rb'
end

every 1.day, ay: '02.25' do
runner 'scripts/cleanup_votes.rb'
end

every 6.hours do
runner 'scripts/recalc_abilities.rb'
end
1 change: 1 addition & 0 deletions scripts/cleanup_votes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CleanupVotesJob.perform_later
Loading