From 6b762ab6e8e9ff6fb7c2eefc616f712ba74169c1 Mon Sep 17 00:00:00 2001
From: Jared Hancock <jared@osticket.com>
Date: Wed, 11 Mar 2015 13:08:07 -0500
Subject: [PATCH] forms: Fix a couple places where validation errors are not
propagated
Reset field content when building a new form
---
include/ajax.tickets.php | 2 +-
include/class.dynamic_forms.php | 71 ++++++++++++++++++++++---------
include/class.filter.php | 16 ++++++-
include/class.forms.php | 2 +-
include/class.ticket.php | 2 +-
include/class.user.php | 12 +++---
include/client/open.inc.php | 6 ++-
include/staff/ticket-open.inc.php | 5 ++-
tickets.php | 2 +
9 files changed, 82 insertions(+), 36 deletions(-)
diff --git a/include/ajax.tickets.php b/include/ajax.tickets.php
index 4ad0d061f..21e93290c 100644
--- a/include/ajax.tickets.php
+++ b/include/ajax.tickets.php
@@ -575,7 +575,7 @@ class TicketsAjaxAPI extends AjaxController {
Http::response(404, 'No such ticket/user');
$errors = array();
- if($user->updateInfo($_POST, $errors))
+ if($user->updateInfo($_POST, $errors, true))
Http::response(201, $user->to_json());
$forms = $user->getForms();
diff --git a/include/class.dynamic_forms.php b/include/class.dynamic_forms.php
index 3d73b862d..fbdb14229 100644
--- a/include/class.dynamic_forms.php
+++ b/include/class.dynamic_forms.php
@@ -47,18 +47,17 @@ class DynamicForm extends VerySimpleModel {
var $_dfields;
function getFields($cache=true) {
- if (!$cache)
- $fields = false;
- else
- $fields = &$this->_fields;
+ if (!$cache) {
+ $this->_fields = null;
+ }
- if (!$fields) {
- $fields = new ListObject();
+ if (!$this->_fields) {
+ $this->_fields = new ListObject();
foreach ($this->getDynamicFields() as $f)
- $fields->append($f->getImpl($f));
+ $this->_fields->append($f->getImpl($f));
}
- return $fields;
+ return $this->_fields;
}
function getDynamicFields() {
@@ -99,13 +98,39 @@ class DynamicForm extends VerySimpleModel {
function getTitle() { return $this->get('title'); }
function getInstructions() { return $this->get('instructions'); }
+ /**
+ * Drop field errors clean info etc. Useful when replacing the source
+ * content of the form. This is necessary because the field listing is
+ * cached under some circumstances.
+ */
+ function reset() {
+ foreach ($this->getFields() as $f)
+ $f->reset();
+ return $this;
+ }
+
function getForm($source=false) {
- if (!$this->_form || $source) {
- $fields = $this->getFields($this->_has_data);
- $this->_form = new Form($fields, $source, array(
- 'title'=>$this->title, 'instructions'=>$this->instructions));
+ if ($source)
+ $this->reset();
+ $fields = $this->getFields();
+ $form = new Form($fields, $source, array(
+ 'title'=>$this->title, 'instructions'=>$this->instructions));
+ return $form;
+ }
+
+ function addErrors(array $formErrors, $replace=false) {
+ $fields = array();
+ foreach ($this->getFields() as $f)
+ $fields[$f->get('id')] = $f;
+ foreach ($formErrors as $id => $fieldErrors) {
+ if (isset($fields[$id])) {
+ if ($replace)
+ $fields[$id]->_errors = $fieldErrors;
+ else
+ foreach ($fieldErrors as $E)
+ $fields[$id]->addError($E);
+ }
}
- return $this->_form;
}
function isDeletable() {
@@ -324,8 +349,7 @@ class TicketForm extends DynamicForm {
return;
$f = $answer->getField();
- $name = $f->get('name') ? $f->get('name')
- : 'field_'.$f->get('id');
+ $name = $f->get('name') ?: ('field_'.$f->get('id'));
$fields = sprintf('`%s`=', $name) . db_input(
implode(',', $answer->getSearchKeys()));
$sql = 'INSERT INTO `'.TABLE_PREFIX.'ticket__cdata` SET '.$fields
@@ -695,12 +719,14 @@ class DynamicFormEntry extends VerySimpleModel {
function getInstructions() { return $this->getForm()->getInstructions(); }
function getForm() {
- if (!isset($this->_form)) {
- $this->_form = DynamicForm::lookup($this->get('form_id'));
- if ($this->_form && isset($this->id))
- $this->_form->data($this);
+ $form = DynamicForm::lookup($this->get('form_id'));
+ if ($form) {
+ if (isset($this->id))
+ $form->data($this);
+ if ($this->errors())
+ $form->addErrors($this->errors(), true);
}
- return $this->_form;
+ return $form;
}
function getFields() {
@@ -710,7 +736,10 @@ class DynamicFormEntry extends VerySimpleModel {
// even when stored elsewhere -- important during validation
foreach ($this->getForm()->getDynamicFields() as $field) {
$field->setForm($this);
- $this->_fields[$field->get('id')] = $field->getImpl($field);
+ $field = $field->getImpl($field);
+ if ($field instanceof ThreadEntryField)
+ continue;
+ $this->_fields[$field->get('id')] = $field;
}
// Get answers to entries
foreach ($this->getAnswers() as $a) {
diff --git a/include/class.filter.php b/include/class.filter.php
index acc3a3590..b05a448ce 100644
--- a/include/class.filter.php
+++ b/include/class.filter.php
@@ -322,7 +322,8 @@ class Filter {
if ($info['reply-to-name'])
$ticket['name'] = $info['reply-to-name'];
if ($changed)
- throw new FilterDataChanged();
+ throw new FilterDataChanged($ticket);
+
}
# Use canned response.
@@ -961,7 +962,18 @@ class RejectedException extends Exception {
}
}
-class FilterDataChanged extends Exception {}
+class FilterDataChanged extends Exception {
+ var $data;
+
+ function __construct($data) {
+ parent::__construct('Ticket filter data changed');
+ $this->data = $data;
+ }
+
+ function getData() {
+ return $this->data;
+ }
+}
/**
* Function: endsWith
diff --git a/include/class.forms.php b/include/class.forms.php
index 70c0510a6..7078692a0 100644
--- a/include/class.forms.php
+++ b/include/class.forms.php
@@ -30,7 +30,7 @@ class Form {
$this->fields = $fields;
foreach ($fields as $k=>$f) {
$f->setForm($this);
- if (!$f->get('name') && $k)
+ if (!$f->get('name') && $k && !is_numeric($k))
$f->set('name', $k);
}
if (isset($options['title']))
diff --git a/include/class.ticket.php b/include/class.ticket.php
index 2619c625b..e7546870e 100644
--- a/include/class.ticket.php
+++ b/include/class.ticket.php
@@ -2378,7 +2378,7 @@ class Ticket {
}
catch (FilterDataChanged $ex) {
// Don't pass user recursively, assume the user has changed
- return self::filterTicketData($origin, $vars, $forms);
+ return self::filterTicketData($origin, $ex->getData(), $forms);
}
return $vars;
}
diff --git a/include/class.user.php b/include/class.user.php
index 99375d65a..87edb06dd 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -342,7 +342,7 @@ class User extends UserModel {
}
}
- $this->_forms[] = $cd->getForm();
+ $this->_forms[] = $cd;
}
}
@@ -497,15 +497,15 @@ class User extends UserModel {
function updateInfo($vars, &$errors, $staff=false) {
$valid = true;
- $forms = $this->getDynamicData();
+ $forms = $this->getForms($vars);
foreach ($forms as $cd) {
$cd->setSource($vars);
if ($staff && !$cd->isValidForStaff())
$valid = false;
- elseif (!$cd->isValidForClient())
+ elseif (!$staff && !$cd->isValidForClient())
$valid = false;
- elseif ($cd->get('type') == 'U'
- && ($form= $cd->getForm())
+ elseif (($form= $cd->getForm())
+ && $form->get('type') == 'U'
&& ($f=$form->getField('email'))
&& $f->getClean()
&& ($u=User::lookup(array('emails__address'=>$f->getClean())))
@@ -518,7 +518,7 @@ class User extends UserModel {
if (!$valid)
return false;
- foreach ($this->getDynamicData() as $cd) {
+ foreach ($forms as $cd) {
if (($f=$cd->getForm()) && $f->get('type') == 'U') {
if (($name = $f->getField('name'))) {
$this->name = $name->getClean();
diff --git a/include/client/open.inc.php b/include/client/open.inc.php
index 821aa6ef9..4024579a8 100644
--- a/include/client/open.inc.php
+++ b/include/client/open.inc.php
@@ -77,8 +77,10 @@ if ($info['topicId'] && ($topic=Topic::lookup($info['topicId']))) {
} ?>
</tbody>
<tbody><?php
- $tform = TicketForm::getInstance()->getForm($_POST);
- if ($_POST) $tform->isValid();
+ $tform = TicketForm::getInstance();
+ if ($_POST) {
+ $tform->isValidForClient();
+ }
$tform->render(false); ?>
</tbody>
<tbody>
diff --git a/include/staff/ticket-open.inc.php b/include/staff/ticket-open.inc.php
index 39acb0864..6041f5573 100644
--- a/include/staff/ticket-open.inc.php
+++ b/include/staff/ticket-open.inc.php
@@ -265,8 +265,9 @@ if ($_POST)
?>
</tbody>
<tbody> <?php
- $tform = TicketForm::getInstance()->getForm($_POST);
- if ($_POST) $tform->isValid();
+ $tform = TicketForm::getInstance();
+ if ($_POST && !$tform->errors())
+ $tform->isValidForStaff();
$tform->render(true);
?>
</tbody>
diff --git a/tickets.php b/tickets.php
index 5b15b0015..fdafd5709 100644
--- a/tickets.php
+++ b/tickets.php
@@ -123,6 +123,8 @@ if($ticket && $ticket->checkUserAccess($thisclient)) {
}
include(CLIENTINC_DIR.'header.inc.php');
include(CLIENTINC_DIR.$inc);
+if ($tform instanceof DynamicFormEntry)
+ $tform = $tform->getForm();
print $tform->getMedia();
include(CLIENTINC_DIR.'footer.inc.php');
?>
--
GitLab