From 1c907dd227a41be8cd25ba0c751f6aa7a308ebfe Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 26 Jan 2016 18:38:42 +1300 Subject: [PATCH] API Support versioned File management API Decouple File and ErrorPage API Link tracking is now only performed on stage (in lieu of versioned relationships) API Refactor versioned API methods out of SiteTree and into Versioned --- _config/config.yml | 3 + code/batchactions/CMSBatchActions.php | 9 +- code/controllers/AssetAdmin.php | 1 + code/controllers/CMSMain.php | 32 +-- .../ErrorPageControllerExtension.php | 7 + code/controllers/ErrorPageFileExtension.php | 18 ++ code/model/SiteTree.php | 199 +++++------------- code/model/SiteTreeFileExtension.php | 62 ++++-- code/model/SiteTreeLinkTracking.php | 23 +- code/model/VirtualPage.php | 21 +- tasks/RemoveOrphanedPagesTask.php | 4 +- tests/model/ErrorPageFileExtensionTest.php | 58 +++++ tests/model/ErrorPageTest.yml | 20 +- tests/model/FileLinkTrackingTest.php | 56 ++++- tests/model/SiteTreeBacklinksTest.php | 12 ++ tests/model/SiteTreeBrokenLinksTest.php | 49 +++-- tests/model/SiteTreeBrokenLinksTest.yml | 42 ++-- tests/model/SiteTreeTest.php | 4 +- tests/model/VirtualPageTest.php | 8 +- tests/search/SearchFormTest.php | 3 + tests/search/SearchFormTest.yml | 78 +++---- 21 files changed, 406 insertions(+), 303 deletions(-) create mode 100644 code/controllers/ErrorPageFileExtension.php create mode 100644 tests/model/ErrorPageFileExtensionTest.php diff --git a/_config/config.yml b/_config/config.yml index e0e480ff..03c711e4 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -7,3 +7,6 @@ Controller: Form: extensions: - ErrorPageControllerExtension +File: + extensions: + - ErrorPageFileExtension diff --git a/code/batchactions/CMSBatchActions.php b/code/batchactions/CMSBatchActions.php index 9f10e2b6..fe38fa24 100644 --- a/code/batchactions/CMSBatchActions.php +++ b/code/batchactions/CMSBatchActions.php @@ -39,7 +39,7 @@ class CMSBatchAction_Unpublish extends CMSBatchAction { } public function applicablePages($ids) { - return $this->applicablePagesHelper($ids, 'canDeleteFromLive', false, true); + return $this->applicablePagesHelper($ids, 'canUnpublish', false, true); } } @@ -182,12 +182,15 @@ class CMSBatchAction_DeleteFromLive extends CMSBatchAction { 'modified'=>array(), 'deleted'=>array() ); - + + /** @var SiteTree $page */ foreach($pages as $page) { $id = $page->ID; // Perform the action - if($page->canDelete()) $page->doDeleteFromLive(); + if($page->canUnpublish()) { + $page->doUnpublish(); + } // check to see if the record exists on the stage site, if it doesn't remove the tree node $stageRecord = Versioned::get_one_by_stage( 'SiteTree', 'Stage', array( diff --git a/code/controllers/AssetAdmin.php b/code/controllers/AssetAdmin.php index b627c05c..d4d7fb84 100644 --- a/code/controllers/AssetAdmin.php +++ b/code/controllers/AssetAdmin.php @@ -63,6 +63,7 @@ class AssetAdmin extends LeftAndMain implements PermissionProvider{ public function init() { parent::init(); + Versioned::reading_stage("Stage"); Requirements::javascript(CMS_DIR . "/javascript/dist/AssetAdmin.js"); Requirements::add_i18n_javascript(CMS_DIR . '/javascript/lang', false, true); diff --git a/code/controllers/CMSMain.php b/code/controllers/CMSMain.php index 15e501a4..420e30f4 100644 --- a/code/controllers/CMSMain.php +++ b/code/controllers/CMSMain.php @@ -533,7 +533,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr * * @param int $id Record ID * @param int $versionID optional Version id of the given record - * @return DataObject + * @return SiteTree */ public function getRecord($id, $versionID = null) { $treeClass = $this->stat('tree_class'); @@ -591,7 +591,6 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr * @return CMSForm */ public function getEditForm($id = null, $fields = null) { - if(!$id) $id = $this->currentPageID(); $form = parent::getEditForm($id); @@ -604,7 +603,6 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr if($record) { $deletedFromStage = $record->getIsDeletedFromStage(); - $deleteFromLive = !$record->getExistsOnLive(); $fields->push($idField = new HiddenField("ID", false, $id)); // Necessary for different subsites @@ -974,13 +972,15 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr */ public function deletefromlive($data, $form) { Versioned::reading_stage('Live'); - $record = DataObject::get_by_id("SiteTree", $data['ID']); - if($record && !($record->canDelete() && $record->canDeleteFromLive())) return Security::permissionFailure($this); - $descRemoved = ''; + /** @var SiteTree $record */ + $record = DataObject::get_by_id("SiteTree", $data['ID']); + if($record && !($record->canDelete() && $record->canUnpublish())) { + return Security::permissionFailure($this); + } + $descendantsRemoved = 0; $recordTitle = $record->Title; - $recordID = $record->ID; // before deleting the records, get the descendants of this tree if($record) { @@ -989,13 +989,14 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr // then delete them from the live site too $descendantsRemoved = 0; foreach( $descendantIDs as $descID ) + /** @var SiteTree $descendant */ if( $descendant = DataObject::get_by_id('SiteTree', $descID) ) { - $descendant->doDeleteFromLive(); + $descendant->doUnpublish(); $descendantsRemoved++; } // delete the record - $record->doDeleteFromLive(); + $record->doUnpublish(); } Versioned::reading_stage('Stage'); @@ -1098,8 +1099,8 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr /** * Delete this page from both live and stage * - * @param type $data - * @param type $form + * @param array $data + * @param Form $form */ public function archive($data, $form) { $id = $data['ID']; @@ -1131,10 +1132,15 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr public function unpublish($data, $form) { $className = $this->stat('tree_class'); + /** @var SiteTree $record */ $record = DataObject::get_by_id($className, $data['ID']); - if($record && !$record->canDeleteFromLive()) return Security::permissionFailure($this); - if(!$record || !$record->ID) throw new SS_HTTPResponse_Exception("Bad record ID #" . (int)$data['ID'], 404); + if($record && !$record->canUnpublish()) { + return Security::permissionFailure($this); + } + if(!$record || !$record->ID) { + throw new SS_HTTPResponse_Exception("Bad record ID #" . (int)$data['ID'], 404); + } $record->doUnpublish(); diff --git a/code/controllers/ErrorPageControllerExtension.php b/code/controllers/ErrorPageControllerExtension.php index 9fc66cd3..a82ba780 100644 --- a/code/controllers/ErrorPageControllerExtension.php +++ b/code/controllers/ErrorPageControllerExtension.php @@ -8,6 +8,13 @@ */ class ErrorPageControllerExtension extends Extension { + /** + * Used by {@see RequestHandler::httpError} + * + * @param int $statusCode + * @param SS_HTTPRequest $request + * @throws SS_HTTPResponse_Exception + */ public function onBeforeHTTPError($statusCode, $request) { $response = ErrorPage::response_for($statusCode); if($response) { diff --git a/code/controllers/ErrorPageFileExtension.php b/code/controllers/ErrorPageFileExtension.php new file mode 100644 index 00000000..574dface --- /dev/null +++ b/code/controllers/ErrorPageFileExtension.php @@ -0,0 +1,18 @@ +filter("ErrorCode", $statusCode)->first(); + } + +} diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 1346eac7..15bde52d 100755 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -26,7 +26,6 @@ * * @method ManyManyList ViewerGroups List of groups that can view this object. * @method ManyManyList EditorGroups List of groups that can edit this object. - * @method ManyManyList BackLinkTracking List of site pages that link to this page. * * @mixin Hierarchy * @mixin Versioned @@ -129,21 +128,10 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid ); private static $many_many = array( - "LinkTracking" => "SiteTree", - "ImageTracking" => "File", "ViewerGroups" => "Group", "EditorGroups" => "Group", ); - private static $belongs_many_many = array( - "BackLinkTracking" => "SiteTree" - ); - - private static $many_many_extraFields = array( - "LinkTracking" => array("FieldName" => "Varchar"), - "ImageTracking" => array("FieldName" => "Varchar") - ); - private static $casting = array( "Breadcrumbs" => "HTMLText", "LastEdited" => "SS_Datetime", @@ -1080,39 +1068,21 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid } /** - * This function should return true if the current user can publish this page. It can be overloaded to customise - * the security model for an application. - * - * Denies permission if any of the following conditions is true: - * - canPublish() on any extension returns false - * - canEdit() returns false - * - * @uses SiteTreeExtension->canPublish() - * - * @param Member $member - * @return bool True if the current user can publish this page. + * @deprecated */ - public function canPublish($member = null) { - if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); - - if($member && Permission::checkMember($member, "ADMIN")) return true; - - // Standard mechanism for accepting permission changes from extensions - $extended = $this->extendedCan('canPublish', $member); - if($extended !== null) return $extended; - - // Normal case - fail over to canEdit() - return $this->canEdit($member); - } - public function canDeleteFromLive($member = null) { - // Standard mechanism for accepting permission changes from extensions - $extended = $this->extendedCan('canDeleteFromLive', $member); - if($extended !==null) return $extended; + Deprecation::notice('4.0', 'Use canUnpublish'); - return $this->canPublish($member); + // Deprecated extension + $extended = $this->extendedCan('canDeleteFromLive', $member); + if($extended !== null) { + Deprecation::notice('4.0', 'Use canUnpublish in your extension instead'); + return $extended; + } + + return $this->canUnpublish($member); } - + /** * Stub method to get the site config, unless the current class can provide an alternate. * @@ -1551,7 +1521,12 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid $this->migrateVersion($this->Version); } } - + + /** + * Trigger synchronisation of link tracking + * + * {@see SiteTreeLinkTracking::augmentSyncLinkTracking} + */ public function syncLinkTracking() { $this->extend('augmentSyncLinkTracking'); } @@ -1737,43 +1712,42 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid } /** - * Rewrites any linked images on this page. + * Rewrites any linked images on this page without creating a new version record. * Non-image files should be linked via shortcodes * Triggers the onRenameLinkedAsset action on extensions. - * TODO: This doesn't work for HTMLText fields on other tables. + * + * @todo Implement image shortcodes and remove this feature */ public function rewriteFileLinks() { + // Skip live stage + if(\Versioned::current_stage() === \Versioned::get_live_stage()) { + return; + } + // Update the content without actually creating a new version - foreach(array("SiteTree_Live", "SiteTree") as $table) { - // Published site - $published = DB::prepared_query( - "SELECT * FROM \"$table\" WHERE \"ID\" = ?", - array($this->ID) - )->record(); - $origPublished = $published; - - foreach($this->db() as $fieldName => $fieldType) { - // Skip if non HTML or if empty - if ($fieldType !== 'HTMLText' || empty($published[$fieldName])) { - continue; - } - - // Regenerate content - $content = Image::regenerate_html_links($published[$fieldName]); - if($content === $published[$fieldName]) { - continue; - } - - $query = sprintf('UPDATE "%s" SET "%s" = ? WHERE "ID" = ?', $table, $fieldName); - DB::prepared_query($query, array($content, $this->ID)); - - // Tell static caching to update itself - if($table == 'SiteTree_Live') { - $publishedClass = $origPublished['ClassName']; - $origPublishedObj = new $publishedClass($origPublished); - $this->invokeWithExtensions('onRenameLinkedAsset', $origPublishedObj); - } + foreach($this->db() as $fieldName => $fieldType) { + // Skip if non HTML or if empty + if ($fieldType !== 'HTMLText') { + continue; } + $fieldValue = $this->{$fieldName}; + if(empty($fieldValue)) { + continue; + } + + // Regenerate content + $content = Image::regenerate_html_links($fieldValue); + if($content === $fieldValue) { + continue; + } + + // Write content directly without updating linked assets + $table = ClassInfo::table_for_object_field($this, $fieldName); + $query = sprintf('UPDATE "%s" SET "%s" = ? WHERE "ID" = ?', $table, $fieldName); + DB::prepared_query($query, array($content, $this->ID)); + + // Update linked assets + $this->invokeWithExtensions('onRenameLinkedAsset'); } } @@ -2267,7 +2241,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid return $actions; } - if($this->isPublished() && $this->canPublish() && !$this->getIsDeletedFromStage() && $this->canDeleteFromLive()) { + if($this->isPublished() && $this->canPublish() && !$this->getIsDeletedFromStage() && $this->canUnpublish()) { // "unpublish" $moreOptions->push( FormAction::create('unpublish', _t('SiteTree.BUTTONUNPUBLISH', 'Unpublish'), 'delete') @@ -2292,7 +2266,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid if($existsOnLive) { // "restore" $majorActions->push(FormAction::create('revert',_t('CMSMain.RESTORE','Restore'))); - if($this->canDelete() && $this->canDeleteFromLive()) { + if($this->canDelete() && $this->canUnpublish()) { // "delete from live" $majorActions->push( FormAction::create('deletefromlive',_t('CMSMain.DELETEFP','Delete')) @@ -2428,11 +2402,13 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid /** * Unpublish this page - remove it from the live site * + * Overrides {@see Versioned::doUnpublish()} + * * @uses SiteTreeExtension->onBeforeUnpublish() * @uses SiteTreeExtension->onAfterUnpublish() */ public function doUnpublish() { - if(!$this->canDeleteFromLive()) return false; + if(!$this->canUnpublish()) return false; if(!$this->ID) return false; $this->invokeWithExtensions('onBeforeUnpublish', $this); @@ -2550,58 +2526,10 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid } /** - * Removes the page from both live and stage - * - * @return bool Success - */ - public function doArchive() { - $this->invokeWithExtensions('onBeforeArchive', $this); - - if($this->doUnpublish()) { - $this->delete(); - $this->invokeWithExtensions('onAfterArchive', $this); - - return true; - } - - return false; - } - - /** - * Check if the current user is allowed to archive this page. - * If extended, ensure that both canDelete and canDeleteFromLive are extended also - * - * @param Member $member - * @return bool - */ - public function canArchive($member = null) { - if(!$member) { - $member = Member::currentUser(); - } - - // Standard mechanism for accepting permission changes from extensions - $extended = $this->extendedCan('canArchive', $member); - if($extended !== null) { - return $extended; - } - - // Check if this page can be deleted - if(!$this->canDelete($member)) { - return false; - } - - // If published, check if we can delete from live - if($this->ExistsOnLive && !$this->canDeleteFromLive($member)) { - return false; - } - - return true; - } - - /** - * Synonym of {@link doUnpublish} + * @deprecated */ public function doDeleteFromLive() { + Deprecation::notice("4.0", "Use doUnpublish instead"); return $this->doUnpublish(); } @@ -2622,21 +2550,6 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid return stripos($this->ID, 'new') === 0; } - - /** - * Check if this page has been published. - * - * @return bool - */ - public function isPublished() { - if($this->isNew()) - return false; - - return (DB::prepared_query("SELECT \"ID\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($this->ID))->value()) - ? true - : false; - } - /** * Get the class dropdown used in the CMS to change the class of a page. This returns the list of options in the * dropdown as a Map from class name to singular name. Filters by {@link SiteTree->canCreate()}, as well as @@ -2920,7 +2833,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid return $classes; } - + /** * Compares current draft with live version, and returns true if no draft version of this page exists but the page * is still published (eg, after triggering "Delete from draft site" in the CMS). @@ -2930,7 +2843,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid public function getIsDeletedFromStage() { if(!$this->ID) return true; if($this->isNew()) return false; - + $stageVersion = Versioned::get_versionnumber_by_stage('SiteTree', 'Stage', $this->ID); // Return true for both completely deleted pages and for pages just deleted from stage diff --git a/code/model/SiteTreeFileExtension.php b/code/model/SiteTreeFileExtension.php index 57b5777b..d2e71904 100644 --- a/code/model/SiteTreeFileExtension.php +++ b/code/model/SiteTreeFileExtension.php @@ -1,12 +1,22 @@ 'SiteTree' + 'BackLinkTracking' => 'SiteTree.ImageTracking' // {@see SiteTreeLinkTracking} ); public function updateCMSFields(FieldList $fields) { @@ -16,8 +26,8 @@ class SiteTreeFileExtension extends DataExtension { _t('AssetTableField.BACKLINKCOUNT', 'Used on:'), $this->BackLinkTracking()->Count() . ' ' . _t('AssetTableField.PAGES', 'page(s)') ) - ->addExtraClass('cms-description-toggle') - ->setDescription($this->BackLinkHTMLList()), + ->addExtraClass('cms-description-toggle') + ->setDescription($this->BackLinkHTMLList()), 'LastEdited' ); } @@ -32,8 +42,8 @@ class SiteTreeFileExtension extends DataExtension { 'SiteTreeFileExtension.BACKLINK_LIST_DESCRIPTION', 'This list shows all pages where the file has been added through a WYSIWYG editor.' ) . ''; - $html .= ''; + return $html; } /** * Extend through {@link updateBackLinkTracking()} in your own {@link Extension}. * - * @param string|array $filter - * @param string $sort - * @param string $join - * @param string $limit * @return ManyManyList */ public function BackLinkTracking() { @@ -88,32 +95,36 @@ class SiteTreeFileExtension extends DataExtension { } /** - * Updates link tracking. + * Updates link tracking in the current stage. */ public function onAfterDelete() { + // Skip live stage + if(\Versioned::current_stage() === \Versioned::get_live_stage()) { + return; + } + // We query the explicit ID list, because BackLinkTracking will get modified after the stage // site does its thing $brokenPageIDs = $this->owner->BackLinkTracking()->column("ID"); if($brokenPageIDs) { - $origStage = Versioned::current_stage(); - - // This will syncLinkTracking on draft - Versioned::reading_stage('Stage'); + // This will syncLinkTracking on the same stage as this file $brokenPages = DataObject::get('SiteTree')->byIDs($brokenPageIDs); foreach($brokenPages as $brokenPage) { $brokenPage->write(); } - - // This will syncLinkTracking on published - Versioned::reading_stage('Live'); - $liveBrokenPages = DataObject::get('SiteTree')->byIDs($brokenPageIDs); - foreach($liveBrokenPages as $brokenPage) { - $brokenPage->write(); - } - - Versioned::reading_stage($origStage); } } + + public function onAfterWrite() { + // Update any database references in the current stage + $this->updateLinks(); + } + + public function onAfterVersionedPublish() { + // Ensure that ->updateLinks is invoked on the draft record + // after ->doPublish() is invoked. + $this->updateLinks(); + } /** * Rewrite links to the $old file to now point to the $new file. @@ -121,6 +132,11 @@ class SiteTreeFileExtension extends DataExtension { * @uses SiteTree->rewriteFileID() */ public function updateLinks() { + // Skip live stage + if(\Versioned::current_stage() === \Versioned::get_live_stage()) { + return; + } + if(class_exists('Subsite')) { Subsite::disable_subsite_filter(true); } diff --git a/code/model/SiteTreeLinkTracking.php b/code/model/SiteTreeLinkTracking.php index 2195173c..9f9d16ce 100644 --- a/code/model/SiteTreeLinkTracking.php +++ b/code/model/SiteTreeLinkTracking.php @@ -12,6 +12,11 @@ * referenced in any HTMLText fields, and two booleans to indicate if there are any broken links. Call * augmentSyncLinkTracking to update those fields with any changes to those fields. * + * Note that since both SiteTree and File are versioned, LinkTracking and ImageTracking will + * only be enabled for the Stage record. + * + * {@see SiteTreeFileExtension} for the extension applied to {@see File} + * * @property SiteTree $owner * * @property bool $HasBrokenFile @@ -19,6 +24,7 @@ * * @method ManyManyList LinkTracking() List of site pages linked on this page. * @method ManyManyList ImageTracking() List of Images linked on this page. + * @method ManyManyList BackLinkTracking List of site pages that link to this page. */ class SiteTreeLinkTracking extends DataExtension { @@ -38,6 +44,10 @@ class SiteTreeLinkTracking extends DataExtension { "ImageTracking" => "File" ); + private static $belongs_many_many = array( + "BackLinkTracking" => "SiteTree.LinkTracking" + ); + private static $many_many_extraFields = array( "LinkTracking" => array("FieldName" => "Varchar"), "ImageTracking" => array("FieldName" => "Varchar") @@ -46,6 +56,8 @@ class SiteTreeLinkTracking extends DataExtension { /** * Scrape the content of a field to detect anly links to local SiteTree pages or files * + * @todo - Replace image tracking with shortcodes + * * @param string $fieldName The name of the field on {@link @owner} to scrape */ public function trackLinksInField($fieldName) { @@ -151,8 +163,15 @@ class SiteTreeLinkTracking extends DataExtension { /** * Find HTMLText fields on {@link owner} to scrape for links that need tracking + * + * @todo Support versioned many_many for per-stage page link tracking */ public function augmentSyncLinkTracking() { + // Skip live tracking + if(\Versioned::current_stage() == \Versioned::get_live_stage()) { + return; + } + // Reset boolean broken flags $this->owner->HasBrokenLink = false; $this->owner->HasBrokenFile = false; @@ -169,7 +188,9 @@ class SiteTreeLinkTracking extends DataExtension { } } - foreach($htmlFields as $field) $this->trackLinksInField($field); + foreach($htmlFields as $field) { + $this->trackLinksInField($field); + } } } diff --git a/code/model/VirtualPage.php b/code/model/VirtualPage.php index 0576c086..6b85f5a3 100644 --- a/code/model/VirtualPage.php +++ b/code/model/VirtualPage.php @@ -12,7 +12,7 @@ class VirtualPage extends Page { public static $virtualFields; /** - * @var Array Define fields that are not virtual - the virtual page must define these fields themselves. + * @var array Define fields that are not virtual - the virtual page must define these fields themselves. * Note that anything in {@link self::config()->initially_copied_fields} is implicitly included in this list. */ private static $non_virtual_fields = array( @@ -34,7 +34,7 @@ class VirtualPage extends Page { ); /** - * @var Array Define fields that are initially copied to virtual pages but left modifiable after that. + * @var array Define fields that are initially copied to virtual pages but left modifiable after that. */ private static $initially_copied_fields = array( 'ShowInMenus', @@ -126,6 +126,7 @@ class VirtualPage extends Page { if($this->CopyContentFrom()) { return $this->CopyContentFrom()->allowedChildren(); } + return array(); } public function syncLinkTracking() { @@ -138,19 +139,14 @@ class VirtualPage extends Page { /** * We can only publish the page if there is a published source page + * + * @param Member $member Member to check + * @return bool */ public function canPublish($member = null) { return $this->isPublishable() && parent::canPublish($member); } - /** - * Return true if we can delete this page from the live site, which is different from can - * we publish it. - */ - public function canDeleteFromLive($member = null) { - return parent::canPublish($member); - } - /** * Returns true if is page is publishable by anyone at all * Return false if the source page isn't published yet. @@ -202,7 +198,7 @@ class VirtualPage extends Page { // Create links back to the original object in the CMS if($this->CopyContentFrom()->exists()) { - $link = "CopyContentFromID\">" + $link = "CopyContentFromID\">" . _t('VirtualPage.EditLink', 'edit') . ""; $msgs[] = _t( @@ -376,6 +372,9 @@ class VirtualPage extends Page { /** * Ensure we have an up-to-date version of everything. + * + * @param DataObject $source + * @param bool $updateImageTracking */ public function copyFrom($source, $updateImageTracking = true) { if($source) { diff --git a/tasks/RemoveOrphanedPagesTask.php b/tasks/RemoveOrphanedPagesTask.php index 313e710d..924ab0b9 100644 --- a/tasks/RemoveOrphanedPagesTask.php +++ b/tasks/RemoveOrphanedPagesTask.php @@ -215,6 +215,7 @@ in the other stage:
$removedOrphans = array(); $orphanBaseClass = ClassInfo::baseDataClass($this->orphanedSearchClass); foreach($orphanIDs as $id) { + /** @var SiteTree $stageRecord */ $stageRecord = Versioned::get_one_by_stage( $this->orphanedSearchClass, 'Stage', @@ -226,6 +227,7 @@ in the other stage:
$stageRecord->destroy(); unset($stageRecord); } + /** @var SiteTree $liveRecord */ $liveRecord = Versioned::get_one_by_stage( $this->orphanedSearchClass, 'Live', @@ -233,7 +235,7 @@ in the other stage:
); if($liveRecord) { $removedOrphans[$liveRecord->ID] = sprintf('Removed %s (#%d) from Live', $liveRecord->Title, $liveRecord->ID); - $liveRecord->doDeleteFromLive(); + $liveRecord->doUnpublish(); $liveRecord->destroy(); unset($liveRecord); } diff --git a/tests/model/ErrorPageFileExtensionTest.php b/tests/model/ErrorPageFileExtensionTest.php new file mode 100644 index 00000000..73e7cd06 --- /dev/null +++ b/tests/model/ErrorPageFileExtensionTest.php @@ -0,0 +1,58 @@ +versionedMode = Versioned::get_reading_mode(); + Versioned::reading_stage('Stage'); + AssetStoreTest_SpyStore::activate('ErrorPageFileExtensionTest'); + $file = new File(); + $file->setFromString('dummy', 'dummy.txt'); + $file->write(); + } + + public function tearDown() { + Versioned::set_reading_mode($this->versionedMode); + AssetStoreTest_SpyStore::reset(); + parent::tearDown(); // TODO: Change the autogenerated stub + } + + public function testErrorPage() { + // Get and publish records + $notFoundPage = $this->objFromFixture('ErrorPage', '404'); + $notFoundPage->publish('Stage', 'Live'); + $notFoundLink = $notFoundPage->Link(); + + $disallowedPage = $this->objFromFixture('ErrorPage', '403'); + $disallowedPage->publish('Stage', 'Live'); + $disallowedLink = $disallowedPage->Link(); + + // Get stage version of file + $file = File::get()->first(); + $fileLink = $file->Link(); + Session::clear("loggedInAs"); + + // Generate shortcode for a file which doesn't exist + $shortcode = File::handle_shortcode(array('id' => 9999), null, new ShortcodeParser(), 'file_link'); + $this->assertEquals($notFoundLink, $shortcode); + $shortcode = File::handle_shortcode(array('id' => 9999), 'click here', new ShortcodeParser(), 'file_link'); + $this->assertEquals(sprintf('%s', $notFoundLink, 'click here'), $shortcode); + + // Test that user cannot view draft file + $shortcode = File::handle_shortcode(array('id' => $file->ID), null, new ShortcodeParser(), 'file_link'); + $this->assertEquals($disallowedLink, $shortcode); + $shortcode = File::handle_shortcode(array('id' => $file->ID), 'click here', new ShortcodeParser(), 'file_link'); + $this->assertEquals(sprintf('%s', $disallowedLink, 'click here'), $shortcode); + + // Authenticated users don't get the same error + $this->logInWithPermission('ADMIN'); + $shortcode = File::handle_shortcode(array('id' => $file->ID), null, new ShortcodeParser(), 'file_link'); + $this->assertEquals($fileLink, $shortcode); + } + +} diff --git a/tests/model/ErrorPageTest.yml b/tests/model/ErrorPageTest.yml index e2152199..b716c73d 100644 --- a/tests/model/ErrorPageTest.yml +++ b/tests/model/ErrorPageTest.yml @@ -1,11 +1,11 @@ ErrorPage: - 404: - Title: Page Not Found - URLSegment: page-not-found - Content: My error page body - ErrorCode: 404 - 403: - Title: Permission Failure - URLSegment: permission-denied - Content: You do not have permission to view this page - ErrorCode: 403 \ No newline at end of file + 404: + Title: Page Not Found + URLSegment: page-not-found + Content: My error page body + ErrorCode: 404 + 403: + Title: Permission Failure + URLSegment: permission-denied + Content: You do not have permission to view this page + ErrorCode: 403 diff --git a/tests/model/FileLinkTrackingTest.php b/tests/model/FileLinkTrackingTest.php index 22296bdb..371034e5 100644 --- a/tests/model/FileLinkTrackingTest.php +++ b/tests/model/FileLinkTrackingTest.php @@ -8,6 +8,9 @@ class FileLinkTrackingTest extends SapphireTest { public function setUp() { parent::setUp(); + + Versioned::reading_stage('Stage'); + AssetStoreTest_SpyStore::activate('FileLinkTrackingTest'); $this->logInWithPermission('ADMIN'); @@ -17,6 +20,8 @@ class FileLinkTrackingTest extends SapphireTest { $destPath = AssetStoreTest_SpyStore::getLocalPath($file); Filesystem::makeFolder(dirname($destPath)); file_put_contents($destPath, str_repeat('x', 1000000)); + // Ensure files are published, thus have public urls + $file->doPublish(); } // Since we can't hard-code IDs, manually inject image tracking shortcode @@ -36,7 +41,13 @@ class FileLinkTrackingTest extends SapphireTest { public function testFileRenameUpdatesDraftAndPublishedPages() { $page = $this->objFromFixture('Page', 'page1'); - $this->assertTrue($page->doPublish()); + $page->doPublish(); + + // Live and stage pages both have link to public file + $this->assertContains( + 'ID))->value() + ); $this->assertContains( 'ID))->value() @@ -45,7 +56,35 @@ class FileLinkTrackingTest extends SapphireTest { $file = $this->objFromFixture('Image', 'file1'); $file->Name = 'renamed-test-file.jpg'; $file->write(); - + + // Staged record now points to secure URL of renamed file, live record remains unchanged + // Note that the "secure" url doesn't have the "FileLinkTrackingTest" component because + // the mocked test location disappears for secure files. + $this->assertContains( + 'ID))->value() + ); + $this->assertContains( + 'ID))->value() + ); + + // Publishing the file should result in a direct public link (indicated by "FileLinkTrackingTest") + // Although the old live page will still point to the old record. + // @todo - Ensure shortcodes are used with all images to prevent live records having broken links + $file->doPublish(); + $this->assertContains( + 'ID))->value() + ); + $this->assertContains( + // Note: Broken link until shortcode-enabled + 'ID))->value() + ); + + // Publishing the page after publishing the asset will resolve any link issues + $page->doPublish(); $this->assertContains( 'ID))->value() @@ -72,11 +111,16 @@ class FileLinkTrackingTest extends SapphireTest { $file->Name = 'renamed-test-file.jpg'; $file->write(); - // Verify that the draft and publish virtual pages both have the corrected link + // Verify that the draft virtual pages have the correct content $this->assertContains( - 'ID))->value() ); + + // Publishing both file and page will update the live record + $file->doPublish(); + $page->doPublish(); + $this->assertContains( 'ID))->value() @@ -119,12 +163,16 @@ class FileLinkTrackingTest extends SapphireTest { $file = DataObject::get_by_id('File', $file->ID); $file->Name = 'renamed-test-file-second-time.jpg'; $file->write(); + $file->doPublish(); // Confirm that the correct image is shown in both the draft and live site $this->assertContains( 'ID))->value() ); + + // Publishing this record also updates live record + $page->doPublish(); $this->assertContains( 'ID))->value() diff --git a/tests/model/SiteTreeBacklinksTest.php b/tests/model/SiteTreeBacklinksTest.php index c64b2678..d5a49ba1 100644 --- a/tests/model/SiteTreeBacklinksTest.php +++ b/tests/model/SiteTreeBacklinksTest.php @@ -1,5 +1,8 @@ markTestSkipped("Test disabled until versioned many_many implemented"); + // publish page 1 & 3 $page1 = $this->objFromFixture('Page', 'page1'); $page3 = $this->objFromFixture('Page', 'page3'); @@ -109,6 +114,8 @@ class SiteTreeBacklinksTest extends SapphireTest { } public function testPublishingPageWithModifiedUrlRewritesLink() { + $this->markTestSkipped("Test disabled until versioned many_many implemented"); + // publish page 1 & 3 $page1 = $this->objFromFixture('Page', 'page1'); $page3 = $this->objFromFixture('Page', 'page3'); @@ -144,6 +151,8 @@ class SiteTreeBacklinksTest extends SapphireTest { } public function testPublishingPageWithModifiedLinksRewritesLinks() { + $this->markTestSkipped("Test disabled until versioned many_many implemented"); + // publish page 1 & 3 $page1 = $this->objFromFixture('Page', 'page1'); $page3 = $this->objFromFixture('Page', 'page3'); @@ -209,6 +218,9 @@ class SiteTreeBacklinksTest extends SapphireTest { $page2 = $this->objFromFixture('Page', 'page2'); $this->assertEquals('

Testing page 1 link

', $page2->obj('ExtraContent')->forTemplate()); + // @todo - Implement versioned many_many + $this->markTestSkipped("Test disabled until versioned many_many implemented"); + // confirm that published link hasn't $page2Live = Versioned::get_one_by_stage("Page", "Live", "\"SiteTree\".\"ID\" = $page2->ID"); Versioned::reading_stage('Live'); diff --git a/tests/model/SiteTreeBrokenLinksTest.php b/tests/model/SiteTreeBrokenLinksTest.php index 7a4b1b28..d786525a 100644 --- a/tests/model/SiteTreeBrokenLinksTest.php +++ b/tests/model/SiteTreeBrokenLinksTest.php @@ -1,11 +1,26 @@ logInWithPermission('ADMIN'); + } + + public function tearDown() { + AssetStoreTest_SpyStore::reset(); + parent::tearDown(); + } + public function testBrokenLinksBetweenPages() { $obj = $this->objFromFixture('Page','content'); @@ -62,7 +77,7 @@ class SiteTreeBrokenLinksTest extends SapphireTest { public function testDeletingFileMarksBackedPagesAsBroken() { // Test entry $file = new File(); - $file->Filename = 'test-file.pdf'; + $file->setFromString('test', 'test-file.txt'); $file->write(); $obj = $this->objFromFixture('Page','content'); @@ -83,65 +98,48 @@ class SiteTreeBrokenLinksTest extends SapphireTest { // Delete the file $file->delete(); - // Confirm that it is marked as broken in both stage and live + // Confirm that it is marked as broken in stage $obj->flushCache(); $obj = DataObject::get_by_id("SiteTree", $obj->ID); $this->assertEquals(1, $obj->HasBrokenFile); + // Publishing this page marks it as broken on live too + $obj->doPublish(); $liveObj = Versioned::get_one_by_stage("SiteTree", "Live", "\"SiteTree\".\"ID\" = $obj->ID"); $this->assertEquals(1, $liveObj->HasBrokenFile); } public function testDeletingMarksBackLinkedPagesAsBroken() { - $this->logInWithPermission('ADMIN'); - // Set up two published pages with a link from content -> about $linkDest = $this->objFromFixture('Page','about'); - $linkDest->doPublish(); $linkSrc = $this->objFromFixture('Page','content'); $linkSrc->Content = "

ID]\">about us

"; $linkSrc->write(); - - $linkSrc->doPublish(); // Confirm no broken link $this->assertEquals(0, (int)$linkSrc->HasBrokenLink); - $this->assertEquals(0, DB::query("SELECT \"HasBrokenLink\" FROM \"SiteTree_Live\" - WHERE \"ID\" = $linkSrc->ID")->value()); // Delete page from draft $linkDestID = $linkDest->ID; $linkDest->delete(); - // Confirm draft has broken link, and published doesn't + // Confirm draft has broken link $linkSrc->flushCache(); $linkSrc = $this->objFromFixture('Page', 'content'); $this->assertEquals(1, (int)$linkSrc->HasBrokenLink); - $this->assertEquals(0, DB::query("SELECT \"HasBrokenLink\" FROM \"SiteTree_Live\" - WHERE \"ID\" = $linkSrc->ID")->value()); - - // Delete from live - $linkDest = Versioned::get_one_by_stage("SiteTree", "Live", "\"SiteTree\".\"ID\" = $linkDestID"); - $linkDest->doDeleteFromLive(); - - // Confirm both draft and published have broken link - $linkSrc->flushCache(); - $linkSrc = $this->objFromFixture('Page', 'content'); - - $this->assertEquals(1, (int)$linkSrc->HasBrokenLink); - $this->assertEquals(1, DB::query("SELECT \"HasBrokenLink\" FROM \"SiteTree_Live\" - WHERE \"ID\" = $linkSrc->ID")->value()); } public function testPublishingSourceBeforeDestHasBrokenLink() { + $this->markTestSkipped("Test disabled until versioned many_many implemented"); + $this->logInWithPermission('ADMIN'); // Set up two draft pages with a link from content -> about $linkDest = $this->objFromFixture('Page','about'); // Ensure that it's not on the published site - $linkDest->doDeleteFromLive(); + $linkDest->doUnpublish(); $linkSrc = $this->objFromFixture('Page','content'); $linkSrc->Content = "

ID]\">about us

