From a4ef711ef00e535734cf9706e15194287574c447 Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Sat, 29 Oct 2016 21:25:42 +0000 Subject: [PATCH] SyncProperty: simplify code and form fixes #12700 --- application/forms/SyncPropertyForm.php | 158 ++++++++++++---------- library/Director/Objects/SyncProperty.php | 31 ----- 2 files changed, 85 insertions(+), 104 deletions(-) diff --git a/application/forms/SyncPropertyForm.php b/application/forms/SyncPropertyForm.php index 55320df0..21b25a1b 100644 --- a/application/forms/SyncPropertyForm.php +++ b/application/forms/SyncPropertyForm.php @@ -71,60 +71,8 @@ class SyncPropertyForm extends DirectorObjectForm } } - if ($destination === 'import') { - $funcTemplates = 'enum' . ucfirst($this->rule->object_type) . 'Templates'; - $templates = $this->db->$funcTemplates(); - if (! empty($templates)) { - $templates = array_combine($templates, $templates); - } + $this->addSourceColumnElement($destination); - $this->addElement('select', 'source_expression', array( - 'label' => $this->translate('Template'), // Objecttype? - 'multiOptions' => $this->optionalEnum($templates), - 'required' => true, - 'class' => 'autosubmit', - )); - } else { - $this->addSourceColumnElement(); - } - - if ($this->hasObject()) { - if (($col = $this->getObject()->getSourceColumn()) === null) { - $this->setElementValue('source_column', self::EXPRESSION); - $this->addElement('text', 'source_expression', array( - 'label' => $this->translate('Source Expression'), - 'required' => true, - )); - if ($this->getSentValue('source_column') === '${' . self::EXPRESSION . '}') { - unset($this->source_column); - } - } else { - $this->setElementValue('source_column', $col); - } - } - - if ($this->getSentValue('source_column') === self::EXPRESSION) { - $this->addElement('text', 'source_expression', array( - 'label' => $this->translate('Source Expression'), - 'required' => true, - )); - if ($this->getSentValue('source_column') === '${' . self::EXPRESSION . '}') { - unset($this->source_column); - } - } - - /* - if ($this->hasObject()) { - // TODO: Add up/down links to table - $this->addElement('text', 'priority', array( - 'label' => $this->translate('Priority'), - 'description' => $this->translate('Priority for the specified source expression'), - 'required' => true, - )); - } - */ - - // TODO: we need modifier $this->addElement('YesNo', 'use_filter', array( 'label' => $this->translate('Set based on filter'), 'ignore' => true, @@ -160,7 +108,8 @@ class SyncPropertyForm extends DirectorObjectForm $this->addElement('select', 'merge_policy', array( 'label' => $this->translate('Merge Policy'), 'description' => $this->translate( - 'Whether you want to merge or replace the destination field. Makes no difference for strings' + 'Whether you want to merge or replace the destination field.' + . ' Makes no difference for strings' ), 'required' => true, 'multiOptions' => $this->optionalEnum(array( @@ -176,25 +125,59 @@ class SyncPropertyForm extends DirectorObjectForm } - protected function addSourceColumnElement() + protected function hasSubOption($options, $key) + { + foreach ($options as $mainKey => $sub) { + if (! is_array($sub)) { + // null -> please choose - or similar + continue; + } + + if (array_key_exists($key, $sub)) { + return true; + } + } + + return false; + } + + protected function addSourceColumnElement($destination) { $error = false; + + $srcTitle = $this->translate('Source columns'); try { - $data = $this->listSourceColumns(); - natsort($data); + $columns[$srcTitle] = $this->listSourceColumns(); + natsort($columns[$srcTitle]); } catch (Exception $e) { - $data = array(); + $srcTitle .= sprintf(' (%s)', $this->translate('failed to fetch')); + $columns[$srcTitle] = array(); $error = sprintf( $this->translate('Unable to fetch data: %s'), $e->getMessage() ); } + if ($destination === 'import') { + $funcTemplates = 'enum' . ucfirst($this->rule->object_type) . 'Templates'; + $templates = $this->db->$funcTemplates(); + if (! empty($templates)) { + $templates = array_combine($templates, $templates); + } + + $importTitle = $this->translate('Existing templates'); + $columns[$importTitle] = $templates; + natsort($columns[$importTitle]); + } + + $xpTitle = $this->translate('Expert mode'); + $columns[$xpTitle][self::EXPRESSION] = $this->translate('Custom expression'); + $this->addElement('select', 'source_column', array( 'label' => $this->translate('Source Column'), - // TODO: List them as ${} ? - 'multiOptions' => $this->optionalEnum($data), + 'multiOptions' => $this->optionalEnum($columns), 'required' => true, + 'ignore' => true, 'class' => 'autosubmit', )); @@ -202,6 +185,35 @@ class SyncPropertyForm extends DirectorObjectForm $this->getElement('source_column')->addError($error); } + $showExpression = false; + + if ($this->hasBeenSent()) { + $sentValue = $this->getSentValue('source_column'); + if ($sentValue === self::EXPRESSION) { + $showExpression = true; + } + } elseif ($this->hasObject()) { + $objectValue = $this->getObject()->source_expression; + if ($this->hasSubOption($columns, $objectValue)) { + $this->setElementValue('source_column', $objectValue); + } else { + $this->setElementValue('source_column', self::EXPRESSION); + $showExpression = true; + } + } + + if ($showExpression) { + $this->addElement('text', 'source_expression', array( + 'label' => $this->translate('Source Expression'), + 'description' => $this->translate( + 'A custom string. Might contain source columns, please use placeholders' + . ' of the form ${columnName} in such case' + ), + 'required' => true, + )); + } + + return $this; } @@ -232,9 +244,11 @@ class SyncPropertyForm extends DirectorObjectForm protected function listSourceColumns() { - $columns = $this->getImportSource()->listColumns(); - $columns = array_combine($columns, $columns); - $columns[self::EXPRESSION] = $this->translate('Custom expression'); + $columns = array(); + foreach ($this->getImportSource()->listColumns() as $col) { + $columns['${' . $col . '}'] = $col; + } + return $columns; } @@ -308,16 +322,6 @@ class SyncPropertyForm extends DirectorObjectForm public function onSuccess() { - $sourceColumn = $this->getValue('source_column'); - if ($sourceColumn === self::EXPRESSION) { - unset($this->source_column); - $this->removeElement('source_column'); - } else { - if (! $this->getElement('source_expression')) { - $this->addHidden('source_expression', '${' . $sourceColumn . '}'); - } - } - $object = $this->getObject(); $object->rule_id = $this->rule->id; // ?! @@ -325,6 +329,14 @@ class SyncPropertyForm extends DirectorObjectForm $object->filter_expression = null; } + $sourceColumn = $this->getValue('source_column'); + unset($this->source_column); + $this->removeElement('source_column'); + + if ($sourceColumn !== self::EXPRESSION) { + $object->source_expression = $sourceColumn; + } + $destination = $this->getValue('destination_field'); if ($destination === 'vars.*') { $destination = $this->getValue('customvar'); diff --git a/library/Director/Objects/SyncProperty.php b/library/Director/Objects/SyncProperty.php index 13abc16e..567719d5 100644 --- a/library/Director/Objects/SyncProperty.php +++ b/library/Director/Objects/SyncProperty.php @@ -22,35 +22,4 @@ class SyncProperty extends DbObject 'filter_expression' => null, 'merge_policy' => null ); - - /** - * Virtual property for source_column - * - * Internally we always use an expression. Form indirectly uses this - * - * Avoid complaints for method names with underscore: - * @codingStandardsIgnoreStart - * - * @return self - */ - public function setSource_column($value) - { - // @codingStandardsIgnoreEnd - $this->source_expression = '${' . $value . '}'; - return $this; - } - - public function sourceIsSingleColumn() - { - return $this->getSourceColumn() !== null; - } - - public function getSourceColumn() - { - if (preg_match('/^\${([A-Za-z0-9_-]+)}$/', $this->source_expression, $m)) { - return $m[1]; - } - - return null; - } }