tonyair
/
silverstripe-versionfeed
mirror of https://github.com/silverstripe/silverstripe-versionfeed
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

FIX: Minor functional alterations and CI improvements 

FIX: PSR-2 codebase. Formatting via phpcbf
FIX: rendering bug in allchanges
FIX: update .gitattributes to not export codecov's config file
FIX: Update SiteTree_versions to the ss4 equivalent SiteTree_Versions
This commit is contained in:
Dylan Wagstaff 2017-12-12 09:47:35 +13:00
parent 17cf3d7487
commit 67e112fd12
12 changed files with 805 additions and 773 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitattributes vendored
View File

@ -4,3 +4,4 @@
/.gitignore export-ignore
/.travis.yml export-ignore
/.scrutinizer.yml export-ignore
/codecov.yml export-ignore

1
.travis.yml
View File

@ -22,7 +22,6 @@ before_script:
# Install composer dependencies
- composer validate
- composer require --no-update silverstripe/recipe-core:1.0.x-dev
- if [[ $DB == PGSQL ]]; then composer require --no-update silverstripe/postgresql 2.0.x-dev; fi
- composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile

4
composer.json
View File

@ -12,7 +12,9 @@
],
"require":
{
"silverstripe/recipe-cms": "^1"
"silverstripe/cms": "^4",
"silverstripe/versioned": "^1",
"silverstripe/siteconfig": "^4"
},
"require-dev": {
"phpunit/phpunit": "^5.7",

2
phpunit.xml.dist
View File

@ -1,4 +1,4 @@
<phpunit bootstrap="vendor/silverstripe/framework/tests/bootstrap.php" colors="true">
<phpunit bootstrap="vendor/silverstripe/cms/tests/bootstrap.php" colors="true">
<testsuite name="Default">
<directory>tests/</directory>
</testsuite>

14
src/Filters/CachedContentFilter.php
View File

@ -2,16 +2,13 @@
namespace SilverStripe\VersionFeed\Filters;
use SilverStripe\Core\Config\Config;
/**
* Caches results of a callback
*/
class CachedContentFilter extends ContentFilter {
class CachedContentFilter extends ContentFilter
{
/**
* Enable caching
@ -21,7 +18,8 @@ class CachedContentFilter extends ContentFilter {
*/
private static $cache_enabled = true;
public function getContent($key, $callback) {
public function getContent($key, $callback)
{
$cache = $this->getCache();
// Return cached value if available
@ -29,7 +27,9 @@ class CachedContentFilter extends ContentFilter {
$result = (isset($_GET['flush']) || !$cacheEnabled)
? null
: $cache->get($key);
if($result) return $result;
if ($result) {
return $result;
}
// Fallback to generate result
$result = parent::getContent($key, $callback);

14
src/Filters/ContentFilter.php
View File

@ -7,13 +7,12 @@ use SilverStripe\Core\Config\Configurable;
use Psr\SimpleCache\CacheInterface;
use SilverStripe\Core\Injector\Injector;
/**
* Conditionally executes a given callback, attempting to return the desired results
* of its execution.
*/
abstract class ContentFilter {
abstract class ContentFilter
{
use Configurable;
@ -32,7 +31,8 @@ abstract class ContentFilter {
*/
private static $cache_lifetime = 300;
public function __construct($nestedContentFilter = null) {
public function __construct($nestedContentFilter = null)
{
$this->nestedContentFilter = $nestedContentFilter;
}
@ -41,7 +41,8 @@ abstract class ContentFilter {
*
* @return Zend_Cache_Frontend
*/
protected function getCache() {
protected function getCache()
{
return Injector::inst()->get(
CacheInterface::class . '.VersionFeedController'
);
@ -54,7 +55,8 @@ abstract class ContentFilter {
* @param callable $callback Callback for evaluating the content
* @return mixed Result of $callback()
*/
public function getContent($key, $callback) {
public function getContent($key, $callback)
{
if ($this->nestedContentFilter) {
return $this->nestedContentFilter->getContent($key, $callback);
} else {

15
src/Filters/RateLimitFilter.php
View File

@ -2,20 +2,15 @@
namespace SilverStripe\VersionFeed\Filters;
use SilverStripe\Core\Config\Config;
use SilverStripe\Control\Controller;
use SilverStripe\Control\HTTPResponse_Exception;
/**
* Provides rate limiting of execution of a callback
*/
class RateLimitFilter extends ContentFilter {
class RateLimitFilter extends ContentFilter
{
/**
* Time duration (in second) to allow for generation of cached results. Requests to
@ -66,7 +61,8 @@ class RateLimitFilter extends ContentFilter {
* @param string $itemkey Input key
* @return string Result key
*/
protected function getCacheKey($itemkey) {
protected function getCacheKey($itemkey)
{
$key = self::CACHE_PREFIX;
// Add global identifier
@ -84,7 +80,8 @@ class RateLimitFilter extends ContentFilter {
}
public function getContent($key, $callback) {
public function getContent($key, $callback)
{
// Bypass rate limiting if flushing, or timeout isn't set
$timeout = $this->config()->get('lock_timeout');
if (isset($_GET['flush']) || !$timeout) {

35
src/VersionFeed.php
View File

@ -13,7 +13,8 @@ use SilverStripe\Forms\LiteralField;
use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\CMS\Model\SiteTreeExtension;
class VersionFeed extends SiteTreeExtension {
class VersionFeed extends SiteTreeExtension
{
private static $db = array(
'PublicHistory' => 'Boolean(true)'
@ -23,7 +24,8 @@ class VersionFeed extends SiteTreeExtension {
'PublicHistory' => true
);
public function updateFieldLabels(&$labels) {
public function updateFieldLabels(&$labels)
{
$labels['PublicHistory'] = _t('RSSHistory.LABEL', 'Make history public');
}
@ -67,10 +69,11 @@ class VersionFeed extends SiteTreeExtension {
*
* @returns ArrayList List of cleaned records.
*/
public function getDiffList($highestVersion = null, $limit = 100) {
public function getDiffList($highestVersion = null, $limit = 100)
{
// This can leak secured content if it was protected via inherited setting.
// For now the users will need to be aware about this shortcoming.
$offset = $highestVersion ? "AND \"SiteTree_versions\".\"Version\"<='".(int)$highestVersion."'" : '';
$offset = $highestVersion ? "AND \"SiteTree_Versions\".\"Version\"<='".(int)$highestVersion."'" : '';
// Get just enough elements for diffing. We need one more than desired to have something to compare to.
$qLimit = (int)$limit + 1;
$versions = $this->owner->allVersions(
@ -88,7 +91,6 @@ class VersionFeed extends SiteTreeExtension {
// Check if we have something to compare with.
if (isset($previous)) {
// Produce the diff fields for use in the template.
if ($version->Title != $previous->Title) {
$diffTitle = Diff::compareHTML($version->Title, $previous->Title);
@ -146,7 +148,8 @@ class VersionFeed extends SiteTreeExtension {
*
* @returns DataObject Object with relevant fields diffed.
*/
public function getDiff() {
public function getDiff()
{
$changes = $this->getDiffList($this->owner->Version, 1);
if ($changes && $changes->Count()) {
return $changes->First();
@ -166,20 +169,24 @@ class VersionFeed extends SiteTreeExtension {
*
* @returns ArrayList List of cleaned records.
*/
public function getDiffedChanges($highestVersion = null, $fullHistory = true, $limit = 100) {
public function getDiffedChanges($highestVersion = null, $fullHistory = true, $limit = 100)
{
return $this->getDiffList(
$highestVersion,
$fullHistory ? $limit : 1
);
}
public function updateSettingsFields(FieldList $fields) {
if(!Config::inst()->get(get_class(), 'changes_enabled')) return;
public function updateSettingsFields(FieldList $fields)
{
if (!Config::inst()->get(get_class(), 'changes_enabled')) {
return;
}
// Add public history field.
$fields->addFieldToTab('Root.Settings', $publicHistory = new FieldGroup(
new CheckboxField('PublicHistory', $this->owner->fieldLabel('PublicHistory')
)));
new CheckboxField('PublicHistory', $this->owner->fieldLabel('PublicHistory'))
));
$warning = _t(
'VersionFeed.Warning',
@ -201,7 +208,8 @@ class VersionFeed extends SiteTreeExtension {
}
}
public function getSiteRSSLink() {
public function getSiteRSSLink()
{
// TODO: This link should be from the homepage, not this page.
if (Config::inst()->get(get_class(), 'allchanges_enabled')
&& SiteConfig::current_site_config()->AllChangesEnabled
@ -210,7 +218,8 @@ class VersionFeed extends SiteTreeExtension {
}
}
public function getDefaultRSSLink() {
public function getDefaultRSSLink()
{
if (Config::inst()->get(get_class(), 'changes_enabled') && $this->owner->PublicHistory) {
return $this->owner->Link('changes');
}

46
src/VersionFeedController.php
View File

@ -16,7 +16,8 @@ use SilverStripe\View\Requirements;
use SilverStripe\Core\Extension;
use SilverStripe\VersionFeed\Filters\ContentFilter;
class VersionFeedController extends Extension {
class VersionFeedController extends Extension
{
private static $allowed_actions = array(
'changes',
@ -35,7 +36,8 @@ class VersionFeedController extends Extension {
*
* @param ContentFilter $contentFilter
*/
public function setContentFilter(ContentFilter $contentFilter) {
public function setContentFilter(ContentFilter $contentFilter)
{
$this->contentFilter = $contentFilter;
}
@ -46,7 +48,8 @@ class VersionFeedController extends Extension {
* @param callable $callback Callback for evaluating the content
* @return mixed Result of $callback()
*/
protected function filterContent($key, $callback) {
protected function filterContent($key, $callback)
{
if ($this->contentFilter) {
return $this->contentFilter->getContent($key, $callback);
} else {
@ -54,7 +57,8 @@ class VersionFeedController extends Extension {
}
}
public function onAfterInit() {
public function onAfterInit()
{
$this->linkToPageRSSFeed();
$this->linkToAllSiteRSSFeed();
}
@ -62,7 +66,8 @@ class VersionFeedController extends Extension {
/**
* Get page-specific changes in a RSS feed.
*/
public function changes() {
public function changes()
{
// Check viewability of changes
if (!Config::inst()->get(VersionFeed::class, 'changes_enabled')
|| !$this->owner->PublicHistory
@ -88,7 +93,8 @@ class VersionFeedController extends Extension {
/**
* Get all changes from the site in a RSS feed.
*/
public function allchanges() {
public function allchanges()
{
// Check viewability of allchanges
if (!Config::inst()->get(VersionFeed::class, 'allchanges_enabled')
|| !SiteConfig::current_site_config()->AllChangesEnabled
@ -98,18 +104,16 @@ class VersionFeedController extends Extension {
$limit = (int)Config::inst()->get(VersionFeed::class, 'allchanges_limit');
$latestChanges = DB::query('
SELECT * FROM "SiteTree_versions"
SELECT * FROM "SiteTree_Versions"
WHERE "WasPublished" = \'1\'
AND "CanViewType" IN (\'Anyone\', \'Inherit\')
AND "ShowInSearch" = 1
AND ("PublicHistory" IS NULL OR "PublicHistory" = \'1\')
ORDER BY "LastEdited" DESC LIMIT ' . $limit
);
ORDER BY "LastEdited" DESC LIMIT ' . $limit);
$lastChange = $latestChanges->record();
$latestChanges->rewind();
if ($lastChange) {
// Cache the diffs to remove DOS possibility.
$key = 'allchanges'
. preg_replace('#[^a-zA-Z0-9_]#', '', $lastChange['LastEdited'])
@ -118,7 +122,6 @@ class VersionFeedController extends Extension {
$changeList = new ArrayList();
$canView = array();
foreach ($latestChanges as $record) {
// Check if the page should be visible.
// WARNING: although we are providing historical details, we check the current configuration.
$id = $record['RecordID'];
@ -126,10 +129,12 @@ class VersionFeedController extends Extension {
$page = SiteTree::get()->byID($id);
$canView[$id] = $page && $page->canView(new Member());
}
if (!$canView[$id]) continue;
if (!$canView[$id]) {
continue;
}
// Get the diff to the previous version.
$version = new Versioned_Version($record);
$version = SiteTree::create($record);
if ($diff = $version->getDiff()) {
$changeList->push($diff);
}
@ -142,7 +147,8 @@ class VersionFeedController extends Extension {
}
// Produce output
$rss = new RSSFeed($changeList, $this->owner->request->getURL(), $this->linkToAllSitesRSSFeedTitle(), '', 'Title', '', null);
$url = $this->owner->getRequest()->getURL();
$rss = new RSSFeed($changeList, $url, $this->linkToAllSitesRSSFeedTitle(), '', 'Title', '', null);
$rss->setTemplate('Page_allchanges_rss');
return $rss->outputToBrowser();
}
@ -150,7 +156,8 @@ class VersionFeedController extends Extension {
/**
* Generates and embeds the RSS header link for the page-specific version rss feed
*/
public function linkToPageRSSFeed() {
public function linkToPageRSSFeed()
{
if (!Config::inst()->get(VersionFeed::class, 'changes_enabled') || !$this->owner->PublicHistory) {
return;
}
@ -167,7 +174,8 @@ class VersionFeedController extends Extension {
/**
* Generates and embeds the RSS header link for the global version rss feed
*/
public function linkToAllSiteRSSFeed() {
public function linkToAllSiteRSSFeed()
{
if (!Config::inst()->get(VersionFeed::class, 'allchanges_enabled')
|| !SiteConfig::current_site_config()->AllChangesEnabled
) {
@ -180,10 +188,12 @@ class VersionFeedController extends Extension {
Requirements::insertHeadTags(
'<link rel="alternate" type="application/rss+xml" title="' . $title .
'" href="' . $url . '" />');
'" href="' . $url . '" />'
);
}
public function linkToAllSitesRSSFeedTitle() {
public function linkToAllSitesRSSFeedTitle()
{
return sprintf(_t('RSSHistory.SITEFEEDTITLE', 'Updates to %s'), SiteConfig::current_site_config()->Title);
}
}

23
src/VersionFeedSiteConfig.php
View File

@ -2,11 +2,6 @@
namespace SilverStripe\VersionFeed;
use SilverStripe\Forms\FieldList;
use SilverStripe\Core\Config\Config;
use SilverStripe\VersionFeed\VersionFeed;
@ -14,12 +9,11 @@ use SilverStripe\Forms\CheckboxField;
use SilverStripe\Forms\FieldGroup;
use SilverStripe\ORM\DataExtension;
/**
* Allows global configuration of all changes
*/
class VersionFeedSiteConfig extends DataExtension {
class VersionFeedSiteConfig extends DataExtension
{
private static $db = array(
'AllChangesEnabled' => 'Boolean(true)'
@ -29,14 +23,19 @@ class VersionFeedSiteConfig extends DataExtension {
'AllChangesEnabled' => true
);
public function updateFieldLabels(&$labels) {
public function updateFieldLabels(&$labels)
{
$labels['AllChangesEnabled'] = _t('VersionFeedSiteConfig.ALLCHANGESLABEL', 'Make global changes feed public');
}
public function updateCMSFields(FieldList $fields) {
if(!Config::inst()->get(VersionFeed::class, 'allchanges_enabled')) return;
public function updateCMSFields(FieldList $fields)
{
if (!Config::inst()->get(VersionFeed::class, 'allchanges_enabled')) {
return;
}
$fields->addFieldToTab('Root.Access',
$fields->addFieldToTab(
'Root.Access',
FieldGroup::create(new CheckboxField('AllChangesEnabled', $this->owner->fieldLabel('AllChangesEnabled')))
->setTitle(_t('VersionFeedSiteConfig.ALLCHANGES', 'All page changes'))
->setDescription(_t(

42
tests/VersionFeedFunctionalTest.php
View File

@ -17,8 +17,8 @@ use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Control\Director;
use Psr\SimpleCache\CacheInterface;
class VersionFeedFunctionalTest extends FunctionalTest {
class VersionFeedFunctionalTest extends FunctionalTest
{
protected $usesDatabase = true;
protected $baseURI = 'http://www.fakesite.test';
@ -30,7 +30,8 @@ class VersionFeedFunctionalTest extends FunctionalTest {
protected $userIP;
protected function setUp() {
protected function setUp()
{
Director::config()->set('alternate_base_url', $this->baseURI);
parent::setUp();
@ -54,7 +55,8 @@ class VersionFeedFunctionalTest extends FunctionalTest {
Config::modify()->set(RateLimitFilter::class, 'lock_cooldown', false);
}
public function tearDown() {
public function tearDown()
{
Director::config()->set('alternate_base_url', null);
$_SERVER['REMOTE_ADDR'] = $this->userIP;
@ -62,7 +64,8 @@ class VersionFeedFunctionalTest extends FunctionalTest {
parent::tearDown();
}
public function testPublicHistory() {
public function testPublicHistory()
{
$page = $this->createPageWithChanges(array('PublicHistory' => false));
$response = $this->get($page->RelativeLink('changes'));
@ -86,7 +89,8 @@ class VersionFeedFunctionalTest extends FunctionalTest {
$this->assertTrue((bool)$xml->channel->item);
}
public function testRateLimiting() {
public function testRateLimiting()
{
// Re-enable locking just for this test
Config::modify()->set(RateLimitFilter::class, 'lock_timeout', 20);
Config::modify()->set(CachedContentFilter::class, 'cache_enabled', true);
@ -145,37 +149,46 @@ class VersionFeedFunctionalTest extends FunctionalTest {
Config::modify()->set(CachedContentFilter::class, 'cache_enabled', false);
}
public function testContainsChangesForPageOnly() {
public function testContainsChangesForPageOnly()
{
$page1 = $this->createPageWithChanges(array('Title' => 'Page1'));
$page2 = $this->createPageWithChanges(array('Title' => 'Page2'));
$response = $this->get($page1->RelativeLink('changes'));
$xml = simplexml_load_string($response->getBody());
$titles = array_map(function($item) {return (string)$item->title;}, $xml->xpath('//item'));
$titles = array_map(function ($item) {
return (string)$item->title;
}, $xml->xpath('//item'));
// TODO Unclear if this should contain the original version
$this->assertContains('Changed: Page1', $titles);
$this->assertNotContains('Changed: Page2', $titles);
$response = $this->get($page2->RelativeLink('changes'));
$xml = simplexml_load_string($response->getBody());
$titles = array_map(function($item) {return (string)$item->title;}, $xml->xpath('//item'));
$titles = array_map(function ($item) {
return (string)$item->title;
}, $xml->xpath('//item'));
// TODO Unclear if this should contain the original version
$this->assertNotContains('Changed: Page1', $titles);
$this->assertContains('Changed: Page2', $titles);
}
public function testContainsAllChangesForAllPages() {
public function testContainsAllChangesForAllPages()
{
$page1 = $this->createPageWithChanges(array('Title' => 'Page1'));
$page2 = $this->createPageWithChanges(array('Title' => 'Page2'));
$response = $this->get($page1->RelativeLink('allchanges'));
$xml = simplexml_load_string($response->getBody());
$titles = array_map(function($item) {return (string)$item->title;}, $xml->xpath('//item'));
$titles = array_map(function ($item) {
return (string)$item->title;
}, $xml->xpath('//item'));
$this->assertContains('Page1', $titles);
$this->assertContains('Page2', $titles);
}
protected function createPageWithChanges($seed = null) {
protected function createPageWithChanges($seed = null)
{
$page = new Page();
$seed = array_merge(array(
@ -212,7 +225,8 @@ class VersionFeedFunctionalTest extends FunctionalTest {
/**
* Tests response code for globally disabled feedss
*/
public function testFeedViewability() {
public function testFeedViewability()
{
// Nested loop through each configuration
foreach (array(true, false) as $publicHistory_Page) {
@ -241,6 +255,4 @@ class VersionFeedFunctionalTest extends FunctionalTest {
}
}
}
}

15
tests/VersionFeedTest.php
View File

@ -2,7 +2,6 @@
namespace SilverStripe\VersionFeed\Tests;
use Page;
use SilverStripe\VersionFeed\VersionFeed;
use SilverStripe\VersionFeed\VersionFeedController;
@ -10,8 +9,8 @@ use SilverStripe\Dev\SapphireTest;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\CMS\Controllers\ContentController;
class VersionFeedTest extends SapphireTest {
class VersionFeedTest extends SapphireTest
{
protected $usesDatabase = true;
@ -24,15 +23,18 @@ class VersionFeedTest extends SapphireTest {
'SiteTree' => ['Translatable']
];
public function testDiffedChangesExcludesRestrictedItems() {
public function testDiffedChangesExcludesRestrictedItems()
{
$this->markTestIncomplete();
}
public function testDiffedChangesIncludesFullHistory() {
public function testDiffedChangesIncludesFullHistory()
{
$this->markTestIncomplete();
}
public function testDiffedChangesTitle() {
public function testDiffedChangesTitle()
{
$page = new Page(['Title' => 'My Title']);
$page->write();
$page->publish('Stage', 'Live');
@ -61,5 +63,4 @@ class VersionFeedTest extends SapphireTest {
'Ignores unpublished title changes'
);
}
}