"; @@ -157,6 +155,7 @@ class SiteTreeBrokenLinksTest extends SapphireTest { } public function testRestoreFixesBrokenLinks() { + $this->markTestSkipped("Test disabled until versioned many_many implemented"); // Create page and virtual page $p = new Page(); $p->Title = "source"; diff --git a/tests/model/SiteTreeBrokenLinksTest.yml b/tests/model/SiteTreeBrokenLinksTest.yml index 0245ca70..46e7e829 100644 --- a/tests/model/SiteTreeBrokenLinksTest.yml +++ b/tests/model/SiteTreeBrokenLinksTest.yml @@ -1,27 +1,21 @@ Page: - content: - Title: ContentPage - Content: 'This is some partially happy content. It has one missing a skiplink, but does have another skiplink here.' - about: - Title: About - URLSegment: about - Content: 'about us here about skiplinks here.' - brokenInternalRedirector: - RedirectionType: Internal - Title: RedirectorPageToBrokenInteralPage - LinkToID: 0 - workingInternalRedirector: - RedirectionType: Internal - Title: RedirectorPageToBrokenInteralPage - LinkTo: =>Page.content - -File: - privacypolicy: - Name: privacypolicy.pdf - Title: privacypolicy.pdf - Filename: assets/privacypolicy.pdf + content: + Title: ContentPage + Content: 'This is some partially happy content. It has one missing a skiplink, but does have another skiplink here.' + about: + Title: About + URLSegment: about + Content: 'about us here about skiplinks here.' + brokenInternalRedirector: + RedirectionType: Internal + Title: RedirectorPageToBrokenInteralPage + LinkToID: 0 + workingInternalRedirector: + RedirectionType: Internal + Title: RedirectorPageToBrokenInteralPage + LinkTo: =>Page.content ErrorPage: - 404: - Title: Page not Found - ErrorCode: 404 + 404: + Title: Page not Found + ErrorCode: 404 diff --git a/tests/model/SiteTreeTest.php b/tests/model/SiteTreeTest.php index 01962240..55b85873 100644 --- a/tests/model/SiteTreeTest.php +++ b/tests/model/SiteTreeTest.php @@ -381,7 +381,7 @@ class SiteTreeTest extends SapphireTest { $parentPage = $this->objFromFixture('Page', 'about'); - $parentPage->doDeleteFromLive(); + $parentPage->doUnpublish(); Versioned::reading_stage('Live'); @@ -425,7 +425,7 @@ class SiteTreeTest extends SapphireTest { $pageStaffDuplicate->doPublish(); $parentPage = $this->objFromFixture('Page', 'about'); - $parentPage->doDeleteFromLive(); + $parentPage->doUnpublish(); Versioned::reading_stage('Live'); $this->assertFalse(DataObject::get_by_id('Page', $pageAbout->ID)); diff --git a/tests/model/VirtualPageTest.php b/tests/model/VirtualPageTest.php index 0e8aece3..da33d251 100644 --- a/tests/model/VirtualPageTest.php +++ b/tests/model/VirtualPageTest.php @@ -198,14 +198,14 @@ class VirtualPageTest extends FunctionalTest { // Delete the source page $this->assertTrue($vp->canPublish()); - $this->assertTrue($p->doDeleteFromLive()); + $this->assertTrue($p->doUnpublish()); // Confirm that we can unpublish, but not publish - $this->assertTrue($vp->canDeleteFromLive()); + $this->assertTrue($vp->canUnpublish()); $this->assertFalse($vp->canPublish()); // Confirm that the action really works - $this->assertTrue($vp->doDeleteFromLive()); + $this->assertTrue($vp->doUnpublish()); $this->assertNull(DB::query("SELECT \"ID\" FROM \"SiteTree_Live\" WHERE \"ID\" = $vp->ID")->value()); } @@ -398,7 +398,7 @@ class VirtualPageTest extends FunctionalTest { // Delete the source page form live, confirm that the virtual page has also been unpublished $pLive = Versioned::get_one_by_stage('SiteTree', 'Live', '"SiteTree"."ID" = ' . $pID); - $this->assertTrue($pLive->doDeleteFromLive()); + $this->assertTrue($pLive->doUnpublish()); $vpLive = Versioned::get_one_by_stage('SiteTree', 'Live', '"SiteTree"."ID" = ' . $vp->ID); $this->assertNull($vpLive); diff --git a/tests/search/SearchFormTest.php b/tests/search/SearchFormTest.php index 5b8dfa0f..8313a800 100644 --- a/tests/search/SearchFormTest.php +++ b/tests/search/SearchFormTest.php @@ -225,7 +225,10 @@ class ZZZSearchFormTest extends FunctionalTest { $sf = new SearchForm($this->mockController, 'SearchForm'); $dontShowInSearchFile = $this->objFromFixture('File', 'dontShowInSearchFile'); + $dontShowInSearchFile->publish('Stage', 'Live'); $showInSearchFile = $this->objFromFixture('File', 'showInSearchFile'); + $showInSearchFile->publish('Stage', 'Live'); + $results = $sf->getResults(null, array('Search'=>'dontShowInSearchFile')); $this->assertNotContains( $dontShowInSearchFile->ID, diff --git a/tests/search/SearchFormTest.yml b/tests/search/SearchFormTest.yml index 6b1cbc0a..1cc8a685 100644 --- a/tests/search/SearchFormTest.yml +++ b/tests/search/SearchFormTest.yml @@ -1,43 +1,43 @@ Group: - websiteusers: - Title: View certain restricted pages + websiteusers: + Title: View certain restricted pages Member: - randomuser: - Email: randomuser@test.com - Password: test - websiteuser: - Email: websiteuser@test.com - Password: test - Groups: =>Group.websiteusers + randomuser: + Email: randomuser@test.com + Password: test + websiteuser: + Email: websiteuser@test.com + Password: test + Groups: =>Group.websiteusers SiteTree: - searchformholder: - URLSegment: searchformholder - Title: searchformholder - publicPublishedPage: - Title: publicPublishedPage - publicUnpublishedPage: - Title: publicUnpublishedPage - restrictedViewLoggedInUsers: - CanViewType: LoggedInUsers - Title: restrictedViewLoggedInUsers - restrictedViewOnlyWebsiteUsers: - CanViewType: OnlyTheseUsers - ViewerGroups: =>Group.websiteusers - Title: restrictedViewOnlyWebsiteUsers - inheritRestrictedView: - CanViewType: Inherit - Parent: =>SiteTree.restrictedViewLoggedInUsers - Title: inheritRestrictedView - dontShowInSearchPage: - Title: dontShowInSearchPage - ShowInSearch: 0 - pageWithSpecialChars: - Title: Brötchen - Content: Frisch vom Bäcker + searchformholder: + URLSegment: searchformholder + Title: searchformholder + publicPublishedPage: + Title: publicPublishedPage + publicUnpublishedPage: + Title: publicUnpublishedPage + restrictedViewLoggedInUsers: + CanViewType: LoggedInUsers + Title: restrictedViewLoggedInUsers + restrictedViewOnlyWebsiteUsers: + CanViewType: OnlyTheseUsers + ViewerGroups: =>Group.websiteusers + Title: restrictedViewOnlyWebsiteUsers + inheritRestrictedView: + CanViewType: Inherit + Parent: =>SiteTree.restrictedViewLoggedInUsers + Title: inheritRestrictedView + dontShowInSearchPage: + Title: dontShowInSearchPage + ShowInSearch: 0 + pageWithSpecialChars: + Title: Brötchen + Content: Frisch vom Bäcker File: - showInSearchFile: - Title: showInSearchFile - ShowInSearch: 1 - dontShowInSearchFile: - Title: dontShowInSearchFile - ShowInSearch: 0 + showInSearchFile: + Title: showInSearchFile + ShowInSearch: 1 + dontShowInSearchFile: + Title: dontShowInSearchFile + ShowInSearch: 0