BUGFIX: Fixed issues with broekn link tracking (from r101138)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@111960 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Sam Minnee 2010-10-12 21:42:25 +00:00
parent 5e00afc9c8
commit e09cc66e94
6 changed files with 32 additions and 27 deletions

View File

@ -1408,9 +1408,8 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
$this->HasBrokenLink = false; $this->HasBrokenLink = false;
$this->HasBrokenFile = false; $this->HasBrokenFile = false;
$formFields = $this->getCMSFields(null);
foreach($htmlFields as $field) { foreach($htmlFields as $field) {
$formField = $formFields->dataFieldByName($field); $formField = new HTMLEditorField($field);
$formField->setValue($this->$field); $formField->setValue($this->$field);
$formField->saveInto($this); $formField->saveInto($this);
} }
@ -2066,7 +2065,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
* @uses SiteTreeDecorator->onAfterUnpublish() * @uses SiteTreeDecorator->onAfterUnpublish()
*/ */
function doUnpublish() { function doUnpublish() {
if(!$this->canDeleteFromLive()) return false; if(!$this->canDeleteFromLive()) return false;
if(!$this->ID) return false; if(!$this->ID) return false;
$this->extend('onBeforeUnpublish'); $this->extend('onBeforeUnpublish');

View File

@ -71,9 +71,6 @@ class HtmlEditorField extends TextareaField {
$linkedPages = array(); $linkedPages = array();
$linkedFiles = array(); $linkedFiles = array();
$record->HasBrokenFile = false;
$record->HasBrokenLink = false;
$htmlValue = new SS_HTMLValue($this->value); $htmlValue = new SS_HTMLValue($this->value);
// Populate link tracking for internal links & links to asset files. // Populate link tracking for internal links & links to asset files.
@ -89,13 +86,18 @@ class HtmlEditorField extends TextareaField {
$link->setAttribute('class', preg_replace('/(^ss-broken|ss-broken$| ss-broken )/', null, $class)); $link->setAttribute('class', preg_replace('/(^ss-broken|ss-broken$| ss-broken )/', null, $class));
} }
if($page = DataObject::get_by_id('SiteTree', $ID)) { $linkedPages[] = $ID;
$linkedPages[] = $page->ID; if(!DataObject::get_by_id('SiteTree', $ID)) $record->HasBrokenLink = true;
} else if(substr($href, 0, strlen(ASSETS_DIR) + 1) == ASSETS_DIR.'/') {
$candidateFile = File::find(Convert::raw2sql(urldecode($href)));
if($candidateFile) {
$linkedFiles[] = $candidateFile->ID;
} else { } else {
$record->HasBrokenLink = true; $record->HasBrokenFile = true;
} }
} elseif($href[0] != '/' && $file = File::find($href)) { } else if($href == '' || $href[0] == '/') {
$linkedFiles[] = $file->ID; $record->HasBrokenLink = true;
} }
} }
} }
@ -142,7 +144,9 @@ class HtmlEditorField extends TextareaField {
DB::query("DELETE FROM \"$tracker->tableName\" WHERE $filter"); DB::query("DELETE FROM \"$tracker->tableName\" WHERE $filter");
if($linkedPages) foreach($linkedPages as $item) { if($linkedPages) foreach($linkedPages as $item) {
$tracker->add($item, array('FieldName' => $this->name)); $SQL_fieldName = Convert::raw2sql($this->name);
DB::query("INSERT INTO \"SiteTree_LinkTracking\" (\"SiteTreeID\",\"ChildID\", \"FieldName\")
VALUES ($record->ID, $item, '$SQL_fieldName')");
} }
} }

View File

