AO3-7264 Allow different rate limits based on the current user #5543
Conversation
| return false unless action_name == "create" || action_name == "update" | ||
|
|
||
| return false unless logged_in? # Guest comment rate limits are not handled here | ||
|
|
||
| parent = find_parent | ||
| return false if parent.is_a?(Tag) || !current_user.should_spam_check_comments? | ||
|
|
||
| !current_user.is_author_of?(parent) |
There was a problem hiding this comment.
I would be very happy about thoughts on how to format all these conditions better
| def should_rate_limit | ||
| return false unless action_name == "create" || action_name == "update" | ||
|
|
||
| return false unless logged_in? # Guest comment rate limits are not handled here |
There was a problem hiding this comment.
I would try and be positive.
return false if logged_out?
There was a problem hiding this comment.
We also need to return false for admins
There was a problem hiding this comment.
return false if logged_out? || logged_in_as_admin_or_whatever?
There was a problem hiding this comment.
aren't admins blocked from commenting in most cases? "Please log out of your admin account to comment." appears for me on tags and works; most news posts are locked to guests so show that banner instead
There was a problem hiding this comment.
My opinion would be:
def should_rate_limit
return false unless action_name == "create" || action_name == "update"
return false unless logged_in? # Guest comment rate limits are not handled here
return false unless current_user.should_spam_check_comments?
parent = find_parent
return false if parent.is_a?(Tag)
return false if current_user.is_author_of?(parent)
return true
endI think grouping the unless and if checks separately makes it read better.
There was a problem hiding this comment.
aren't admins blocked from commenting in most cases?
Yes, but I am not entirely sure whether we're guaranteed to do that check before we run this code here. We should be, but I'd rather be safe than sorry
| @@ -0,0 +1,4 @@ | |||
| <h2 class="heading">Error 429</h2> | |||
There was a problem hiding this comment.
yeah, I completely forgot
4 participants
Issue
https://otwarchive.atlassian.net/browse/AO3-7264
Purpose
Use Rails' new rate limiting to apply commenting rate limits based on the current user.
Credit
Bilka