oops: ticket filters

Refactor filter.yaml and related code base
Move isBanned routine to banlist class and make it banlist specific
Refactor how canned responses get list of filters

include/class.banlist.php

@@ -25,8 +25,31 @@ class Banlist {
         return self::getSystemBanList()->removeRule('email','equal',$email);
     }
 
-    function isbanned($email) {
-        return TicketFilter::isBanned($email);
+    /**
+     * Quick function to determine if the received email-address is in the
+     * banlist. Returns the filter of the filter that has the address
+     * blacklisted and FALSE if the email is not blacklisted.
+     *
+     */
+    static function isBanned($addr) {
+
+        if (!($filter=self::getFilter()))
+            return false;
+
+        $sql='SELECT filter.id '
+            .' FROM '.FILTER_TABLE.' filter'
+            .' INNER JOIN '.FILTER_RULE_TABLE.' rule'
+            .'  ON (filter.id=rule.filter_id)'
+            .' WHERE filter.id='.db_input($filter->getId())
+            .'   AND filter.isactive'
+            .'   AND rule.isactive '
+            .'   AND rule.what="email"'
+            .'   AND rule.val='.db_input($addr);
+
+        if (($res=db_query($sql)) && db_num_rows($res))
+            return $filter;
+
+        return false;
     }
 
     function includes($email) {
@@ -61,7 +84,7 @@ class Banlist {
         return new Filter(self::ensureSystemBanList());
     }
 
-    function getFilter() {
+    static function getFilter() {
         return self::getSystemBanList();
     }
 }

include/class.canned.php

@@ -84,9 +84,28 @@ class Canned {
         return $this->isEnabled();
     }
 
+    function getFilters() {
+
+        if (!isset($this->_filters)) {
+            $this->_filters = array();
+            $cid = sprintf('"canned_id":%d', $this->getId());
+            $sql='SELECT filter.id, filter.name '
+                .' FROM '.FILTER_TABLE.' filter'
+                .' INNER JOIN '.FILTER_ACTION_TABLE.' action'
+                .'  ON (filter.id=action.filter_id)'
+                .' WHERE action.type="canned"'
+                ."  AND action.configuration LIKE '%$cid%'";
+
+            if (($res=db_query($sql)) && db_num_rows($res))
+                while (list($id, $name) = db_fetch_row($res))
+                    $this->_filters[$id] = $name;
+        }
+
+        return $this->_filters;
+    }
+
     function getNumFilters() {
-        //XXX : Query the filter action table and cache the results
-        return $this->ht['filters'];
+        return count($this->getFilters());
     }
 
     function getTitle() {
@@ -174,19 +193,6 @@ class Canned {
         return $this->getHashtable();
     }
 
-    function getFilters() {
-        if (!$this->_filters) {
-            $this->_filters = array();
-            $res = db_query(
-                  'SELECT name FROM '.FILTER_TABLE
-                .' WHERE canned_response_id = '.db_input($this->getId())
-                .' ORDER BY name');
-            while ($row = db_fetch_row($res))
-                $this->_filters[] = $row[0];
-        }
-        return $this->_filters;
-    }
-
     function update($vars, &$errors) {
 
         if(!$this->save($this->getId(),$vars,$errors))

include/class.filter.php

@@ -503,18 +503,8 @@ class Filter {
             .',email_id='.db_input($emailId)
             .',match_all_rules='.db_input($vars['match_all_rules'])
             .',stop_onmatch='.db_input(isset($vars['stop_onmatch'])?1:0)
-            .',reject_ticket='.db_input(isset($vars['reject_ticket'])?1:0)
             .',notes='.db_input(Format::sanitize($vars['notes']));
 
-
-        //Auto assign ID is overloaded...
-        if($vars['assign'] && $vars['assign'][0]=='s')
-             $sql.=',team_id=0,staff_id='.db_input(preg_replace("/[^0-9]/", "",$vars['assign']));
-        elseif($vars['assign'] && $vars['assign'][0]=='t')
-            $sql.=',staff_id=0,team_id='.db_input(preg_replace("/[^0-9]/", "",$vars['assign']));
-        else
-            $sql.=',staff_id=0,team_id=0 '; //no auto-assignment!
-
         if($id) {
             $sql='UPDATE '.FILTER_TABLE.' SET '.$sql.' WHERE id='.db_input($id);
             if(!db_query($sql))
@@ -543,8 +533,16 @@ class Filter {
             return;
 
         foreach ($vars['actions'] as $sort=>$v) {
-            $info = substr($v, 1);
-            switch ($v[0]) {
+            if (is_array($v)) {
+                $info = $v['type'];
+                $sort = $v['sort'] ?: $sort;
+                $action = 'N';
+            } else {
+                $action = $v[0];
+                $info = substr($v, 1);
+            }
+
+            switch ($action) {
             case 'N': # new filter action
                 $I = FilterAction::create(array(
                     'type'=>$info,
@@ -806,51 +804,6 @@ class TicketFilter {
         return db_query($sql);
     }
 
-    /**
-     * Quick function to determine if the received email-address is
-     * indicated by an active email filter to be banned. Returns the id of
-     * the filter that has the address blacklisted and FALSE if the email is
-     * not blacklisted.
-     *
-     * XXX: If more detailed matching is to be supported, perhaps this
-     *      should receive an array like the constructor and
-     *      Filter::matches() method.
-     *      Peter - Let's keep it as a quick scan for obviously banned emails.
-     */
-    /* static */
-    function isBanned($addr) {
-
-        $sql='SELECT filter.id, what, how, UPPER(val) '
-            .' FROM '.FILTER_TABLE.' filter'
-            .' INNER JOIN '.FILTER_RULE_TABLE.' rule'
-            .' ON (filter.id=rule.filter_id)'
-            .' WHERE filter.reject_ticket'
-            .'   AND filter.match_all_rules=0'
-            .'   AND filter.email_id=0'
-            .'   AND filter.isactive'
-            .'   AND rule.isactive '
-            .'   AND rule.what="email"'
-            .'   AND LOCATE(rule.val,'.db_input($addr).')';
-
-        if(!($res=db_query($sql)) || !db_num_rows($res))
-            return false;
-
-        # XXX: Use MB_xxx function for proper unicode support
-        $addr = strtoupper($addr);
-        $how=array('equal'      => array('strcmp', 0),
-                   'contains'   => array('strpos', null, false));
-
-        while ($row=db_fetch_array($res)) {
-            list($func, $pos, $neg) = $how[$row['how']];
-            if (!$func) continue;
-            $result = call_user_func($func, $addr, $row['val']);
-            if (($neg === null && $result === $pos) || $result !== $neg)
-                return $row['id'];
-        }
-
-        return false;
-    }
-
     /**
      * Simple true/false if the headers of the email indicate that the email
      * is an automatic response.

include/class.mailfetch.php

@@ -19,6 +19,7 @@ require_once(INCLUDE_DIR.'class.ticket.php');
 require_once(INCLUDE_DIR.'class.dept.php');
 require_once(INCLUDE_DIR.'class.email.php');
 require_once(INCLUDE_DIR.'class.filter.php');
+require_once(INCLUDE_DIR.'class.banlist.php');
 require_once(INCLUDE_DIR.'tnef_decoder.php');
 
 class MailFetcher {
@@ -600,7 +601,7 @@ class MailFetcher {
         }
 
 	    //Is the email address banned?
-        if($mailinfo['email'] && TicketFilter::isBanned($mailinfo['email'])) {
+        if($mailinfo['email'] && Banlist::isBanned($mailinfo['email'])) {
 	        //We need to let admin know...
             $ost->logWarning(_S('Ticket denied'),
                 sprintf(_S('Banned email — %s'),$mailinfo['email']), false);

include/class.ticket.php

@@ -2669,8 +2669,8 @@ class Ticket {
 
         try {
             // Make sure the email address is not banned
-            if (TicketFilter::isBanned($vars['email'])) {
-                throw new RejectedException(Banlist::getFilter(), $vars);
+            if (($filter=Banlist::isBanned($vars['email']))) {
+                throw new RejectedException($filter, $vars);
             }
 
             // Init ticket filters...

include/i18n/en_US/filter.yaml

@@ -8,8 +8,6 @@
 # Fields:
 # isactive - (bool:0|1) true or false if the filter is initially enabled
 # execorder - (int) order the filters should be executed in (lowest first)
-# reject_ticket - (bool:0|1) if a ticket matches the filter it should be
-#       rejected (ie. not be created).
 # name - (string) Descriptive name for the filter
 # notes - (string) Administrative notes (viewable internally only)
 # rules - (list<FilterRule>) List of rules for the filter
@@ -19,19 +17,25 @@
 # what - (enum<email|>) field to check
 # how - (enum<equals|contains|dncontain>) how to check for <val>
 # val - (string) search value
+#
+# Fields for FilterAction
+# type - type of filter action
+#
 ---
 - isactive: 1
   execorder: 99
-  reject_ticket: 1
-  # NOTE: Don't translate 'Email'
-  target: Email
-  # NOTE: Don't translate 'SYSTEM BAN LIST'
-  name: SYSTEM BAN LIST
+  match_all_rules: 0
+  email_id: 0
+  target: Email #notrans
+  name: SYSTEM BAN LIST #notrans
   notes: |
     Internal list for email banning. Do not remove
 
   rules:
     - isactive: 1
-      what: email
-      how: equal
-      val: test@example.com
+      what: email #notrans
+      how: equal #notrans
+      val: test@example.com #notrans
+  actions:
+    - sort: 1
+      type: reject #notrans