@ -23,7 +23,7 @@ class ClassInfoTest extends SapphireTest {
$classes = ClassInfo::classes_for_folder('sapphire/tests'); $classes = ClassInfo::classes_for_folder('sapphire/tests');
$this->assertContains( $this->assertContains(
'ClassInfoTest', 'classinfotest',
$classes, $classes,
'ClassInfo::classes_for_folder() returns classes matching the filename' 'ClassInfo::classes_for_folder() returns classes matching the filename'
); );

View File

@ -9,8 +9,8 @@ class ManifestBuilderTest extends SapphireTest {
$manifestInfo = ManifestBuilder::get_manifest_info($baseFolder); $manifestInfo = ManifestBuilder::get_manifest_info($baseFolder);
global $project; global $project;
$this->assertEquals("$baseFolder/sapphire/MyClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['MyClass']); $this->assertEquals("$baseFolder/sapphire/MyClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['myclass']);
$this->assertEquals("$baseFolder/sapphire/subdir/SubDirClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['SubDirClass']); $this->assertEquals("$baseFolder/sapphire/subdir/SubDirClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['subdirclass']);
$this->assertNotContains('OtherFile', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); $this->assertNotContains('OtherFile', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));
$this->assertContains('MyClass', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); $this->assertContains('MyClass', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
@ -38,12 +38,12 @@ class ManifestBuilderTest extends SapphireTest {
$manifestInfo = ManifestBuilder::get_manifest_info($baseFolder); $manifestInfo = ManifestBuilder::get_manifest_info($baseFolder);
/* Our fixture defines the class MyClass_InComment inside a comment, so it shouldn't be included in the class manifest. */ /* Our fixture defines the class MyClass_InComment inside a comment, so it shouldn't be included in the class manifest. */
$this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); $this->assertNotContains('myclass_incomment', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));
$this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); $this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
$this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); $this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents']));
/* Our fixture defines the class MyClass_InSlashSlashComment inside a //-style comment, so it shouldn't be included in the class manifest. */ /* Our fixture defines the class MyClass_InSlashSlashComment inside a //-style comment, so it shouldn't be included in the class manifest. */
$this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); $this->assertNotContains('myclass_inslashslashcomment', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));
$this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); $this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
$this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); $this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents']));
} }
@ -53,17 +53,17 @@ class ManifestBuilderTest extends SapphireTest {
$manifestInfo = ManifestBuilder::get_manifest_info($baseFolder); $manifestInfo = ManifestBuilder::get_manifest_info($baseFolder);
/* If a class defintion is listed in a single quote string, then it shouldn't be inlcuded. Here we have put a class definition for MyClass_InSingleQuoteString inside a single-quoted string */ /* If a class defintion is listed in a single quote string, then it shouldn't be inlcuded. Here we have put a class definition for MyClass_InSingleQuoteString inside a single-quoted string */
$this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); $this->assertNotContains('myclass_insinglequotestring', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));
$this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); $this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
$this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); $this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents']));
/* Ditto for double quotes. Here we have put a class definition for MyClass_InDoubleQuoteString inside a double-quoted string. */ /* Ditto for double quotes. Here we have put a class definition for MyClass_InDoubleQuoteString inside a double-quoted string. */
$this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); $this->assertNotContains('myclass_indoublequotestring', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));
$this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); $this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
$this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); $this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents']));
/* Finally, we need to ensure that class definitions inside heredoc strings aren't included. Here, we have defined the class MyClass_InHeredocString inside a heredoc string. */ /* Finally, we need to ensure that class definitions inside heredoc strings aren't included. Here, we have defined the class MyClass_InHeredocString inside a heredoc string. */
$this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); $this->assertNotContains('myclass_inheredocstring', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));
$this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); $this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
$this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); $this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents']));
} }

View File

