Merge facff3c into 2017028

APY · May 16, 2017 · 05f0794 · 05f0794
2 parents 2017028 + facff3c
commit 05f0794
Show file tree

Hide file tree

Showing 55 changed files with 5,284 additions and 281 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -19,6 +19,13 @@ before_install:
 install:
   - travis_retry composer self-update
   - travis_retry composer update ${COMPOSER_FLAGS} --no-interaction
+  - curl -s http://getcomposer.org/installer | php
+  - php composer.phar install --dev --no-interaction
 
 script:
   - vendor/bin/phpunit
+  - mkdir -p build/logs
+  - php vendor/bin/phpunit -c phpunit.xml.dist
+
+after_success:
+  - travis_retry php vendor/bin/coveralls
diff --git a/Grid/Action/MassAction.php b/Grid/Action/MassAction.php
@@ -14,11 +14,22 @@
 
 class MassAction implements MassActionInterface
 {
+    /** @var string */
     protected $title;
+
+    /** @var string|null */
     protected $callback;
+
+    /** @var bool */
     protected $confirm;
+
+    /** @var string */
     protected $confirmMessage;
+
+    /** @var array  */
     protected $parameters = [];
+
+    /** @var string|null  */
     protected $role;
 
     /**
@@ -40,10 +51,11 @@ public function __construct($title, $callback = null, $confirm = false, $paramet
         $this->role = $role;
     }
 
+    // @todo: has this setter sense? we passed the title from constructor
     /**
      * Set action title.
      *
-     * @param $title
+     * @param string $title
      *
      * @return self
      */
@@ -67,7 +79,7 @@ public function getTitle()
     /**
      * Set action callback.
      *
-     * @param  $callback
+     * @param string $callback
      *
      * @return self
      */
@@ -88,10 +100,12 @@ public function getCallback()
         return $this->callback;
     }
 
+    // @todo: we should change this to something like "enableConfirm" as "false" is the default value and has pretty much
+    // nosense to use setConfirm with false parameter.
     /**
      * Set action confirm.
      *
-     * @param  $confirm
+     * @param bool $confirm
      *
      * @return self
      */
@@ -102,6 +116,7 @@ public function setConfirm($confirm)
         return $this;
     }
 
+    // @todo: could we change this to neddConfirm?
     /**
      * Get action confirm.
      *
@@ -163,7 +178,7 @@ public function getParameters()
     /**
      * set role.
      *
-     * @param mixed $role
+     * @param string $role
      *
      * @return self
      */
