From 82a5cd7d08a6d81a00f15156f548762895138757 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 08:47:16 -0300
Subject: [PATCH 01/33] repeating function for check if bans need a review

---
 Bantracker/plugin.py | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 834ead1..ad5c381 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -48,6 +48,7 @@ import supybot.callbacks as callbacks
 import supybot.ircmsgs as ircmsgs
 import supybot.conf as conf
 import supybot.ircdb as ircdb
+import supybot.schedule as schedule
 from fnmatch import fnmatch
 import sqlite
 import pytz
@@ -187,6 +188,9 @@ class Bantracker(callbacks.Plugin):
             self.db = None
         self.get_bans(irc)
         self.get_nicks(irc)
+        # schedule
+        schedule.addPeriodicEvent(lambda : self.reviewBans(irc), 20,
+                name=self.name)
 
     def get_nicks(self, irc):
         self.hosts.clear()
@@ -293,6 +297,10 @@ class Bantracker(callbacks.Plugin):
         except:
             pass
         queue.clear()
+        try:
+            schedule.removeEvent(self.name)
+        except:
+            pass
 
     def reset(self):
         global queue
@@ -372,6 +380,29 @@ class Bantracker(callbacks.Plugin):
                 %(type, mask, channel, prefix, ban.id)
         irc.reply(s, to=ban.who, private=True)
 
+    def reviewBans(self, irc):
+        self.log.debug('Checking for bans that need review ...')
+        now = time.mktime(time.gmtime())
+        try:
+            for channel, bans in self.bans.iteritems():
+                for ban in bans:
+                    age = now - ban.when
+                    self.log.debug('  channel %s ban %s (%s)', channel, ban.mask, age)
+                    # FIXME doesn't check if op is not online
+                    # FIXME doesn't mark if the review was sent
+                    if age > 120: # lets use mins for now
+                        op = ban.who
+                        op = op[:op.find('!')]
+                        s = "Please review ban '%s' in %s" %(ban.mask, channel)
+                        msg = ircmsgs.privmsg(op, s)
+                        irc.queueMsg(msg)
+                    else:
+                        # the bans left are even more recent
+                        break
+        except Exception, e:
+            # I need to catch exceptions as they are silenced
+            self.log.error('Except: %s' %e)
+
     def doLog(self, irc, channel, s):
         if not self.registryValue('enabled', channel):
             return

From 7c042b4a799ed6f13a38b61810d4f8890185c89d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 13:32:28 -0300
Subject: [PATCH 02/33] use a config option for store when we last checked for
 ban review. That way we can skip bans already reviewed.

---
 Bantracker/config.py |  6 ++++++
 Bantracker/plugin.py | 28 ++++++++++++++++++----------
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index 2a0bc34..32bcd78 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -52,3 +52,9 @@ conf.registerChannelValue(Bantracker.commentRequest.forward, 'channels',
             "List of channels/nicks to forward the request if the op that set the ban/quiet"\
             " is in the forward list."))
 
+
+# temp config
+conf.registerGlobalValue(Bantracker, 'reviewTime',
+        registry.Integer(0, "", ))
+conf.registerGlobalValue(Bantracker, 'reviewAfterTime',
+        registry.Integer(2, "", ))
diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index ad5c381..32c5b8f 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -381,27 +381,35 @@ class Bantracker(callbacks.Plugin):
         irc.reply(s, to=ban.who, private=True)
 
     def reviewBans(self, irc):
-        self.log.debug('Checking for bans that need review ...')
-        now = time.mktime(time.gmtime())
         try:
+            self.log.debug('Checking for bans that need review ...')
+            now = time.mktime(time.gmtime())
+            lastreview = self.registryValue('reviewTime')
+            reviewAfterTime = self.registryValue('reviewAfterTime') * 60 # time in mins
+            if not lastreview:
+                lastreview = now
             for channel, bans in self.bans.iteritems():
                 for ban in bans:
-                    age = now - ban.when
-                    self.log.debug('  channel %s ban %s (%s)', channel, ban.mask, age)
-                    # FIXME doesn't check if op is not online
-                    # FIXME doesn't mark if the review was sent
-                    if age > 120: # lets use mins for now
+                    banTime = now - ban.when
+                    reviewTime = lastreview - ban.when
+                    self.log.debug('  channel %s ban %s (%s/%s/%s)', channel, ban.mask, reviewTime,
+                            reviewAfterTime, banTime)
+                    if reviewTime <= reviewAfterTime < banTime:
                         op = ban.who
-                        op = op[:op.find('!')]
+                        # ban.who can be a nick or IRC hostmask
+                        if ircutils.isUserHostmask(op):
+                            op = op[:op.find('!')]
                         s = "Please review ban '%s' in %s" %(ban.mask, channel)
+                        # FIXME doesn't check if op is not online
                         msg = ircmsgs.privmsg(op, s)
                         irc.queueMsg(msg)
-                    else:
+                    elif banTime < reviewAfterTime:
                         # the bans left are even more recent
                         break
+            self.setRegistryValue('reviewTime', now) # update last time reviewed
         except Exception, e:
             # I need to catch exceptions as they are silenced
-            self.log.error('Except: %s' %e)
+            self.log.debug('Except: %s' %e)
 
     def doLog(self, irc, channel, s):
         if not self.registryValue('enabled', channel):

From 5dfe161bac27a1b4690acb097aaefe86a438ebb0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 14:02:53 -0300
Subject: [PATCH 03/33] tweak reviewTime initialisation and comments++

---
 Bantracker/plugin.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 32c5b8f..beed1ae 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -188,7 +188,7 @@ class Bantracker(callbacks.Plugin):
             self.db = None
         self.get_bans(irc)
         self.get_nicks(irc)
-        # schedule
+        # add scheduled event for check bans that need review
         schedule.addPeriodicEvent(lambda : self.reviewBans(irc), 20,
                 name=self.name)
 
@@ -387,7 +387,8 @@ class Bantracker(callbacks.Plugin):
             lastreview = self.registryValue('reviewTime')
             reviewAfterTime = self.registryValue('reviewAfterTime') * 60 # time in mins
             if not lastreview:
-                lastreview = now
+                # initialize last time reviewed timestamp
+                lastreview = now - reviewAfterTime
             for channel, bans in self.bans.iteritems():
                 for ban in bans:
                     banTime = now - ban.when
@@ -395,6 +396,7 @@ class Bantracker(callbacks.Plugin):
                     self.log.debug('  channel %s ban %s (%s/%s/%s)', channel, ban.mask, reviewTime,
                             reviewAfterTime, banTime)
                     if reviewTime <= reviewAfterTime < banTime:
+                        # ban is old enough, and inside the "review window"
                         op = ban.who
                         # ban.who can be a nick or IRC hostmask
                         if ircutils.isUserHostmask(op):

From 6b3a33ce4c7e8baef84f6e0f5e539e2f0e990621 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 14:56:35 -0300
Subject: [PATCH 04/33] instead of sending reviews immediately, enqueue them
 and send then the next time the op says something, that way not only we make
 sure op is online, but active as well.

---
 Bantracker/plugin.py | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index beed1ae..abaf08f 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -189,8 +189,9 @@ class Bantracker(callbacks.Plugin):
         self.get_bans(irc)
         self.get_nicks(irc)
         # add scheduled event for check bans that need review
-        schedule.addPeriodicEvent(lambda : self.reviewBans(irc), 20,
+        schedule.addPeriodicEvent(self.reviewBans, 20,
                 name=self.name)
+        self.pendingReviews = ircutils.IrcDict()
 
     def get_nicks(self, irc):
         self.hosts.clear()
@@ -380,7 +381,7 @@ class Bantracker(callbacks.Plugin):
                 %(type, mask, channel, prefix, ban.id)
         irc.reply(s, to=ban.who, private=True)
 
-    def reviewBans(self, irc):
+    def reviewBans(self):
         try:
             self.log.debug('Checking for bans that need review ...')
             now = time.mktime(time.gmtime())
@@ -401,10 +402,19 @@ class Bantracker(callbacks.Plugin):
                         # ban.who can be a nick or IRC hostmask
                         if ircutils.isUserHostmask(op):
                             op = op[:op.find('!')]
+                        elif not ircutils.isNick(op, strictRfc=True):
+                            # probably a ban restored by IRC server
+                            continue
+                        if nickMatch(op, self.registryValue('commentRequest.ignore'):
+                            # in the ignore list
+                            continue
                         s = "Please review ban '%s' in %s" %(ban.mask, channel)
-                        # FIXME doesn't check if op is not online
                         msg = ircmsgs.privmsg(op, s)
-                        irc.queueMsg(msg)
+                        self.log.debug('  adding ban to the pending review list ...')
+                        if op in self.pendingReviews:
+                            self.pendingReviews[op].append(msg)
+                        else:
+                            self.pendingReviews[op] = [msg]
                     elif banTime < reviewAfterTime:
                         # the bans left are even more recent
                         break
@@ -413,6 +423,13 @@ class Bantracker(callbacks.Plugin):
             # I need to catch exceptions as they are silenced
             self.log.debug('Except: %s' %e)
 
+    def _sendReviews(self, irc, msg):
+        if msg.nick in self.pendingReviews:
+            op = msg.nick
+            for m in self.pendingReviews[op]:
+                irc.queueMsg(m)
+            del self.pendingReviews[op]
+
     def doLog(self, irc, channel, s):
         if not self.registryValue('enabled', channel):
             return
@@ -470,6 +487,7 @@ class Bantracker(callbacks.Plugin):
                                '* %s %s\n' % (nick, ircmsgs.unAction(msg)))
                 else:
                     self.doLog(irc, channel, '<%s> %s\n' % (nick, text))
+        self._sendReviews(irc, msg)
 
     def doNotice(self, irc, msg):
         (recipients, text) = msg.args

From e60e72a6204dc1ea7e88a82fb46f79c4b759e244 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 16:22:10 -0300
Subject: [PATCH 05/33] fix: - self.name => self.name() - syntax error

---
 Bantracker/plugin.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index abaf08f..eb9fd26 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -190,7 +190,7 @@ class Bantracker(callbacks.Plugin):
         self.get_nicks(irc)
         # add scheduled event for check bans that need review
         schedule.addPeriodicEvent(self.reviewBans, 20,
-                name=self.name)
+                name=self.name())
         self.pendingReviews = ircutils.IrcDict()
 
     def get_nicks(self, irc):
@@ -299,7 +299,7 @@ class Bantracker(callbacks.Plugin):
             pass
         queue.clear()
         try:
-            schedule.removeEvent(self.name)
+            schedule.removeEvent(self.name())
         except:
             pass
 
@@ -405,7 +405,7 @@ class Bantracker(callbacks.Plugin):
                         elif not ircutils.isNick(op, strictRfc=True):
                             # probably a ban restored by IRC server
                             continue
-                        if nickMatch(op, self.registryValue('commentRequest.ignore'):
+                        if nickMatch(op, self.registryValue('commentRequest.ignore')):
                             # in the ignore list
                             continue
                         s = "Please review ban '%s' in %s" %(ban.mask, channel)

From 3dd2f09769a7f6fd82b906fcf92940c44b189458 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 17:13:00 -0300
Subject: [PATCH 06/33] reviewAfterTime is now stored in seconds, but when
 setting the value the unit is days.

This saves the user for calculating how many seconds a day is, while allowing
me to set times of some seconds for automatic testing.
---
 Bantracker/config.py | 18 +++++++++++++++++-
 Bantracker/plugin.py |  5 ++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index 32bcd78..5d6ab91 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -23,6 +23,20 @@ class SpaceSeparatedListOfTypes(registry.SpaceSeparatedListOf):
     Value = ValidTypes
 
 
+# This registry translates days to seconds
+# storing seconds instead of days is more convenient for testing
+class DaysToSeconds(registry.Integer):
+    """Value must be an integer and not higher than 100"""
+    def set(self, s):
+        try:
+            n = int(s)
+            if n > 100:
+                raise ValueError
+            self.setValue(n*84600)
+        except ValueError:
+            self.error()
+
+
 def configure(advanced):
     conf.registerPlugin('Bantracker', True)
 
@@ -57,4 +71,6 @@ conf.registerChannelValue(Bantracker.commentRequest.forward, 'channels',
 conf.registerGlobalValue(Bantracker, 'reviewTime',
         registry.Integer(0, "", ))
 conf.registerGlobalValue(Bantracker, 'reviewAfterTime',
-        registry.Integer(2, "", ))
+        DaysToSeconds(7*84600,
+            "Days after which the bot will request for review a ban. NOTE: the number of days is"
+            " stored in seconds, but when configuring it the time unit is in days."))
diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index eb9fd26..38891ac 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -383,10 +383,13 @@ class Bantracker(callbacks.Plugin):
 
     def reviewBans(self):
         try:
+            reviewAfterTime = self.registryValue('reviewAfterTime')
+            if not reviewAfterTime:
+                # time is zero, do nothing
+                return
             self.log.debug('Checking for bans that need review ...')
             now = time.mktime(time.gmtime())
             lastreview = self.registryValue('reviewTime')
-            reviewAfterTime = self.registryValue('reviewAfterTime') * 60 # time in mins
             if not lastreview:
                 # initialize last time reviewed timestamp
                 lastreview = now - reviewAfterTime

From 0eb45f782243b8cf3431385149d06697065f0579 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Tue, 30 Mar 2010 17:13:51 -0300
Subject: [PATCH 07/33] TestCases for comment request and review request.

run then with "supybot-test Bantracker"
---
 Bantracker/test.py | 55 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/Bantracker/test.py b/Bantracker/test.py
index 49ca8d5..f4f1282 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -1,4 +1,57 @@
 from supybot.test import *
 
-class BantrackerTestCase(PluginTestCase):
+import supybot.conf as conf
+import supybot.ircmsgs as ircmsgs
+import supybot.schedule as schedule
+
+import time
+
+
+bConf = conf.supybot.plugins.Bantracker
+bConf.enabled.setValue(True)
+
+
+class BantrackerTestCase(ChannelPluginTestCase):
     plugins = ('Bantracker',)
+
+    def getCallback(self):
+        for cb in self.irc.callbacks:
+            if cb.name() == 'Bantracker':
+                break
+        return cb
+
+    def testCommentRequest(self):
+        ban = ircmsgs.ban('#test', 'asd!*@*', prefix='op!user@host.net')
+        self.irc.feedMsg(ban)
+        msg = self.irc.takeMsg()
+        # ban id is None is because there's no database for this TestCase
+        self.assertEqual(str(msg).strip(), 
+            "PRIVMSG op :Please comment on the ban of asd!*@* in #test, use: @comment None"
+            " <comment>")
+
+    def testReviewResquest(self):
+        cb = self.getCallback()
+        ban = ircmsgs.ban('#test', 'asd!*@*', prefix='op!user@host.net')
+        self.irc.feedMsg(ban)
+        self.irc.takeMsg() # ignore comment request comment
+        bConf.reviewAfterTime.setValue(1)
+        cb.reviewBans()
+        self.assertFalse(cb.pendingReviews)
+        print 'waiting 2 secs..'
+        time.sleep(2)
+        cb.reviewBans()
+        # check is pending
+        self.assertTrue(cb.pendingReviews)
+        # check msg if op and only op says something
+        self.feedMsg('Hi!', to='#test', frm='dude!user@host.net') 
+        msg = self.irc.takeMsg()
+        self.assertEqual(msg, None)
+        self.feedMsg('Hi!', to='#test', frm='op!user@host.net') 
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(),
+                "PRIVMSG op :Please review ban 'asd!*@*' in #test")
+        # don't ask again
+        cb.reviewBans()
+        self.assertFalse(cb.pendingReviews)
+
+

