Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

FIX: Broken file link tracking (fixes #996) #998

Merged
merged 1 commit into from

3 participants

@kinglozzer
Collaborator

List of changes:

  • Switched SiteTree linking to use SiteTree::get()->byID($id) instead of DataObject::get_by_id()
  • Removed the attempted ‘ss-broken’ class removal - this never worked: the class attribute was changed but this modified field data was never added to the record.
  • Fixed link tracking for files to be parsed from shortcodes instead of scanning for the presence of ASSETS_DIR in the href
  • Allowed spaces in the link shortcodes for legacy CMS support

If it’d be preferred, I can re-add the old file link tracking (that searches the href for ASSETS_DIR) in case any legacy CMS content is relying on that.

@kinglozzer
Collaborator

Removed SiteTreeBrokenLinksTest::testBrokenAssetLinks() as this is now covered in SiteTreeHTMLEditorFieldTest

@SparkGreen

Following on from your comments from #954, can you change the regex on line 41 to accept either a comma or whitespace between the sitetree_link and id fields in the shortcode? A previous version of the CMS set the shortcode with a space instead of a comma; adding for backwards compatibility. eg:
'/[sitetree_link[,\s]id=([0-9]+)]/i'

@kinglozzer
Collaborator

Pull request and tests updated to accept spaces in the shortcode :)

@tractorcow tractorcow merged commit 341eeb7 into silverstripe:3.1
@kinglozzer kinglozzer deleted the kinglozzer:pulls/sitetree-file-linktracking branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2014
  1. @kinglozzer
This page is out of date. Refresh to see the latest.
View
34 code/model/SiteTreeLinkTracking.php
@@ -38,24 +38,22 @@ function trackLinksInField($field) {
$href = Director::makeRelative($link->getAttribute('href'));
if($href) {
- if(preg_match('/\[sitetree_link,id=([0-9]+)\]/i', $href, $matches)) {
- $ID = $matches[1];
-
- // clear out any broken link classes
- if($class = $link->getAttribute('class')) {
- $link->setAttribute('class',
- preg_replace('/(^ss-broken|ss-broken$| ss-broken )/', null, $class));
- }
-
- $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->HasBrokenFile = true;
+ if(preg_match('/\[(sitetree|file)_link[,\s]id=([0-9]+)\]/i', $href, $matches)) {
+ $type = $matches[1];
+ $id = $matches[2];
+
+ if($type === 'sitetree') {
+ if(SiteTree::get()->byID($id)) {
+ $linkedPages[] = $id;
+ } else {
+ $record->HasBrokenLink = true;
+ }
+ } else if($type === 'file') {
+ if(File::get()->byID($id)) {
+ $linkedFiles[] = $id;
+ } else {
+ $record->HasBrokenFile = true;
+ }
}
} else if($href == '' || $href[0] == '/') {
$record->HasBrokenLink = true;
View
17 tests/model/SiteTreeBrokenLinksTest.php
@@ -46,18 +46,6 @@ public function testBrokenInternalRedirectorPages() {
$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 @@ public function testDeletingFileMarksBackedPagesAsBroken() {
$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
View
70 tests/model/SiteTreeHTMLEditorFieldTest.php
@@ -31,6 +31,16 @@ public function testLinkTracking() {
$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 @@ public function testFileLinkTracking() {
$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 @@ public function testFileLinkTracking() {
$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 @@ public function testImageTracking() {
);
}
- public function testBrokenLinkTracking() {
+ public function testBrokenSiteTreeLinkTracking() {
$sitetree = new SiteTree();
$editor = new HtmlEditorField('Content');
@@ -117,14 +143,38 @@ public function testBrokenLinkTracking() {
$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 @@ public function testBrokenLinkHighlighting() {
$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']);
}
}
Something went wrong with that request. Please try again.