set ban duration with @comment command, also allow multiple ban ids

example:
 @comment 1,2 this is a comment, 1 week

Bantracker/plugin.py

@@ -119,8 +119,11 @@ def readTimeDelta(s):
                 # completed an unit, add to seconds
                 string = string.strip()
                 if string:
-                    mult = timeUnits[string]
+                    try:
-                    seconds += number * mult
+                        unit = timeUnits[string]
+                    except KeyError:
+                        raise ValueError(string)
+                    seconds += number * unit
                     string = ''
             string += c
         else:
@@ -139,8 +142,11 @@ def readTimeDelta(s):
     else:
         string = string.strip()
         if string:
-            mult = timeUnits[string]
+            try:
-            seconds += number * mult
+                unit = timeUnits[string]
+            except KeyError:
+                raise ValueError(string)
+            seconds += number * unit
 
     return seconds
 
@@ -1466,12 +1472,54 @@ class Bantracker(callbacks.Plugin):
 
     updatebt = wrap(updatebt, [optional('anything', default=None)])
 
-    def comment(self, irc, msg, args, id, kickmsg):
+    def _getBan(self, id):
-        """<id> [<comment>]
+        """gets mask, channel and removal date of ban"""
+        L = self.db_run("SELECT mask, channel, removal FROM bans WHERE id = %s",
+                        id, expect_result=True)
+        if not L:
+            raise ValueError
+        return L[0]
+
+    def _setBanDuration(self, id, duration):
+        # check if ban has already a duration time
+        for idx, br in enumerate(self.managedBans):
+            if id == br.ban.id:
+                ban = br.ban
+                del self.managedBans.shelf[idx]
+                break
+        else:
+            # ban obj ins't in self.managedBans
+            try:
+                mask, channel, removal = self._getBan(id)
+            except ValueError:
+                raise Exception("unknow id")
+
+            type = guessBanType(mask)
+            if type not in ('ban', 'quiet'):
+                raise Exception("not a ban or quiet")
+
+            if removal:
+                raise Exception("ban was removed")
+
+            for ban in self.bans[channel]:
+                if mask == ban.mask:
+                    if ban.id is None:
+                        ban.id = id
+                    break
+            else:
+                # ban not in sync it seems, shouldn't happen normally.
+                raise Exception("bans not in sync")
+
+        # add ban duration
+        self.managedBans.add(BanRemoval(ban, duration))
+
+    def comment(self, irc, msg, args, ids, kickmsg):
+        """<id>[,<id> ...] [<comment>][, <duration>]
 
         Reads or adds the <comment> for the ban with <id>,
         use @bansearch to find the id of a ban
         """
+
         def addComment(id, nick, msg):
             n = now()
             self.db_run("INSERT INTO comments (ban_id, who, comment, time) values(%s,%s,%s,%s)", (id, nick, msg, n))
@@ -1479,74 +1527,74 @@ class Bantracker(callbacks.Plugin):
             return self.db_run("SELECT who, comment, time FROM comments WHERE ban_id=%i", (id,), True)
 
         nick = msg.nick
-        if kickmsg:
+        duration, banset = None, []
-            addComment(id, nick, kickmsg)
+        if kickmsg and ',' in kickmsg:
-            irc.replySuccess()
+            s = kickmsg[kickmsg.rfind(','):]
-        else:
+            try:
-            data = readComment(id)
+                duration = readTimeDelta(s)
-            if data:
+            except ValueError:
-                for c in data:
+                pass
-                    irc.reply("%s %s: %s" % (cPickle.loads(c[2]).astimezone(pytz.timezone('UTC')).strftime("%b %d %Y %H:%M:%S"), c[0], c[1].strip()) )
+
+        for id in [ int(i) for i in ids.split(',') if i.isdigit() and int(i) > 0 ]:
+            try:
+                mask, channel, removal = self._getBan(id)
+            except ValueError:
+                irc.reply("I don't know any ban with id %s." % id)
+                continue
+
+            if kickmsg:
+                addComment(id, nick, kickmsg)
+                if duration is not None:
+                    # set duration time
+                    type = guessBanType(mask)
+                    if type not in ('ban', 'quiet'):
+                        continue
+
+                    try:
+                        self._setBanDuration(id, duration)
+                        banset.append(str(id))
+                    except Exception as exc:
+                        irc.reply("Failed to set duration time on ban %s (%s)" % (id, exc))
             else:
-                irc.error("No comments recorded for ban %i" % id)
+                data = readComment(id)
-    comment = wrap(comment, ['id', optional('text')])
+                if data:
+                    for c in data:
+                        date = cPickle.loads(c[2]).astimezone(pytz.timezone('UTC')).strftime("%b %d %Y %H:%M")
+                        irc.reply("%s %s: %s" % (date, c[0], c[1].strip()))
+                else:
+                    irc.reply("No comments recorded for ban %s" % id)
+
+        # success reply. If duration time used, say which ones.
+        if kickmsg:
+            if duration is not None:
+                if banset:
+                    irc.reply("Ban set for auto removal: %s" % ', '.join(banset))
+            else:
+                irc.replySuccess()
+
+    comment = wrap(comment, ['something', optional('text')])
 
     def banremove(self, irc, msg, args, ids, timespec):
-        """<id>[,<id> ...] <time>
+        """<id>[,<id> ...] <duration>
 
         Sets expiration time.
         """
         try:
             seconds = readTimeDelta(timespec)