From 1807883da27ce799b3d5dee2af4cc9bc7674cfd4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Wed, 31 Mar 2010 00:03:13 -0300
Subject: [PATCH 08/33] Use floats in reviewAfterTime, this is less confusing.
 And make reviewTime option readonly

---
 Bantracker/config.py | 27 ++++++++++++++++-----------
 Bantracker/plugin.py |  2 +-
 Bantracker/test.py   |  2 +-
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index 5d6ab91..4da64c4 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -23,16 +23,21 @@ class SpaceSeparatedListOfTypes(registry.SpaceSeparatedListOf):
     Value = ValidTypes
 
 
-# This registry translates days to seconds
-# storing seconds instead of days is more convenient for testing
-class DaysToSeconds(registry.Integer):
-    """Value must be an integer and not higher than 100"""
+# This little hack allows me to store last time bans were checked for review
+# and guarantee that nobody will edit it.
+# I'm using a registry option instead of the SQL db because this is much simpler.
+class ReadOnlyValue(registry.Integer):
+    """This is a read only option."""
+    def __init__(self, *args, **kwargs):
+        super(ReadOnlyValue, self).__init__(*args, **kwargs)
+        self.value = None
+
     def set(self, s):
         try:
-            n = int(s)
-            if n > 100:
+            if not self.value:
+                self.setValue(int(s))
+            else:
                 raise ValueError
-            self.setValue(n*84600)
         except ValueError:
             self.error()
 
@@ -69,8 +74,8 @@ conf.registerChannelValue(Bantracker.commentRequest.forward, 'channels',
 
 # temp config
 conf.registerGlobalValue(Bantracker, 'reviewTime',
-        registry.Integer(0, "", ))
+        ReadOnlyValue(0,
+            "Timestamp used internally for identify bans that need review. Can't and shouldn't be edited."))
 conf.registerGlobalValue(Bantracker, 'reviewAfterTime',
-        DaysToSeconds(7*84600,
-            "Days after which the bot will request for review a ban. NOTE: the number of days is"
-            " stored in seconds, but when configuring it the time unit is in days."))
+        registry.Float(7,
+            "Days after which the bot will request for review a ban. Can be an integer or decimal value."))
diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 38891ac..0830003 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -383,7 +383,7 @@ class Bantracker(callbacks.Plugin):
 
     def reviewBans(self):
         try:
-            reviewAfterTime = self.registryValue('reviewAfterTime')
+            reviewAfterTime = int(self.registryValue('reviewAfterTime') * 84600)
             if not reviewAfterTime:
                 # time is zero, do nothing
                 return
diff --git a/Bantracker/test.py b/Bantracker/test.py
index f4f1282..c3e1a35 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -34,7 +34,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
         ban = ircmsgs.ban('#test', 'asd!*@*', prefix='op!user@host.net')
         self.irc.feedMsg(ban)
         self.irc.takeMsg() # ignore comment request comment
-        bConf.reviewAfterTime.setValue(1)
+        bConf.reviewAfterTime.setValue(1.0/84600) # one second
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
         print 'waiting 2 secs..'

From a7447c0bef78665b3dd34485517c775b5f4fafb8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Thu, 1 Apr 2010 12:39:46 -0300
Subject: [PATCH 09/33] if not dabatase is set, db_run would return None and
 raise exception. fixed

---
 Bantracker/plugin.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 0830003..9313b0f 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -424,7 +424,9 @@ class Bantracker(callbacks.Plugin):
             self.setRegistryValue('reviewTime', now) # update last time reviewed
         except Exception, e:
             # I need to catch exceptions as they are silenced
-            self.log.debug('Except: %s' %e)
+            import traceback
+            self.log.error('Except: %s' %e)
+            self.log.error(traceback.format_exc())
 
     def _sendReviews(self, irc, msg):
         if msg.nick in self.pendingReviews:
@@ -726,8 +728,10 @@ class Bantracker(callbacks.Plugin):
         return mutes + bans
 
     def get_banId(self, mask, channel):
-        data = self.db_run("SELECT MAX(id) FROM bans WHERE mask=%s AND channel=%s", (mask, channel), True)[0]
-        if not data[0]:
+        data = self.db_run("SELECT MAX(id) FROM bans WHERE mask=%s AND channel=%s", (mask, channel), True)
+        if data:
+            data = data[0]
+        if not data or not data[0]:
             return
         return int(data[0])
 

From bfd6a055c1d8169892f18a6d3c4bdc5f40b3b34b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Thu, 1 Apr 2010 12:47:56 -0300
Subject: [PATCH 10/33] complete string message

---
 Bantracker/plugin.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 9313b0f..a4274e7 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -411,7 +411,11 @@ class Bantracker(callbacks.Plugin):
                         if nickMatch(op, self.registryValue('commentRequest.ignore')):
                             # in the ignore list
                             continue
-                        s = "Please review ban '%s' in %s" %(ban.mask, channel)
+                        if not ban.id:
+                            ban.id = self.get_banId(ban.mask, channel)
+                        s = "Hi, please review the ban '%s' that you set on %s in %s,"\
+                        " link: %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel,
+                                self.registryValue('bansite'), ban.id)
                         msg = ircmsgs.privmsg(op, s)
                         self.log.debug('  adding ban to the pending review list ...')
                         if op in self.pendingReviews:

From 62ee6cf346f8c57db056076c39e3ba50cf5877d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Thu, 1 Apr 2010 12:48:19 -0300
Subject: [PATCH 11/33] skip mutes

---
 Bantracker/plugin.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index a4274e7..63eab3d 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -395,9 +395,12 @@ class Bantracker(callbacks.Plugin):
                 lastreview = now - reviewAfterTime
             for channel, bans in self.bans.iteritems():
                 for ban in bans:
+                    if ban.mask[0] == '%':
+                        # skip mutes
+                        continue
                     banTime = now - ban.when
                     reviewTime = lastreview - ban.when
-                    self.log.debug('  channel %s ban %s (%s/%s/%s)', channel, ban.mask, reviewTime,
+                    self.log.debug('  channel %s ban %s (%s/%s/%s)', channel, str(ban), reviewTime,
                             reviewAfterTime, banTime)
                     if reviewTime <= reviewAfterTime < banTime:
                         # ban is old enough, and inside the "review window"

From 6ec87d68ddef1e9d36a03309406fbfff3323c3a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Thu, 1 Apr 2010 16:34:59 -0300
Subject: [PATCH 12/33] more testcases for BanTracker

---
 Bantracker/test.py | 107 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 94 insertions(+), 13 deletions(-)

diff --git a/Bantracker/test.py b/Bantracker/test.py
index c3e1a35..d67020c 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -2,39 +2,108 @@ from supybot.test import *
 
 import supybot.conf as conf
 import supybot.ircmsgs as ircmsgs
-import supybot.schedule as schedule
 
 import time
 
 
-bConf = conf.supybot.plugins.Bantracker
-bConf.enabled.setValue(True)
+pluginConf = conf.supybot.plugins.Bantracker
+pluginConf.enabled.setValue(True)
+pluginConf.bansite.setValue('http://foo.bar.com')
+pluginConf.database.setValue('bantracker-test.db')
 
+def quiet(channel, hostmask, prefix='', msg=None):
+    """Returns a MODE to quiet nick on channel."""
+    return ircmsgs.mode(channel, ('+q', hostmask), prefix, msg)
 
 class BantrackerTestCase(ChannelPluginTestCase):
     plugins = ('Bantracker',)
 
+    def setUp(self):
+        super(BantrackerTestCase, self).setUp()
+        self.setDb()
+        pluginConf.commentRequest.ignore.set('*') # disable comments
+
+    def setDb(self):
+        import sqlite, os
+        dbfile = os.path.join(os.curdir, pluginConf.database())
+        try:
+            os.remove(dbfile)
+        except:
+            pass
+        db = sqlite.connect(dbfile)
+        cursor = db.cursor()
+        cursor.execute('CREATE TABLE bans ('
+                'id INTEGER PRIMARY KEY,'
+                'channel VARCHAR(30) NOT NULL,'
+                'mask VARCHAR(100) NOT NULL,'
+                'operator VARCHAR(30) NOT NULL,'
+                'time VARCHAR(300) NOT NULL,'
+                'removal DATETIME,'
+                'removal_op VARCHAR(30),'
+                'log TEXT)')
+        cursor.execute('CREATE TABLE comments ('
+                'ban_id INTEGER,'
+                'who VARCHAR(100) NOT NULL,'
+                'comment MEDIUMTEXT NOT NULL,'
+                'time VARCHAR(300) NOT NULL)')
+        cursor.execute('CREATE TABLE sessions ('
+                'session_id VARCHAR(50) PRIMARY KEY,'
+                'user MEDIUMTEXT NOT NULL,'
+                'time INT NOT NULL)')
+        cursor.execute('CREATE TABLE users ('
+                'username VARCHAR(50) PRIMARY KEY,'
+                'salt VARCHAR(8),'
+                'password VARCHAR(50))')
+        db.commit()
+        cursor.close()
+        db.close()
+
     def getCallback(self):
         for cb in self.irc.callbacks:
             if cb.name() == 'Bantracker':
                 break
         return cb
 
-    def testCommentRequest(self):
-        ban = ircmsgs.ban('#test', 'asd!*@*', prefix='op!user@host.net')
+    def getDb(self):
+        return self.getCallback().db
+
+    def query(self, query, parms=()):
+        cursor = self.getDb().cursor()
+        cursor.execute(query, parms)
+        return cursor.fetchall()
+
+    def feedBan(self, hostmask, prefix='', channel=None, mode='b'):
+        if not channel:
+            channel = self.channel
+        if not prefix:
+            prefix = 'op!user@host.net'
+        if mode == 'b':
+            ban = ircmsgs.ban(channel, hostmask, prefix=prefix)
+        elif mode == 'q':
+            ban = quiet(channel, hostmask, prefix=prefix)
         self.irc.feedMsg(ban)
+
+    def testCommentRequest(self):
+        pluginConf.commentRequest.ignore.set('')
+        # test bans
+        self.feedBan('asd!*@*')
         msg = self.irc.takeMsg()
-        # ban id is None is because there's no database for this TestCase
         self.assertEqual(str(msg).strip(), 
-            "PRIVMSG op :Please comment on the ban of asd!*@* in #test, use: @comment None"
+            "PRIVMSG op :Please comment on the ban of asd!*@* in #test, use: @comment 1"
+            " <comment>")
+        # test quiets
+        self.feedBan('dude!*@*', mode='q')
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(), 
+            "PRIVMSG op :Please comment on the quiet of dude!*@* in #test, use: @comment 2"
             " <comment>")
 
     def testReviewResquest(self):
+        pluginConf.commentRequest.ignore.set('')
         cb = self.getCallback()
-        ban = ircmsgs.ban('#test', 'asd!*@*', prefix='op!user@host.net')
-        self.irc.feedMsg(ban)
+        self.feedBan('asd!*@*')
         self.irc.takeMsg() # ignore comment request comment
-        bConf.reviewAfterTime.setValue(1.0/84600) # one second
+        pluginConf.reviewAfterTime.setValue(1.0/84600) # one second
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
         print 'waiting 2 secs..'
@@ -43,15 +112,27 @@ class BantrackerTestCase(ChannelPluginTestCase):
         # check is pending
         self.assertTrue(cb.pendingReviews)
         # check msg if op and only op says something
-        self.feedMsg('Hi!', to='#test', frm='dude!user@host.net') 
+        self.feedMsg('Hi!', frm='dude!user@host.net') 
         msg = self.irc.takeMsg()
         self.assertEqual(msg, None)
-        self.feedMsg('Hi!', to='#test', frm='op!user@host.net') 
+        self.feedMsg('Hi!', frm='op!user@host.net') 
         msg = self.irc.takeMsg()
         self.assertEqual(str(msg).strip(),
-                "PRIVMSG op :Please review ban 'asd!*@*' in #test")
+            "PRIVMSG op :Hi, please review the ban 'asd!*@*' that you set on %s in #test, link: "\
+            "%s/bans.cgi?log=1" %(cb.bans['#test'][-1].ascwhen, pluginConf.bansite()))
         # don't ask again
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
 
+    def testBan(self):
+        self.feedBan('asd!*@*')
+        fetch = self.query("SELECT id,channel,mask,operator FROM bans")
+        self.assertEqual((1, '#test', 'asd!*@*', 'op'), fetch[0])
+
+    def testQuiet(self):
+        self.feedBan('asd!*@*', mode='q')
+        fetch = self.query("SELECT id,channel,mask,operator FROM bans")
+        self.assertEqual((1, '#test', '%asd!*@*', 'op'), fetch[0])
+
+
 

