FIX: Use a logging table for daily likes given. Use it for badges.

This commit is contained in:
Robin Ward 2016-03-17 14:41:00 -04:00
parent f15d463eb8
commit 1fba835d4f
9 changed files with 165 additions and 60 deletions

View File

@ -23,15 +23,6 @@ class PostActionsController < ApplicationController
@post.reload
render_post_json(@post, _add_raw = false)
end
rescue RateLimiter::LimitExceeded => e
# Special case: if we hit the create like rate limit, record it in user history
# so we can award a badge
if e.type == "create_like"
UserHistory.create!(action: UserHistory.actions[:rate_limited_like],
target_user_id: current_user.id,
post_id: @post.id)
end
render_rate_limit_error(e)
end
def destroy

View File

@ -286,11 +286,11 @@ SQL
def self.like_rate_limit(count)
<<-SQL
SELECT uh.target_user_id AS user_id, MAX(uh.created_at) AS granted_at
FROM user_histories AS uh
WHERE uh.action = #{UserHistory.actions[:rate_limited_like]}
AND (:backfill OR uh.target_user_id IN (:user_ids))
GROUP BY uh.target_user_id
SELECT gdl.user_id, current_timestamp AS granted_at
FROM given_daily_likes AS gdl
WHERE gdl.limit_reached
AND (:backfill OR gdl.user_id IN (:user_ids))
GROUP BY gdl.user_id
HAVING COUNT(*) >= #{count}
SQL
end

View File

@ -0,0 +1,34 @@
class GivenDailyLike < ActiveRecord::Base
belongs_to :user
def self.find_for(user_id, date)
where(user_id: user_id, given_date: date)
end
def self.increment_for(user_id)
return unless user_id
given_date = Date.today
# upsert would be nice here
rows = find_for(user_id, given_date).update_all('likes_given = likes_given + 1')
if rows == 0
create(user_id: user_id, given_date: given_date, likes_given: 1)
else
find_for(user_id, given_date)
.where('limit_reached = false AND likes_given >= :limit', limit: SiteSetting.max_likes_per_day)
.update_all(limit_reached: true)
end
end
def self.decrement_for(user_id)
return unless user_id
given_date = Date.today
find_for(user_id, given_date).update_all('likes_given = likes_given - 1')
find_for(user_id, given_date)
.where('limit_reached = true AND likes_given < :limit', limit: SiteSetting.max_likes_per_day)
.update_all(limit_reached: false)
end
end

View File

@ -286,12 +286,11 @@ SQL
BadgeGranter.queue_badge_grant(Badge::Trigger::PostAction, post_action: post_action)
end
end
GivenDailyLike.increment_for(user.id)
# agree with other flags
if staff_took_action
PostAction.agree_flags!(post, user)
# update counters
post_action.try(:update_counters)
end
@ -311,6 +310,7 @@ SQL
if action = finder.first
action.remove_act!(user)
action.post.unhide! if action.staff_took_action
GivenDailyLike.decrement_for(user.id)
end
end

View File

@ -52,7 +52,8 @@ class UserHistory < ActiveRecord::Base
grant_moderation: 34,
revoke_moderation: 35,
backup_operation: 36,
rate_limited_like: 37)
rate_limited_like: 37 # not used anymore
)
end
# Staff actions is a subset of all actions, used to audit actions taken by staff users.

View File

@ -0,0 +1,34 @@
class CreateGivenDailyLikes < ActiveRecord::Migration
def up
create_table :given_daily_likes, id: false, force: true do |t|
t.integer :user_id, null: false
t.integer :likes_given, null: false
t.date :given_date, null: false
t.boolean :limit_reached, null: false, default: false
end
add_index :given_daily_likes, [:user_id, :given_date], unique: true
add_index :given_daily_likes, [:limit_reached, :user_id]
max_likes_rows = execute("SELECT value FROM site_settings WHERE name = 'max_likes_per_day'")
if max_likes_rows && max_likes_rows.cmd_tuples > 0
max_likes = max_likes_rows[0]['value'].to_i
end
max_likes ||= 50
execute "INSERT INTO given_daily_likes (user_id, likes_given, limit_reached, given_date)
SELECT pa.user_id,
COUNT(*),
CASE WHEN COUNT(*) >= #{max_likes} THEN true
ELSE false
END,
pa.created_at::date
FROM post_actions AS pa
WHERE pa.post_action_type_id = 2
AND pa.deleted_at IS NULL
GROUP BY pa.user_id, pa.created_at::date"
end
def down
drop_table :given_daily_likes
end
end

View File

