Merge pull request #998 from kinglozzer/pulls/sitetree-file-linktracking

FIX: Broken file link tracking (fixes #996)

code/model/SiteTreeLinkTracking.php

@@ -38,25 +38,23 @@ class SiteTreeLinkTracking extends DataExtension {
 			$href = Director::makeRelative($link->getAttribute('href'));
 
 			if($href) {
-				if(preg_match('/\[sitetree_link,id=([0-9]+)\]/i', $href, $matches)) {
+				if(preg_match('/\[(sitetree|file)_link[,\s]id=([0-9]+)\]/i', $href, $matches)) {
-					$ID = $matches[1];
+					$type = $matches[1];
+					$id = $matches[2];
 
-					// clear out any broken link classes
+					if($type === 'sitetree') {
-					if($class = $link->getAttribute('class')) {
+						if(SiteTree::get()->byID($id)) {
-						$link->setAttribute('class',
+							$linkedPages[] = $id;
-							preg_replace('/(^ss-broken|ss-broken$| ss-broken )/', null, $class));
+						} else {
+							$record->HasBrokenLink = true;
 						}
-
+					} else if($type === 'file') {
-					$linkedPages[] = $ID;
+						if(File::get()->byID($id)) {
-					if(!DataObject::get_by_id('SiteTree', $ID))  $record->HasBrokenLink = true;
+							$linkedFiles[] = $id;
-
-				} 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->HasBrokenFile = true;
 						}
+					}
 				} else if($href == '' || $href[0] == '/') {
 					$record->HasBrokenLink = true;
 				}

tests/model/SiteTreeBrokenLinksTest.php

@@ -46,18 +46,6 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
 		$this->assertTrue($rp->HasBrokenLink, 'Broken redirector page IS marked as such');
 	}
 
-	public function testBrokenAssetLinks() {
-		$obj = $this->objFromFixture('Page','content');
-		
-		$obj->Content = '<a href="assets/nofilehere.pdf">this is a broken link to a pdf file</a>';
-		$obj->syncLinkTracking();
-		$this->assertTrue($obj->HasBrokenFile, 'Page has a broken file');
-		
-		$obj->Content = '<a href="assets/privacypolicy.pdf">this is not a broken file link</a>';
-		$obj->syncLinkTracking();
-		$this->assertFalse($obj->HasBrokenFile, 'Page does NOT have a broken file');
-	}
-
 	public function testDeletingFileMarksBackedPagesAsBroken() {
 		// Test entry
 		$file = new File();
@@ -65,7 +53,10 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
 		$file->write();
 
 		$obj = $this->objFromFixture('Page','content');
-		$obj->Content = '<a href="assets/test-file.pdf">link to a pdf file</a>';
+		$obj->Content = sprintf(
+			'<p><a href="[file_link,id=%d]">Working Link</a></p>',
+			$file->ID
+		);
 		$obj->write();
 		$this->assertTrue($obj->doPublish());
 		// Confirm that it isn't marked as broken to begin with

tests/model/SiteTreeHTMLEditorFieldTest.php

@@ -31,6 +31,16 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		$editor->saveInto($sitetree);
 		$sitetree->write();
 		$this->assertEquals(array(), $sitetree->LinkTracking()->getIdList(), 'Link tracking is removed when links are.');
+
+		// Legacy support - old CMS versions added link shortcodes with spaces instead of commas
+		$editor->setValue("<a href=\"[sitetree_link id=$aboutID]\">Example Link</a>");
+		$editor->saveInto($sitetree);
+		$sitetree->write();
+		$this->assertEquals(
+			array($aboutID => $aboutID),
+			$sitetree->LinkTracking()->getIdList(),
+			'Link tracking with space instead of comma in shortcode works.'
+		);
 	}
 
 	public function testFileLinkTracking() {
@@ -38,7 +48,10 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		$editor   = new HtmlEditorField('Content');
 		$fileID   = $this->idFromFixture('File', 'example_file');
 
-		$editor->setValue('<a href="assets/example.pdf">Example File</a>');
+		$editor->setValue(sprintf(
+			'<p><a href="[file_link,id=%d]">Example File</a></p>',
+			$fileID
+		));
 		$editor->saveInto($sitetree);
 		$sitetree->write();
 		$this->assertEquals (
@@ -49,6 +62,19 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		$editor->saveInto($sitetree);
 		$sitetree->write();
 		$this->assertEquals(array(), $sitetree->ImageTracking()->getIdList(), 'Asset tracking is removed with links.');
+
+		// Legacy support - old CMS versions added link shortcodes with spaces instead of commas
+		$editor->setValue(sprintf(
+			'<p><a href="[file_link id=%d]">Example File</a></p>',
+			$fileID
+		));
+		$editor->saveInto($sitetree);
+		$sitetree->write();
+		$this->assertEquals(
+			array($fileID => $fileID),
+			$sitetree->ImageTracking()->getIDList(),
+			'Link tracking with space instead of comma in shortcode works.'
+		);
 	}
 
 	public function testImageInsertion() {
@@ -94,7 +120,7 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		);
 	}
 
-	public function testBrokenLinkTracking() {
+	public function testBrokenSiteTreeLinkTracking() {
 		$sitetree = new SiteTree();
 		$editor   = new HtmlEditorField('Content');
 
@@ -117,14 +143,38 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		$this->assertFalse((bool) $sitetree->HasBrokenLink);
 	}
 
+	public function testBrokenFileLinkTracking() {
+		$sitetree = new SiteTree();
+		$editor   = new HtmlEditorField('Content');
+
+		$this->assertFalse((bool) $sitetree->HasBrokenFile);
+
+		$editor->setValue('<p><a href="[file_link,id=0]">Broken Link</a></p>');
+		$editor->saveInto($sitetree);
+		$sitetree->write();
+
+		$this->assertTrue($sitetree->HasBrokenFile);
+
+		$editor->setValue(sprintf (
+			'<p><a href="[file_link,id=%d]">Working Link</a></p>',
+			$this->idFromFixture('File', 'example_file')
+		));
+		$sitetree->HasBrokenFile = false;
+		$editor->saveInto($sitetree);
+		$sitetree->write();
+
+		$this->assertFalse((bool) $sitetree->HasBrokenFile);
+	}
+
 	public function testBrokenLinkHighlighting() {
 		$sitetree = new SiteTree();
 		$editor   = new HtmlEditorField('Content');
 
+		// SiteTree link highlighting
 		$editor->setValue('<a href="[sitetree_link,id=0]">Broken Link</a>');
 
 		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
-		$this->assertContains('ss-broken', (string) $element['class'], 'A broken link class is added to broken links');
+		$this->assertContains('ss-broken', (string) $element['class'], 'A broken SiteTree link is highlighted');
 
 		$editor->setValue(sprintf (
 			'<a href="[sitetree_link,id=%d]">Working Link</a>',
@@ -133,6 +183,20 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 
 		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
 		$this->assertNotContains('ss-broken', (string) $element['class']);
+
+		// File link highlighting
+		$editor->setValue('<a href="[file_link,id=0]">Broken Link</a>');
+
+		$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 (
+			'<a href="[file_link,id=%d]">Working Link</a>',
+			$this->idFromFixture('File', 'example_file')
+		));
+
+		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
+		$this->assertNotContains('ss-broken', (string) $element['class']);
 	}
 
 }