tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

BUG Test case for versioned now correctly checks IDs returned from Versioned::get_including_deleted

Browse Source 
BUG Issue with deleted records not being queried properly.
API DataQuery::expressionForField no longer requires a second parameter. Rather the query object is inferred from the DataQuery itself. This should improve consistency of use of this function.
This commit is contained in:
Damian Mooyman 2012-12-17 17:15:26 +13:00
parent 75b0c3ec8f
commit 6aa16e1f01
3 changed files with 50 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
model/DataQuery.php
View File

@ -647,12 +647,12 @@ class DataQuery {
* @param string $field
*/
public function subtract(DataQuery $subtractQuery, $field='ID') {
$subSelect= $subtractQuery->getFinalisedQuery();
$fieldExpression = $this->expressionForField($field, $subSelect);
$fieldExpression = $subtractQuery->expressionForField($field);
$subSelect = $subtractQuery->getFinalisedQuery();
$subSelect->setSelect(array());
$subSelect->selectField($fieldExpression, $field);
$subSelect->setOrderBy(null);
$this->where($this->expressionForField($field, $this).' NOT IN ('.$subSelect->sql().')');
$this->where($this->expressionForField($field).' NOT IN ('.$subSelect->sql().')');
return $this;
}
@ -679,9 +679,9 @@ class DataQuery {
* @param String $field See {@link expressionForField()}.
*/
public function column($field = 'ID') {
$fieldExpression = $this->expressionForField($field);
$query = $this->getFinalisedQuery(array($field));
$originalSelect = $query->getSelect();
$fieldExpression = $this->expressionForField($field, $query);
$query->setSelect(array());
$query->selectField($fieldExpression, $field);
$this->ensureSelectContainsOrderbyColumns($query, $originalSelect);
@ -692,17 +692,21 @@ class DataQuery {
/**
* @param String $field Select statement identifier, either the unquoted column name,
* the full composite SQL statement, or the alias set through {@link SQLQuery->selectField()}.
* @param SQLQuery $query
* @return String
* @return String The expression used to query this field via this DataQuery
*/
protected function expressionForField($field, $query) {
// Special case for ID
if($field == 'ID') {
protected function expressionForField($field) {
// Prepare query object for selecting this field
$query = $this->getFinalisedQuery(array($field));
// Allow query to define the expression for this field
$expression = $query->expressionForField($field);
if(!empty($expression)) return $expression;
// Special case for ID, if not provided
if($field === 'ID') {
$baseClass = ClassInfo::baseDataClass($this->dataClass);
return "\"$baseClass\".\"ID\"";
} else {
return $query->expressionForField($field);
}
}

16
model/Versioned.php
View File

@ -232,8 +232,19 @@ class Versioned extends DataExtension {
foreach(self::$db_for_versions_table as $name => $type) {
$query->selectField(sprintf('"%s_versions"."%s"', $baseTable, $name), $name);
}
// Alias the record ID as the row ID
$query->selectField(sprintf('"%s_versions"."%s"', $baseTable, 'RecordID'), "ID");
$query->addOrderBy(sprintf('"%s_versions"."%s"', $baseTable, 'Version'));
// Ensure that any sort order referring to this ID is correctly aliased
$orders = $query->getOrderBy();
foreach($orders as $order => $dir) {
if($order === "\"$baseTable\".\"ID\"") {
unset($orders[$order]);
$orders["\"{$baseTable}_versions\".\"RecordID\""] = $dir;
}
}
$query->setOrderBy($orders);
// latest_version has one more step
// Return latest version instances, regardless of whether they are on a particular stage
@ -250,6 +261,9 @@ class Versioned extends DataExtension {
) AS \"{$alias}_versions_latest\"
WHERE \"{$alias}_versions_latest\".\"RecordID\" = \"{$alias}_versions\".\"RecordID\"
)");
} else {
// If all versions are requested, ensure that records are sorted by this field
$query->addOrderBy(sprintf('"%s_versions"."%s"', $baseTable, 'Version'));
}
break;
default:

16
tests/model/VersionedTest.php
View File

@ -74,8 +74,15 @@ class VersionedTest extends SapphireTest {
* Test Versioned::get_including_deleted()
*/
public function testGetIncludingDeleted() {
// Delete a page
$this->objFromFixture('VersionedTest_DataObject', 'page3')->delete();
// Get all ids of pages
$allPageIDs = DataObject::get('VersionedTest_DataObject', "\"ParentID\" = 0", "\"VersionedTest_DataObject\".\"ID\" ASC")->column('ID');
// Modify a page, ensuring that the Version ID and Record ID will differ,
// and then subsequently delete it
$targetPage = $this->objFromFixture('VersionedTest_DataObject', 'page3');
$targetPage->Content = 'To be deleted';
$targetPage->write();
$targetPage->delete();
// Get all items, ignoring deleted
$remainingPages = DataObject::get("VersionedTest_DataObject", "\"ParentID\" = 0",
@ -90,12 +97,17 @@ class VersionedTest extends SapphireTest {
// Check that page 3 is still there
$this->assertEquals(array("Page 1", "Page 2", "Page 3"), $allPages->column('Title'));
// Check that the returned pages have the correct IDs
$this->assertEquals($allPageIDs, $allPages->column('ID'));
// Check that this still works if we switch to reading the other stage
Versioned::reading_stage("Live");
$allPages = Versioned::get_including_deleted("VersionedTest_DataObject", "\"ParentID\" = 0",
"\"VersionedTest_DataObject\".\"ID\" ASC");
$this->assertEquals(array("Page 1", "Page 2", "Page 3"), $allPages->column('Title'));
// Check that the returned pages still have the correct IDs
$this->assertEquals($allPageIDs, $allPages->column('ID'));
}
public function testVersionedFieldsAdded() {