@ -7,20 +7,6 @@ describe PostActionsController do
expect { xhr :post, :create }.to raise_error(Discourse::NotLoggedIn)
end
describe 'logged in as regular user' do
before do
@user = log_in(:user)
@post = Fabricate(:post, user: Fabricate(:coding_horror))
end
it 'creates user history if the rate limit for a like is hit' do
PostAction.expects(:act).once.raises(RateLimiter::LimitExceeded.new(60, 'create_like'))
expect(-> {
xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like]
}).to change(UserHistory, :count).by(1)
end
end
describe 'logged in as moderator' do
before do
@user = log_in(:moderator)

View File

@ -0,0 +1,50 @@
require 'rails_helper'
describe GivenDailyLike do
it 'no errors without a user' do
expect(-> { GivenDailyLike.increment_for(nil) }).not_to raise_error
expect(-> { GivenDailyLike.decrement_for(nil) }).not_to raise_error
end
context 'with a user' do
let(:user) { Fabricate(:user) }
def value_for(user_id, date)
GivenDailyLike.find_for(user_id, date).pluck(:likes_given)[0] || 0
end
def limit_reached_for(user_id, date)
GivenDailyLike.find_for(user_id, date).pluck(:limit_reached)[0] || false
end
it 'can be incremented and decremented' do
SiteSetting.max_likes_per_day = 2
Timecop.freeze(Date.today) do
dt = Date.today
expect(value_for(user.id, dt)).to eq(0)
expect(limit_reached_for(user.id, dt)).to eq(false)
GivenDailyLike.increment_for(user.id)
expect(value_for(user.id, dt)).to eq(1)
expect(limit_reached_for(user.id, dt)).to eq(false)
GivenDailyLike.increment_for(user.id)
expect(value_for(user.id, dt)).to eq(2)
expect(limit_reached_for(user.id, dt)).to eq(true)
GivenDailyLike.decrement_for(user.id)
expect(value_for(user.id, dt)).to eq(1)
expect(limit_reached_for(user.id, dt)).to eq(false)
GivenDailyLike.decrement_for(user.id)
expect(value_for(user.id, dt)).to eq(0)
expect(limit_reached_for(user.id, dt)).to eq(false)
end
end
end
end

View File

@ -168,19 +168,24 @@ describe PostAction do
describe "update_counters" do
it "properly updates topic counters" do
# we need this to test it
TopicUser.change(codinghorror, post.topic, posted: true)
Timecop.freeze(Date.today) do
# we need this to test it
TopicUser.change(codinghorror, post.topic, posted: true)
PostAction.act(moderator, post, PostActionType.types[:like])
PostAction.act(codinghorror, second_post, PostActionType.types[:like])
expect(GivenDailyLike.value_for(moderator.id, Date.today)).to eq(0)
post.topic.reload
expect(post.topic.like_count).to eq(2)
PostAction.act(moderator, post, PostActionType.types[:like])
PostAction.act(codinghorror, second_post, PostActionType.types[:like])
tu = TopicUser.get(post.topic, codinghorror)
expect(tu.liked).to be true
expect(tu.bookmarked).to be false
post.topic.reload
expect(post.topic.like_count).to eq(2)
expect(GivenDailyLike.value_for(moderator.id, Date.today)).to eq(1)
tu = TopicUser.get(post.topic, codinghorror)
expect(tu.liked).to be true
expect(tu.bookmarked).to be false
end
end
end
@ -239,29 +244,33 @@ describe PostAction do
end
it 'should increase the `like_count` and `like_score` when a user likes something' do
PostAction.act(codinghorror, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(1)
expect(post.like_score).to eq(1)
post.topic.reload
expect(post.topic.like_count).to eq(1)
Timecop.freeze(Date.today) do
PostAction.act(codinghorror, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(1)
expect(post.like_score).to eq(1)
post.topic.reload
expect(post.topic.like_count).to eq(1)
expect(GivenDailyLike.value_for(codinghorror.id, Date.today)).to eq(1)
# When a staff member likes it
PostAction.act(moderator, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(2)
expect(post.like_score).to eq(4)
# When a staff member likes it
PostAction.act(moderator, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(2)
expect(post.like_score).to eq(4)
# Removing likes
PostAction.remove_act(codinghorror, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(1)
expect(post.like_score).to eq(3)
# Removing likes
PostAction.remove_act(codinghorror, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(1)
expect(post.like_score).to eq(3)
expect(GivenDailyLike.value_for(codinghorror.id, Date.today)).to eq(0)
PostAction.remove_act(moderator, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(0)
expect(post.like_score).to eq(0)
PostAction.remove_act(moderator, post, PostActionType.types[:like])
post.reload
expect(post.like_count).to eq(0)
expect(post.like_score).to eq(0)
end
end
end