From e0c327cc0cc991f1deecd847ef45ef98e5400178 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Thu, 1 Apr 2010 17:52:22 -0300
Subject: [PATCH 13/33] match operators by cloak/host instead of nick

---
 Bantracker/plugin.py | 87 ++++++++++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 63eab3d..66ad23b 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -160,6 +160,14 @@ class Ban(object):
     def time(self):
         return datetime.datetime.fromtimestamp(self.when)
 
+def guessBanType(mask):
+    if mask[0] == '%':
+        return 'quiet'
+    elif ircutils.isUserHostmask(mask) or mask.endswith('(realname)'):
+        return 'ban'
+    return 'removal'
+
+
 class Bantracker(callbacks.Plugin):
     """Plugin to manage bans.
        See '@list Bantracker' and '@help <command>' for commands"""
@@ -327,6 +335,7 @@ class Bantracker(callbacks.Plugin):
             self.lastMsgs[irc] = msg
 
     def db_run(self, query, parms, expect_result = False, expect_id = False):
+        self.log.debug("SQL: %q %s", query, parms)
         if not self.db:
             self.log.error("Bantracker database not open")
             return
@@ -334,7 +343,8 @@ class Bantracker(callbacks.Plugin):
         try:
             cur = self.db.cursor()
             cur.execute(query, parms)
-        except:
+        except Exception, e:
+            self.log.error('error: %s', e)
             cur = None
             if n_tries > 5:
                 print "Tried more than 5 times, aborting"
@@ -345,6 +355,7 @@ class Bantracker(callbacks.Plugin):
         if expect_result and cur: data = cur.fetchall()
         if expect_id: data = self.db.insert_id()
         self.db.commit()
+        self.log.debug("SQL return: %q", data)
         return data
 
     def requestComment(self, irc, channel, ban, type=None):
@@ -354,13 +365,9 @@ class Bantracker(callbacks.Plugin):
         # check the type of the action taken
         mask = ban.mask
         if not type:
-            if mask[0] == '%':
-                type = 'quiet'
+            type = guessBanType(mask)
+            if type == 'quiet':
                 mask = mask[1:]
-            elif ircutils.isUserHostmask(mask) or mask.endswith('(realname)'):
-                type = 'ban'
-            else:
-                type = 'removal'
         # check if type is enabled
         if type not in self.registryValue('commentRequest.type', channel=channel):
             return
@@ -379,7 +386,11 @@ class Bantracker(callbacks.Plugin):
         # send to op
         s = "Please comment on the %s of %s in %s, use: %scomment %s <comment>" \
                 %(type, mask, channel, prefix, ban.id)
-        irc.reply(s, to=ban.who, private=True)
+        try:
+            op = ircutils.nickFromHostmask(ban.who)
+        except:
+            op = ban.who
+        irc.reply(s, to=op, private=True)
 
     def reviewBans(self):
         try:
@@ -395,8 +406,9 @@ class Bantracker(callbacks.Plugin):
                 lastreview = now - reviewAfterTime
             for channel, bans in self.bans.iteritems():
                 for ban in bans:
-                    if ban.mask[0] == '%':
-                        # skip mutes
+                    type = guessBanType(ban.mask)
+                    if type in ('quiet', 'removal'):
+                        # skip mutes and kicks
                         continue
                     banTime = now - ban.when
                     reviewTime = lastreview - ban.when
@@ -404,12 +416,13 @@ class Bantracker(callbacks.Plugin):
                             reviewAfterTime, banTime)
                     if reviewTime <= reviewAfterTime < banTime:
                         # ban is old enough, and inside the "review window"
-                        op = ban.who
-                        # ban.who can be a nick or IRC hostmask
-                        if ircutils.isUserHostmask(op):
-                            op = op[:op.find('!')]
-                        elif not ircutils.isNick(op, strictRfc=True):
-                            # probably a ban restored by IRC server
+                        try:
+                            # ban.who should be a user hostmask
+                            op = ircutils.nickFromHostmask(ban.who)
+                            host = ircutils.hostFromHostmask(ban.who)
+                        except:
+                            # probably a ban restored by IRC server in a netsplit
+                            # XXX see if something can be done about this
                             continue
                         if nickMatch(op, self.registryValue('commentRequest.ignore')):
                             # in the ignore list
@@ -419,12 +432,11 @@ class Bantracker(callbacks.Plugin):
                         s = "Hi, please review the ban '%s' that you set on %s in %s,"\
                         " link: %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel,
                                 self.registryValue('bansite'), ban.id)
-                        msg = ircmsgs.privmsg(op, s)
                         self.log.debug('  adding ban to the pending review list ...')
-                        if op in self.pendingReviews:
-                            self.pendingReviews[op].append(msg)
+                        if host in self.pendingReviews:
+                            self.pendingReviews[host].append((op, s))
                         else:
-                            self.pendingReviews[op] = [msg]
+                            self.pendingReviews[host] = [(op, s)]
                     elif banTime < reviewAfterTime:
                         # the bans left are even more recent
                         break
@@ -435,12 +447,22 @@ class Bantracker(callbacks.Plugin):
             self.log.error('Except: %s' %e)
             self.log.error(traceback.format_exc())
 
-    def _sendReviews(self, irc, msg):
-        if msg.nick in self.pendingReviews:
-            op = msg.nick
-            for m in self.pendingReviews[op]:
-                irc.queueMsg(m)
-            del self.pendingReviews[op]
+    def _sendReviews(self, irc, msg=None):
+        if msg is None:
+            # try to send them all
+            for host, values in self.pendingReviews.iteritems():
+                op, s = values
+                msg = ircmsgs.privmsg(op, s)
+                irc.queueMsg(msg)
+            self.pendingReviews.clear()
+        else:
+            host = ircutils.hostFromHostmask(msg.prefix)
+            if host in self.pendingReviews:
+                op = msg.nick
+                for _, s in self.pendingReviews[host]:
+                    msg = ircmsgs.privmsg(op, s)
+                    irc.queueMsg(msg)
+                del self.pendingReviews[host]
 
     def doLog(self, irc, channel, s):
         if not self.registryValue('enabled', channel):
@@ -454,12 +476,13 @@ class Bantracker(callbacks.Plugin):
 #        self.logs[channel] = self.logs[channel][-199:] + [s.strip()]
         self.logs[channel].append(s.strip())
 
-    def doKickban(self, irc, channel, nick, target, kickmsg = None, use_time = None, extra_comment = None):
+    def doKickban(self, irc, channel, operator, target, kickmsg = None, use_time = None, extra_comment = None):
         if not self.registryValue('enabled', channel):
             return
         n = now()
         if use_time:
             n = fromTime(use_time)
+        nick = ircutils.nickFromHostmask(operator)
         id = self.db_run("INSERT INTO bans (channel, mask, operator, time, log) values(%s, %s, %s, %s, %s)", 
                           (channel, target, nick, n, '\n'.join(self.logs[channel])), expect_id=True)
         if kickmsg and id and not (kickmsg == nick):
@@ -468,7 +491,7 @@ class Bantracker(callbacks.Plugin):
             self.db_run("INSERT INTO comments (ban_id, who, comment, time) values(%s,%s,%s,%s)", (id, nick, extra_comment, n))
         if channel not in self.bans:
             self.bans[channel] = []
-        ban = Ban(mask=target, who=nick, when=time.mktime(time.gmtime()), id=id)
+        ban = Ban(mask=target, who=operator, when=time.mktime(time.gmtime()), id=id)
         self.bans[channel].append(ban)
         self.requestComment(irc, channel, ban)
         return id
@@ -551,14 +574,14 @@ class Bantracker(callbacks.Plugin):
         else:
             self.doLog(irc, channel,
                        '*** %s was kicked by %s\n' % (target, msg.nick))
-        self.doKickban(irc, channel, msg.nick, target, kickmsg, extra_comment=host)
+        self.doKickban(irc, channel, msg.prefix, target, kickmsg, extra_comment=host)
 
     def doPart(self, irc, msg):
         for channel in msg.args[0].split(','):
             self.doLog(irc, channel, '*** %s (%s) has left %s (%s)\n' % (msg.nick, msg.prefix, channel, len(msg.args) > 1 and msg.args[1] or ''))
             if len(msg.args) > 1 and msg.args[1].startswith('requested by'):
                 args = msg.args[1].split()
-                self.doKickban(irc, channel, args[2], msg.nick, ' '.join(args[3:]).strip(), extra_comment=msg.prefix)
+                self.doKickban(irc, channel, args[2], msg.prefix, ' '.join(args[3:]).strip(), extra_comment=msg.prefix)
 
     def doMode(self, irc, msg):
         channel = msg.args[0]
@@ -585,7 +608,7 @@ class Bantracker(callbacks.Plugin):
 
                 if param[0] in ('+b', '+q'):
                     comment = self.getHostFromBan(irc, msg, mask)
-                    self.doKickban(irc, channel, msg.nick, mask + realname, extra_comment=comment)
+                    self.doKickban(irc, channel, msg.prefix, mask + realname, extra_comment=comment)
                 elif param[0] in ('-b', '-q'):
                     self.doUnban(irc,channel, msg.nick, mask + realname)
 
@@ -721,7 +744,7 @@ class Bantracker(callbacks.Plugin):
         hostmask = self.nick_to_host(irc, target)
 
         self.doLog(irc, channel.lower(), '*** %s requested a mark for %s\n' % (msg.nick, target))
-        self.doKickban(irc, channel.lower(), msg.nick, hostmask, kickmsg)
+        self.doKickban(irc, channel.lower(), msg.prefix, hostmask, kickmsg)
         irc.replySuccess()
 
     mark = wrap(mark, [optional('channel'), 'something', additional('text')])

From 621a73919893a694f54b6c4e48e0490131b95952 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Thu, 1 Apr 2010 17:52:53 -0300
Subject: [PATCH 14/33] improved testcases

---
 Bantracker/test.py | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/Bantracker/test.py b/Bantracker/test.py
index d67020c..53eeb2c 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -106,23 +106,45 @@ class BantrackerTestCase(ChannelPluginTestCase):
         pluginConf.reviewAfterTime.setValue(1.0/84600) # one second
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
-        print 'waiting 2 secs..'
+        print 'waiting 4 secs..'
         time.sleep(2)
         cb.reviewBans()
         # check is pending
         self.assertTrue(cb.pendingReviews)
-        # check msg if op and only op says something
-        self.feedMsg('Hi!', frm='dude!user@host.net') 
+        # send msg if a user with a matching host says something
+        self.feedMsg('Hi!', frm='op!user@fakehost.net') 
         msg = self.irc.takeMsg()
         self.assertEqual(msg, None)
-        self.feedMsg('Hi!', frm='op!user@host.net') 
+        self.feedMsg('Hi!', frm='op_!user@host.net') 
         msg = self.irc.takeMsg()
         self.assertEqual(str(msg).strip(),
-            "PRIVMSG op :Hi, please review the ban 'asd!*@*' that you set on %s in #test, link: "\
-            "%s/bans.cgi?log=1" %(cb.bans['#test'][-1].ascwhen, pluginConf.bansite()))
+            "PRIVMSG op_ :Hi, please review the ban 'asd!*@*' that you set on %s in #test, link: "\
+            "%s/bans.cgi?log=1" %(cb.bans['#test'][0].ascwhen, pluginConf.bansite()))
         # don't ask again
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
+        self.feedBan('asd2!*@*')
+        self.irc.takeMsg()
+        self.feedBan('qwe!*@*', prefix='otherop!user@home.net')
+        self.irc.takeMsg()
+        time.sleep(2)
+        cb.reviewBans()
+        self.assertTrue(len(cb.pendingReviews) == 2)
+        self.feedMsg('Hi!', frm='op!user@fakehost.net') 
+        msg = self.irc.takeMsg()
+        self.assertEqual(msg, None)
+        self.feedMsg('Hi!', frm='mynickissocreative!user@home.net') 
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(),
+            "PRIVMSG mynickissocreative :Hi, please review the ban 'qwe!*@*' that you set on %s in #test, link: "\
+            "%s/bans.cgi?log=3" %(cb.bans['#test'][2].ascwhen, pluginConf.bansite()))
+        self.feedMsg('ping', to='test', frm='op!user@host.net') # in a query
+        self.irc.takeMsg() # drop pong reply
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(),
+            "PRIVMSG op :Hi, please review the ban 'asd2!*@*' that you set on %s in #test, link: "\
+            "%s/bans.cgi?log=2" %(cb.bans['#test'][1].ascwhen, pluginConf.bansite()))
+
 
     def testBan(self):
         self.feedBan('asd!*@*')

From b82f1ae2bca84e293b079f37fb2335c41e4036b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 13:51:49 -0300
Subject: [PATCH 15/33] not use super or raise error, causes some problems

---
 Bantracker/config.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index 4da64c4..2bc4bb6 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -29,7 +29,7 @@ class SpaceSeparatedListOfTypes(registry.SpaceSeparatedListOf):
 class ReadOnlyValue(registry.Integer):
     """This is a read only option."""
     def __init__(self, *args, **kwargs):
-        super(ReadOnlyValue, self).__init__(*args, **kwargs)
+        registry.Integer.__init__(self, *args, **kwargs)
         self.value = None
 
     def set(self, s):
@@ -39,7 +39,8 @@ class ReadOnlyValue(registry.Integer):
             else:
                 raise ValueError
         except ValueError:
-            self.error()
+            #self.error() # commented, this causes a lot of trouble.
+            pass
 
 
 def configure(advanced):

From fffc4b58ed8b3a004bb55dc8e721d65468565ade Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 13:59:34 -0300
Subject: [PATCH 16/33] run reviewBans every 10min

---
 Bantracker/plugin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 66ad23b..e8fb3a0 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -197,7 +197,7 @@ class Bantracker(callbacks.Plugin):
         self.get_bans(irc)
         self.get_nicks(irc)
         # add scheduled event for check bans that need review
