[tor-bugs] #1860 [BridgeDB]: bridgedb email rate limiting needs review

[Tor Bug Tracker & Wiki]

#1860: bridgedb email rate limiting needs review
----------------------+-----------------------------------------------------
 Reporter:  aagbsn    |       Owner:  nickm                    
     Type:  task      |      Status:  needs_review             
 Priority:  normal    |   Milestone:  BridgeDB Upgrades Phase 1
Component:  BridgeDB  |     Version:                           
 Keywords:            |      Parent:                           
----------------------+-----------------------------------------------------

Comment(by nickm):

 Looks good: some issues...

   * It's okay to use "git rebase -i" to squash patch series that nobody
 has looked at before sending them for review.

   * It looks like when you're the EMAIL_MESSAGE_RATELIMIT message, you
 tell the user that the rate limit is a number of seconds.  Most non-
 programmers won't automatically assume that an integer like this is a
 number of seconds; probably the message should say something more like
 "the minimum time between emails is %d seconds" rather than just "the
 minimum time between emails is %d".

   * You're implementing booleans in the db in a slightly odd way; it looks
 like you insert the value "True" to indicate true in setWasWarned, but you
 treat _any_ value as indicating truth in getWasWarned.

   * Also in the sql code, WarnedEmails is a better table name than
 WasWarned.  If we have more than one warning in the future, WasWarned will
 be a downright odd table name.

   * It looks like warnings stay in the table forever unless the user later
 sends a message that _doesn't_ get a warning, and the database gets a
 complete list of all the email addresses it ever sent a warning to without
 later sending them a non-warning message.  That can't be right, can it?

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/1860#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online