From 2ba1c46bc86b3c8fdea723e6b936ede4806314be Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Fri, 15 Aug 2014 12:04:48 +1200 Subject: [PATCH] API Change broken link hihglighting to write to database. Before it would be applied on the fly during the rendering of the HtmlEditorField, and only be written to the database during the subsequent write. We just shift the behaviour to apply just-in-time. --- _config/config.yml | 3 - code/model/SiteTreeLinkTracking.php | 83 +++++++------------ tests/model/SiteTreeBrokenLinksTest.php | 7 -- tests/model/SiteTreeHTMLEditorFieldTest.php | 33 -------- tests/model/SiteTreeLinkTrackingTest.php | 21 ++--- .../tasks/MigrateSiteTreeLinkingTaskTest.php | 4 +- .../tasks/MigrateSiteTreeLinkingTaskTest.yml | 4 +- 7 files changed, 43 insertions(+), 112 deletions(-) diff --git a/_config/config.yml b/_config/config.yml index 8e169ffa..8b91fc39 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -4,6 +4,3 @@ LeftAndMain: Security: extensions: - ErrorPageControllerExtension -HtmlEditorField: - extensions: - - SiteTreeLinkTracking_Highlighter diff --git a/code/model/SiteTreeLinkTracking.php b/code/model/SiteTreeLinkTracking.php index 5286e572..7d57cc0d 100644 --- a/code/model/SiteTreeLinkTracking.php +++ b/code/model/SiteTreeLinkTracking.php @@ -37,15 +37,39 @@ class SiteTreeLinkTracking extends DataExtension { "ImageTracking" => array("FieldName" => "Varchar") ); - function trackLinksInField($field) { + public function trackLinksInField($fieldName) { $record = $this->owner; $linkedPages = array(); $linkedFiles = array(); - $htmlValue = Injector::inst()->create('HTMLValue', $record->$field); + $htmlValue = Injector::inst()->create('HTMLValue', $record->$fieldName); $links = $this->parser->process($htmlValue); + // Highlight broken links in the content. + foreach ($links as $link) { + $classStr = trim($link['DOMReference']->getAttribute('class')); + if (!$classStr) { + $classes = array(); + } else { + $classes = explode(' ', $classStr); + } + + // Add or remove the broken class from the link, depending on the link status. + if ($link['Broken']) { + $classes = array_unique(array_merge($classes, array('ss-broken'))); + } else { + $classes = array_diff($classes, array('ss-broken')); + } + + if (!empty($classes)) { + $link['DOMReference']->setAttribute('class', implode(' ', $classes)); + } else { + $link['DOMReference']->removeAttribute('class'); + } + } + $record->$fieldName = $htmlValue->getContent(); + // Populate link tracking for internal links & links to asset files. foreach ($links as $link) { switch ($link['Type']) { @@ -88,13 +112,13 @@ class SiteTreeLinkTracking extends DataExtension { if($record->ID && $record->many_many('LinkTracking') && $tracker = $record->LinkTracking()) { $tracker->removeByFilter(sprintf( '"FieldName" = \'%s\' AND "%s" = %d', - $field, + $fieldName, $tracker->getForeignKey(), $record->ID )); if($linkedPages) foreach($linkedPages as $item) { - $tracker->add($item, array('FieldName' => $field)); + $tracker->add($item, array('FieldName' => $fieldName)); } } @@ -102,18 +126,18 @@ class SiteTreeLinkTracking extends DataExtension { if($record->ID && $record->many_many('ImageTracking') && $tracker = $record->ImageTracking()) { $tracker->removeByFilter(sprintf( '"FieldName" = \'%s\' AND "%s" = %d', - $field, + $fieldName, $tracker->getForeignKey(), $record->ID )); if($linkedFiles) foreach($linkedFiles as $item) { - $tracker->add($item, array('FieldName' => $field)); + $tracker->add($item, array('FieldName' => $fieldName)); } } } - function augmentSyncLinkTracking() { + public function augmentSyncLinkTracking() { // Reset boolean broken flags $this->owner->HasBrokenLink = false; $this->owner->HasBrokenFile = false; @@ -134,51 +158,6 @@ class SiteTreeLinkTracking extends DataExtension { } } -/** - * Extension for enabling highlighting of broken links in the HtmlEditorFields. - */ -class SiteTreeLinkTracking_Highlighter extends Extension { - - public $parser; - - private static $dependencies = array( - 'parser' => '%$SiteTreeLinkTracking_Parser' - ); - - /** - * Adds an ability to highlight broken links in the content. - * It reuses the parser the SiteTreeLinkTracking uses for maintaining the references and the "broken" flags - * to make sure all pages listed in the BrokenLinkChecker highlight these in their content. - */ - public function onBeforeRender($field) { - // Handle situation when the field has been customised, i.e. via $properties on the HtmlEditorField::Field call. - $obj = $this->owner->getCustomisedObj() ?: $this->owner; - $value = $obj->value; - - // Parse the text as DOM. - $htmlValue = Injector::inst()->create('HTMLValue', $value); - $links = $this->parser->process($htmlValue); - - foreach ($links as $link) { - $classStr = $link['DOMReference']->getAttribute('class'); - $classes = explode(' ', $classStr); - - // Add or remove the broken class from the link, depending on the link status. - if ($link['Broken']) { - $classes = array_unique(array_merge($classes, array('ss-broken'))); - } else { - $classes = array_diff($classes, array('ss-broken')); - } - $link['DOMReference']->setAttribute('class', implode(' ', $classes)); - } - - $obj->customise(array( - 'Value' => htmlentities($htmlValue->getContent(), ENT_COMPAT, 'UTF-8') - )); - } - -} - /** * A helper object for extracting information about links. */ diff --git a/tests/model/SiteTreeBrokenLinksTest.php b/tests/model/SiteTreeBrokenLinksTest.php index 90fb9144..c3f8ccc6 100644 --- a/tests/model/SiteTreeBrokenLinksTest.php +++ b/tests/model/SiteTreeBrokenLinksTest.php @@ -292,13 +292,6 @@ class SiteTreeBrokenLinksTest extends SapphireTest { $this->assertFalse((bool)$vp->HasBrokenLink); $this->assertFalse((bool)$rp->HasBrokenLink); - // However, the page isn't marked as modified on stage - $this->assertFalse($p2->IsModifiedOnStage); - $this->assertFalse($rp->IsModifiedOnStage); - - // This is something that we know to be broken - //$this->assertFalse($vp->IsModifiedOnStage); - } public function testBrokenAnchorLinksInAPage() { diff --git a/tests/model/SiteTreeHTMLEditorFieldTest.php b/tests/model/SiteTreeHTMLEditorFieldTest.php index daf5c77d..c3a63a5b 100644 --- a/tests/model/SiteTreeHTMLEditorFieldTest.php +++ b/tests/model/SiteTreeHTMLEditorFieldTest.php @@ -166,37 +166,4 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest { $this->assertFalse((bool) $sitetree->HasBrokenFile); } - public function testBrokenLinkHighlighting() { - $sitetree = new SiteTree(); - $editor = new HtmlEditorField('Content'); - - // SiteTree link highlighting - $editor->setValue('Broken Link'); - - $element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field()))); - $this->assertContains('ss-broken', (string) $element['class'], 'A broken SiteTree link is highlighted'); - - $editor->setValue(sprintf ( - 'Working Link', - $this->idFromFixture('SiteTree', 'home') - )); - - $element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field()))); - $this->assertNotContains('ss-broken', (string) $element['class']); - - // File link highlighting - $editor->setValue('Broken Link'); - - $element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field()))); - $this->assertContains('ss-broken', (string) $element['class'], 'A broken File link is highlighted'); - - $editor->setValue(sprintf ( - 'Working Link', - $this->idFromFixture('File', 'example_file') - )); - - $element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field()))); - $this->assertNotContains('ss-broken', (string) $element['class']); - } - } diff --git a/tests/model/SiteTreeLinkTrackingTest.php b/tests/model/SiteTreeLinkTrackingTest.php index 7fc43f9f..98ccb71f 100644 --- a/tests/model/SiteTreeLinkTrackingTest.php +++ b/tests/model/SiteTreeLinkTrackingTest.php @@ -35,10 +35,10 @@ class SiteTreeLinkTrackingTest extends SapphireTest { } function highlight($content) { - $field = new SiteTreeLinkTrackingTest_Field('Test'); - $field->setValue($content); - $newContent = html_entity_decode($field->Field(), ENT_COMPAT, 'UTF-8'); - return $newContent; + $page = new Page(); + $page->Content = $content; + $page->write(); + return $page->Content; } function testHighlighter() { @@ -49,20 +49,15 @@ class SiteTreeLinkTrackingTest extends SapphireTest { $content = $this->highlight('link'); $this->assertEquals(substr_count($content, 'ss-broken'), 1, 'ss-broken class is added to the broken link.'); - $page = new Page(); - $page->Content = ''; - $page->write(); + $otherPage = new Page(); + $otherPage->Content = ''; + $otherPage->write(); $content = $this->highlight( - "ID]\" class=\"existing-class ss-broken ss-broken\">link" + "ID]\" class=\"existing-class ss-broken ss-broken\">link" ); $this->assertEquals(substr_count($content, 'ss-broken'), 0, 'All ss-broken classes are removed from good link'); $this->assertEquals(substr_count($content, 'existing-class'), 1, 'Existing class is not removed.'); } -} -class SiteTreeLinkTrackingTest_Field extends HtmlEditorField implements TestOnly { - private static $extensions = array( - 'SiteTreeLinkTracking_Highlighter' - ); } diff --git a/tests/tasks/MigrateSiteTreeLinkingTaskTest.php b/tests/tasks/MigrateSiteTreeLinkingTaskTest.php index 537d2467..6239d687 100644 --- a/tests/tasks/MigrateSiteTreeLinkingTaskTest.php +++ b/tests/tasks/MigrateSiteTreeLinkingTaskTest.php @@ -29,12 +29,12 @@ class MigrateSiteTreeLinkingTaskTest extends SapphireTest { $hashID = $this->idFromFixture('SiteTree', 'hash_link'); $homeContent = sprintf ( - 'AboutStaffExternal Link', + 'AboutStaffExternal Link', $aboutID, $staffID ); $aboutContent = sprintf ( - 'HomeStaff', + 'HomeStaff', $homeID, $staffID ); diff --git a/tests/tasks/MigrateSiteTreeLinkingTaskTest.yml b/tests/tasks/MigrateSiteTreeLinkingTaskTest.yml index 084ae190..3d14a8b9 100644 --- a/tests/tasks/MigrateSiteTreeLinkingTaskTest.yml +++ b/tests/tasks/MigrateSiteTreeLinkingTaskTest.yml @@ -2,11 +2,11 @@ SiteTree: home: Title: Home Page URLSegment: home - Content: 'AboutStaffExternal Link' + Content: 'AboutStaffExternal Link' about: Title: About Us URLSegment: about - Content: 'HomeStaff' + Content: 'HomeStaff' staff: Title: Staff URLSegment: staff