-        schedule.addPeriodicEvent(self.reviewBans, 20,
+        schedule.addPeriodicEvent(self.reviewBans, 60*10,
                 name=self.name())
         self.pendingReviews = ircutils.IrcDict()
 

From 44b07e6c1057df5ca482f554ffc06a448b88dccf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 14:01:07 -0300
Subject: [PATCH 17/33] use case insensible dicts here

---
 Bantracker/plugin.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index e8fb3a0..0653a5d 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -181,10 +181,10 @@ class Bantracker(callbacks.Plugin):
         self.lastMsgs = {}
         self.lastStates = {}
         self.replies = {}
-        self.logs = {}
+        self.logs = ircutils.IrcDict()
         self.nicks = {}
         self.hosts = {}
-        self.bans = {}
+        self.bans = ircutils.IrcDict()
 
         self.thread_timer = threading.Timer(10.0, dequeue, args=(self,irc))
         self.thread_timer.start()

From b31a2669330da117547c0810cd4c5433ecdb1cac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 14:01:57 -0300
Subject: [PATCH 18/33] keep ban list sorted by time

---
 Bantracker/plugin.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 0653a5d..648d355 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -271,7 +271,9 @@ class Bantracker(callbacks.Plugin):
         """Got ban"""
         if msg.args[1] not in self.bans.keys():
             self.bans[msg.args[1]] = []
-        self.bans[msg.args[1]].append(Ban(msg.args))
+        bans = self.bans[msg.args[1]]
+        bans.append(Ban(msg.args))
+        bans.sort(key=lambda x: x.when)
 
     def nick_to_host(self, irc=None, target='', with_nick=True, reply_now=True):
         target = target.lower()

From 8bb046e84c605d571d21d36805d4561bfa6a5e07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 14:03:01 -0300
Subject: [PATCH 19/33] refactoring, requested reviews now follow forward
 options, some fixes

---
 Bantracker/plugin.py | 84 ++++++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 35 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 648d355..a9350a0 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -376,37 +376,43 @@ class Bantracker(callbacks.Plugin):
         # send msg
         prefix = conf.supybot.reply.whenAddressedBy.chars()[0] # prefix char for commands
         # check to who send the request
-        if nickMatch(ban.who, self.registryValue('commentRequest.forward', channel=channel)):
+        try:
+            op = ircutils.nickFromHostmask(ban.who)
+        except:
+            op = ban.who
+        if nickMatch(op, self.registryValue('commentRequest.forward', channel=channel)):
             channels = self.registryValue('commentRequest.forward.channels', channel=channel)
             if channels:
                 s = "Please somebody comment on the %s of %s in %s done by %s, use:"\
-                    " %scomment %s <comment>" %(type, mask, channel, ban.who, prefix, ban.id)
+                    " %scomment %s <comment>" %(type, mask, channel, op, prefix, ban.id)
                 for chan in channels:
                     msg = ircmsgs.notice(chan, s)
+                    self.log.info('SENDING: %s' %msg)
                     irc.queueMsg(msg)
                 return
         # send to op
         s = "Please comment on the %s of %s in %s, use: %scomment %s <comment>" \
                 %(type, mask, channel, prefix, ban.id)
-        try:
-            op = ircutils.nickFromHostmask(ban.who)
-        except:
-            op = ban.who
-        irc.reply(s, to=op, private=True)
+        self.log.info('SENDING: %s %s' %(op, s))
+        #irc.reply(s, to=op, private=True)
 
     def reviewBans(self):
         try:
-            reviewAfterTime = int(self.registryValue('reviewAfterTime') * 84600)
+            reviewAfterTime = int(self.registryValue('reviewAfterTime') * 86400)
             if not reviewAfterTime:
                 # time is zero, do nothing
                 return
             self.log.debug('Checking for bans that need review ...')
             now = time.mktime(time.gmtime())
             lastreview = self.registryValue('reviewTime')
+            bansite = self.registryValue('bansite')
             if not lastreview:
                 # initialize last time reviewed timestamp
                 lastreview = now - reviewAfterTime
             for channel, bans in self.bans.iteritems():
+                ignore = self.registryValue('commentRequest.ignore', channel=channel)
+                forward = self.registryValue('commentRequest.forward', channel=channel)
+                fchannels = self.registryValue('commentRequest.forward.channels', channel=channel)
                 for ban in bans:
                     type = guessBanType(ban.mask)
                     if type in ('quiet', 'removal'):
@@ -414,8 +420,8 @@ class Bantracker(callbacks.Plugin):
                         continue
                     banTime = now - ban.when
                     reviewTime = lastreview - ban.when
-                    self.log.debug('  channel %s ban %s (%s/%s/%s)', channel, str(ban), reviewTime,
-                            reviewAfterTime, banTime)
+                    self.log.debug('  channel %s ban %s (%s %s %s)', channel, str(ban), reviewTime,
+                            reviewAfterTime, reviewAfterTime-reviewTime)
                     if reviewTime <= reviewAfterTime < banTime:
                         # ban is old enough, and inside the "review window"
                         try:
@@ -426,21 +432,30 @@ class Bantracker(callbacks.Plugin):
                             # probably a ban restored by IRC server in a netsplit
                             # XXX see if something can be done about this
                             continue
-                        if nickMatch(op, self.registryValue('commentRequest.ignore')):
+                        if nickMatch(op, ignore):
                             # in the ignore list
                             continue
+                        self.log.debug('  adding ban to the pending review list ...')
                         if not ban.id:
                             ban.id = self.get_banId(ban.mask, channel)
-                        s = "Hi, please review the ban '%s' that you set on %s in %s,"\
-                        " link: %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel,
-                                self.registryValue('bansite'), ban.id)
-                        self.log.debug('  adding ban to the pending review list ...')
-                        if host in self.pendingReviews:
-                            self.pendingReviews[host].append((op, s))
+                        if nickMatch(op, forward):
+                            msgs = []
+                            s = "Hi, please somebody review the ban '%s' set by %s on %s in"\
+                            " %s, link: %s/bans.cgi?log=%s" %(ban.mask, op, ban.ascwhen, channel,
+                                    bansite, ban.id)
+                            for chan in fchannels:
+                                msgs.append(ircmsgs.notice(chan, s))
+                                # FIXME forwards should be sent now, not later.
                         else:
-                            self.pendingReviews[host] = [(op, s)]
+                            s = "Hi, please review the ban '%s' that you set on %s in %s, link:"\
+                            " %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel, bansite, ban.id)
+                            msgs = [ircmsgs.privmsg(op, s)]
+                        if host not in self.pendingReviews:
+                            self.pendingReviews[host] = []
+                        for msg in msgs:
+                            self.pendingReviews[host].append((op, msg))
                     elif banTime < reviewAfterTime:
-                        # the bans left are even more recent
+                        # since we made sure bans are sorted by time, the bans left are more recent
                         break
             self.setRegistryValue('reviewTime', now) # update last time reviewed
         except Exception, e:
@@ -449,22 +464,18 @@ class Bantracker(callbacks.Plugin):
             self.log.error('Except: %s' %e)
             self.log.error(traceback.format_exc())
 
-    def _sendReviews(self, irc, msg=None):
-        if msg is None:
-            # try to send them all
-            for host, values in self.pendingReviews.iteritems():
-                op, s = values
-                msg = ircmsgs.privmsg(op, s)
-                irc.queueMsg(msg)
-            self.pendingReviews.clear()
-        else:
-            host = ircutils.hostFromHostmask(msg.prefix)
-            if host in self.pendingReviews:
-                op = msg.nick
-                for _, s in self.pendingReviews[host]:
-                    msg = ircmsgs.privmsg(op, s)
+    def _sendReviews(self, irc, msg):
+        host = ircutils.hostFromHostmask(msg.prefix)
+        if host in self.pendingReviews:
+            op = msg.nick
+            for nick, msg in self.pendingReviews[host]:
+                if msg.command == 'NOTICE':
+                    self.log.info('SENDING: %s' %msg)
                     irc.queueMsg(msg)
-                del self.pendingReviews[host]
+                else:
+                    m = ircmsgs.privmsg(op, msg.args[1])
+                    #irc.queueMsg(m)
+            del self.pendingReviews[host]
 
     def doLog(self, irc, channel, s):
         if not self.registryValue('enabled', channel):
@@ -484,7 +495,10 @@ class Bantracker(callbacks.Plugin):
         n = now()
         if use_time:
             n = fromTime(use_time)
-        nick = ircutils.nickFromHostmask(operator)
+        try:
+            nick = ircutils.nickFromHostmask(operator)
+        except:
+            nick = operator
         id = self.db_run("INSERT INTO bans (channel, mask, operator, time, log) values(%s, %s, %s, %s, %s)", 
                           (channel, target, nick, n, '\n'.join(self.logs[channel])), expect_id=True)
         if kickmsg and id and not (kickmsg == nick):

From 155d1ae47d3891413299c039fba9d9e48101fa3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 14:03:44 -0300
Subject: [PATCH 20/33] banreview command

---
 Bantracker/plugin.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index a9350a0..305f65a 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -1050,4 +1050,16 @@ class Bantracker(callbacks.Plugin):
             irc.reply("%s/bans.cgi?log=%s&mark=%s" % (self.registryValue('bansite'), id, highlight), private=True)
     banlink = wrap(banlink, ['id', optional('somethingWithoutSpaces')])
 
+    def banReview(self, irc, msg, args):
+        """Lists pending ban reviews."""
+        count = []
+        for reviews in self.pendingReviews.itervalues():
+            count.append((reviews[0][0], len(reviews)))
+        total = sum([ x[1] for x in count ])
+        s = ' '.join([ '%s:%s' %pair for pair in count ])
+        s = 'Pending ban reviews (%s): %s' %(total, s)
+        irc.reply(s)
+
+    banreview = wrap(banReview)
+
 Class = Bantracker

From 9b0a5d418027e3054dff5f6065b7feed2a1b629a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 17:28:43 -0300
Subject: [PATCH 21/33] save pending reviews on close so we won't lose them

---
 Bantracker/plugin.py | 46 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 305f65a..39f81d1 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -167,6 +167,48 @@ def guessBanType(mask):
         return 'ban'
     return 'removal'
 
