Skip to content

Commit

Permalink
BUGFIX: Fixed issues with broekn link tracking (from r101138)
Browse files Browse the repository at this point in the history 
git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@111960 467b73ca-7a2a-4603-9d3b-597d59a354a9
  • Loading branch information
Sam Minnee committed Oct 12, 2010
1 parent 5e00afc commit e09cc66
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 27 deletions.
5 changes: 2 additions & 3 deletions core/model/SiteTree.php
View file
Expand Up @@ -1408,9 +1408,8 @@ function syncLinkTracking() {
$this->HasBrokenLink = false;
$this->HasBrokenFile = false;

$formFields = $this->getCMSFields(null);
foreach($htmlFields as $field) {
$formField = $formFields->dataFieldByName($field);
$formField = new HTMLEditorField($field);
$formField->setValue($this->$field);
$formField->saveInto($this);
}
Expand Down Expand Up @@ -2066,7 +2065,7 @@ static function generate_homepage_domain_map() {
* @uses SiteTreeDecorator->onAfterUnpublish()
*/
function doUnpublish() {
if(!$this->canDeleteFromLive()) return false;
if(!$this->canDeleteFromLive()) return false;
if(!$this->ID) return false;

$this->extend('onBeforeUnpublish');
Expand Down
22 changes: 13 additions & 9 deletions forms/HtmlEditorField.php
View file
Expand Up @@ -71,9 +71,6 @@ public function saveInto($record) {
$linkedPages = array();
$linkedFiles = array();

$record->HasBrokenFile = false;
$record->HasBrokenLink = false;

$htmlValue = new SS_HTMLValue($this->value);

// Populate link tracking for internal links & links to asset files.
Expand All @@ -89,13 +86,18 @@ public function saveInto($record) {
$link->setAttribute('class', preg_replace('/(^ss-broken|ss-broken$| ss-broken )/', null, $class));
}

if($page = DataObject::get_by_id('SiteTree', $ID)) {
$linkedPages[] = $page->ID;
$linkedPages[] = $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 {
$record->HasBrokenLink = true;
$record->HasBrokenFile = true;
}
} elseif($href[0] != '/' && $file = File::find($href)) {
$linkedFiles[] = $file->ID;
} else if($href == '' || $href[0] == '/') {
$record->HasBrokenLink = true;
}
}
}
Expand Down Expand Up @@ -142,7 +144,9 @@ public function saveInto($record) {
DB::query("DELETE FROM \"$tracker->tableName\" WHERE $filter");

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')");
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ClassInfoTest.php
View file
Expand Up @@ -23,7 +23,7 @@ function testClassesForFolder() {

$classes = ClassInfo::classes_for_folder('sapphire/tests');
$this->assertContains(
'ClassInfoTest',
'classinfotest',
$classes,
'ClassInfo::classes_for_folder() returns classes matching the filename'
);
Expand Down
14 changes: 7 additions & 7 deletions tests/ManifestBuilderTest.php
View file
Expand Up @@ -9,8 +9,8 @@ function testManifest() {
$manifestInfo = ManifestBuilder::get_manifest_info($baseFolder);
global $project;

$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/MyClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['myclass']);
$this->assertEquals("$baseFolder/sapphire/subdir/SubDirClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['subdirclass']);
$this->assertNotContains('OtherFile', array_keys($manifestInfo['globals']['_CLASS_MANIFEST']));

$this->assertContains('MyClass', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists']));
Expand Down Expand Up @@ -38,12 +38,12 @@ function testManifestIgnoresClassesInComments() {
$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. */
$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']['parents']));

/* 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']['parents']));
}
Expand All @@ -53,17 +53,17 @@ function testManifestIgnoresClassesInStrings() {
$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 */
$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']['parents']));

/* 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']['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. */
$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']['parents']));
}
Expand Down
15 changes: 8 additions & 7 deletions tests/SiteTreeBrokenLinksTest.php
View file
Expand Up @@ -21,11 +21,11 @@ static function tear_down_once() {
function testBrokenLinksBetweenPages() {
$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();
$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();
$this->assertFalse($obj->HasBrokenLink, 'Page does NOT have a broken link');
}
Expand Down Expand Up @@ -107,10 +107,11 @@ function testDeletingMarksBackLinkedPagesAsBroken() {
$linkDest->doPublish();

$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->doPublish();

// Confirm no broken link
$this->assertEquals(0, (int)$linkSrc->HasBrokenLink);
$this->assertEquals(0, DB::query("SELECT \"HasBrokenLink\" FROM \"SiteTree_Live\"
Expand Down Expand Up @@ -150,7 +151,7 @@ function testPublishingSourceBeforeDestHasBrokenLink() {
$linkDest->doDeleteFromLive();

$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();

// Publish the source of the link, while the dest is still unpublished.
Expand All @@ -174,7 +175,7 @@ function testRestoreFixesBrokenLinks() {
// Content links are one kind of link to pages
$p2 = new Page();
$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();
$this->assertTrue($p2->doPublish());

Expand Down Expand Up @@ -249,7 +250,7 @@ function testRevertToLiveFixesBrokenLinks() {
// Content links are one kind of link to pages
$p2 = new Page();
$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();
$this->assertTrue($p2->doPublish());

Expand Down
1 change: 1 addition & 0 deletions tests/forms/HtmlEditorFieldTest.php
View file
Expand Up @@ -151,6 +151,7 @@ public function testBrokenLinkTracking() {
'<p><a href="[sitetree_link id=%d]">Working Link</a></p>',
$this->idFromFixture('SiteTree', 'home')
));
$sitetree->HasBrokenLink = false;
$editor->saveInto($sitetree);

$this->assertFalse((bool) $sitetree->HasBrokenLink);
Expand Down

0 comments on commit e09cc66

Please sign in to comment.