From 4efef0171aa0ee538f344f19e0c0bf6e34ac88cc Mon Sep 17 00:00:00 2001
From: Jared Hancock <jared@osticket.com>
Date: Thu, 9 Apr 2015 11:15:08 -0500
Subject: [PATCH] custom-data: Address major confusion

This feature addresses a major issue with the initial implementation of the
custom data system. The original system confused the usage of
database-backed field (dynamic-fields) and their corresponding
implementation. This created the need to crate awkward caching pieces to
ensure that validation errors and data was maintained. Furthermore, the
system confused the linking between form instances (dynamic-entry) and the
form used to represent that entry.

This patch addresses the confusion in two ways:

Dynamic form entries do not link directly to the dynamic form. Instead, the
::getForm() method returns something from the forms API directly.
Furthermore, the ::getFields() method does not return dynamic field
instances (database backed / designed fields). Instead, the actual
implementation of the fields from the forms API is retrieved. This allows
the fields to *always* be cached, which helps preserve data and validation
state.

Secondly, the dynamic form uses the same system, so that requests to turn a
dynamic form into a form (via ::getForm) will also result in the same
behavior, again, where the fields are represented as forms API fields rather
than the dynamic fields.

So going forward, the dynamic fields are *only* used to create corresponding
forms API field implementations. The are associated with the dynamic
counterparts as sparingly as possible.
---
 include/class.company.php                     |   2 +-
 include/class.dynamic_forms.php               | 316 ++++++++----------
 include/class.forms.php                       |  12 +
 include/class.organization.php                |  44 ++-
 include/class.orm.php                         |   1 +
 include/class.user.php                        |  55 ++-
 include/staff/dynamic-form.inc.php            |   2 +-
 include/staff/templates/dynamic-form.tmpl.php |   2 +-
 include/staff/templates/form-manage.tmpl.php  |   4 +-
 include/staff/templates/org.tmpl.php          |   2 +-
 include/staff/templates/user-delete.tmpl.php  |   2 +-
 include/staff/templates/user-lookup.tmpl.php  |   2 +-
 include/staff/templates/user.tmpl.php         |   4 +-
 include/staff/ticket-view.inc.php             |   8 +-
 .../streams/core/b26f29a6-1ee831c8.patch.sql  |   5 +
 scp/forms.php                                 |   4 +-
 scp/lists.php                                 |  10 +-
 17 files changed, 228 insertions(+), 247 deletions(-)