+class PersistentCache(dict):
+    def __init__(self, filename):
+        self.filename = conf.supybot.directories.data.dirize(filename)
+
+    def open(self):
+        import csv
+        try:
+            reader = csv.reader(open(self.filename, 'rb'))
+        except IOError:
+            return
+        for row in reader:
+            host, value = self.deserialize(*row)
+            try:
+                self[host].append(value)
+            except KeyError:
+                self[host] = [value]
+
+    def close(self):
+        import csv
+        try:
+            writer = csv.writer(open(self.filename, 'wb'))
+        except IOError:
+            return
+        for host, values in self.iteritems():
+            for v in values:
+                writer.writerow(self.serialize(host, v))
+
+    def deserialize(self, host, nick, command, channel, text):
+        if command == 'PRIVMSG':
+            msg = ircmsgs.privmsg(channel, text)
+        elif command == 'NOTICE':
+            msg = ircmsgs.notice(channel, text)
+        else:
+            return
+        return (host, (nick, msg))
+
+    def serialize(self, host, value):
+        nick, msg = value
+        command, channel, text = msg.command, msg.args[0], msg.args[1]
+        return (host, nick, command, channel, text)
+
+
 
 class Bantracker(callbacks.Plugin):
     """Plugin to manage bans.
@@ -199,7 +241,8 @@ class Bantracker(callbacks.Plugin):
         # add scheduled event for check bans that need review
         schedule.addPeriodicEvent(self.reviewBans, 60*10,
                 name=self.name())
-        self.pendingReviews = ircutils.IrcDict()
+        self.pendingReviews = PersistentCache('bt.reviews.db')
+        self.pendingReviews.open()
 
     def get_nicks(self, irc):
         self.hosts.clear()
@@ -312,6 +355,7 @@ class Bantracker(callbacks.Plugin):
             schedule.removeEvent(self.name())
         except:
             pass
+        self.pendingReviews.close()
 
     def reset(self):
         global queue

From fd55f019e1267a559d08217d2d9e9a984757d5d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 18:32:19 -0300
Subject: [PATCH 22/33] remove our ReadOnly option, which was an ugly hack, and
 use PersistentCache instead

---
 Bantracker/config.py | 23 -----------------------
 Bantracker/plugin.py | 11 +++++++----
 2 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index 2bc4bb6..7d97520 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -23,26 +23,6 @@ class SpaceSeparatedListOfTypes(registry.SpaceSeparatedListOf):
     Value = ValidTypes
 
 
-# This little hack allows me to store last time bans were checked for review
-# and guarantee that nobody will edit it.
-# I'm using a registry option instead of the SQL db because this is much simpler.
-class ReadOnlyValue(registry.Integer):
-    """This is a read only option."""
-    def __init__(self, *args, **kwargs):
-        registry.Integer.__init__(self, *args, **kwargs)
-        self.value = None
-
-    def set(self, s):
-        try:
-            if not self.value:
-                self.setValue(int(s))
-            else:
-                raise ValueError
-        except ValueError:
-            #self.error() # commented, this causes a lot of trouble.
-            pass
-
-
 def configure(advanced):
     conf.registerPlugin('Bantracker', True)
 
@@ -74,9 +54,6 @@ conf.registerChannelValue(Bantracker.commentRequest.forward, 'channels',
 
 
 # temp config
-conf.registerGlobalValue(Bantracker, 'reviewTime',
-        ReadOnlyValue(0,
-            "Timestamp used internally for identify bans that need review. Can't and shouldn't be edited."))
 conf.registerGlobalValue(Bantracker, 'reviewAfterTime',
         registry.Float(7,
             "Days after which the bot will request for review a ban. Can be an integer or decimal value."))
diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 39f81d1..b0cb8a8 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -170,6 +170,7 @@ def guessBanType(mask):
 class PersistentCache(dict):
     def __init__(self, filename):
         self.filename = conf.supybot.directories.data.dirize(filename)
+        self.time = 0
 
     def open(self):
         import csv
@@ -177,6 +178,7 @@ class PersistentCache(dict):
             reader = csv.reader(open(self.filename, 'rb'))
         except IOError:
             return
+        self.time = int(reader.next()[1])
         for row in reader:
             host, value = self.deserialize(*row)
             try:
@@ -190,6 +192,7 @@ class PersistentCache(dict):
             writer = csv.writer(open(self.filename, 'wb'))
         except IOError:
             return
+        writer.writerow(('time', str(int(self.time))))
         for host, values in self.iteritems():
             for v in values:
                 writer.writerow(self.serialize(host, v))
@@ -238,11 +241,11 @@ class Bantracker(callbacks.Plugin):
             self.db = None
         self.get_bans(irc)
         self.get_nicks(irc)
+        self.pendingReviews = PersistentCache('bt.reviews.db')
+        self.pendingReviews.open()
         # add scheduled event for check bans that need review
         schedule.addPeriodicEvent(self.reviewBans, 60*10,
                 name=self.name())
-        self.pendingReviews = PersistentCache('bt.reviews.db')
-        self.pendingReviews.open()
 
     def get_nicks(self, irc):
         self.hosts.clear()
@@ -448,7 +451,7 @@ class Bantracker(callbacks.Plugin):
                 return
             self.log.debug('Checking for bans that need review ...')
             now = time.mktime(time.gmtime())
-            lastreview = self.registryValue('reviewTime')
+            lastreview = self.pendingReviews.time
             bansite = self.registryValue('bansite')
             if not lastreview:
                 # initialize last time reviewed timestamp
@@ -501,7 +504,7 @@ class Bantracker(callbacks.Plugin):
                     elif banTime < reviewAfterTime:
                         # since we made sure bans are sorted by time, the bans left are more recent
                         break
-            self.setRegistryValue('reviewTime', now) # update last time reviewed
+            self.pendingReviews.time = now # update last time reviewed
         except Exception, e:
             # I need to catch exceptions as they are silenced
             import traceback

From b0b823fef150d8822fd54ecaf459195540928328 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Fri, 2 Apr 2010 23:43:26 -0300
Subject: [PATCH 23/33] config rename:

Bantracker.commetRequest => Bantracker.request
Bantracker.reviewAfterTime => Bantracker.request.review
---
 Bantracker/config.py | 19 +++++++++----------
 Bantracker/plugin.py | 29 ++++++++++++++---------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index 7d97520..a5c081c 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -34,26 +34,25 @@ conf.registerGlobalValue(Bantracker, 'database',
 conf.registerGlobalValue(Bantracker, 'bansite',
         registry.String('', "Web site for the bantracker, without the 'bans.cgi' appended", private=True))
 
-conf.registerGroup(Bantracker, 'commentRequest')
-conf.registerChannelValue(Bantracker.commentRequest, 'type',
+conf.registerGroup(Bantracker, 'request')
+conf.registerChannelValue(Bantracker.request, 'type',
         SpaceSeparatedListOfTypes(['removal', 'ban', 'quiet'],
             "List of events for which the bot should request a comment."))
-conf.registerChannelValue(Bantracker.commentRequest, 'ignore',
-        registry.SpaceSeparatedListOfStrings([],
+conf.registerChannelValue(Bantracker.request, 'ignore',
+        registry.SpaceSeparatedListOfStrings(['FloodBot?', 'FloodBotK?', 'ChanServ'],
             "List of nicks for which the bot won't request to comment a ban/quiet/removal."\
             " Is case insensible and wildcards * ? are accepted."))
-conf.registerChannelValue(Bantracker.commentRequest, 'forward',
+conf.registerChannelValue(Bantracker.request, 'forward',
         registry.SpaceSeparatedListOfStrings([],
             "List of nicks for which the bot will forward the request to"\
             " the channels/nicks defined in forwards.channels option."\
             " Is case insensible and wildcards * ? are accepted."))
-conf.registerChannelValue(Bantracker.commentRequest.forward, 'channels',
+conf.registerChannelValue(Bantracker.request.forward, 'channels',
         registry.SpaceSeparatedListOfStrings([],
             "List of channels/nicks to forward the request if the op that set the ban/quiet"\
             " is in the forward list."))
-
-
-# temp config
-conf.registerGlobalValue(Bantracker, 'reviewAfterTime',
+conf.registerChannelValue(Bantracker.request, 'review',
         registry.Float(7,
             "Days after which the bot will request for review a ban. Can be an integer or decimal value."))
+
+
diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index b0cb8a8..a9ee452 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -212,7 +212,6 @@ class PersistentCache(dict):
         return (host, nick, command, channel, text)
 
 
-
 class Bantracker(callbacks.Plugin):
     """Plugin to manage bans.
        See '@list Bantracker' and '@help <command>' for commands"""
@@ -409,7 +408,7 @@ class Bantracker(callbacks.Plugin):
 
     def requestComment(self, irc, channel, ban, type=None):
         # check if we should request a comment
-        if nickMatch(ban.who, self.registryValue('commentRequest.ignore', channel=channel)):
+        if nickMatch(ban.who, self.registryValue('request.ignore', channel=channel)):
                 return
         # check the type of the action taken
         mask = ban.mask
@@ -418,7 +417,7 @@ class Bantracker(callbacks.Plugin):
             if type == 'quiet':
                 mask = mask[1:]
         # check if type is enabled
-        if type not in self.registryValue('commentRequest.type', channel=channel):
+        if type not in self.registryValue('request.type', channel=channel):
             return
         # send msg
         prefix = conf.supybot.reply.whenAddressedBy.chars()[0] # prefix char for commands
@@ -427,8 +426,8 @@ class Bantracker(callbacks.Plugin):
             op = ircutils.nickFromHostmask(ban.who)
         except:
             op = ban.who
-        if nickMatch(op, self.registryValue('commentRequest.forward', channel=channel)):
-            channels = self.registryValue('commentRequest.forward.channels', channel=channel)
+        if nickMatch(op, self.registryValue('request.forward', channel=channel)):
+            channels = self.registryValue('request.forward.channels', channel=channel)
             if channels:
                 s = "Please somebody comment on the %s of %s in %s done by %s, use:"\
                     " %scomment %s <comment>" %(type, mask, channel, op, prefix, ban.id)
@@ -441,25 +440,25 @@ class Bantracker(callbacks.Plugin):
         s = "Please comment on the %s of %s in %s, use: %scomment %s <comment>" \
                 %(type, mask, channel, prefix, ban.id)
         self.log.info('SENDING: %s %s' %(op, s))
-        #irc.reply(s, to=op, private=True)
+        irc.reply(s, to=op, private=True)
 
     def reviewBans(self):
         try:
-            reviewAfterTime = int(self.registryValue('reviewAfterTime') * 86400)
-            if not reviewAfterTime:
-                # time is zero, do nothing
-                return
             self.log.debug('Checking for bans that need review ...')
             now = time.mktime(time.gmtime())
             lastreview = self.pendingReviews.time
             bansite = self.registryValue('bansite')
             if not lastreview:
                 # initialize last time reviewed timestamp
-                lastreview = now - reviewAfterTime
+                lastreview = now - self.registryValue('request.review')
             for channel, bans in self.bans.iteritems():
-                ignore = self.registryValue('commentRequest.ignore', channel=channel)
-                forward = self.registryValue('commentRequest.forward', channel=channel)
-                fchannels = self.registryValue('commentRequest.forward.channels', channel=channel)
+                reviewAfterTime = int(self.registryValue('request.review', channel=channel) * 86400)
+                if not reviewAfterTime:
+                    # time is zero, do nothing
+                    continue
+                ignore = self.registryValue('request.ignore', channel=channel)
+                forward = self.registryValue('request.forward', channel=channel)
+                fchannels = self.registryValue('request.forward.channels', channel=channel)
                 for ban in bans:
                     type = guessBanType(ban.mask)
                     if type in ('quiet', 'removal'):
@@ -521,7 +520,7 @@ class Bantracker(callbacks.Plugin):
                     irc.queueMsg(msg)
                 else:
                     m = ircmsgs.privmsg(op, msg.args[1])
-                    #irc.queueMsg(m)
+                    irc.queueMsg(m)
             del self.pendingReviews[host]
 
     def doLog(self, irc, channel, s):

From fd4516f077e53883d1c06cf4067242e3395eeae0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 00:17:26 -0300
Subject: [PATCH 24/33] fixed testcases

---
 Bantracker/test.py | 73 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/Bantracker/test.py b/Bantracker/test.py
index 53eeb2c..2bcd393 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -21,7 +21,8 @@ class BantrackerTestCase(ChannelPluginTestCase):
     def setUp(self):
         super(BantrackerTestCase, self).setUp()
         self.setDb()
-        pluginConf.commentRequest.ignore.set('*') # disable comments
+        pluginConf.request.ignore.set('*') # disable comments
+        pluginConf.request.forward.set('')
 
     def setDb(self):
         import sqlite, os
@@ -81,10 +82,16 @@ class BantrackerTestCase(ChannelPluginTestCase):
             ban = ircmsgs.ban(channel, hostmask, prefix=prefix)
         elif mode == 'q':
             ban = quiet(channel, hostmask, prefix=prefix)
+        elif mode == 'k':
+            ban = ircmsgs.kick(channel, hostmask, s='kthxbye!', prefix=prefix)
+        elif mode == 'p':
+            ban = ircmsgs.part(channel, prefix=hostmask,
+                    s='requested by %s (kthxbye!)' %prefix[:prefix.find('!')])
         self.irc.feedMsg(ban)
+        return ban
 
     def testCommentRequest(self):
-        pluginConf.commentRequest.ignore.set('')
+        pluginConf.request.ignore.set('')
         # test bans
         self.feedBan('asd!*@*')
         msg = self.irc.takeMsg()
@@ -97,13 +104,37 @@ class BantrackerTestCase(ChannelPluginTestCase):
         self.assertEqual(str(msg).strip(), 
             "PRIVMSG op :Please comment on the quiet of dude!*@* in #test, use: @comment 2"
             " <comment>")
+        # test kick/part
+        self.feedBan('dude', mode='k')
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(), 
+            "PRIVMSG op :Please comment on the removal of dude in #test, use: @comment 3"
+            " <comment>")
+        self.feedBan('dude', mode='p')
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(), 
+            "PRIVMSG op :Please comment on the removal of dude in #test, use: @comment 4"
+            " <comment>")
+        # test forwards
+        pluginConf.request.forward.set('bot')
+        pluginConf.request.forward.channels.set('#channel')
+        self.feedBan('qwe!*@*')
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(), 
+            "PRIVMSG op :Please comment on the ban of qwe!*@* in #test, use: @comment 5"
+            " <comment>")
+        self.feedBan('zxc!*@*', prefix='bot!user@host.com')
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(), 
+            "NOTICE #channel :Please somebody comment on the ban of zxc!*@* in #test done by bot,"
+            " use: @comment 6 <comment>")
 
-    def testReviewResquest(self):
-        pluginConf.commentRequest.ignore.set('')
+    def testReviewRequest(self):
+        pluginConf.request.ignore.set('')
         cb = self.getCallback()
         self.feedBan('asd!*@*')
         self.irc.takeMsg() # ignore comment request comment
-        pluginConf.reviewAfterTime.setValue(1.0/84600) # one second
+        pluginConf.request.review.setValue(1.0/86400) # one second
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
         print 'waiting 4 secs..'
@@ -133,6 +164,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
         self.feedMsg('Hi!', frm='op!user@fakehost.net') 
         msg = self.irc.takeMsg()
         self.assertEqual(msg, None)
+        self.assertResponse('banreview', 'Pending ban reviews (2): otherop:1 op:1')
         self.feedMsg('Hi!', frm='mynickissocreative!user@home.net') 
         msg = self.irc.takeMsg()
         self.assertEqual(str(msg).strip(),
@@ -145,6 +177,26 @@ class BantrackerTestCase(ChannelPluginTestCase):
             "PRIVMSG op :Hi, please review the ban 'asd2!*@*' that you set on %s in #test, link: "\
             "%s/bans.cgi?log=2" %(cb.bans['#test'][1].ascwhen, pluginConf.bansite()))
 
+    def testPersistentCache(self):
+        msg1 = ircmsgs.privmsg('nick', 'Hello World')
+        msg2 = ircmsgs.privmsg('nick', 'Hello World')
+        msg3 = ircmsgs.notice('#chan', 'Hello World')
+        msg4 = ircmsgs.privmsg('nick_', 'Hello World')
+        cb = self.getCallback()
+        pr = cb.pendingReviews
+        pr['host.net'] = [('op', msg1), ('op', msg2), ('op_', msg3)]
+        pr['home.net'] = [('dude', msg4)]
+        self.assertResponse('banreview', 'Pending ban reviews (4): dude:1 op:3')
+        pr.close()
+        pr.clear()
+        pr.open()
+        self.assertResponse('banreview', 'Pending ban reviews (4): dude:1 op:3')
+        items = pr['host.net']
+        self.assertTrue(items[0][0] == 'op' and items[0][1] == msg1)
+        self.assertTrue(items[1][0] == 'op' and items[1][1] == msg2)
+        self.assertTrue(items[2][0] == 'op_' and items[2][1] == msg3)
+        items = pr['home.net']
+        self.assertTrue(items[0][0] == 'dude' and items[0][1] == msg4)
 
     def testBan(self):
         self.feedBan('asd!*@*')
@@ -156,5 +208,16 @@ class BantrackerTestCase(ChannelPluginTestCase):
         fetch = self.query("SELECT id,channel,mask,operator FROM bans")
         self.assertEqual((1, '#test', '%asd!*@*', 'op'), fetch[0])
 
+    def testKick(self):
+        self.feedBan('troll', mode='k')
+        fetch = self.query("SELECT id,channel,mask,operator FROM bans")
+        self.assertEqual((1, '#test', 'troll', 'op'), fetch[0])
+
+    def testPart(self):
+        self.feedBan('troll!user@trollpit.net', mode='p')
+        fetch = self.query("SELECT id,channel,mask,operator FROM bans")
+        self.assertEqual((1, '#test', 'troll!user@trollpit.net', 'op'), fetch[0])
+
+
 
 

From dda5154237fa15be3efb284c50e38c8b5a6e11dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 00:26:20 -0300
Subject: [PATCH 25/33] removed try: except: block

---
 Bantracker/plugin.py | 123 +++++++++++++++++++++----------------------
 1 file changed, 59 insertions(+), 64 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index a9ee452..763c356 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -212,6 +212,7 @@ class PersistentCache(dict):
         return (host, nick, command, channel, text)
 
 
+
 class Bantracker(callbacks.Plugin):
     """Plugin to manage bans.
        See '@list Bantracker' and '@help <command>' for commands"""
@@ -443,72 +444,66 @@ class Bantracker(callbacks.Plugin):
         irc.reply(s, to=op, private=True)
 
     def reviewBans(self):
-        try:
-            self.log.debug('Checking for bans that need review ...')
-            now = time.mktime(time.gmtime())
-            lastreview = self.pendingReviews.time
-            bansite = self.registryValue('bansite')
-            if not lastreview:
-                # initialize last time reviewed timestamp
-                lastreview = now - self.registryValue('request.review')
-            for channel, bans in self.bans.iteritems():
-                reviewAfterTime = int(self.registryValue('request.review', channel=channel) * 86400)
-                if not reviewAfterTime:
-                    # time is zero, do nothing
+        self.log.debug('Checking for bans that need review ...')
+        now = time.mktime(time.gmtime())
+        lastreview = self.pendingReviews.time
+        bansite = self.registryValue('bansite')
+        if not lastreview:
+            # initialize last time reviewed timestamp
+            lastreview = now - self.registryValue('request.review')
+        for channel, bans in self.bans.iteritems():
+            reviewAfterTime = int(self.registryValue('request.review', channel=channel) * 86400)
+            if not reviewAfterTime:
+                # time is zero, do nothing
+                continue
+            ignore = self.registryValue('request.ignore', channel=channel)
+            forward = self.registryValue('request.forward', channel=channel)
+            fchannels = self.registryValue('request.forward.channels', channel=channel)
+            for ban in bans:
+                type = guessBanType(ban.mask)
+                if type in ('quiet', 'removal'):
+                    # skip mutes and kicks
                     continue
-                ignore = self.registryValue('request.ignore', channel=channel)
-                forward = self.registryValue('request.forward', channel=channel)
-                fchannels = self.registryValue('request.forward.channels', channel=channel)
-                for ban in bans:
-                    type = guessBanType(ban.mask)
-                    if type in ('quiet', 'removal'):
-                        # skip mutes and kicks
+                banTime = now - ban.when
+                reviewTime = lastreview - ban.when
+                self.log.debug('  channel %s ban %s (%s %s %s)', channel, str(ban), reviewTime,
+                        reviewAfterTime, reviewAfterTime-reviewTime)
+                if reviewTime <= reviewAfterTime < banTime:
+                    # ban is old enough, and inside the "review window"
+                    try:
+                        # ban.who should be a user hostmask
+                        op = ircutils.nickFromHostmask(ban.who)
+                        host = ircutils.hostFromHostmask(ban.who)
+                    except:
+                        # probably a ban restored by IRC server in a netsplit
+                        # XXX see if something can be done about this
                         continue
-                    banTime = now - ban.when
-                    reviewTime = lastreview - ban.when
-                    self.log.debug('  channel %s ban %s (%s %s %s)', channel, str(ban), reviewTime,
-                            reviewAfterTime, reviewAfterTime-reviewTime)
-                    if reviewTime <= reviewAfterTime < banTime:
-                        # ban is old enough, and inside the "review window"
-                        try:
-                            # ban.who should be a user hostmask
-                            op = ircutils.nickFromHostmask(ban.who)
-                            host = ircutils.hostFromHostmask(ban.who)
-                        except:
-                            # probably a ban restored by IRC server in a netsplit
-                            # XXX see if something can be done about this
-                            continue
-                        if nickMatch(op, ignore):
-                            # in the ignore list
-                            continue
-                        self.log.debug('  adding ban to the pending review list ...')
-                        if not ban.id:
-                            ban.id = self.get_banId(ban.mask, channel)
-                        if nickMatch(op, forward):
-                            msgs = []
-                            s = "Hi, please somebody review the ban '%s' set by %s on %s in"\
-                            " %s, link: %s/bans.cgi?log=%s" %(ban.mask, op, ban.ascwhen, channel,
-                                    bansite, ban.id)
-                            for chan in fchannels:
-                                msgs.append(ircmsgs.notice(chan, s))
-                                # FIXME forwards should be sent now, not later.
-                        else:
-                            s = "Hi, please review the ban '%s' that you set on %s in %s, link:"\
-                            " %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel, bansite, ban.id)
-                            msgs = [ircmsgs.privmsg(op, s)]
-                        if host not in self.pendingReviews:
-                            self.pendingReviews[host] = []
-                        for msg in msgs:
-                            self.pendingReviews[host].append((op, msg))
-                    elif banTime < reviewAfterTime:
-                        # since we made sure bans are sorted by time, the bans left are more recent
-                        break
-            self.pendingReviews.time = now # update last time reviewed
-        except Exception, e:
-            # I need to catch exceptions as they are silenced
-            import traceback
-            self.log.error('Except: %s' %e)
-            self.log.error(traceback.format_exc())
+                    if nickMatch(op, ignore):
+                        # in the ignore list
+                        continue
+                    self.log.debug('  adding ban to the pending review list ...')
+                    if not ban.id:
+                        ban.id = self.get_banId(ban.mask, channel)
+                    if nickMatch(op, forward):
+                        msgs = []
+                        s = "Hi, please somebody review the ban '%s' set by %s on %s in"\
+                        " %s, link: %s/bans.cgi?log=%s" %(ban.mask, op, ban.ascwhen, channel,
+                                bansite, ban.id)
+                        for chan in fchannels:
+                            msgs.append(ircmsgs.notice(chan, s))
+                            # FIXME forwards should be sent now, not later.
+                    else:
+                        s = "Hi, please review the ban '%s' that you set on %s in %s, link:"\
+                        " %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel, bansite, ban.id)
+                        msgs = [ircmsgs.privmsg(op, s)]
+                    if host not in self.pendingReviews:
+                        self.pendingReviews[host] = []
+                    for msg in msgs:
+                        self.pendingReviews[host].append((op, msg))
+                elif banTime < reviewAfterTime:
+                    # since we made sure bans are sorted by time, the bans left are more recent
+                    break
+        self.pendingReviews.time = now # update last time reviewed
 
     def _sendReviews(self, irc, msg):
         host = ircutils.hostFromHostmask(msg.prefix)

From d1d0fc86abb9e2a6aeaadc3a1b1b4b7e6b8f736c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 01:44:57 -0300
Subject: [PATCH 26/33] refactoring and getting ready for production, added
 testcase

---
 Bantracker/plugin.py | 29 +++++++++++++----------------
 Bantracker/test.py   | 33 +++++++++++++++++++++++++++------
 2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 763c356..6147298 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -243,8 +243,8 @@ class Bantracker(callbacks.Plugin):
         self.get_nicks(irc)
         self.pendingReviews = PersistentCache('bt.reviews.db')
         self.pendingReviews.open()
-        # add scheduled event for check bans that need review
-        schedule.addPeriodicEvent(self.reviewBans, 60*10,
+        # add scheduled event for check bans that need review, check every hour
+        schedule.addPeriodicEvent(lambda : self.reviewBans(irc), 60*60,
                 name=self.name())
 
     def get_nicks(self, irc):
@@ -443,7 +443,7 @@ class Bantracker(callbacks.Plugin):
         self.log.info('SENDING: %s %s' %(op, s))
         irc.reply(s, to=op, private=True)
 
-    def reviewBans(self):
+    def reviewBans(self, irc=None):
         self.log.debug('Checking for bans that need review ...')
         now = time.mktime(time.gmtime())
         lastreview = self.pendingReviews.time
@@ -485,20 +485,20 @@ class Bantracker(callbacks.Plugin):
                     if not ban.id:
                         ban.id = self.get_banId(ban.mask, channel)
                     if nickMatch(op, forward):
-                        msgs = []
+                        if not irc:
+                            continue
                         s = "Hi, please somebody review the ban '%s' set by %s on %s in"\
                         " %s, link: %s/bans.cgi?log=%s" %(ban.mask, op, ban.ascwhen, channel,
                                 bansite, ban.id)
                         for chan in fchannels:
-                            msgs.append(ircmsgs.notice(chan, s))
-                            # FIXME forwards should be sent now, not later.
+                            msg = ircmsgs.notice(chan, s)
+                            irc.queueMsg(msg)
                     else:
                         s = "Hi, please review the ban '%s' that you set on %s in %s, link:"\
                         " %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel, bansite, ban.id)
-                        msgs = [ircmsgs.privmsg(op, s)]
-                    if host not in self.pendingReviews:
-                        self.pendingReviews[host] = []
-                    for msg in msgs:
+                        msg = ircmsgs.privmsg(op, s)
+                        if host not in self.pendingReviews:
+                            self.pendingReviews[host] = []
                         self.pendingReviews[host].append((op, msg))
                 elif banTime < reviewAfterTime:
                     # since we made sure bans are sorted by time, the bans left are more recent
@@ -510,12 +510,9 @@ class Bantracker(callbacks.Plugin):
         if host in self.pendingReviews:
             op = msg.nick
             for nick, msg in self.pendingReviews[host]:
-                if msg.command == 'NOTICE':
-                    self.log.info('SENDING: %s' %msg)
-                    irc.queueMsg(msg)
-                else:
-                    m = ircmsgs.privmsg(op, msg.args[1])
-                    irc.queueMsg(m)
+                if op != nick:
+                    msg = ircmsgs.privmsg(op, msg.args[1])
+                irc.queueMsg(msg)
             del self.pendingReviews[host]
 
     def doLog(self, irc, channel, s):
diff --git a/Bantracker/test.py b/Bantracker/test.py
index 2bcd393..6ad3b1b 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -90,7 +90,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
         self.irc.feedMsg(ban)
         return ban
 
-    def testCommentRequest(self):
+    def testComment(self):
         pluginConf.request.ignore.set('')
         # test bans
         self.feedBan('asd!*@*')
@@ -115,21 +115,23 @@ class BantrackerTestCase(ChannelPluginTestCase):
         self.assertEqual(str(msg).strip(), 
             "PRIVMSG op :Please comment on the removal of dude in #test, use: @comment 4"
             " <comment>")
-        # test forwards
+
+    def testCommentForward(self):
+        pluginConf.request.ignore.set('')
         pluginConf.request.forward.set('bot')
         pluginConf.request.forward.channels.set('#channel')
         self.feedBan('qwe!*@*')
         msg = self.irc.takeMsg()
         self.assertEqual(str(msg).strip(), 
-            "PRIVMSG op :Please comment on the ban of qwe!*@* in #test, use: @comment 5"
+            "PRIVMSG op :Please comment on the ban of qwe!*@* in #test, use: @comment 1"
             " <comment>")
         self.feedBan('zxc!*@*', prefix='bot!user@host.com')
         msg = self.irc.takeMsg()
         self.assertEqual(str(msg).strip(), 
             "NOTICE #channel :Please somebody comment on the ban of zxc!*@* in #test done by bot,"
-            " use: @comment 6 <comment>")
+            " use: @comment 2 <comment>")
 
-    def testReviewRequest(self):
+    def testReview(self):
         pluginConf.request.ignore.set('')
         cb = self.getCallback()
         self.feedBan('asd!*@*')
@@ -177,6 +179,26 @@ class BantrackerTestCase(ChannelPluginTestCase):
             "PRIVMSG op :Hi, please review the ban 'asd2!*@*' that you set on %s in #test, link: "\
             "%s/bans.cgi?log=2" %(cb.bans['#test'][1].ascwhen, pluginConf.bansite()))
 
+    def testReviewForward(self):
+        pluginConf.request.ignore.set('')
+        pluginConf.request.forward.set('bot')
+        pluginConf.request.forward.channels.set('#channel')
+        cb = self.getCallback()
+        self.feedBan('asd!*@*', prefix='bot!user@host.net')
+        self.irc.takeMsg() # ignore comment request comment
+        pluginConf.request.review.setValue(1.0/86400) # one second
+        cb.reviewBans(self.irc)
+        self.assertFalse(cb.pendingReviews)
+        print 'waiting 2 secs..'
+        time.sleep(2)
+        cb.reviewBans(self.irc)
+        # since it's a forward, it was sent already
+        self.assertFalse(cb.pendingReviews)
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(),
+            "NOTICE #channel :Hi, please somebody review the ban 'asd!*@*' set by bot on %s in #test, link: "\
+            "%s/bans.cgi?log=1" %(cb.bans['#test'][0].ascwhen, pluginConf.bansite()))
+
     def testPersistentCache(self):
         msg1 = ircmsgs.privmsg('nick', 'Hello World')
         msg2 = ircmsgs.privmsg('nick', 'Hello World')
@@ -220,4 +242,3 @@ class BantrackerTestCase(ChannelPluginTestCase):
 
 
 
-

From 75bf9e209cd09ad6ee735dde5d4cf5ffe1613fb9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 01:57:08 -0300
Subject: [PATCH 27/33] refactor and removed some self.log calls

---
 Bantracker/plugin.py | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 6147298..fb7fe4c 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -384,7 +384,7 @@ class Bantracker(callbacks.Plugin):
             self.lastMsgs[irc] = msg
 
     def db_run(self, query, parms, expect_result = False, expect_id = False):
-        self.log.debug("SQL: %q %s", query, parms)
+        #self.log.debug("SQL: %q %s", query, parms)
         if not self.db:
             self.log.error("Bantracker database not open")
             return
@@ -404,7 +404,7 @@ class Bantracker(callbacks.Plugin):
         if expect_result and cur: data = cur.fetchall()
         if expect_id: data = self.db.insert_id()
         self.db.commit()
-        self.log.debug("SQL return: %q", data)
+        #self.log.debug("SQL return: %q", data)
         return data
 
     def requestComment(self, irc, channel, ban, type=None):
@@ -429,22 +429,18 @@ class Bantracker(callbacks.Plugin):
             op = ban.who
         if nickMatch(op, self.registryValue('request.forward', channel=channel)):
             channels = self.registryValue('request.forward.channels', channel=channel)
-            if channels:
-                s = "Please somebody comment on the %s of %s in %s done by %s, use:"\
-                    " %scomment %s <comment>" %(type, mask, channel, op, prefix, ban.id)
-                for chan in channels:
-                    msg = ircmsgs.notice(chan, s)
-                    self.log.info('SENDING: %s' %msg)
-                    irc.queueMsg(msg)
-                return
+            s = "Please somebody comment on the %s of %s in %s done by %s, use:"\
+                " %scomment %s <comment>" %(type, mask, channel, op, prefix, ban.id)
+            for chan in channels:
+                msg = ircmsgs.notice(chan, s)
+                irc.queueMsg(msg)
+            return
         # send to op
         s = "Please comment on the %s of %s in %s, use: %scomment %s <comment>" \
                 %(type, mask, channel, prefix, ban.id)
-        self.log.info('SENDING: %s %s' %(op, s))
         irc.reply(s, to=op, private=True)
 
     def reviewBans(self, irc=None):
-        self.log.debug('Checking for bans that need review ...')
         now = time.mktime(time.gmtime())
         lastreview = self.pendingReviews.time
         bansite = self.registryValue('bansite')
@@ -481,7 +477,6 @@ class Bantracker(callbacks.Plugin):
                     if nickMatch(op, ignore):
                         # in the ignore list
                         continue
-                    self.log.debug('  adding ban to the pending review list ...')
                     if not ban.id:
                         ban.id = self.get_banId(ban.mask, channel)
                     if nickMatch(op, forward):

From 44b2133f1cee043a8f6bf02537f59181301f79d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 13:20:15 -0300
Subject: [PATCH 28/33] Added a fallback: if for some reason we don't have op's
 full hostmask, revert to match by nick for send the review. This may be
 needed in the future as operator's full hostmask aren't stored in the db.

---
 Bantracker/plugin.py | 37 +++++++++++++++++++++++++++++--------
 Bantracker/test.py   | 35 +++++++++++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index fb7fe4c..d3f7a8a 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -471,9 +471,14 @@ class Bantracker(callbacks.Plugin):
                         op = ircutils.nickFromHostmask(ban.who)
                         host = ircutils.hostFromHostmask(ban.who)
                     except:
-                        # probably a ban restored by IRC server in a netsplit
-                        # XXX see if something can be done about this
-                        continue
+                        if ircutils.isNick(ban.who, strictRfc=True):
+                            # ok, op's nick, use it
+                            op = ban.who
+                            host = None
+                        else:
+                            # probably a ban restored by IRC server in a netsplit
+                            # XXX see if something can be done about this
+                            continue
                     if nickMatch(op, ignore):
                         # in the ignore list
                         continue
@@ -505,10 +510,22 @@ class Bantracker(callbacks.Plugin):
         if host in self.pendingReviews:
             op = msg.nick
             for nick, msg in self.pendingReviews[host]:
-                if op != nick:
+                if op != nick and not irc.isChannel(nick): # be extra careful
+                    # correct nick in msg
                     msg = ircmsgs.privmsg(op, msg.args[1])
                 irc.queueMsg(msg)
             del self.pendingReviews[host]
+        # check if we have any reviews by nick to send
+        if None in self.pendingReviews:
+            op = msg.nick
+            L = self.pendingReviews[None]
+            for i, v in enumerate(L):
+                nick, msg = v
+                if nickMatch(op, nick):
+                    irc.queueMsg(msg)
+                    del L[i]
+            if not L:
+                del self.pendingReviews[None]
 
     def doLog(self, irc, channel, s):
         if not self.registryValue('enabled', channel):
@@ -1085,11 +1102,15 @@ class Bantracker(callbacks.Plugin):
 
     def banReview(self, irc, msg, args):
         """Lists pending ban reviews."""
-        count = []
+        count = {}
         for reviews in self.pendingReviews.itervalues():
-            count.append((reviews[0][0], len(reviews)))
-        total = sum([ x[1] for x in count ])
-        s = ' '.join([ '%s:%s' %pair for pair in count ])
+            for nick, msg in reviews:
+                try:
+                    count[nick] += 1
+                except KeyError:
+                    count[nick] = 1
+        total = sum(count.itervalues())
+        s = ' '.join([ '%s:%s' %pair for pair in count.iteritems() ])
         s = 'Pending ban reviews (%s): %s' %(total, s)
         irc.reply(s)
 
diff --git a/Bantracker/test.py b/Bantracker/test.py
index 6ad3b1b..16e8e73 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -23,6 +23,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
         self.setDb()
         pluginConf.request.ignore.set('*') # disable comments
         pluginConf.request.forward.set('')
+        pluginConf.request.review.setValue(1.0/86400) # one second
 
     def setDb(self):
         import sqlite, os
@@ -136,7 +137,6 @@ class BantrackerTestCase(ChannelPluginTestCase):
         cb = self.getCallback()
         self.feedBan('asd!*@*')
         self.irc.takeMsg() # ignore comment request comment
-        pluginConf.request.review.setValue(1.0/86400) # one second
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
         print 'waiting 4 secs..'
@@ -186,7 +186,6 @@ class BantrackerTestCase(ChannelPluginTestCase):
         cb = self.getCallback()
         self.feedBan('asd!*@*', prefix='bot!user@host.net')
         self.irc.takeMsg() # ignore comment request comment
-        pluginConf.request.review.setValue(1.0/86400) # one second
         cb.reviewBans(self.irc)
         self.assertFalse(cb.pendingReviews)
         print 'waiting 2 secs..'
@@ -199,7 +198,35 @@ class BantrackerTestCase(ChannelPluginTestCase):
             "NOTICE #channel :Hi, please somebody review the ban 'asd!*@*' set by bot on %s in #test, link: "\
             "%s/bans.cgi?log=1" %(cb.bans['#test'][0].ascwhen, pluginConf.bansite()))
 
+    def testReviewNickFallback(self):
+        """If for some reason we don't have ops full hostmask, revert to nick match. This may be
+        needed in the future as hostmasks aren't stored in the db."""
+        pluginConf.request.ignore.set('')
+        cb = self.getCallback()
+        self.feedBan('asd!*@*')
+        self.irc.takeMsg() # ignore comment request comment
+        cb.bans['#test'][0].who = 'op' # replace hostmask by nick
+        print 'waiting 2 secs..'
+        time.sleep(2)
+        cb.reviewBans()
+        # check is pending
+        self.assertTrue(cb.pendingReviews)
+        self.assertResponse('banreview', 'Pending ban reviews (1): op:1')
+        # send msg if a user with a matching nick says something
+        self.feedMsg('Hi!', frm='op_!user@host.net') 
+        msg = self.irc.takeMsg()
+        self.assertEqual(msg, None)
+        self.feedMsg('Hi!', frm='op!user@host.net') 
+        msg = self.irc.takeMsg()
+        self.assertEqual(str(msg).strip(),
+            "PRIVMSG op :Hi, please review the ban 'asd!*@*' that you set on %s in #test, link: "\
+            "%s/bans.cgi?log=1" %(cb.bans['#test'][0].ascwhen, pluginConf.bansite()))
+        # check not pending anymore
+        self.assertFalse(cb.pendingReviews)
+
     def testPersistentCache(self):
