From 91068c23b5cb448fe63ae9f40875a8f0818dbe1f Mon Sep 17 00:00:00 2001 From: Al Twohill Date: Thu, 5 Jul 2018 21:26:31 +1200 Subject: [PATCH 1/3] FIX Make column query not distinct When selecting a column, it doesn't make sense to have it distinct. As an alternative, the API could be changed to give the end user the option (eg `->column($field, $distinct = false)`) However if we did that, we would need to make sure we do something similar with `SilverStripe\ORM\UnsavedRelationList` to ensure consistency. --- src/ORM/DataQuery.php | 1 + tests/php/ORM/DataQueryTest.php | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/ORM/DataQuery.php b/src/ORM/DataQuery.php index e403f90b3..f953aca35 100644 --- a/src/ORM/DataQuery.php +++ b/src/ORM/DataQuery.php @@ -1147,6 +1147,7 @@ class DataQuery { $fieldExpression = $this->expressionForField($field); $query = $this->getFinalisedQuery(array($field)); + $query->setDistinct(false); $originalSelect = $query->getSelect(); $query->setSelect(array()); $query->selectField($fieldExpression, $field); diff --git a/tests/php/ORM/DataQueryTest.php b/tests/php/ORM/DataQueryTest.php index b9278a496..8f2b2a3fd 100644 --- a/tests/php/ORM/DataQueryTest.php +++ b/tests/php/ORM/DataQueryTest.php @@ -379,4 +379,22 @@ class DataQueryTest extends SapphireTest $this->assertEquals('Last', $second['Title']); $this->assertEmpty(array_shift($arrayResult)); } + + public function testColumnReturnsAllValues() + { + $first = new DataQueryTest\ObjectA(); + $first->Name = 'Bar'; + $first->write(); + + $second = new DataQueryTest\ObjectA(); + $second->Name = 'Foo'; + $second->write(); + + $third = new DataQueryTest\ObjectA(); + $third->Name = 'Bar'; + $third->write(); + + $result = DataQueryTest\ObjectA::get()->column('Name'); + $this->assertEquals(['Bar', 'Foo', 'Bar'], $result); + } } From 3292a8b773c5b29a69b72718f996a36f3daead1d Mon Sep 17 00:00:00 2001 From: Al Twohill Date: Thu, 12 Jul 2018 13:44:32 +1200 Subject: [PATCH 2/3] NEW Add `columnUnique` API SS_List classes. --- src/ORM/ArrayList.php | 11 ++++++++++ src/ORM/DataList.php | 13 +++++++++++- src/ORM/DataQuery.php | 1 - src/ORM/ListDecorator.php | 5 +++++ src/ORM/UnsavedRelationList.php | 12 +++++++++++ .../GridField/GridFieldSortableHeaderTest.php | 2 +- tests/php/ORM/DataQueryTest.php | 20 +++++++++++++++++++ 7 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/ORM/ArrayList.php b/src/ORM/ArrayList.php index a012d4009..952bbcc03 100644 --- a/src/ORM/ArrayList.php +++ b/src/ORM/ArrayList.php @@ -370,6 +370,17 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L return $result; } + /** + * Returns a unique array of a single field value for all the items in the list + * + * @param string $colName + * @return array + */ + public function columnUnique($colName = 'ID') + { + return array_unique($this->column($colName)); + } + /** * You can always sort a ArrayList * diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index f9d1e87cf..996f78edf 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -990,7 +990,18 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li */ public function column($colName = "ID") { - return $this->dataQuery->column($colName); + return $this->dataQuery->distinct(false)->column($colName); + } + + /** + * Returns a unque array of a single field value for all items in the list. + * + * @param string $colName + * @return array + */ + public function columnUnique($colName = "ID") + { + return $this->dataQuery->distinct(true)->column($colName); } // Member altering methods diff --git a/src/ORM/DataQuery.php b/src/ORM/DataQuery.php index f953aca35..e403f90b3 100644 --- a/src/ORM/DataQuery.php +++ b/src/ORM/DataQuery.php @@ -1147,7 +1147,6 @@ class DataQuery { $fieldExpression = $this->expressionForField($field); $query = $this->getFinalisedQuery(array($field)); - $query->setDistinct(false); $originalSelect = $query->getSelect(); $query->setSelect(array()); $query->selectField($fieldExpression, $field); diff --git a/src/ORM/ListDecorator.php b/src/ORM/ListDecorator.php index 8d1397fcd..b141d548e 100644 --- a/src/ORM/ListDecorator.php +++ b/src/ORM/ListDecorator.php @@ -142,6 +142,11 @@ abstract class ListDecorator extends ViewableData implements SS_List, Sortable, return $this->list->column($value); } + public function columnUnique($value = "ID") + { + return $this->list->columnUnique($value); + } + public function each($callback) { return $this->list->each($callback); diff --git a/src/ORM/UnsavedRelationList.php b/src/ORM/UnsavedRelationList.php index 2885c89fc..5b1479b40 100644 --- a/src/ORM/UnsavedRelationList.php +++ b/src/ORM/UnsavedRelationList.php @@ -275,6 +275,18 @@ class UnsavedRelationList extends ArrayList implements Relation return $list->column($colName); } + /** + * Returns a unique array of a single field value for all items in the list. + * + * @param string $colName + * @return array + */ + public function columnUnique($colName = "ID") + { + $list = new ArrayList($this->toArray()); + return $list->columnUnique($colName); + } + /** * Returns a copy of this list with the relationship linked to the given foreign ID. * @param int|array $id An ID or an array of IDs. diff --git a/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php b/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php index c39ee1788..937731607 100644 --- a/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php +++ b/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php @@ -68,7 +68,7 @@ class GridFieldSortableHeaderTest extends SapphireTest public function testGetManipulatedData() { - $list = new DataList(Team::class); + $list = Team::get()->filter([ 'ClassName' => Team::class ]); $config = new GridFieldConfig_RecordEditor(); $gridField = new GridField('testfield', 'testfield', $list, $config); diff --git a/tests/php/ORM/DataQueryTest.php b/tests/php/ORM/DataQueryTest.php index 8f2b2a3fd..ef3a72c96 100644 --- a/tests/php/ORM/DataQueryTest.php +++ b/tests/php/ORM/DataQueryTest.php @@ -397,4 +397,24 @@ class DataQueryTest extends SapphireTest $result = DataQueryTest\ObjectA::get()->column('Name'); $this->assertEquals(['Bar', 'Foo', 'Bar'], $result); } + + public function testColumnUniqueReturnsAllValues() + { + $first = new DataQueryTest\ObjectA(); + $first->Name = 'Bar'; + $first->write(); + + $second = new DataQueryTest\ObjectA(); + $second->Name = 'Foo'; + $second->write(); + + $third = new DataQueryTest\ObjectA(); + $third->Name = 'Bar'; + $third->write(); + + $result = DataQueryTest\ObjectA::get()->columnUnique('Name'); + $this->assertCount(2, $result); + $this->assertContains('Bar', $result); + $this->assertContains('Foo', $result); + } } From 89a2ce5d8f5f2a0e18f1ad124eca6548961fc56e Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Mon, 16 Jul 2018 11:43:27 +0100 Subject: [PATCH 3/3] DOCS Add changes to 4.3.0 changelog --- docs/en/04_Changelogs/4.3.0.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 docs/en/04_Changelogs/4.3.0.md diff --git a/docs/en/04_Changelogs/4.3.0.md b/docs/en/04_Changelogs/4.3.0.md new file mode 100644 index 000000000..ca03e829f --- /dev/null +++ b/docs/en/04_Changelogs/4.3.0.md @@ -0,0 +1,13 @@ +# 4.2.0 + +## Overview {#overview} + + - `DataList::column()` now returns all values and not just "distinct" values from a column as per the API docs + - `DataList`, `ArrayList` and `UnsavedRalationList` all have `columnUnique()` method for fetching distinct column values + +## Upgrading {#upgrading} + +### Fetching distinct column values on DataList + +Prior to this release `DataList` would erroneously return a distinct list of values from a column on an object. +If this behaviour is still required, please use `columnUnique()` instead.