@@ -177,7 +192,7 @@ public function setRole($role)
     /**
      * Get role.
      *
-     * @return mixed
+     * @return string
      */
     public function getRole()
     {

diff --git a/Grid/Action/RowAction.php b/Grid/Action/RowAction.php
@@ -12,19 +12,44 @@
 
 namespace APY\DataGridBundle\Grid\Action;
 
+use APY\DataGridBundle\Grid\Row;
+
 class RowAction implements RowActionInterface
 {
+    /** @var string */
     protected $title;
+
+    /** @var string */
     protected $route;
+
+    /** @var bool */
     protected $confirm;
+
+    /** @var string */
     protected $confirmMessage;
+
+    /** @var string */
     protected $target;
+
+    /** @var string */
     protected $column = '__actions';
+
+    /** @var array */
     protected $routeParameters = [];
+
+    /** @var array */
     protected $routeParametersMapping = [];
+
+    /** @var array */
     protected $attributes = [];
+
+    /** @var string|null */
     protected $role;
+
+    /** @var array */
     protected $callbacks = [];
+
+    /** @var bool */
     protected $enabled = true;
 
     /**
@@ -50,6 +75,7 @@ public function __construct($title, $route, $confirm = false, $target = '_self',
         $this->role = $role;
     }
 
+    // @todo: has this setter real sense? we passed this value from constructor
     /**
      * Set action title.
      *
@@ -65,15 +91,14 @@ public function setTitle($title)
     }
 
     /**
-     * get action title.
-     *
-     * @return string
+     * {@inheritdoc}
      */
     public function getTitle()
     {
         return $this->title;
     }
 
+    // @todo: has this setter real sense? we passed this value from constructor
     /**
      * Set action route.
      *
@@ -89,19 +114,19 @@ public function setRoute($route)
     }
 
     /**
-     * get action route.
-     *
-     * @return string
+     * {@inheritdoc}
      */
     public function getRoute()
     {
         return $this->route;
     }
 
+    // @todo: we should change this to something like "enableConfirm" as "false" is the default value and has pretty much
+    // nosense to use setConfirm with false parameter.
     /**
      * Set action confirm.
      *
-     * @param  $confirm
+     * @param bool $confirm
      *
      * @return self
      */
@@ -113,9 +138,7 @@ public function setConfirm($confirm)
     }
 
     /**
-     * get action confirm.
-     *
-     * @return bool
+     * {@inheritdoc}
      */
     public function getConfirm()
     {
@@ -137,9 +160,7 @@ public function setConfirmMessage($confirmMessage)
     }
 
     /**
-     * get action confirmMessage.
-     *
-     * @return string
+     * {@inheritdoc}
      */
     public function getConfirmMessage()
     {
@@ -161,9 +182,7 @@ public function setTarget($target)
     }
 
     /**
-     * get action target.
-     *
-     * @return string
+     * {@inheritdoc}
      */
     public function getTarget()
     {
@@ -185,9 +204,7 @@ public function setColumn($column)
     }
 
     /**
-     * get action column.
-     *
-     * @return \APY\DataGridBundle\Grid\Column\Column
+     * {@inheritdoc}
      */
     public function getColumn()
     {
@@ -231,15 +248,14 @@ public function setRouteParameters($routeParameters)
     }
 
     /**
-     * get route parameters.
-     *
-     * @return array
+     * {@inheritdoc}
      */
     public function getRouteParameters()
     {
         return $this->routeParameters;
     }
 
+    // @todo: why is this accepting string? it seems pretty useless, isn't it?
     /**
      * Set route parameters mapping.
      *
@@ -296,9 +312,7 @@ public function addAttribute($name, $value)
     }
 
     /**
-     * Get attributes.
-     *
-     * @return array
+     * {@inheritdoc}
      */
     public function getAttributes()
     {
@@ -308,7 +322,7 @@ public function getAttributes()
     /**
      * set role.
      *
-     * @param mixed $role
+     * @param string $role
      *
      * @return self
      */
@@ -322,7 +336,7 @@ public function setRole($role)
     /**
      * Get role.
      *
-     * @return mixed
+     * @return string
      */
     public function getRole()
     {
@@ -332,21 +346,21 @@ public function getRole()
     /**
      * Set render callback.
      *
-     * @deprecated This is deprecated and will be removed in 2.4. Use addManipulateRender instead.
+     * @deprecated This is deprecated and will be removed in 3.0; use addManipulateRender instead.
      *
-     * @param  $callback
+     * @param \Closure $callback
      *
      * @return self
      */
-    public function manipulateRender($callback)
+    public function manipulateRender(\Closure $callback)
     {
         return $this->addManipulateRender($callback);
     }
 
     /**
      * Add a callback to render callback stack.
      *
-     * @param $callback
+     * @param \Closure $callback
      *
      * @return self
      */
@@ -360,9 +374,9 @@ public function addManipulateRender($callback)
     /**
      * Render action for row.
      *
-     * @param \APY\DataGridBundle\Grid\Row $row
+     * @param Row $row
      *
-     * @return null|RowAction
+     * @return RowAction|null
      */
     public function render($row)
     {
@@ -377,22 +391,22 @@ public function render($row)
         return $this;
     }
 
+    // @todo: should not this be "isEnabled"?
     /**
-     * Get the enabled state of this action.
-     *
-     * @return bool
+     * {@inheritdoc}
      */
     public function getEnabled()
     {
         return $this->enabled;
     }
 
+    // @todo: should not this be "enable" as default value is false?
     /**
      * Set the enabled state of this action.
      *
      * @param bool $enabled
      *
-     * @return \APY\DataGridBundle\Grid\Action\RowAction
+     * @return self
      */
     public function setEnabled($enabled)
     {

diff --git a/Grid/Action/RowActionInterface.php b/Grid/Action/RowActionInterface.php
@@ -12,6 +12,9 @@
 
 namespace APY\DataGridBundle\Grid\Action;
 
+// @todo: implementation seems to be more specific than interface. It obviously be the case but I've noticed that
+// only one method of this interface is used in our code. So I wonder if this interface is "updated" and is the mimimum
+// API methods that should be provided as a contract or not.
 interface RowActionInterface
 {
     /**
@@ -38,21 +41,21 @@ public function getConfirm();
     /**
      * get action confirmMessage.
      *
-     * @return bool
+     * @return string
      */
     public function getConfirmMessage();
 
     /**
      * get action target.
      *
-     * @return bool
+     * @return string
      */
     public function getTarget();
 
     /**
      * get the action column id.
      *
-     * @return bool
+     * @return string
      */
     public function getColumn();
 

diff --git a/Grid/Column/ArrayColumn.php b/Grid/Column/ArrayColumn.php
@@ -85,6 +85,7 @@ public function renderCell($values, $row, $router)
         $return = [];
         if (is_array($values) || $values instanceof \Traversable) {
             foreach ($values as $key => $value) {
+                // @todo: this seems like dead code
                 if (!is_array($value) && isset($this->values[(string) $value])) {
                     $value = $this->values[$value];
                 }