diff --git a/include/class.company.php b/include/class.company.php
index 049ecbb21..c9aa22f04 100644
--- a/include/class.company.php
+++ b/include/class.company.php
@@ -48,7 +48,7 @@ class Company {
     }
 
     function getInfo() {
-        return $this->getForm()->getSaved();
+        return $this->getForm()->getClean();
     }
 
     function getName() {
diff --git a/include/class.dynamic_forms.php b/include/class.dynamic_forms.php
index 23829e8a4..36dfce384 100644
--- a/include/class.dynamic_forms.php
+++ b/include/class.dynamic_forms.php
@@ -51,34 +51,37 @@ class DynamicForm extends VerySimpleModel {
     var $_has_data = false;
     var $_dfields;
 
-    function getFields($cache=true) {
-        if (!$cache) {
-            $this->_fields = null;
-        }
+    function getInfo() {
+        $base = $this->ht;
+        unset($base['fields']);
+        return $base;
+    }
 
+    /**
+     * Fetch a list of field implementations for the fields defined in this
+     * form. This method should *always* be preferred over
+     * ::getDynamicFields() to avoid caching confusion
+     */
+    function getFields() {
         if (!$this->_fields) {
             $this->_fields = new ListObject();
             foreach ($this->getDynamicFields() as $f)
                 $this->_fields->append($f->getImpl($f));
         }
-
         return $this->_fields;
     }
 
+    /**
+     * Fetch the dynamic fields associated with this dynamic form. Do not
+     * use this list for data processing or validation. Use ::getFields()
+     * for that.
+     */
     function getDynamicFields() {
-        if (!isset($this->id))
-            return array();
-        elseif (!$this->_dfields) {
-            $this->_dfields = DynamicFormField::objects()
-                ->filter(array('form_id'=>$this->id))
-                ->all();
-            foreach ($this->_dfields as $f)
-                $f->setForm($this);
-        }
-        return $this->_dfields;
+        return $this->fields;
     }
 
-    // Multiple inheritance -- delegate to Form
+    // Multiple inheritance -- delegate methods not defined to a forms API
+    // Form
     function __call($what, $args) {
         $delegate = array($this->getForm(), $what);
         if (!is_callable($delegate))
@@ -86,23 +89,14 @@ class DynamicForm extends VerySimpleModel {
         return call_user_func_array($delegate, $args);
     }
 
-    function getField($name, $cache=true) {
-        foreach ($this->getFields($cache) as $f) {
-            if (!strcasecmp($f->get('name'), $name))
-                return $f;
-        }
-        if ($cache)
-            return $this->getField($name, false);
+    function getTitle() {
+        return $this->getLocal('title');
     }
 
-    function hasField($name) {
-        return ($this->getField($name));
+    function getInstructions() {
+        return $this->getLocal('instructions');
     }
 
-
-    function getTitle() { return $this->getLocal('title'); }
-    function getInstructions() { return $this->getLocal('instructions'); }
-
     /**
      * Drop field errors clean info etc. Useful when replacing the source
      * content of the form. This is necessary because the field listing is
@@ -119,37 +113,16 @@ class DynamicForm extends VerySimpleModel {
             $this->reset();
         $fields = $this->getFields();
         $form = new Form($fields, $source, array(
-            'title'=>$this->getLocal('title'), 'instructions'=>$this->getLocal('instructions')));
+            'title' => $this->getLocal('title'),
+            'instructions' => $this->getLocal('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);
-            }
-        }
-    }
-
     function isDeletable() {
         return $this->get('deletable');
     }
 
-    function disableFields(array $ids) {
-        foreach ($this->getFields() as $F) {
-            if (in_array($F->get('id'), $ids)) {
-                $F->disable();
-            }
-        }
-    }
-
     function hasAnyVisibleFields($user=false) {
         global $thisstaff, $thisclient;
         $user = $user ?: $thisstaff ?: $thisclient;
@@ -167,16 +140,12 @@ class DynamicForm extends VerySimpleModel {
         return $visible > 0;
     }
 
-    function instanciate($sort=1) {
-        return DynamicFormEntry::create(array(
+    function instanciate($sort=1, $data=false) {
+        $entry = DynamicFormEntry::create(array(
             'form_id'=>$this->get('id'), 'sort'=>$sort));
-    }
-
-    function data($data) {
-        if ($data instanceof DynamicFormEntry) {
-            $this->_fields = $data->getFields();
-            $this->_has_data = true;
-        }
+        if ($data)
+            $entry->setSource($data);
+        return $entry;
     }
 
     function getTranslateTag($subtag) {
@@ -205,33 +174,29 @@ class DynamicForm extends VerySimpleModel {
             return parent::delete();
     }
 
-
     function getExportableFields($exclude=array()) {
-
         $fields = array();
         foreach ($this->getFields() as $f) {
             // Ignore core fields
             if ($exclude && in_array($f->get('name'), $exclude))
                 continue;
             // Ignore non-data fields
+            // FIXME: Consider ::isStorable() too
             elseif (!$f->hasData() || $f->isPresentationOnly())
                 continue;
 
             $fields['__field_'.$f->get('id')] = $f;
         }
-
         return $fields;
     }
 
-
     static function create($ht=false) {
         $inst = parent::create($ht);
         $inst->set('created', new SqlFunction('NOW'));
         if (isset($ht['fields'])) {
             $inst->save();
             foreach ($ht['fields'] as $f) {
-                $f = DynamicFormField::create($f);
-                $f->form_id = $inst->id;
+                $f = DynamicFormField::create(array('form' => $inst));
                 $f->setForm($inst);
                 $f->save();
             }
@@ -427,7 +392,7 @@ class TicketForm extends DynamicForm {
     static function updateDynamicDataView($answer, $data) {
         // TODO: Detect $data['dirty'] for value and value_id
         // We're chiefly concerned with Ticket form answers
-        if (!($e = $answer->getEntry()) || $e->getForm()->get('type') != 'T')
+        if (!($e = $answer->getEntry()) || $e->form->get('type') != 'T')
             return;
 
         // $record = array();
@@ -477,11 +442,11 @@ Signal::connect('model.updated',
 Signal::connect('model.created',
     array('TicketForm', 'dropDynamicDataView'),
     'DynamicFormField',
-    function($o) { return $o->getForm()->get('type') == 'T'; });
+    function($o) { return $o->form->type == 'T'; });
 Signal::connect('model.deleted',
     array('TicketForm', 'dropDynamicDataView'),
     'DynamicFormField',
-    function($o) { return $o->getForm()->get('type') == 'T'; });
+    function($o) { return $o->form->type == 'T'; });
 // If the `name` column is in the dirty list, we would be renaming a
 // column. Delete the view instead.
 Signal::connect('model.updated',
@@ -556,13 +521,18 @@ class DynamicFormField extends VerySimpleModel {
 
     const MASK_AGENT_FULL       = 0x7000;
 
-    // Multiple inheritance -- delegate to FormField
+    // Multiple inheritance -- delegate methods not defined here to the
+    // forms API FormField instance
     function __call($what, $args) {
         return call_user_func_array(
             array($this->getField(), $what), $args);
     }
 
-    function getField($cache=true) {
+    /**
+     * Fetch a forms API FormField instance which represents this designable
+     * DynamicFormField.
+     */
+    function getField() {
         global $thisstaff;
 
         // Finagle the `required` flag for the FormField instance
@@ -570,16 +540,11 @@ class DynamicFormField extends VerySimpleModel {
         $ht['required'] = ($thisstaff) ? $this->isRequiredForStaff()
             : $this->isRequiredForUsers();
 
-        if (!$cache)
-            return new FormField($ht);
-
         if (!isset($this->_field))
             $this->_field = new FormField($ht);
         return $this->_field;
     }
 
-    function getAnswer() { return $this->answer; }
-
     /**
      * setConfiguration
      *
@@ -642,6 +607,10 @@ class DynamicFormField extends VerySimpleModel {
         return (isset($this->flags) && ($this->flags & $flag) != 0);
     }
 
+    /**
+     * Describes the current visibility settings for this field. Returns a
+     * comma-separated, localized list of flag descriptions.
+     */
     function getVisibilityDescription() {
         $F = $this->flags;
 
@@ -683,6 +652,16 @@ class DynamicFormField extends VerySimpleModel {
         return $T != $tag ? $T : ($default ?: $this->get($subtag));
     }
 
+    /**
+     * Fetch a list of names to flag settings to make configuring new fields
+     * a bit easier.
+     *
+     * Returns:
+     * <Array['desc', 'flags']>, where the 'desc' key is a localized
+     * description of the flag set, and the 'flags' key is a bit mask of
+     * flags which should be set on the new field to implement the
+     * requirement / visibility mode.
+     */
     function allRequirementModes() {
         return array(
             'a' => array('desc' => __('Optional'),
@@ -711,6 +690,19 @@ class DynamicFormField extends VerySimpleModel {
         );
     }
 
+    /**
+     * Fetch a list of valid requirement modes for this field. This list
+     * will be filtered based on flags which are not supported or not
+     * allowed for this field.
+     *
+     * Deprecated:
+     * This was used in previous versions when a drop-down list was
+     * presented for editing a field's visibility. The current software
+     * version presents the drop-down list for new fields only.
+     *
+     * Returns:
+     * <Array['desc', 'flags']> Filtered list from ::allRequirementModes
+     */
     function getAllRequirementModes() {
         $modes = static::allRequirementModes();
         if ($this->isPrivacyForced()) {
@@ -844,7 +836,6 @@ class DynamicFormEntry extends VerySimpleModel {
         ),
     );
 
-    var $_values;
     var $_fields;
     var $_form;
     var $_errors = false;
@@ -856,14 +847,7 @@ class DynamicFormEntry extends VerySimpleModel {
     }
 
     function getAnswers() {
-        if (!isset($this->_values)) {
-            $this->_values = DynamicFormEntryAnswer::objects()
-                ->filter(array('entry_id'=>$this->get('id')))
-                ->all();
-            foreach ($this->_values as $v)
-                $v->entry = $this;
-        }
-        return $this->_values;
+        return $this->answers;
     }
 
     function getAnswer($name) {
@@ -890,44 +874,69 @@ class DynamicFormEntry extends VerySimpleModel {
         return $this->_errors;
     }
 
-    function getTitle() { return $this->getForm()->getTitle(); }
-    function getInstructions() { return $this->getForm()->getInstructions(); }
+    function getTitle() {
+        return $this->form->getTitle();
+    }
+
+    function getInstructions() {
+        return $this->form->getInstructions();
+    }
+
+    function getDynamicForm() {
+        return $this->form;
+    }
 
     function getForm() {
-        $form = DynamicForm::lookup($this->get('form_id'));
-        if ($form) {
-            if (isset($this->id))
-                $form->data($this);
+        if (!isset($this->_form)) {
+            // XXX: Should source be $this?
+            $form = new Form($this->getFields(), $this->getSource(),
+            array(
+                'title' => $this->getTitle(),
+                'instructions' => $this->getInstructions(),
+            ));
             if (isset($this->extra)) {
                 $x = JsonDataParser::decode($this->extra) ?: array();
                 $form->disableFields($x['disable'] ?: array());
             }
-            if ($this->errors())
-                $form->addErrors($this->errors(), true);
+            $this->_form = $form;
         }
-        return $form;
+        return $this->_form;
+    }
+
+    function getDynamicFields() {
+        return $this->form->fields;
+    }
+
+    function getMedia() {
+        return $this->getForm()->getMedia();
     }
 
     function getFields() {
         if (!isset($this->_fields)) {
             $this->_fields = array();
             // Get all dynamic fields associated with the form
-            //  even when stored elsewhere -- important during validation
-            foreach ($this->getForm()->getDynamicFields() as $field) {
-                $field = $field->getImpl($field);
-                if ($field instanceof ThreadEntryField)
+            // even when stored elsewhere -- important during validation
+            foreach ($this->getDynamicFields() as $f) {
+                $f = $f->getImpl($f);
+                if ($f instanceof ThreadEntryField)
                     continue;
-                $this->_fields[$field->get('id')] = $field;
+                $this->_fields[$f->get('id')] = $f;
+                $f->isnew = true;
             }
-            // Get answers to entries
+            // Include any other answers included in this entry, which may
+            // be for fields which have since been deleted
             foreach ($this->getAnswers() as $a) {
-                if (!($f = $a->getField())) continue;
-                $this->_fields[$f->get('id')] = $f;
+                $f = $a->getField();
+                $id = $f->get('id');
+                if (!isset($this->_fields[$id])) {
+                    // This field is not currently on the associated form
+                    $a->deleted = true;
+                }
+                $this->_fields[$id] = $f;
+                // This field has an answer, so it isn't new (to this entry)
+                $f->isnew = false;
             }
         }
-        foreach ($this->_fields as $F)
-            $F->setForm($this);
-
         return $this->_fields;
     }
 
@@ -939,7 +948,6 @@ class DynamicFormEntry extends VerySimpleModel {
     }
 
     function getField($name) {
-
         foreach ($this->getFields() as $field)
             if (!strcasecmp($field->get('name'), $name))
                 return $field;
@@ -981,13 +989,7 @@ class DynamicFormEntry extends VerySimpleModel {
     }
 
     function getClean() {
-        if (!$this->_clean) {
-            $this->_clean = array();
-            foreach ($this->getFields() as $field)
-                $this->_clean[$field->get('id')]
-                    = $this->_clean[$field->get('name')] = $field->getClean();
-        }
-        return $this->_clean;
+        return $this->getForm()->getClean();
     }
 
     /**
@@ -1023,16 +1025,6 @@ class DynamicFormEntry extends VerySimpleModel {
         return $vars;
     }
 
-    function getSaved() {
-        $info = array();
-        foreach ($this->getAnswers() as $a) {
-            $field = $a->getField();
-            $info[$field->get('id')]
-                = $info[$field->get('name')] = $a->getValue();
-        }
-        return $info;
-    }
-
     function forTicket($ticket_id, $force=false) {
         static $entries = array();
         if (!isset($entries[$ticket_id]) || $force) {
@@ -1077,29 +1069,14 @@ class DynamicFormEntry extends VerySimpleModel {
      * entry.
      */
     function addMissingFields() {
-        // Track deletions
-        foreach ($this->getAnswers() as $answer)
-            $answer->deleted = true;
-
-        foreach ($this->getForm()->getDynamicFields() as $field) {
-            $found = false;
-            foreach ($this->getAnswers() as $answer) {
-                if ($answer->get('field_id') == $field->get('id')) {
-                    $answer->deleted = false; $found = true; break;
-                }
-            }
-            if (!$found && ($fImpl = $field->getImpl($field))
-                    && $field->isEnabled()
-                    && !$fImpl->isPresentationOnly()) {
+        foreach ($this->getFields() as $field) {
+            if ($field->isnew && $field->isEnabled()
+                    && !$field->isPresentationOnly()) {
                 $a = DynamicFormEntryAnswer::create(
-                    array('field_id'=>$field->get('id'), 'entry_id'=>$this->id));
-                $a->field = $field;
-                $a->entry = $this;
-                $a->deleted = false;
+                    array('field_id'=>$field->get('id'), 'entry'=>$this));
+
                 // Add to list of answers
-                $this->_values[] = $a;
-                $this->_fields[$field->get('id')] = $fImpl;
-                $this->_form = null;
+                $this->answers->add($a);
 
                 // Omit fields without data and non-storable fields.
                 if (!$field->hasData() || !$field->isStorable())
@@ -1121,48 +1098,55 @@ class DynamicFormEntry extends VerySimpleModel {
     function save($refetch=false) {
         if (count($this->dirty))
             $this->set('updated', new SqlFunction('NOW'));
+
         if (!parent::save($refetch || count($this->dirty)))
             return false;
 
-        foreach ($this->getFields() as $field) {
-            if (!($a = $field->getAnswer()) || !$field->isStorable())
-                continue;
+        foreach ($this->getAnswers() as $a) {
+            $field = $a->getField();
 
-            $a = $field->getAnswer();
+            // Don't save answers for presentation-only fields or fields
+            // which are stored elsewhere
+            if (!$field->hasData() || !$field->isStorable()
+                || $field->isPresentationOnly()
+            ) {
+                continue;
+            }
             // Set the entry ID here so that $field->getClean() can use the
             // entry-id if necessary
-            $a->set('entry_id', $this->get('id'));
+            $a->entry = $this;
             $val = $field->to_database($field->getClean());
             if (is_array($val)) {
                 $a->set('value', $val[0]);
                 $a->set('value_id', $val[1]);
             }
-            else
+            else {
                 $a->set('value', $val);
-            // Don't save answers for presentation-only fields
-            if ($field->hasData() && !$field->isPresentationOnly())
-                $a->save();
+            }
+            $a->save();
         }
-        $this->_values = null;
     }
 
     function delete() {
+        if (!parent::delete())
+            return false;
+
         foreach ($this->getAnswers() as $a)
             $a->delete();
-        return parent::delete();
+
+        return true;
     }
 
     static function create($ht=false) {
         $inst = parent::create($ht);
         $inst->set('created', new SqlFunction('NOW'));
-        foreach ($inst->getForm()->getDynamicFields() as $f) {
-            if (!$f->hasData()) continue;
+        foreach ($inst->getDynamicFields() as $field) {
+            if (!$field->getImpl()->hasData())
+                continue;
             $a = DynamicFormEntryAnswer::create(
-                array('field_id'=>$f->get('id')));
-            $a->field = $f;
+                array('field'=>$field, 'entry'=>$inst));
             $a->field->setAnswer($a);
-            $a->entry = $inst;
-            $inst->_values[] = $a;
+            $inst->answers->add($a);
         }
         return $inst;
     }
@@ -1192,8 +1176,6 @@ class DynamicFormEntryAnswer extends VerySimpleModel {
     );
 
     var $_field;
-    var $form;
-    var $entry;
     var $deleted = false;
     var $_value;
 
@@ -1202,9 +1184,7 @@ class DynamicFormEntryAnswer extends VerySimpleModel {
     }
 
     function getForm() {
-        if (!$this->form)
-            $this->form = $this->getEntry()->getForm();
-        return $this->form;
+        return $this->getEntry()->getForm();
     }
 
     function getField() {
diff --git a/include/class.forms.php b/include/class.forms.php
index efa445efc..00d9a1c12 100644
--- a/include/class.forms.php
+++ b/include/class.forms.php
@@ -61,6 +61,10 @@ class Form {
             return $fields[$name];
     }
 
+    function hasField($name) {
+        return $this->getField($name);
+    }
+
     function getTitle() { return $this->title; }
     function getInstructions() { return $this->instructions; }
     function getSource() { return $this->_source; }
@@ -104,6 +108,14 @@ class Form {
         return $this->_clean;
     }
 
+    function disableFields(array $ids) {
+        foreach ($this->getFields() as $F) {
+            if (in_array($F->get('id'), $ids)) {
+                $F->disable();
+            }
+        }
+    }
+
     function errors($formOnly=false) {
         return ($formOnly) ? $this->_errors['form'] : $this->_errors;
     }
diff --git a/include/class.organization.php b/include/class.organization.php
index b5fed6c32..c5b8de4d1 100644
--- a/include/class.organization.php
+++ b/include/class.organization.php
@@ -180,18 +180,18 @@ class Organization extends OrganizationModel {
 
         if (!isset($this->_forms)) {
             $this->_forms = array();
-            foreach ($this->getDynamicData() as $cd) {
-                $cd->addMissingFields();
+            foreach ($this->getDynamicData() as $entry) {
+                $entry->addMissingFields();
                 if(!$data
-                        && ($form = $cd->getForm())
+                        && ($form = $entry->getDynamicForm())
                         && $form->get('type') == 'O' ) {
-                    foreach ($cd->getFields() as $f) {
+                    foreach ($entry->getFields() as $f) {
                         if ($f->get('name') == 'name')
                             $f->value = $this->getName();
                     }
                 }
 
-                $this->_forms[] = $cd->getForm();
+                $this->_forms[] = $entry;
             }
         }
 
@@ -251,11 +251,11 @@ class Organization extends OrganizationModel {
     function getFilterData() {
         $vars = array();
         foreach ($this->getDynamicData() as $entry) {
-            if ($entry->getForm()->get('type') != 'O')
+            if ($entry->getDynamicForm()->get('type') != 'O')
                 continue;
             $vars += $entry->getFilterData();
             // Add special `name` field
-            $f = $entry->getForm()->getField('name');
+            $f = $entry->getField('name');
             $vars['field.'.$f->get('id')] = $this->getName();
         }
         return $vars;
@@ -308,12 +308,11 @@ class Organization extends OrganizationModel {
 
         $valid = true;
         $forms = $this->getForms($vars);
-        foreach ($forms as $cd) {
-            if (!$cd->isValid())
+        foreach ($forms as $entry) {
+            if (!$entry->isValid())
                 $valid = false;
-            if ($cd->get('type') == 'O'
-                        && ($form= $cd->getForm($vars))
-                        && ($f=$form->getField('name'))
+            if ($entry->getDynamicForm()->get('type') == 'O'
+                        && ($f = $entry->getField('name'))
                         && $f->getClean()
                         && ($o=Organization::lookup(array('name'=>$f->getClean())))
                         && $o->id != $this->getId()) {
@@ -347,14 +346,14 @@ class Organization extends OrganizationModel {
         if (!$valid || $errors)
             return false;
 
-        foreach ($this->getDynamicData() as $cd) {
-            if (($f=$cd->getForm())
-                    && ($f->get('type') == 'O')
-                    && ($name = $f->getField('name'))) {
-                    $this->name = $name->getClean();
-                    $this->save();
-                }
-            $cd->save();
+        foreach ($this->getDynamicData() as $entry) {
+            if ($entry->getDynamicForm()->get('type') == 'O'
+               && ($name = $entry->getField('name'))
+            ) {
+                $this->name = $name->getClean();
+                $this->save();
+            }
+            $entry->save();
         }
 
         // Set flags
@@ -379,11 +378,6 @@ class Organization extends OrganizationModel {
             }
         }
 
-        // Send signal for search engine updating if not modifying the
-        // fields specific to the organization
-        if (count($this->dirty) === 0)
-            Signal::send('model.updated', $this);
-
         return $this->save();
     }
 
diff --git a/include/class.orm.php b/include/class.orm.php
index ce9c830fc..43795a8e7 100644
--- a/include/class.orm.php
+++ b/include/class.orm.php
@@ -383,6 +383,7 @@ class VerySimpleModel {
         // First, if any foreign properties of this object are connected to
         // another *new* object, then save those objects first and set the
         // local foreign key field values
+        static::_inspect();
         foreach (static::$meta['joins'] as $prop => $j) {
             if (isset($this->ht[$prop]) 
                 && ($foreign = $this->ht[$prop])
diff --git a/include/class.user.php b/include/class.user.php
index 08e2ce3b4..195e0de47 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -285,12 +285,12 @@ class User extends UserModel {
         return $this->created;
     }
 
-    function addForm($form, $sort=1) {
-        $form = $form->instanciate();
-        $form->set('sort', $sort);
-        $form->set('object_type', 'U');
-        $form->set('object_id', $this->getId());
-        $form->save();
+    function addForm($form, $sort=1, $data=false) {
+        $entry = $form->instanciate($sort, $data);
+        $entry->set('object_type', 'U');
+        $entry->set('object_id', $this->getId());
+        $entry->save();
+        return $entry;
     }
 
     function getLanguage($flags=false) {
@@ -328,14 +328,7 @@ class User extends UserModel {
     }
 
     function addDynamicData($data) {
-        $uf = UserForm::getNewInstance();
-        $uf->setClientId($this->id);
-        foreach ($uf->getFields() as $f)
-            if (isset($data[$f->get('name')]))
-                $uf->setAnswer($f->get('name'), $data[$f->get('name')]);
-        $uf->save();
-
-        return $uf;
+        return $this->addForm(UserForm::objects()->one(), 1, $data);
     }
 
     function getDynamicData($create=true) {
@@ -355,12 +348,12 @@ class User extends UserModel {
     function getFilterData() {
         $vars = array();
         foreach ($this->getDynamicData() as $entry) {
-            if ($entry->getForm()->get('type') != 'U')
+            if ($entry->getDynamicForm()->get('type') != 'U')
                 continue;
             $vars += $entry->getFilterData();
             // Add in special `name` and `email` fields
             foreach (array('name', 'email') as $name) {
-                if ($f = $entry->getForm()->getField($name))
+                if ($f = $entry->getField($name))
                     $vars['field.'.$f->get('id')] =
                         $name == 'name' ? $this->getName() : $this->getEmail();
             }
@@ -372,12 +365,12 @@ class User extends UserModel {
 
         if (!isset($this->_forms)) {
             $this->_forms = array();
-            foreach ($this->getDynamicData() as $cd) {
-                $cd->addMissingFields();
+            foreach ($this->getDynamicData() as $entry) {
+                $entry->addMissingFields();
                 if(!$data
-                        && ($form = $cd->getForm())
+                        && ($form = $entry->getDynamicForm())
                         && $form->get('type') == 'U' ) {
-                    foreach ($cd->getFields() as $f) {
+                    foreach ($entry->getFields() as $f) {
                         if ($f->get('name') == 'name')
                             $f->value = $this->getFullName();
                         elseif ($f->get('name') == 'email')
@@ -385,7 +378,7 @@ class User extends UserModel {
                     }
                 }
 
-                $this->_forms[] = $cd;
+                $this->_forms[] = $entry;
             }
         }
 
@@ -549,13 +542,13 @@ class User extends UserModel {
 
         $valid = true;
         $forms = $this->getForms($vars);
-        foreach ($forms as $cd) {
-            $cd->setSource($vars);
-            if ($staff && !$cd->isValidForStaff())
+        foreach ($forms as $entry) {
+            $entry->setSource($vars);
+            if ($staff && !$entry->isValidForStaff())
                 $valid = false;
-            elseif (!$staff && !$cd->isValidForClient())
+            elseif (!$staff && !$entry->isValidForClient())
                 $valid = false;
-            elseif (($form= $cd->getForm())
+            elseif (($form= $entry->getDynamicForm())
                         && $form->get('type') == 'U'
                         && ($f=$form->getField('email'))
                         && $f->getClean()
@@ -569,8 +562,8 @@ class User extends UserModel {
         if (!$valid)
             return false;
 
-        foreach ($forms as $cd) {
-            if (($f=$cd->getForm()) && $f->get('type') == 'U') {
+        foreach ($forms as $entry) {
+            if (($f=$entry->getDynamicForm()) && $f->get('type') == 'U') {
                 if (($name = $f->getField('name'))) {
                     $this->name = $name->getClean();
                     $this->save();
@@ -581,7 +574,7 @@ class User extends UserModel {
                     $this->default_email->save();
                 }
             }
-            $cd->save();
+            $entry->save();
         }
 
         return true;
@@ -631,8 +624,8 @@ class User extends UserModel {
         $this->emails->expunge();
 
         // Drop dynamic data
-        foreach ($this->getDynamicData() as $cd) {
-            $cd->delete();
+        foreach ($this->getDynamicData() as $entry) {
+            $entry->delete();
         }
 
         // Delete user
diff --git a/include/staff/dynamic-form.inc.php b/include/staff/dynamic-form.inc.php
index a25f8ef27..e194d4d85 100644
--- a/include/staff/dynamic-form.inc.php
+++ b/include/staff/dynamic-form.inc.php
@@ -6,7 +6,7 @@ if($form && $_REQUEST['a']!='add') {
     $action = 'update';
     $url = "?id=".urlencode($_REQUEST['id']);
     $submit_text=__('Save Changes');
-    $info = $form->ht;
+    $info = $form->getInfo();
     $trans = array(
         'title' => $form->getTranslateTag('title'),
         'instructions' => $form->getTranslateTag('instructions'),
diff --git a/include/staff/templates/dynamic-form.tmpl.php b/include/staff/templates/dynamic-form.tmpl.php
index b6abadd3a..7fac7eb12 100644
--- a/include/staff/templates/dynamic-form.tmpl.php
+++ b/include/staff/templates/dynamic-form.tmpl.php
@@ -24,7 +24,7 @@ if (isset($options['entry']) && $options['mode'] == 'edit') { ?>
 <?php if ($options['mode'] == 'edit') { ?>
         <div class="pull-right">
     <?php if ($options['entry']
-                && $options['entry']->getForm()->get('type') == 'G') { ?>
+                && $options['entry']->getDynamicForm()->get('type') == 'G') { ?>
             <a href="#" title="Delete Entry" onclick="javascript:
                 $(this).closest('tbody').remove();
                 return false;"><i class="icon-trash"></i></a>&nbsp;
diff --git a/include/staff/templates/form-manage.tmpl.php b/include/staff/templates/form-manage.tmpl.php
index 05b15e89e..e002ef165 100644
--- a/include/staff/templates/form-manage.tmpl.php
+++ b/include/staff/templates/form-manage.tmpl.php
@@ -12,9 +12,9 @@ $current_list = array();
 foreach ($forms as $e) { ?>
 <div class="sortable row-item" data-id="<?php echo $e->get('id'); ?>">
     <input type="hidden" name="forms[]" value="<?php echo $e->get('form_id'); ?>" />
-    <i class="icon-reorder"></i> <?php echo $e->getForm()->getTitle();
+    <i class="icon-reorder"></i> <?php echo $e->getTitle();
     $current_list[] = $e->get('form_id');
-    if ($e->getForm()->get('type') == 'G') { ?>
+    if ($e->getDynamicForm()->get('type') == 'G') { ?>
     <div class="button-group">
     <div class="delete"><a href="#"><i class="icon-trash"></i></a></div>
     </div>
diff --git a/include/staff/templates/org.tmpl.php b/include/staff/templates/org.tmpl.php
index 561cba8f6..41fcec1ee 100644
--- a/include/staff/templates/org.tmpl.php
+++ b/include/staff/templates/org.tmpl.php
@@ -28,7 +28,7 @@ if ($info['error']) {
 <?php foreach ($org->getDynamicData() as $entry) {
 ?>
     <tr><td colspan="2" style="border-bottom: 1px dotted black"><strong><?php
-         echo $entry->getForm()->get('title'); ?></strong></td></tr>
+         echo $entry->getTitle(); ?></strong></td></tr>
 <?php foreach ($entry->getAnswers() as $a) { ?>
     <tr style="vertical-align:top"><td style="width:30%;border-bottom: 1px dotted #ccc"><?php echo Format::htmlchars($a->getField()->get('label'));
          ?>:</td>
diff --git a/include/staff/templates/user-delete.tmpl.php b/include/staff/templates/user-delete.tmpl.php
index b563643d2..35bb685be 100644
--- a/include/staff/templates/user-delete.tmpl.php
+++ b/include/staff/templates/user-delete.tmpl.php
@@ -35,7 +35,7 @@ if ($info['error']) {
 <?php foreach ($user->getDynamicData() as $entry) {
 ?>
     <tr><td colspan="2" style="border-bottom: 1px dotted black"><strong><?php
-         echo $entry->getForm()->get('title'); ?></strong></td></tr>
+         echo $entry->getTitle(); ?></strong></td></tr>
 <?php foreach ($entry->getAnswers() as $a) { ?>
     <tr style="vertical-align:top"><td style="width:30%;border-bottom: 1px dotted #ccc"><?php echo Format::htmlchars($a->getField()->get('label'));
          ?>:</td>
diff --git a/include/staff/templates/user-lookup.tmpl.php b/include/staff/templates/user-lookup.tmpl.php
index 568df91f8..948026e8f 100644
--- a/include/staff/templates/user-lookup.tmpl.php
+++ b/include/staff/templates/user-lookup.tmpl.php
@@ -44,7 +44,7 @@ if ($user) { ?>
     <table style="margin-top: 1em;">
 <?php foreach ($user->getDynamicData() as $entry) { ?>
     <tr><td colspan="2" style="border-bottom: 1px dotted black"><strong><?php
-         echo $entry->getForm()->get('title'); ?></strong></td></tr>
+         echo $entry->getTitle(); ?></strong></td></tr>
 <?php foreach ($entry->getAnswers() as $a) { ?>
     <tr style="vertical-align:top"><td style="width:30%;border-bottom: 1px dotted #ccc"><?php echo Format::htmlchars($a->getField()->get('label'));
          ?>:</td>
diff --git a/include/staff/templates/user.tmpl.php b/include/staff/templates/user.tmpl.php
index 555385020..8c8974999 100644
--- a/include/staff/templates/user.tmpl.php
+++ b/include/staff/templates/user.tmpl.php
@@ -62,7 +62,7 @@ if ($info['error']) {
 <?php foreach ($user->getDynamicData() as $entry) {
 ?>
     <tr><th colspan="2"><strong><?php
-         echo $entry->getForm()->get('title'); ?></strong></td></tr>
+         echo $entry->getTitle(); ?></strong></td></tr>
 <?php foreach ($entry->getAnswers() as $a) { ?>
     <tr><td style="width:30%;"><?php echo Format::htmlchars($a->getField()->get('label'));
          ?>:</td>
@@ -86,7 +86,7 @@ if ($info['error']) {
 <?php foreach ($org->getDynamicData() as $entry) {
 ?>
     <tr><th colspan="2"><strong><?php
-         echo $entry->getForm()->get('title'); ?></strong></td></tr>
+         echo $entry->getTitle(); ?></strong></td></tr>
 <?php foreach ($entry->getAnswers() as $a) { ?>
     <tr><td style="width:30%"><?php echo Format::htmlchars($a->getField()->get('label'));
          ?>:</td>
diff --git a/include/staff/ticket-view.inc.php b/include/staff/ticket-view.inc.php
index 55241ed18..a054b841b 100644
--- a/include/staff/ticket-view.inc.php
+++ b/include/staff/ticket-view.inc.php
@@ -350,10 +350,10 @@ foreach (DynamicFormEntry::forTicket($ticket->getId()) as $form) {
     // TODO: Rewrite getAnswers() so that one could write
     //       ->getAnswers()->filter(not(array('field__name__in'=>
     //           array('email', ...))));
-    $answers = array_filter($form->getAnswers(), function ($a) {
-        return !in_array($a->getField()->get('name'),
-                array('email','subject','name','priority'));
-        });
+    $answers = $form->getAnswers()->exclude(Q::any(array(
+        'field__flags__hasbit' => DynamicFormField::FLAG_EXT_STORED,
+        'field__name__in' => array('subject', 'priority')
+    )));
     if (count($answers) == 0)
         continue;
     ?>
diff --git a/include/upgrader/streams/core/b26f29a6-1ee831c8.patch.sql b/include/upgrader/streams/core/b26f29a6-1ee831c8.patch.sql
index bb7f38eda..62621faf4 100644
--- a/include/upgrader/streams/core/b26f29a6-1ee831c8.patch.sql
+++ b/include/upgrader/streams/core/b26f29a6-1ee831c8.patch.sql
@@ -185,6 +185,11 @@ UPDATE `%TABLE_PREFIX%form_field` A1 JOIN `%TABLE_PREFIX%form` A2 ON(A2.id=A1.fo
     SET A1.`flags`=3
     WHERE A2.`type`='O' AND A1.`name` IN('name');
 
+-- Thread entry field is stored externally
+UPDATE `%TABLE_PREFIX%form_field` A1 JOIN `%TABLE_PREFIX%form` A2 ON(A2.id=A1.form_id)
+    SET A1.`flags`=3
+    WHERE A2.`type`='T' AND A1.`name` IN ('message');
+
 -- Coalesce to zero here in case the config option has never been saved
 set @client_edit = coalesce(
     (select value from `%TABLE_PREFIX%config` where `key` =
diff --git a/scp/forms.php b/scp/forms.php
index 3b7e6af04..2e164f243 100644
--- a/scp/forms.php
+++ b/scp/forms.php
@@ -113,7 +113,7 @@ if($_POST) {
                 'name'=>trim($_POST["name-new-$i"]),
             ));
             $field->setRequirementMode($_POST["visibility-new-$i"]);
-            $field->setForm($form);
+            $form->fields->add($field);
             if (in_array($field->get('name'), $names))
                 $field->addError(__('Field variable name is not unique'), 'name');
             if ($field->isValid()) {
@@ -124,9 +124,7 @@ if($_POST) {
             else
                 $errors["new-$i"] = $field->errors();
         }
-        // XXX: Move to an instrumented list that can handle this better
         if (!$errors) {
-            $form->_dfields = $form->_fields = null;
             $form->save(true);
             foreach ($form_fields as $field) {
                 $field->set('form_id', $form->get('id'));
diff --git a/scp/lists.php b/scp/lists.php
index 628e60b05..89173a88e 100644
--- a/scp/lists.php
+++ b/scp/lists.php
@@ -95,6 +95,7 @@ if($_POST) {
             break;
         case 'add':
             if ($list=DynamicList::add($_POST, $errors)) {
+                 $form = $list->getForm(true);
                  $msg = sprintf(__('Successfully added %s'),
                     __('this custom list'));
             } elseif ($errors) {
@@ -154,7 +155,6 @@ if($_POST) {
             if (!$_POST["prop-label-new-$i"])
                 continue;
             $field = DynamicFormField::create(array(
-                'form_id' => $form->get('id'),
                 'sort' => $_POST["prop-sort-new-$i"] ?: ++$max_sort,
                 'label' => $_POST["prop-label-new-$i"],
                 'type' => $_POST["type-new-$i"],
@@ -163,15 +163,13 @@ if($_POST) {
                     | DynamicFormField::FLAG_AGENT_VIEW
                     | DynamicFormField::FLAG_AGENT_EDIT,
             ));
-            $field->setForm($form);
-            if ($field->isValid())
+            if ($field->isValid()) {
+                $form->fields->add($field);
                 $field->save();
+            }
             else
                 $errors["new-$i"] = $field->errors();
         }
-        // XXX: Move to an instrumented list that can handle this better
-        if (!$errors)
-            $form->_dfields = $form->_fields = null;
     }
 }
 
-- 
GitLab