From 6ff319a0e1c4cfc15e24580bac07dfef38702942 Mon Sep 17 00:00:00 2001
From: Maxime Rainville <maxime@silverstripe.com>
Date: Wed, 27 Feb 2019 11:14:47 +1300
Subject: [PATCH] BUG Implement peer review feedback,

---
 .../MockDynamicAssignmentDBField.php              | 15 ++++++++-------
 .../MockDynamicAssignmentDataObject.php           |  4 ++--
 tests/php/ORM/ManyManyListTest.php                |  3 ++-
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDBField.php b/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDBField.php
index 5ab87481c..25828f9dd 100644
--- a/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDBField.php
+++ b/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDBField.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\ORM\Tests\DataObjectTest;
 
+use SilverStripe\Dev\TestOnly;
 use SilverStripe\ORM\FieldType\DBBoolean;
 use SilverStripe\ORM\FieldType\DBField;
 
@@ -13,16 +14,16 @@ use SilverStripe\ORM\FieldType\DBField;
  * If the field is set to false, it will try to do a plain assignment. This is so you can save the initial value no
  * matter what. If the field is set to true, it will try to do a dynamic assignment.
  */
-class MockDynamicAssignmentDBField extends DBBoolean
+class MockDynamicAssignmentDBField extends DBBoolean implements TestOnly
 {
 
     private $scalarOnly;
     private $dynamicAssignment;
 
     /**
-     * @param $name
-     * @param $scalarOnly Whether our fake field should be scalar only
-     * @param $dynamicAssigment Wheter our fake field will try to do a dynamic assignement
+     * @param string $name
+     * @param boolean $scalarOnly Whether our fake field should be scalar only.
+     * @param boolean $dynamicAssignment Whether our fake field will try to do a dynamic assignment.
      */
     public function __construct($name = '', $scalarOnly = false, $dynamicAssignment = false)
     {
@@ -32,15 +33,15 @@ class MockDynamicAssignmentDBField extends DBBoolean
     }
 
     /**
-     * If the field value and dynamicAssignment are true, we'll try to do a dynamic assignement
+     * If the field value and $dynamicAssignment are true, we'll try to do a dynamic assignment.
      * @param $value
-     * @return array|int|mixed
+     * @return array|int
      */
     public function prepValueForDB($value)
     {
         if ($value) {
             return $this->dynamicAssignment
-                ? ['GREATEST(?, ?)' => [0, 1]]
+                ? ['ABS(?)' => [1]]
                 : 1;
         }
 
diff --git a/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDataObject.php b/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDataObject.php
index 8baa26cb8..1617734cf 100644
--- a/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDataObject.php
+++ b/tests/php/ORM/DataObjectTest/MockDynamicAssignmentDataObject.php
@@ -30,11 +30,11 @@ class MockDynamicAssignmentDataObject extends DataObject implements TestOnly
     ];
 
     private static $many_many = [
-        "MockManyMany" => self::class,
+        'MockManyMany' => self::class,
     ];
 
     private static $belongs_many_many = [
-        "MockBelongsManyMany" => self::class,
+        'MockBelongsManyMany' => self::class,
     ];
 
     private static $many_many_extraFields = [
diff --git a/tests/php/ORM/ManyManyListTest.php b/tests/php/ORM/ManyManyListTest.php
index ab7d30859..008ec8189 100644
--- a/tests/php/ORM/ManyManyListTest.php
+++ b/tests/php/ORM/ManyManyListTest.php
@@ -10,6 +10,7 @@ use SilverStripe\ORM\Tests\DataObjectTest\Player;
 use SilverStripe\ORM\Tests\DataObjectTest\Team;
 use SilverStripe\ORM\Tests\ManyManyListTest\ExtraFieldsObject;
 use SilverStripe\ORM\Tests\ManyManyListTest\Product;
+use InvalidArgumentException;
 
 class ManyManyListTest extends SapphireTest
 {
@@ -402,7 +403,7 @@ class ManyManyListTest extends SapphireTest
 
     public function testWriteManipulationWithNonScalarValuesDisallowed()
     {
-        $this->expectException(\InvalidArgumentException::class);
+        $this->expectException(InvalidArgumentException::class);
 
         $left = DataObjectTest\MockDynamicAssignmentDataObject::create();
         $left->write();