@ -21,11 +21,11 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
function testBrokenLinksBetweenPages() { function testBrokenLinksBetweenPages() {
$obj = $this->objFromFixture('Page','content'); $obj = $this->objFromFixture('Page','content');
$obj->Content = '<a href="no-page-here/">this is a broken link</a>'; $obj->Content = '<a href="[sitetree_link id=3423423]">this is a broken link</a>';
$obj->syncLinkTracking(); $obj->syncLinkTracking();
$this->assertTrue($obj->HasBrokenLink, 'Page has a broken link'); $this->assertTrue($obj->HasBrokenLink, 'Page has a broken link');
$obj->Content = '<a href="about/">this is not a broken link</a>'; $obj->Content = '<a href="[sitetree_link id=' . $this->idFromFixture('Page','about') .']">this is not a broken link</a>';
$obj->syncLinkTracking(); $obj->syncLinkTracking();
$this->assertFalse($obj->HasBrokenLink, 'Page does NOT have a broken link'); $this->assertFalse($obj->HasBrokenLink, 'Page does NOT have a broken link');
} }
@ -107,10 +107,11 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
$linkDest->doPublish(); $linkDest->doPublish();
$linkSrc = $this->objFromFixture('Page','content'); $linkSrc = $this->objFromFixture('Page','content');
$linkSrc->Content = "<p><a href=\"" . $linkDest->URLSegment . "/\">about us</a></p>"; $linkSrc->Content = "<p><a href=\"[sitetree_link id=$linkDest->ID]\">about us</a></p>";
$linkSrc->write(); $linkSrc->write();
$linkSrc->doPublish(); $linkSrc->doPublish();
// Confirm no broken link // Confirm no broken link
$this->assertEquals(0, (int)$linkSrc->HasBrokenLink); $this->assertEquals(0, (int)$linkSrc->HasBrokenLink);
$this->assertEquals(0, DB::query("SELECT \"HasBrokenLink\" FROM \"SiteTree_Live\" $this->assertEquals(0, DB::query("SELECT \"HasBrokenLink\" FROM \"SiteTree_Live\"
@ -150,7 +151,7 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
$linkDest->doDeleteFromLive(); $linkDest->doDeleteFromLive();
$linkSrc = $this->objFromFixture('Page','content'); $linkSrc = $this->objFromFixture('Page','content');
$linkSrc->Content = "<p><a href=\"" . $linkDest->URLSegment . "/\">about us</a></p>"; $linkSrc->Content = "<p><a href=\"[sitetree_link id=$linkDest->ID]\">about us</a></p>";
$linkSrc->write(); $linkSrc->write();
// Publish the source of the link, while the dest is still unpublished. // Publish the source of the link, while the dest is still unpublished.
@ -174,7 +175,7 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
// Content links are one kind of link to pages // Content links are one kind of link to pages
$p2 = new Page(); $p2 = new Page();
$p2->Title = "regular link"; $p2->Title = "regular link";
$p2->Content = "<a href=\"" . Director::makeRelative($p->Link()) . "\">test</a>"; $p2->Content = "<a href=\"[sitetree_link id=$p->ID]\">test</a>";
$p2->write(); $p2->write();
$this->assertTrue($p2->doPublish()); $this->assertTrue($p2->doPublish());
@ -249,7 +250,7 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
// Content links are one kind of link to pages // Content links are one kind of link to pages
$p2 = new Page(); $p2 = new Page();
$p2->Title = "regular link"; $p2->Title = "regular link";
$p2->Content = "<a href=\"" . Director::makeRelative($p->Link()) . "\">test</a>"; $p2->Content = "<a href=\"[sitetree_link id=$p->ID]\">test</a>";
$p2->write(); $p2->write();
$this->assertTrue($p2->doPublish()); $this->assertTrue($p2->doPublish());

View File

@ -151,6 +151,7 @@ class HtmlEditorFieldTest extends FunctionalTest {
'<p><a href="[sitetree_link id=%d]">Working Link</a></p>', '<p><a href="[sitetree_link id=%d]">Working Link</a></p>',
$this->idFromFixture('SiteTree', 'home') $this->idFromFixture('SiteTree', 'home')
)); ));
$sitetree->HasBrokenLink = false;
$editor->saveInto($sitetree); $editor->saveInto($sitetree);
$this->assertFalse((bool) $sitetree->HasBrokenLink); $this->assertFalse((bool) $sitetree->HasBrokenLink);