From 573697d8c7d3eb09bb4fe39f7cde2fe5ef5efac0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julien=20Fastr=C3=A9?= <julien.fastre@champs-libres.coop>
Date: Fri, 18 Dec 2015 00:22:09 +0100
Subject: [PATCH] improve function isEmptyValue + tests

---
 CustomFields/CustomFieldChoice.php            | 18 ++---
 Tests/CustomFields/CustomFieldsChoiceTest.php | 65 +++++++++++++++++--
 2 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/CustomFields/CustomFieldChoice.php b/CustomFields/CustomFieldChoice.php
index bc8c89d78..782f5ab17 100644
--- a/CustomFields/CustomFieldChoice.php
+++ b/CustomFields/CustomFieldChoice.php
@@ -269,11 +269,6 @@ class CustomFieldChoice extends AbstractCustomField
             return true;
         }
         
-        // if only one choice...
-        if (is_string($value)) {
-            return empty($value);
-        }
-        
         // if multiple choice OR multiple/single choice with other
         if (is_array($value))
         {
@@ -282,17 +277,14 @@ class CustomFieldChoice extends AbstractCustomField
                 if ($value['_choices'] === NULL) {
                     return true;
                 }
-                if (is_string($value['_choices'])) {
-                    return empty($value);
-                }
-                if (is_array($value['_choices'])){
-                    return count($value['_choices']) > 0;
-                }
+                return empty($value['_choices']);
             } else { // we do not have 'allow other'
-                return count($value) > .0;
+                return empty($value);
             }
+        } else {
+            return empty($value);
         }
-        
+
         throw \LogicException("This case is not expected.");
     }
 
diff --git a/Tests/CustomFields/CustomFieldsChoiceTest.php b/Tests/CustomFields/CustomFieldsChoiceTest.php
index f01a283ee..6fbc0b67e 100644
--- a/Tests/CustomFields/CustomFieldsChoiceTest.php
+++ b/Tests/CustomFields/CustomFieldsChoiceTest.php
@@ -344,22 +344,79 @@ class CustomFieldsChoiceTest extends KernelTestCase
     ////////////////////////////////////////
     
     /**
+     * test the not empty with the not-empty data provider
      * 
      * @param mixed $data
      * @dataProvider serializedRepresentationDataProvider
      */
     public function testIsEmptyValueNotEmpty($data)
     {
-        $this->markTestSkipped("We have to improve the isEmptyFunction");
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => false,
+            CustomFieldChoice::MULTIPLE => true
+        ));
+
+        $isEmpty = $this->cfChoice->isEmptyValue($data, $customField);
+        
+        $this->assertFalse($isEmpty);
+    }
+    
+    /**
+     * 
+     * @dataProvider emptyDataProvider
+     * @param mixed $data
+     */
+    public function testIsEmptyValueEmpty($data)
+    {
         $customField = $this->generateCustomField(array(
             CustomFieldChoice::ALLOW_OTHER => false,
             CustomFieldChoice::MULTIPLE => true
         ));
         
-        $deserialized = $this->cfChoice->deserialize($data, $customField);
-        $isEmpty = $this->cfChoice->isEmptyValue($deserialized, $customField);
+        $isEmpty = $this->cfChoice->isEmptyValue($data, $customField);
         
-        $this->assertFalse($isEmpty);
+        $this->assertTrue($isEmpty);
+    }
+    
+    /**
+     * provide empty data in different possible reprsentation
+     * 
+     * @return array
+     */
+    public function emptyDataProvider()
+    {
+        return array(
+            // 0
+            array(
+                // signle
+                ''
+            ),
+            // 1
+            array(
+                // single
+                null
+            ),
+            // 2
+            array(
+                // signle with allow other
+                array('_other' => 'something', '_choices' => '')
+            ),
+            // 3
+            array(
+                // multiple
+                array()
+            ),
+            // 4
+            array(
+                // multiple with allow other
+                array('_other' => 'something', '_choices' => array())
+            ),
+            // 5
+             array(
+                // multiple with allow other
+                array('_other' => '', '_choices' => array())
+            ),
+        );
     }
     
 }