From 2fafff084f425a7d47197f2fed72064cc424ee2e Mon Sep 17 00:00:00 2001 From: Christopher Joe Date: Tue, 28 Feb 2017 13:39:30 +1300 Subject: [PATCH 1/2] Fix history comparison fields will now show diff properly, rather than escaped html diff --- code/Controllers/CMSPageHistoryController.php | 34 +++++++++++++------ .../CMSPageHistoryControllerTest.php | 34 +++++++++++++++++++ 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/code/Controllers/CMSPageHistoryController.php b/code/Controllers/CMSPageHistoryController.php index b21545d8..abf5b296 100644 --- a/code/Controllers/CMSPageHistoryController.php +++ b/code/Controllers/CMSPageHistoryController.php @@ -7,10 +7,12 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Forms\CheckboxField; +use SilverStripe\Forms\CompositeField; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormAction; use SilverStripe\Forms\HiddenField; +use SilverStripe\Forms\HTMLReadonlyField; use SilverStripe\Forms\LiteralField; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\FieldType\DBHTMLText; @@ -450,22 +452,17 @@ class CMSPageHistoryController extends CMSMain $form->setActions(new FieldList()); $form->addExtraClass('compare'); - // Comparison views shouldn't be editable. - // Its important to convert fields *before* loading data, - // as the comparison output is HTML and not valid values for the various field types - $readonlyFields = $form->Fields()->makeReadonly(); - $form->setFields($readonlyFields); - $form->loadDataFrom($record); $form->loadDataFrom(array( "ID" => $id, "Version" => $fromVersion, )); - - foreach ($form->Fields()->dataFields() as $field) { - $field->dontEscape = true; - } - + + // Comparison views shouldn't be editable. + // As the comparison output is HTML and not valid values for the various field types + $readonlyFields = $this->transformReadonly($form->Fields()); + $form->setFields($readonlyFields); + return $form; } @@ -475,4 +472,19 @@ class CMSPageHistoryController extends CMSMain $crumbs[0]->Title = _t('CMSPagesController.MENUTITLE', 'Pages'); return $crumbs; } + + public function transformReadonly(FieldList $fields) + { + foreach($fields as &$field) { + if ($field instanceof CompositeField) { + $subfields = $this->transformReadonly($field->FieldList()); + $field->setChildren($subfields); + } + if ($field->hasData() && !$field instanceof HiddenField) { + $newField = $field->castedCopy(HTMLReadonlyField::class); + $fields->replaceField($field->getName(), $newField); + } + } + return $fields; + } } diff --git a/tests/controller/CMSPageHistoryControllerTest.php b/tests/controller/CMSPageHistoryControllerTest.php index d85acda5..cbfbe74d 100755 --- a/tests/controller/CMSPageHistoryControllerTest.php +++ b/tests/controller/CMSPageHistoryControllerTest.php @@ -1,5 +1,10 @@ assertThat($checkbox[0], $this->logicalNot($this->isNull())); $this->assertEquals('checked', (string) $checkbox[0]->attributes()->checked); } + + public function testTransformReadonly() + { + /** @var CMSPageHistoryController $history */ + $history = singleton(CMSPageHistoryController::class); + + $fieldList = FieldList::create([ + FieldGroup::create('group', [ + TextField::create('childField', 'child field'), + ]), + TextField::create('field', 'field', 'My valuechange'), + HiddenField::create('hiddenField', 'hidden field'), + ]); + + $newList = $history->transformReadonly($fieldList); + + $field = $newList->dataFieldByName('field'); + $this->assertTrue($field instanceof HTMLReadonlyField); + $this->assertContains('', $field->forTemplate()); + + $groupField = $newList->fieldByName('group'); + $this->assertTrue($groupField instanceof FieldGroup); + + $childField = $newList->dataFieldByName('childField'); + $this->assertTrue($childField instanceof HTMLReadonlyField); + + $hiddenField = $newList->dataFieldByName('hiddenField'); + $this->assertTrue($hiddenField instanceof HiddenField); + } } From 8537d6ddb12754c85e7cba6c046b1d2993097720 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 28 Feb 2017 16:34:46 +1300 Subject: [PATCH 2/2] Simplify code to use dataFields() --- code/Controllers/CMSPageHistoryController.php | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/code/Controllers/CMSPageHistoryController.php b/code/Controllers/CMSPageHistoryController.php index abf5b296..7ac7c3f9 100644 --- a/code/Controllers/CMSPageHistoryController.php +++ b/code/Controllers/CMSPageHistoryController.php @@ -457,12 +457,12 @@ class CMSPageHistoryController extends CMSMain "ID" => $id, "Version" => $fromVersion, )); - + // Comparison views shouldn't be editable. // As the comparison output is HTML and not valid values for the various field types $readonlyFields = $this->transformReadonly($form->Fields()); $form->setFields($readonlyFields); - + return $form; } @@ -472,18 +472,21 @@ class CMSPageHistoryController extends CMSMain $crumbs[0]->Title = _t('CMSPagesController.MENUTITLE', 'Pages'); return $crumbs; } - + + /** + * Replace all data fields with HTML readonly fields to display diff + * + * @param FieldList $fields + * @return FieldList + */ public function transformReadonly(FieldList $fields) { - foreach($fields as &$field) { - if ($field instanceof CompositeField) { - $subfields = $this->transformReadonly($field->FieldList()); - $field->setChildren($subfields); - } - if ($field->hasData() && !$field instanceof HiddenField) { - $newField = $field->castedCopy(HTMLReadonlyField::class); - $fields->replaceField($field->getName(), $newField); + foreach ($fields->dataFields() as $field) { + if ($field instanceof HiddenField) { + continue; } + $newField = $field->castedCopy(HTMLReadonlyField::class); + $fields->replaceField($field->getName(), $newField); } return $fields; }