+        """Save pending reviews and when bans were last checked. This is needed for plugin
+        reloads"""
         msg1 = ircmsgs.privmsg('nick', 'Hello World')
         msg2 = ircmsgs.privmsg('nick', 'Hello World')
         msg3 = ircmsgs.notice('#chan', 'Hello World')
@@ -208,11 +235,11 @@ class BantrackerTestCase(ChannelPluginTestCase):
         pr = cb.pendingReviews
         pr['host.net'] = [('op', msg1), ('op', msg2), ('op_', msg3)]
         pr['home.net'] = [('dude', msg4)]
-        self.assertResponse('banreview', 'Pending ban reviews (4): dude:1 op:3')
+        self.assertResponse('banreview', 'Pending ban reviews (4): op_:1 dude:1 op:2')
         pr.close()
         pr.clear()
         pr.open()
-        self.assertResponse('banreview', 'Pending ban reviews (4): dude:1 op:3')
+        self.assertResponse('banreview', 'Pending ban reviews (4): op_:1 dude:1 op:2')
         items = pr['host.net']
         self.assertTrue(items[0][0] == 'op' and items[0][1] == msg1)
         self.assertTrue(items[1][0] == 'op' and items[1][1] == msg2)

From 8e05e8748a459a57a6ccf69c3651508f1a4ef142 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 21:55:59 -0300
Subject: [PATCH 29/33] refactoring, and fix: config option request.review must
 be global, since the review timestamp isn't channel specific

