Changes for saving Filter Actions:

- Save Filter Actions at the top of Filter::save
- Validate the Filter Actions before commiting the save

This is done to fix the issue where Depts/HT's said 'Unchanged' when initially saved. Now their configuration is set correctly before doing additional validations for active depts/hts

include/class.filter.php

@@ -471,24 +471,28 @@ class Filter {
     }
 
     function save($id,$vars,&$errors) {
+      //get current filter actions (they're validated before saving)
+      self::save_actions($id, $vars, $errors);
+
       if ($this) {
         foreach ($this->getActions() as $A) {
+          $config = JsonDataParser::parse($A->configuration);
           if ($A->type == 'dept') {
-            $dept = Dept::lookup($A->parseConfiguration($vars)['dept_id']);
+            $dept = Dept::lookup($config['dept_id']);
             $dept_action = $A->getId();
           }
 
           if ($A->type == 'topic') {
-            $topic = Topic::lookup($A->parseConfiguration($vars)['topic_id']);
+            $topic = Topic::lookup($config['topic_id']);
             $topic_action = $A->getId();
           }
         }
       }
 
-      if($dept && !$dept->isActive() && !in_array('D' . $dept_action,$vars['actions']))
+      if($dept && !$dept->isActive() && (is_array($vars['actions']) && !in_array('D' . $dept_action,$vars['actions'])))
         $errors['err'] = sprintf(__('%s selected for %s must be active'), __('Department'), __('Filter Action'));
 
-      if($topic && !$topic->isActive() && !in_array('D' . $topic_action,$vars['actions']))
+      if($topic && !$topic->isActive() && (is_array($vars['actions']) && !in_array('D' . $topic_action,$vars['actions'])))
         $errors['err'] = sprintf(__('%s selected for %s must be active'), __('Help Topic'), __('Filter Action'));
 
         if(!$vars['execorder'])
@@ -547,7 +551,6 @@ class Filter {
         # Don't care about errors stashed in $xerrors
         $xerrors = array();
         self::save_rules($id,$vars,$xerrors);
-        self::save_actions($id, $vars, $errors);
 
         return count($errors) == 0;
     }
@@ -555,20 +558,31 @@ class Filter {
     function validate_actions($action) {
       $errors = array();
       $config = json_decode($action->ht['configuration'], true);
-      if ($action->ht['type'] == 'dept') {
-        $dept = Dept::lookup($config['dept_id']);
-        if (!$dept || !$dept->isActive()) {
-          $errors['err'] = sprintf(__('Unable to save: Please choose an active %s'), 'Department');
-          return $errors;
-        }
-      }
+      switch ($action->ht['type']) {
+        case 'dept':
+          $dept = Dept::lookup($config['dept_id']);
+          if (!$dept || !$dept->isActive()) {
+            $errors['err'] = sprintf(__('Unable to save: Please choose an active %s'), 'Department');
+            return $errors;
+          }
+          break;
 
-      if ($action->ht['type'] == 'topic') {
-        $topic = Topic::lookup($config['topic_id']);
-        if (!$topic || !$topic->isActive()) {
-          $errors['err'] = sprintf(__('Unable to save: Please choose an active %s'), 'Help Topic');
-          return $errors;
-        }
+        case 'topic':
+          $topic = Topic::lookup($config['topic_id']);
+          if (!$topic || !$topic->isActive()) {
+            $errors['err'] = sprintf(__('Unable to save: Please choose an active %s'), 'Help Topic');
+            return $errors;
+          }
+          break;
+
+        default:
+          foreach ($config as $key => $value) {
+            if (!$value) {
+              $errors['err'] = sprintf(__('Unable to save: Please insert a value for %s'), ucfirst($action->ht['type']));
+              return $errors;
+            }
+          }
+          break;
       }
 
       return false;
@@ -597,7 +611,6 @@ class Filter {
                     'sort' => (int) $sort,
                 ));
                 $I->setConfiguration($errors, $vars);
-                $config = json_decode($I->ht['configuration'], true);
 
                 $invalid = self::validate_actions($I);
                 if ($invalid) {
@@ -611,8 +624,6 @@ class Filter {
                 if ($I = FilterAction::lookup($info)) {
                     $I->setConfiguration($errors, $vars);
 
-                    $config = json_decode($I->ht['configuration'], true);
-
                     $invalid = self::validate_actions($I);
                     if ($invalid) {
                       $errors['err'] = sprintf($invalid['err']);

include/staff/filter.inc.php

@@ -240,17 +240,16 @@ $info=Format::htmlchars(($errors && $_POST)?$_POST:$info);
                 if ($filter) { foreach ($filter->getActions() as $A) {
                     $_warn = '';
                     $existing[] = $A->type;
+                    $config = JsonDataParser::parse($A->configuration);
                     if($A->type == 'dept') {
                       $errors['topic_id'] = '';
-                      $dept_config = $A->parseConfiguration($_POST);
-                      $dept = Dept::lookup($dept_config['dept_id']);
+                      $dept = Dept::lookup($config['dept_id']);
                       if($dept && !$dept->isActive())
                         $_warn = sprintf(__('%s must be active'), __('Department'));
                     }
                     elseif($A->type == 'topic') {
                       $errors['dept_id'] = '';
-                      $topic_config = $A->parseConfiguration($_POST);
-                      $topic = Topic::lookup($topic_config['topic_id']);
+                      $topic = Topic::lookup($config['topic_id']);
                       if($topic && !$topic->isActive())
                         $_warn = sprintf(__('%s must be active'), __('Help Topic'));
                     }

scp/filters.php

@@ -120,11 +120,12 @@ $tip_namespace = 'manage.filter';
 if($filter || ($_REQUEST['a'] && !strcasecmp($_REQUEST['a'],'add'))) {
   if($filter) {
     foreach ($filter->getActions() as $A) {
+      $config = JsonDataParser::parse($A->configuration);
       if($A->type == 'dept')
-        $dept = Dept::lookup($A->parseConfiguration($_POST)['dept_id']);
+        $dept = Dept::lookup($config['dept_id']);
 
       if($A->type == 'topic')
-        $topic = Topic::lookup($A->parseConfiguration($_POST)['topic_id']);
+        $topic = Topic::lookup($config['topic_id']);
     }
   }