-        except KeyError:
+        except ValueError:
             irc.error("bad time format.")
             return
 
-        ids = [ int(i) for i in ids.split(',') if i.isdigit() and int(i) > 0 ]
+        banset = []
+        for id in [ int(i) for i in ids.split(',') if i.isdigit() and int(i) > 0 ]:
+            try:
+                self._setBanDuration(id, seconds)
+                banset.append(str(id))
+            except Exception as exc:
+                irc.reply("Failed to set duration time on ban %s (%s)" % (id, exc))
 
-        for id in ids:
+        if banset:
-            # lets check if is already managed first
+            irc.reply("Ban set for auto removal: %s" % ', '.join(banset))
-            for idx, remove in enumerate(self.managedBans):
-                if id == remove.ban.id:
-                    ban = remove.ban
-                    del self.managedBans.shelf[idx]
-                    break
-            else:
-                L = self.db_run("SELECT mask, channel, removal FROM bans WHERE id = %s",
-                                id, expect_result=True)
-                if not L:
-                    irc.reply("I don't know any ban with id %s." % id)
-                    continue
-
-                mask, channel, removal = L[0]
-                type = guessBanType(mask)
-                if type not in ('ban', 'quiet'):
-                    irc.reply("Id %s is a %s, only bans or quiets can be autoremoved." % (id, type))
-                    continue
-
-                if removal:
-                    irc.reply("Ban %s (%s) was already removed in %s." % (id, mask, channel))
-                    continue
-
-                for ban in self.bans[channel]:
-                    if mask == ban.mask:
-                        if ban.id is None:
-                            ban.id = id
-                        break
-                else:
-                    # ban not in sync it seems, shouldn't happen normally.
-                    irc.reply("Ban %s (%s) isn't active in %s." % (id, mask, channel))
-                    continue
-
-            self.managedBans.add(BanRemoval(ban, seconds))
-
-        # check bans set
-        done = []
-        for br in self.managedBans:
-            if br.ban.id in ids:
-                done.append(str(br.ban.id))
-        if done:
-            irc.reply("Ban set for auto removal: %s" % ', '.join(done))
 
     banremove = wrap(banremove, ['something', 'text'])
 

Bantracker/test.py

@@ -125,6 +125,30 @@ class BantrackerTestCase(ChannelPluginTestCase):
         return ban
 
     def testComment(self):
+        self.assertResponse('comment 1', "I don't know any ban with id 1.")
+        self.feedBan('asd!*@*')
+        self.assertResponse('comment 1', 'No comments recorded for ban 1')
+        self.assertResponse('comment 1 this is a test', 'The operation succeeded.')
+        self.assertRegexp('comment 1', 'test: this is a test$')
+
+    def testMultiComment(self):
+        self.feedBan('asd!*@*')
+        self.feedBan('qwe!*@*')
+        self.assertResponse('comment 1,2,3 this is a test',
+                            "I don't know any ban with id 3.")
+        msg = self.irc.takeMsg()
+        self.assertEqual(msg.args[1], "test: The operation succeeded.")
+        self.assertRegexp('comment 1,2', 'test: this is a test$')
+        msg = self.irc.takeMsg()
+        self.assertTrue(msg.args[1].endswith("test: this is a test"))
+
+    def testCommentDuration(self):
+        self.feedBan('asd!*@*')
+        self.assertResponse('comment 1 this is a test, 1 week 10', 'Ban set for auto removal: 1')
+        self.assertRegexp('comment 1', 'test: this is a test, 1 week 10$')
+        self.assertRegexp('baninfo 1', 'expires in 1 week$')
+
+    def testCommentRequest(self):
         pluginConf.request.setValue(True)
         # test bans
         self.feedBan('asd!*@*')
@@ -382,7 +406,7 @@ class BantrackerTestCase(ChannelPluginTestCase):
     def testBanremoveMultiSet(self):
         self.feedBan('asd!*@*')
         self.assertResponse('banremove 1,2 10',
-            "I don't know any ban with id 2.")
+                            "Failed to set duration time on ban 2 (unknow id)")
         msg = self.irc.takeMsg()
         self.assertEqual(msg.args[1], "test: Ban set for auto removal: 1")
 
@@ -400,19 +424,19 @@ class BantrackerTestCase(ChannelPluginTestCase):
     def testBanremoveBadType(self):
         self.feedBan('nick', mode='k')
         self.assertResponse('banremove 1 0',
-            "Id 1 is a removal, only bans or quiets can be autoremoved.")
+            "Failed to set duration time on ban 1 (not a ban or quiet)")
         self.feedBan('$a:nick')
         self.assertResponse('banremove 2 0', 'Ban set for auto removal: 2')
 
     def testBanremoveBadId(self):
-        self.assertResponse('banremove 1 0', "I don't know any ban with id 1.")
+        self.assertResponse('banremove 1 0', "Failed to set duration time on ban 1 (unknow id)")
 
     def testBanremoveInactiveBan(self):
         self.feedBan('asd!*@*')
         self.irc.feedMsg(ircmsgs.unban(self.channel, 'asd!*@*', 
                                        'op!user@host.net'))
         self.assertResponse('banremove 1 0', 
-                            "Ban 1 (asd!*@*) was already removed in #test.")
+                            "Failed to set duration time on ban 1 (ban was removed)")
 
     def testBanremoveTimeFormat(self):
         cb = self.getCallback()