---
 Bantracker/config.py |  2 +-
 Bantracker/plugin.py | 36 ++++++++++++++++++++----------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index b8f183f..b77e7b2 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -53,7 +53,7 @@ conf.registerChannelValue(Bantracker.request.forward, 'channels',
         registry.SpaceSeparatedListOfStrings([],
             "List of channels/nicks to forward the request if the op that set the ban/quiet"\
             " is in the forward list."))
-conf.registerChannelValue(Bantracker.request, 'review',
+conf.registerGlobalValue(Bantracker.request, 'review',
         registry.Float(7,
             "Days after which the bot will request for review a ban. Can be an integer or decimal value."))
 
diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index f809fe8..7b5581e 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -244,6 +244,10 @@ class Bantracker(callbacks.Plugin):
         self.pendingReviews = PersistentCache('bt.reviews.db')
         self.pendingReviews.open()
         # add scheduled event for check bans that need review, check every hour
+        try:
+            schedule.removeEvent(self.name())
+        except:
+            pass
         schedule.addPeriodicEvent(lambda : self.reviewBans(irc), 60*60,
                 name=self.name())
 
@@ -319,7 +323,7 @@ class Bantracker(callbacks.Plugin):
             self.bans[msg.args[1]] = []
         bans = self.bans[msg.args[1]]
         bans.append(Ban(msg.args))
-        bans.sort(key=lambda x: x.when)
+        bans.sort(key=lambda x: x.when) # needed for self.reviewBans
 
     def nick_to_host(self, irc=None, target='', with_nick=True, reply_now=True):
         target = target.lower()
@@ -354,10 +358,7 @@ class Bantracker(callbacks.Plugin):
         except:
             pass
         queue.clear()
-        try:
-            schedule.removeEvent(self.name())
-        except:
-            pass
+        schedule.removeEvent(self.name())
         self.pendingReviews.close()
 
     def reset(self):
@@ -443,16 +444,19 @@ class Bantracker(callbacks.Plugin):
         irc.reply(s, to=op, private=True)
 
     def reviewBans(self, irc=None):
+        reviewTime = int(self.registryValue('request.review') * 86400)
+        if not reviewTime:
+            # time is zero, do nothing
+            return
         now = time.mktime(time.gmtime())
         lastreview = self.pendingReviews.time
-        bansite = self.registryValue('bansite')
         if not lastreview:
             # initialize last time reviewed timestamp
-            lastreview = now - self.registryValue('request.review')
+            lastreview = now - reviewTime
+        bansite = self.registryValue('bansite')
         for channel, bans in self.bans.iteritems():
-            reviewAfterTime = int(self.registryValue('request.review', channel=channel) * 86400)
-            if not reviewAfterTime:
-                # time is zero, do nothing
+            if not self.registryValue('enabled', channel=channel) \
+                    or not self.registryValue('request', channel=channel):
                 continue
             ignore = self.registryValue('request.ignore', channel=channel)
             forward = self.registryValue('request.forward', channel=channel)
@@ -462,11 +466,11 @@ class Bantracker(callbacks.Plugin):
                 if type in ('quiet', 'removal'):
                     # skip mutes and kicks
                     continue
-                banTime = now - ban.when
-                reviewTime = lastreview - ban.when
-                self.log.debug('  channel %s ban %s (%s %s %s)', channel, str(ban), reviewTime,
-                        reviewAfterTime, reviewAfterTime-reviewTime)
-                if reviewTime <= reviewAfterTime < banTime:
+                banAge = now - ban.when
+                reviewWindow = lastreview - ban.when
+                self.log.debug('  channel %s ban %s by %s (%s/%s/%s %s)', channel, ban.mask,
+                        ban.who, reviewWindow, reviewTime, banAge, reviewTime-reviewWindow)
+                if reviewWindow <= reviewTime < banAge:
                     # ban is old enough, and inside the "review window"
                     try:
                         # ban.who should be a user hostmask
@@ -502,7 +506,7 @@ class Bantracker(callbacks.Plugin):
                         if host not in self.pendingReviews:
                             self.pendingReviews[host] = []
                         self.pendingReviews[host].append((op, msg))
-                elif banTime < reviewAfterTime:
+                elif banAge < reviewTime:
                     # since we made sure bans are sorted by time, the bans left are more recent
                     break
         self.pendingReviews.time = now # update last time reviewed

From ca60ba5aa2b48ab5132f897a932b5c2dc2e37c3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 23:48:54 -0300
Subject: [PATCH 30/33] refactor

---
 Bantracker/plugin.py | 113 ++++++++++++++++++++-----------------------
 Bantracker/test.py   |   1 +
 2 files changed, 54 insertions(+), 60 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 7b5581e..abcfd4b 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -408,40 +408,33 @@ class Bantracker(callbacks.Plugin):
         #self.log.debug("SQL return: %q", data)
         return data
 
-    def requestComment(self, irc, channel, ban, type=None):
-        if not ban or not self.registryValue('request', channel):
-            return
-        # check if we should request a comment
-        if nickMatch(ban.who, self.registryValue('request.ignore', channel=channel)):
+    def requestComment(self, irc, channel, ban):
+        if not ban or not self.registryValue('request', channel) \
+                or nickMatch(ban.who, self.registryValue('request.ignore', channel)):
             return
         # check the type of the action taken
         mask = ban.mask
-        if not type:
-            type = guessBanType(mask)
-            if type == 'quiet':
-                mask = mask[1:]
+        type = guessBanType(mask)
+        if type == 'quiet':
+            mask = mask[1:]
         # check if type is enabled
-        if type not in self.registryValue('request.type', channel=channel):
+        if type not in self.registryValue('request.type', channel):
             return
-        # send msg
         prefix = conf.supybot.reply.whenAddressedBy.chars()[0] # prefix char for commands
         # check to who send the request
         try:
-            op = ircutils.nickFromHostmask(ban.who)
+            nick = ircutils.nickFromHostmask(ban.who)
         except:
-            op = ban.who
-        if nickMatch(op, self.registryValue('request.forward', channel=channel)):
-            channels = self.registryValue('request.forward.channels', channel=channel)
+            nick = ban.who
+        if nickMatch(nick, self.registryValue('request.forward', channel)):
             s = "Please somebody comment on the %s of %s in %s done by %s, use:"\
-                " %scomment %s <comment>" %(type, mask, channel, op, prefix, ban.id)
-            for chan in channels:
-                msg = ircmsgs.notice(chan, s)
-                irc.queueMsg(msg)
-            return
-        # send to op
-        s = "Please comment on the %s of %s in %s, use: %scomment %s <comment>" \
-                %(type, mask, channel, prefix, ban.id)
-        irc.reply(s, to=op, private=True)
+                " %scomment %s <comment>" %(type, mask, channel, nick, prefix, ban.id)
+            self._sendForward(irc, s, channel)
+        else:
+            # send to op
+            s = "Please comment on the %s of %s in %s, use: %scomment %s <comment>" \
+                    %(type, mask, channel, prefix, ban.id)
+            irc.reply(s, to=nick, private=True)
 
     def reviewBans(self, irc=None):
         reviewTime = int(self.registryValue('request.review') * 86400)
@@ -450,85 +443,85 @@ class Bantracker(callbacks.Plugin):
             return
         now = time.mktime(time.gmtime())
         lastreview = self.pendingReviews.time
+        self.pendingReviews.time = now # update last time reviewed
         if not lastreview:
             # initialize last time reviewed timestamp
             lastreview = now - reviewTime
-        bansite = self.registryValue('bansite')
+
         for channel, bans in self.bans.iteritems():
-            if not self.registryValue('enabled', channel=channel) \
-                    or not self.registryValue('request', channel=channel):
+            if not self.registryValue('enabled', channel) \
+                    or not self.registryValue('request', channel):
                 continue
-            ignore = self.registryValue('request.ignore', channel=channel)
-            forward = self.registryValue('request.forward', channel=channel)
-            fchannels = self.registryValue('request.forward.channels', channel=channel)
+
             for ban in bans:
-                type = guessBanType(ban.mask)
-                if type in ('quiet', 'removal'):
+                if guessBanType(ban.mask) in ('quiet', 'removal'):
                     # skip mutes and kicks
                     continue
                 banAge = now - ban.when
                 reviewWindow = lastreview - ban.when
-                self.log.debug('  channel %s ban %s by %s (%s/%s/%s %s)', channel, ban.mask,
-                        ban.who, reviewWindow, reviewTime, banAge, reviewTime-reviewWindow)
+                #self.log.debug('review ban: %s ban %s by %s (%s/%s/%s %s)', channel, ban.mask,
+                #        ban.who, reviewWindow, reviewTime, banAge, reviewTime - reviewWindow)
                 if reviewWindow <= reviewTime < banAge:
                     # ban is old enough, and inside the "review window"
                     try:
                         # ban.who should be a user hostmask
-                        op = ircutils.nickFromHostmask(ban.who)
+                        nick = ircutils.nickFromHostmask(ban.who)
                         host = ircutils.hostFromHostmask(ban.who)
                     except:
                         if ircutils.isNick(ban.who, strictRfc=True):
                             # ok, op's nick, use it
-                            op = ban.who
+                            nick = ban.who
                             host = None
                         else:
                             # probably a ban restored by IRC server in a netsplit
                             # XXX see if something can be done about this
                             continue
-                    if nickMatch(op, ignore):
+                    if nickMatch(nick, self.registryValue('request.ignore', channel)):
                         # in the ignore list
                         continue
                     if not ban.id:
                         ban.id = self.get_banId(ban.mask, channel)
-                    if nickMatch(op, forward):
-                        if not irc:
-                            continue
+                    if nickMatch(nick, self.registryValue('request.forward', channel)):
                         s = "Hi, please somebody review the ban '%s' set by %s on %s in"\
-                        " %s, link: %s/bans.cgi?log=%s" %(ban.mask, op, ban.ascwhen, channel,
-                                bansite, ban.id)
-                        for chan in fchannels:
-                            msg = ircmsgs.notice(chan, s)
-                            irc.queueMsg(msg)
+                        " %s, link: %s/bans.cgi?log=%s" %(ban.mask, nick, ban.ascwhen, channel,
+                                self.registryValue('bansite'), ban.id)
+                        self._sendForward(irc, s, channel)
                     else:
                         s = "Hi, please review the ban '%s' that you set on %s in %s, link:"\
-                        " %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel, bansite, ban.id)
-                        msg = ircmsgs.privmsg(op, s)
-                        if host not in self.pendingReviews:
-                            self.pendingReviews[host] = []
-                        self.pendingReviews[host].append((op, msg))
+                        " %s/bans.cgi?log=%s" %(ban.mask, ban.ascwhen, channel,
+                                self.registryValue('bansite'), ban.id)
+                        msg = ircmsgs.privmsg(nick, s)
+                        if host in self.pendingReviews:
+                            self.pendingReviews[host].append((nick, msg))
+                        else:
+                            self.pendingReviews[host] = [(nick, msg)]
                 elif banAge < reviewTime:
                     # since we made sure bans are sorted by time, the bans left are more recent
                     break
-        self.pendingReviews.time = now # update last time reviewed
+
+    def _sendForward(self, irc, s, channel=None):
+        if not irc:
+            return
+        for chan in self.registryValue('request.forward.channels', channel=channel):
+            msg = ircmsgs.notice(chan, s)
+            irc.queueMsg(msg)
 
     def _sendReviews(self, irc, msg):
         host = ircutils.hostFromHostmask(msg.prefix)
         if host in self.pendingReviews:
-            op = msg.nick
-            for nick, msg in self.pendingReviews[host]:
-                if op != nick and not irc.isChannel(nick): # be extra careful
+            for nick, m in self.pendingReviews[host]:
+                if msg.nick != nick and not irc.isChannel(nick): # I'm a bit extra careful here
                     # correct nick in msg
-                    msg = ircmsgs.privmsg(op, msg.args[1])
-                irc.queueMsg(msg)
+                    m = ircmsgs.privmsg(msg.nick, m.args[1])
+                irc.queueMsg(m)
             del self.pendingReviews[host]
         # check if we have any reviews by nick to send
         if None in self.pendingReviews:
-            op = msg.nick
             L = self.pendingReviews[None]
             for i, v in enumerate(L):
-                nick, msg = v
-                if nickMatch(op, nick):
-                    irc.queueMsg(msg)
+                nick, m = v
+                if ircutils.strEqual(msg.nick, nick):
+                    irc.queueMsg(m)
                     del L[i]
             if not L:
                 del self.pendingReviews[None]
diff --git a/Bantracker/test.py b/Bantracker/test.py
index 77906ac..4093045 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -157,6 +157,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
         # don't ask again
         cb.reviewBans()
         self.assertFalse(cb.pendingReviews)
+        # test again with two ops
         self.feedBan('asd2!*@*')
         self.irc.takeMsg()
         self.feedBan('qwe!*@*', prefix='otherop!user@home.net')

From b6887f5ede65bf2e525c3b2a921b321656201252 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sat, 3 Apr 2010 23:53:05 -0300
Subject: [PATCH 31/33] config help review

---
 Bantracker/config.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Bantracker/config.py b/Bantracker/config.py
index b77e7b2..afc665f 100644
--- a/Bantracker/config.py
+++ b/Bantracker/config.py
@@ -15,10 +15,12 @@
 import supybot.conf as conf
 import supybot.registry as registry
 
+
 class ValidTypes(registry.OnlySomeStrings):
     """Invalid type, valid types are: 'removal', 'ban' or 'quiet'."""
     validStrings = ('removal', 'ban', 'quiet')
 
+
 class SpaceSeparatedListOfTypes(registry.SpaceSeparatedListOf):
     Value = ValidTypes
 
@@ -42,7 +44,7 @@ conf.registerChannelValue(Bantracker.request, 'type',
             "List of events for which the bot should request a comment."))
 conf.registerChannelValue(Bantracker.request, 'ignore',
         registry.SpaceSeparatedListOfStrings(['FloodBot?', 'FloodBotK?', 'ChanServ'],
-            "List of nicks for which the bot won't request to comment a ban/quiet/removal."\
+            "List of nicks for which the bot won't request to comment or review."\
             " Is case insensible and wildcards * ? are accepted."))
 conf.registerChannelValue(Bantracker.request, 'forward',
         registry.SpaceSeparatedListOfStrings([],
@@ -51,10 +53,10 @@ conf.registerChannelValue(Bantracker.request, 'forward',
             " Is case insensible and wildcards * ? are accepted."))
 conf.registerChannelValue(Bantracker.request.forward, 'channels',
         registry.SpaceSeparatedListOfStrings([],
-            "List of channels/nicks to forward the request if the op that set the ban/quiet"\
-            " is in the forward list."))
+            "List of channels/nicks to forward the request if the op is in the forward list."))
 conf.registerGlobalValue(Bantracker.request, 'review',
         registry.Float(7,
-            "Days after which the bot will request for review a ban. Can be an integer or decimal value."))
+            "Days after which the bot will request for review a ban. Can be an integer or decimal"
+            " value. Zero disables reviews."))
 
 

