Merge pull request #29 from codete/additional-fields-v2

Add (better) option to specify additional fields through annotation

Author: malarzm

Annotations/Display.php

@@ -6,6 +6,8 @@
  * @Annotation
  * 
  * @author Maciej Malarz <malarzm@gmail.com>
+ *
+ * @deprecated use Field instead
  */
 class Display extends \Doctrine\Common\Annotations\Annotation
 {

Annotations/Embed.php

@@ -5,7 +5,7 @@
 /**
  * @Annotation
  */
-class Embed extends Display
+class Embed extends Field
 {
     public $view = 'default';
 

Annotations/Field.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Codete\FormGeneratorBundle\Annotations;
+
+/**
+ * @Annotation
+ *
+ * @author Maciej Malarz <malarzm@gmail.com>
+ */
+class Field extends Display
+{
+
+}

FormConfigurationFactory.php

@@ -3,6 +3,7 @@
 namespace Codete\FormGeneratorBundle;
 
 use Codete\FormGeneratorBundle\Annotations\Display;
+use Codete\FormGeneratorBundle\Annotations\Field;
 use Codete\FormGeneratorBundle\Annotations\Form;
 use Codete\FormGeneratorBundle\Form\Type\EmbedType;
 use Doctrine\Common\Annotations\AnnotationReader;
@@ -80,31 +81,60 @@ private function getFieldsConfiguration($model, $fields = [])
             $properties = $ro->getProperties();
         } else {
             foreach (array_keys($fields) as $field) {
+                // setting the configuration to null guarantees the order of elements in there
+                // to match the order specified in $fields
+                $configuration[$field] = null;
+                if (! $ro->hasProperty($field)) {
+                    continue; // most prob a class-level field, order will be maintained due to trick above
+                }
                 $properties[] = $ro->getProperty($field);
             }
         }
+        $fieldConfigurations = [];
+        // first are coming properties
         foreach ($properties as $property) {
-            $propertyIsListed = array_key_exists($property->getName(), $fields);
+            $propertyName = $property->getName();
+            $propertyIsListed = array_key_exists($propertyName, $fields);
             if (!empty($fields) && !$propertyIsListed) {
-                continue;
+                continue; // list of fields was specified and current one is not there
             }
             $fieldConfiguration = $this->annotationReader->getPropertyAnnotation($property, Display::class);
             if ($fieldConfiguration === null && !$propertyIsListed) {
                 continue;
             }
-            $configuration[$property->getName()] = (array)$fieldConfiguration;
-            if (isset($fields[$property->getName()])) {
-                $configuration[$property->getName()] = array_replace_recursive($configuration[$property->getName()], $fields[$property->getName()]);
+            $fieldConfigurations[$propertyName] = $fieldConfiguration;
+        }
+        // later are coming class-level fields. We need to iterate through all annotations as there's no method
+        // to get *all* occurrences of chosen annotation.
+        foreach ($this->annotationReader->getClassAnnotations($ro) as $annotation) {
+            if (! $annotation instanceof Display) {
+                continue;
+            }
+            $propertyName = $annotation->value;
+            if (!empty($fields) && !array_key_exists($propertyName, $fields)) {
+                continue; // list of fields was specified and current one is not there
+            }
+            // @todo this was a mistake originally, need to drop default required at all in 2.0
+            unset($annotation->required);
+            $fieldConfigurations[$propertyName] = $annotation;
+        }
+        foreach ($fieldConfigurations as $propertyName => $fieldConfiguration) {
+            if ($fieldConfiguration instanceof Display && ! $fieldConfiguration instanceof Field) {
+                @trigger_error("Display annotation has been deprecated in 1.3 and will be removed in 2.0 - please use Field instead.", E_USER_DEPRECATED);
+            }
+            $configuration[$propertyName] = (array)$fieldConfiguration;
+            if (isset($fields[$propertyName])) {
+                $configuration[$propertyName] = array_replace_recursive($configuration[$propertyName], $fields[$propertyName]);
             }
-            if ($configuration[$property->getName()]['type'] === EmbedType::TYPE) {
-                if (($value = $property->getValue($model)) === null) {
-                    $value = $this->instantiator->instantiate($configuration[$property->getName()]['class']);
+            if ($configuration[$propertyName]['type'] === EmbedType::TYPE) {
+                if (! $ro->hasProperty($propertyName) || ($value = $ro->getProperty($propertyName)->getValue($model)) === null) {
+                    $value = $this->instantiator->instantiate($configuration[$propertyName]['class']);
                 }
-                $configuration[$property->getName()]['data_class'] = $configuration[$property->getName()]['class'];
-                $configuration[$property->getName()]['model'] = $value;
+                $configuration[$propertyName]['data_class'] = $configuration[$propertyName]['class'];
+                $configuration[$propertyName]['model'] = $value;
             }
             // this variable comes from Doctrine\Common\Annotations\Annotation
-            unset($configuration[$property->getName()]['value']);
+            unset($configuration[$propertyName]['value']);
         }
         return $configuration;
     }

README.md

@@ -15,6 +15,9 @@ forms' configuration although YAML/XML support is planned.
 Basic Usages
 ------------
 
+**`@Form\Display` annotation was deprecated in 1.3 and will be removed
+in 2.0. Please use `@Form\Field` instead.**
+
 Consider a class
 
 ``` php
@@ -30,32 +33,32 @@ class Person
     public $id;
 
     /**
-     * @Form\Display(type="choice", choices = { "mr" = "Mr.", "ms" = "Ms." })
+     * @Form\Field(type="choice", choices = { "mr" = "Mr.", "ms" = "Ms." })
      */
     public $title;
 
     /**
-     * @Form\Display(type="text")
+     * @Form\Field(type="text")
      */
     public $name;
 
     /**
-     * @Form\Display(type="text")
+     * @Form\Field(type="text")
      */
     public $surname;
 
     /**
-     * @Form\Display(type="file")
+     * @Form\Field(type="file")
      */
     public $photo;
 
     /**
-     * @Form\Display(type="checkbox")
+     * @Form\Field(type="checkbox")
      */
     public $active;
 
     /**
-     * @Form\Display(type="money")
+     * @Form\Field(type="money")
      */
     public $salary;
 }
@@ -87,12 +90,12 @@ namespace when specifying type**
 Specifying Field Options
 ------------------------
 
-By default everything you specify in `@Form\Display` (except for `type`) annotation
+By default everything you specify in `@Form\Field` (except for `type`) annotation
 will be passed as an option to generated form type. To illustrate:
 
 ```php
 /**
- * @Form\Display(type="choice", choices = { "mr" = "Mr.", "ms" = "Ms." }, "attr" = { "class" = "foo" })
+ * @Form\Field(type="choice", choices = { "mr" = "Mr.", "ms" = "Ms." }, "attr" = { "class" = "foo" })
  */
 public $title;
 ```
@@ -118,7 +121,7 @@ all Symfony fields' options into an `options` property:
 
 ```php
 /**
- * @Form\Display(
+ * @Form\Field(
  *   type="choice",
  *   options={ "choices" = { "mr" = "Mr.", "ms" = "Ms." }, "attr" = { "class" = "foo" } }
  * )
@@ -131,6 +134,28 @@ them as that field's options to the `FormBuilder`. Effectively this allows you t
 separate field's options from options for your configuration modifiers which can be
 a gain on its own.
 
+Adding fields not mapped to a property
+--------------------------------------
+
+**This feature was added in 1.3.0.**
+
+Sometimes you may need to add a field that will not be mapped to a property. An example
+of such use case is adding buttons to the form:
+
+```php
+/**
+ * The first value in Field annotation specifies field's name.
+ *
+ * @Form\Field("reset", type="reset")
+ * @Form\Field("submit", type="submit", "label"="Save")
+ */
+class Person
+```
+
+All fields added on the class level come last in the generated form, unless a form view 
+(described below) specifies otherwise. Contrary to other class-level settings, `@Field`s
+will not be inherited by child classes.
+
 Form Views
 ----------
 

Tests/FormGeneratorTest.php

@@ -39,6 +39,7 @@ public function testDefaultForm($model, $expectedFields, $additionalCheck = null
     public function provideDefaultForm()
     {
         return [
+            [new Model\DeprecatedDisplay(), ['title']],
             [new Model\Simple(), ['title']],
             [new Model\SimpleNotOverridingDefaultView(), ['author', 'title']],
             [new Model\SimpleOverridingDefaultView(), ['title', 'author'], function($phpunit, $form) {
@@ -56,7 +57,8 @@ public function provideDefaultForm()
                 $phpunit->assertEquals('foo', $options['attr']['class']);
                 $optionsIgnoreInlinedFields = $form->get('optionsIgnoreInlinedFields')->getConfig()->getOptions();
                 $phpunit->assertEquals('foo', $optionsIgnoreInlinedFields['attr']['class']);
-            }]
+            }],
+            [new Model\ClassLevelFields(), ['title', 'reset', 'submit']],
         ];
     }
 
@@ -195,7 +197,7 @@ public function testFormAnnotationViewIsInherited()
         $this->checkForm(new Model\Director(), ['title', 'name', 'surname', 'photo', 'active'], null, 'personal');
     }
 
-    public function testFormAnnotationViewCanBeOverriden()
+    public function testFormAnnotationViewCanBeOverridden()
     {
         $this->checkForm(new Model\Director(), ['salary', 'department'], null, 'work');
     }
@@ -204,6 +206,14 @@ public function testAllParentsAreCheckedForDefaultFormView()
     {
         $this->checkForm(new Model\InheritanceTest(), ['title', 'author']);
     }
+
+    public function testFormViewCanAffectClassLevelFields()
+    {
+        $this->checkForm(new Model\ClassLevelFields(), ['submit', 'title'], function($phpunit, $form) {
+            $normal = $form->get('submit')->getConfig()->getOptions();
+            $phpunit->assertEquals('Click me', $normal['label']);
+        }, 'tweaked');
+    }
 
     protected function checkForm($model, $expectedFields, callable $additionalCheck = null, $form = 'default', $context = [])
     {

Tests/Model/ClassLevelFields.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace Codete\FormGeneratorBundle\Tests\Model;
+
+use Codete\FormGeneratorBundle\Annotations as Form;
+
+/**
+ * @Form\Form(
+ *   tweaked = { "submit" = { "label" = "Click me" }, "title" }
+ * )
+ * @Form\Field("reset", type="reset")
+ * @Form\Field("submit", type="submit")
+ */
+class ClassLevelFields
+{
+    /**
+     * @Form\Field(type="text")
+     */
+    public $title;
+}

Tests/Model/DeprecatedDisplay.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace Codete\FormGeneratorBundle\Tests\Model;
+
+use Codete\FormGeneratorBundle\Annotations as Form;
+
+class DeprecatedDisplay
+{
+    public $id;
+
+    /**
+     * @Form\Display(type="text")
+     */
+    public $title;
+}

Tests/Model/Director.php

@@ -12,7 +12,7 @@
 class Director extends Person
 {
     /**
-     * @Form\Display(type="text")
+     * @Form\Field(type="text")
      */
     public $department;
 }

Tests/Model/DisplayOptions.php

@@ -7,17 +7,17 @@
 class DisplayOptions
 {
     /**
-     * @Form\Display(type="text", attr = { "class" = "foo" })
+     * @Form\Field(type="text", attr = { "class" = "foo" })
      */
     public $normal;
 
     /**
-     * @Form\Display(type="text", options = { "attr" = { "class" = "foo" } })
+     * @Form\Field(type="text", options = { "attr" = { "class" = "foo" } })
      */
     public $options;
 
     /**
-     * @Form\Display(type="text", options = { "attr" = { "class" = "foo" } }, attr = { "class" = "bar" })
+     * @Form\Field(type="text", options = { "attr" = { "class" = "foo" } }, attr = { "class" = "bar" })
      */
     public $optionsIgnoreInlinedFields;
 }