fix bug in changing custom field option

bug description :

> if you change the options of a custom field choice (i.e. from a
> multiple to a single, removing or adding allow_other), the data
> representation change and do not match with the expected
> representation of the form.

This commit fix this bug by switching the data representation to the
current options.

CustomFields/CustomFieldChoice.php

@@ -167,8 +167,96 @@ class CustomFieldChoice extends AbstractCustomField
 
     public function deserialize($serialized, CustomField $customField)
     {
+        // we always have to adapt to what the current data should be
+        $options = $customField->getOptions();
+        
+        if ($options[self::MULTIPLE]) {
+            return $this->deserializeToMultiple($serialized, $options[self::ALLOW_OTHER]);
+        } else {
+            return $this->deserializeToUnique($serialized, $options[self::ALLOW_OTHER]);
+        }
         return $serialized;
     }
+    
+    private function deserializeToUnique($serialized, $allowOther)
+    {
+        $value = $this->guessValue($serialized);
+        
+        // set in a single value. We must have a single string
+        $fixedValue = is_array($value) ? 
+                // check if the array has an element, if not replace by empty string
+                count($value) > 0 ? end($value) : '' 
+                : 
+                $value;
+        
+        if ($allowOther) {
+            return $this->deserializeWithAllowOther($serialized, $fixedValue);
+        } else {
+            return $fixedValue;
+        }
+    }
+    
+    /**
+     * deserialized the data from the database to a multiple 
+     * field
+     * 
+     * @param mixed $serialized
+     * @param boolean $allowOther
+     */
+    private function deserializeToMultiple($serialized, $allowOther)
+    {
+        $value = $this->guessValue($serialized);
+        
+        // set in an array : we want a multiple
+        $fixedValue = is_array($value) ? $value : array($value);
+        
+        if ($allowOther) {
+            return $this->deserializeWithAllowOther($serialized, $fixedValue);
+        } else {
+            return $fixedValue;
+        }
+    }
+    
+    private function deserializeWithAllowOther($serialized, $value)
+    {
+        $existingOther = isset($serialized['_other']) ? $serialized['_other'] : '';
+        
+        return array(
+            '_other' => $existingOther,
+            '_choices' => $value
+        );
+    }
+    
+    /**
+     * Guess the value from the representation of it.
+     * 
+     * If the value had an 'allow_other' = true option, the returned value
+     * **is not** the content of the _other field, but the `_other` string.
+     * 
+     * @param array|string $value
+     * @return mixed
+     * @throws \LogicException if the case is not covered by this
+     */
+    private function guessValue($value)
+    {
+        if ($value === NULL) {
+            return NULL;
+        }
+        
+        if (!is_array($value)) {
+            return $value;
+        } else {
+            // we have a field with "allow other"
+            if (isset($value['_choices'])) {
+                return $value['_choices'];
+            } else {
+                // we have a field with "multiple"
+                return $value;
+            }
+        }
+        
+        throw \LogicException("This case is not expected.");
+    }
 
     public function getName()
     {

Tests/CustomFields/CustomFieldsChoiceTest.php

@@ -0,0 +1,365 @@
+<?php
+
+/*
+ * Copyright (C) 2015 Julien Fastré <julien.fastre@champs-libres.coop>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace Chill\CustomFieldsBundle\Tests;
+
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Chill\CustomFieldsBundle\Entity\CustomField;
+use Chill\CustomFieldsBundle\CustomFields\CustomFieldChoice;
+
+/**
+ * This class cover the test of CustomFieldChoice.
+ * 
+ * Function currently covered: 
+ * 
+ * - deserialize
+ *
+ * @author Julien Fastré <julien.fastre@champs-libres.coop>
+ */
+class CustomFieldsChoiceTest extends KernelTestCase
+{
+    
+    /**
+     *
+     * @var \Chill\CustomFieldsBundle\Service\CustomFieldProvider
+     */
+    private $cfProvider;
+    
+    /**
+     *
+     * @var \Chill\CustomFieldsBundle\CustomFields\CustomFieldChoice
+     */
+    private $cfChoice;
+    
+    public function setUp()
+    {
+        static::bootKernel();
+        
+        $this->cfProvider = static::$kernel->getContainer()
+                ->get('chill.custom_field.provider');
+        $this->cfChoice = $this->cfProvider->getCustomFieldByType('choice');
+    }
+    
+    public function tearDown()
+    {
+        parent::tearDown();
+    }
+    
+    /**
+     * 
+     * @param array $options
+     * @return CustomField
+     */
+    private function generateCustomField($options)
+    {
+        return (new CustomField())
+            ->setActive(true)
+            ->setSlug('slug')
+            ->setOptions($options)
+            ->setType('choice')
+                ;
+    }
+    
+    /////////////////////////////////////////
+    //
+    // test function deserialize 
+    //
+    ////////////////////////////////////////
+    
+    /**
+     * Test if the representation of the data is deserialized to a single text.
+     * 
+     * If the value is in _other, the _other value should not be returned.
+     * 
+     * @param type $data
+     * @dataProvider serializedRepresentationDataProvider
+     */
+    public function testDeserializeSingleChoiceWithoutOther($data)
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => false,
+            CustomFieldChoice::MULTIPLE => false
+        ));
+        
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame('my-value', $deserialized);
+    }
+    
+    
+    public function testDeserializeSingleChoiceWithoutOtherDataIsNull()
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => false,
+            CustomFieldChoice::MULTIPLE => false
+        ));
+        
+        $deserialized = $this->cfChoice->deserialize(null, $customField);
+        
+        $this->assertSame(null, $deserialized);
+        
+        $deserialized = $this->cfChoice->deserialize('', $customField);
+        
+        $this->assertSame('', $deserialized);
+    }
+    
+    /**
+     * Test if the representation of the data is deserialized to a single text
+     * with an "allow_other" field.
+     * 
+     * If the value is in _other, the _other value should be in the _other field.
+     * 
+     * @param type $data
+     * @dataProvider serializedRepresentationDataProvider
+     */
+    public function testDeserializeSingleChoiceWithOther($data)
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => true,
+            CustomFieldChoice::MULTIPLE => false
+        ));
+        
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => '', '_choices' => 'my-value'), $deserialized);
+    }
+    
+    /**
+     * Other cases : 
+     * 
+     *  - Test if the selected value is '_other
+     * - Test with null data
+     * 
+     * @param type $data
+     */
+    public function testDeserializeSingleChoiceWithOtherOtherCases()
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => true,
+            CustomFieldChoice::MULTIPLE => false
+        ));
+        
+        // from a single to a single
+        $data = array('_other' => 'something', '_choices' => '_other');
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => '_other'), $deserialized);
+        
+        
+        // from a multiple to a single
+        $data = array('_other' => 'something', '_choices' => array('some', '_other'));
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => '_other'), $deserialized);
+        
+        //test with null data
+        //from a single to a single :
+        $data = array('_other' => 'something', '_choices' => null);
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => null), $deserialized);
+        
+        $data = array('_other' => 'something', '_choices' => '');
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => ''), $deserialized);
+        
+        // from a multiple to a signle
+        $data = array('_other' => 'something', '_choices' => array());
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => ''), $deserialized);
+        
+        $data = array('_other' => 'something', '_choices' => array(''));
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => ''), $deserialized);
+        
+    }
+    
+    /**
+     * Test if the representation of the data is deserialized to an array text
+     * with an "allow_other" field.
+     * 
+     * This test does not covers the case when the selected value is `_other`
+     * 
+     * @param type $data
+     * @dataProvider serializedRepresentationDataProvider
+     */
+    public function testDeserializeMultipleChoiceWithOther($data)
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => true,
+            CustomFieldChoice::MULTIPLE => true
+        ));
+        
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => '', '_choices' => array('my-value')), 
+                $deserialized);
+    }
+    
+    /**
+     * Test if the representation of the data is deserialized to an array text
+     * with an "allow_other" field.
+     * 
+     * This test covers :
+     * - the case when the selected value is `_other`
+     * - result is null
+     * 
+     * @param type $data
+     */
+    public function testDeserializeMultipleChoiceWithOtherOtherCases()
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => true,
+            CustomFieldChoice::MULTIPLE => true
+        ));
+        
+        // selected value is _other
+        // from single to multiple
+        $data = array('_other' => 'something', '_choices' => '_other');
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => array('_other')), 
+                $deserialized);
+        
+        // from multiple to multiple
+        $data = array('_other' => 'something', '_choices' => array('_other', 'something'));
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => 'something', '_choices' => array('_other', 'something')), 
+                $deserialized);
+        
+        // test with null value
+        // from single to multiple
+        $data = array('_other' => '', '_choices' => '');
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => '', '_choices' => array('')), 
+                $deserialized);
+        
+        // from multiple to multiple
+        $data = array('_other' => '', '_choices' => array());
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('_other' => '', '_choices' => array()), 
+                $deserialized);
+    }
+    
+    /**
+     * Test if the representation of the data is deserialized to an array text
+     * **without** an "allow_other" field.
+     * 
+     * 
+     * @param type $data
+     * @dataProvider serializedRepresentationDataProvider
+     */
+    public function testDeserializeMultipleChoiceWithoutOther($data)
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => false,
+            CustomFieldChoice::MULTIPLE => true
+        ));
+        
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('my-value'), $deserialized);
+    }
+    
+    /**
+     * Test if the representation of the data is deserialized to an array text
+     * **without** an "allow_other" field.
+     * 
+     * Covered cases :
+     * - NULL values
+     * 
+     * 
+     * @param type $data
+     */
+    public function testDeserializeMultipleChoiceWithoutOtherOtherCases()
+    {
+        $customField = $this->generateCustomField(array(
+            CustomFieldChoice::ALLOW_OTHER => false,
+            CustomFieldChoice::MULTIPLE => true
+        ));
+        
+        // from single to multiple
+        $data = 'my-value';
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('my-value'), $deserialized);
+        
+        // from multiple to multiple
+        $data = array('my-value');
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        
+        $this->assertSame(array('my-value'), $deserialized);
+    }
+    
+    public function serializedRepresentationDataProvider()
+    {
+        return array(
+            array(
+                // multiple => false, allow_other => false
+                'my-value' 
+            ),
+            array(
+                // multiple => true, allow_ther => false
+                array('my-value')
+            ),
+            array(
+                // multiple => false, allow_other => true, current value not in other
+                array('_other' => '', '_choices' => 'my-value')
+            ),
+            array(
+                // multiple => true, allow_other => true, current value not in other
+                array('_other' => '', '_choices'=> array('my-value'))
+            ),
+        );
+    }
+    
+    
+    
+    /////////////////////////////////////////
+    //
+    // test function isEmptyValue 
+    //
+    ////////////////////////////////////////
+    
+    /**
+     * 
+     * @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
+        ));
+        
+        $deserialized = $this->cfChoice->deserialize($data, $customField);
+        $isEmpty = $this->cfChoice->isEmptyValue($deserialized, $customField);
+        
+        $this->assertFalse($isEmpty);
+    }
+    
+}