From 484d45e7055404ec3d6c243904192afd99396372 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sun, 4 Apr 2010 01:31:15 -0300
Subject: [PATCH 32/33] use check_auth in banreview, and made a workaround so
 testcases still pass

---
 Bantracker/plugin.py |  9 ++++++---
 Bantracker/test.py   | 24 +++++++++++++++++++++---
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index abcfd4b..9ff8c3d 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -1103,8 +1103,11 @@ class Bantracker(callbacks.Plugin):
             irc.reply("%s/bans.cgi?log=%s&mark=%s" % (self.registryValue('bansite'), id, highlight), private=True)
     banlink = wrap(banlink, ['id', optional('somethingWithoutSpaces')])
 
-    def banReview(self, irc, msg, args):
-        """Lists pending ban reviews."""
+    def banreview(self, irc, msg, args):
+        """
+        Lists pending ban reviews."""
+        if not self.check_auth(irc, msg, args):
+            return
         count = {}
         for reviews in self.pendingReviews.itervalues():
             for nick, msg in reviews:
@@ -1117,6 +1120,6 @@ class Bantracker(callbacks.Plugin):
         s = 'Pending ban reviews (%s): %s' %(total, s)
         irc.reply(s)
 
-    banreview = wrap(banReview)
+    banreview = wrap(banreview)
 
 Class = Bantracker
diff --git a/Bantracker/test.py b/Bantracker/test.py
index 4093045..d8a36be 100644
--- a/Bantracker/test.py
+++ b/Bantracker/test.py
@@ -2,6 +2,7 @@ from supybot.test import *
 
 import supybot.conf as conf
 import supybot.ircmsgs as ircmsgs
+import supybot.world as world
 
 import time
 
@@ -20,11 +21,22 @@ class BantrackerTestCase(ChannelPluginTestCase):
 
     def setUp(self):
         super(BantrackerTestCase, self).setUp()
-        self.setDb()
         pluginConf.request.setValue(False) # disable comments
         pluginConf.request.ignore.set('')
         pluginConf.request.forward.set('')
         pluginConf.request.review.setValue(1.0/86400) # one second
+        self.setDb()
+        # Bantracker for some reason doesn't use Supybot's own methods for check capabilities,
+        # so it doesn't have a clue about testing and screws my tests by default.
+        # This would fix it until I bring myself to take a look
+        cb = self.getCallback()
+        f = cb.check_auth
+        def test_check_auth(*args, **kwargs):
+            if world.testing:
+                return True
+            else:
+                return f(*args, **kwargs)
+        cb.check_auth = test_check_auth
 
     def setDb(self):
         import sqlite, os
@@ -233,8 +245,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
         msg2 = ircmsgs.privmsg('nick', 'Hello World')
         msg3 = ircmsgs.notice('#chan', 'Hello World')
         msg4 = ircmsgs.privmsg('nick_', 'Hello World')
-        cb = self.getCallback()
-        pr = cb.pendingReviews
+        pr = self.getCallback().pendingReviews
         pr['host.net'] = [('op', msg1), ('op', msg2), ('op_', msg3)]
         pr['home.net'] = [('dude', msg4)]
         self.assertResponse('banreview', 'Pending ban reviews (4): op_:1 dude:1 op:2')
@@ -249,6 +260,13 @@ class BantrackerTestCase(ChannelPluginTestCase):
         items = pr['home.net']
         self.assertTrue(items[0][0] == 'dude' and items[0][1] == msg4)
 
+    def testReviewBanreview(self):
+        pr = self.getCallback().pendingReviews
+        m = ircmsgs.privmsg('#test', 'asd')
+        pr['host.net'] = [('op', m), ('op_', m), ('op', m)]
+        pr['home.net'] = [('dude', m)]
+        self.assertResponse('banreview', 'Pending ban reviews (4): op_:1 dude:1 op:2')
+
     def testBan(self):
         self.feedBan('asd!*@*')
         fetch = self.query("SELECT id,channel,mask,operator FROM bans")

From f1b72a7f57120a2c9c67691af764aef9a42f748e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eli=C3=A1n=20Hanisch?= <lambdae2@gmail.com>
Date: Sun, 4 Apr 2010 01:39:50 -0300
Subject: [PATCH 33/33] forgot to remove this

---
 Bantracker/plugin.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Bantracker/plugin.py b/Bantracker/plugin.py
index 9ff8c3d..60ef297 100644
--- a/Bantracker/plugin.py
+++ b/Bantracker/plugin.py
@@ -385,7 +385,6 @@ class Bantracker(callbacks.Plugin):
             self.lastMsgs[irc] = msg
 
     def db_run(self, query, parms, expect_result = False, expect_id = False):
-        #self.log.debug("SQL: %q %s", query, parms)
         if not self.db:
             self.log.error("Bantracker database not open")
             return
@@ -393,8 +392,7 @@ class Bantracker(callbacks.Plugin):
         try:
             cur = self.db.cursor()
             cur.execute(query, parms)
-        except Exception, e:
-            self.log.error('error: %s', e)
+        except:
             cur = None
             if n_tries > 5:
                 print "Tried more than 5 times, aborting"
@@ -405,7 +403,6 @@ class Bantracker(callbacks.Plugin):
         if expect_result and cur: data = cur.fetchall()
         if expect_id: data = self.db.insert_id()
         self.db.commit()
-        #self.log.debug("SQL return: %q", data)
         return data
 
     def requestComment(self, irc, channel, ban):