FIX Do not use cached SiteTree object but ensure record ID is set before diff

This commit is contained in:
Robbie Averill 2017-12-18 16:38:16 +13:00
parent 6b6f4ec622
commit 5bde86198b
3 changed files with 25 additions and 11 deletions

View File

@ -128,8 +128,8 @@ class VersionFeedController extends Extension
// Check if the page should be visible.
// WARNING: although we are providing historical details, we check the current configuration.
$id = $record['RecordID'];
$page = DataObject::get_by_id(SiteTree::class, $id);
if (!isset($canView[$id])) {
$page = DataObject::get_by_id(SiteTree::class, $id);
$canView[$id] = $page && $page->canView(Security::getCurrentUser());
}
if (!$canView[$id]) {
@ -137,7 +137,8 @@ class VersionFeedController extends Extension
}
// Get the diff to the previous version.
$version = $page;
$record['ID'] = $record['RecordID'];
$version = SiteTree::create($record);
if ($diff = $version->getDiff()) {
$changeList->push($diff);
}

View File

@ -54,7 +54,7 @@ class VersionFeedFunctionalTest extends FunctionalTest
Config::modify()->set(RateLimitFilter::class, 'lock_cooldown', false);
}
public function tearDown()
protected function tearDown()
{
Director::config()->set('alternate_base_url', null);
@ -68,24 +68,37 @@ class VersionFeedFunctionalTest extends FunctionalTest
$page = $this->createPageWithChanges(array('PublicHistory' => false));
$response = $this->get($page->RelativeLink('changes'));
$this->assertEquals(404, $response->getStatusCode());
$this->assertEquals(
404,
$response->getStatusCode(),
'With Page\'s "PublicHistory" disabled, `changes` action response code should be 404'
);
$response = $this->get($page->RelativeLink('allchanges'));
$this->assertEquals(200, $response->getStatusCode());
$xml = simplexml_load_string($response->getBody());
$this->assertFalse((bool)$xml->channel->item);
$this->assertFalse(
(bool)$xml->channel->item,
'With Page\'s "PublicHistory" disabled, `allchanges` action should not have an item in the channel'
);
$page = $this->createPageWithChanges(array('PublicHistory' => true));
$response = $this->get($page->RelativeLink('changes'));
$this->assertEquals(200, $response->getStatusCode());
$xml = simplexml_load_string($response->getBody());
$this->assertTrue((bool)$xml->channel->item);
$this->assertTrue(
(bool)$xml->channel->item,
'With Page\'s "PublicHistory" enabled, `changes` action should have an item in the channel'
);
$response = $this->get($page->RelativeLink('allchanges'));
$this->assertEquals(200, $response->getStatusCode());
$xml = simplexml_load_string($response->getBody());
$this->assertTrue((bool)$xml->channel->item);
$this->assertTrue(
(bool)$xml->channel->item,
'With "PublicHistory" enabled, `allchanges` action should have an item in the channel'
);
}
public function testRateLimiting()
@ -148,7 +161,7 @@ class VersionFeedFunctionalTest extends FunctionalTest
Config::modify()->set(CachedContentFilter::class, 'cache_enabled', false);
}
public function testContainsChangesForPageOnly()
public function testChangesActionContainsChangesForCurrentPageOnly()
{
$page1 = $this->createPageWithChanges(array('Title' => 'Page1'));
$page2 = $this->createPageWithChanges(array('Title' => 'Page2'));
@ -172,7 +185,7 @@ class VersionFeedFunctionalTest extends FunctionalTest
$this->assertContains('Changed: Page2', $titles);
}
public function testContainsAllChangesForAllPages()
public function testAllChangesActionContainsAllChangesForAllPages()
{
$page1 = $this->createPageWithChanges(array('Title' => 'Page1'));
$page2 = $this->createPageWithChanges(array('Title' => 'Page2'));

View File

@ -33,11 +33,11 @@ class VersionFeedTest extends SapphireTest
{
$page = new Page(['Title' => 'My Title']);
$page->write();
$page->copyVersionToStage(Versioned::STAGE, Versioned::LIVE);
$page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
$page->Title = 'My Changed Title';
$page->write();
$page->copyVersionToStage(Versioned::STAGE, Versioned::LIVE);
$page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
$page->Title = 'My Unpublished Changed Title';
$page->write();