bug #230 Fixed an issue when custom actions override default actions …

…without defining all their options (javiereguiluz)

This PR was squashed before being merged into the master branch (closes #230).

Discussion
----------

Fixed an issue when custom actions override default actions without defining all their options

This fixes #222

Commits
-------

0e78935 Fixed an issue when custom actions override default actions without defining all their options

DependencyInjection/EasyAdminExtension.php

@@ -149,13 +149,13 @@ public function processEntityActions(array $backendConfiguration)
  foreach (array('edit', 'list', 'new', 'show') as $view) {
  $defaultActions = $this->getDefaultActions($view);
  $backendActions = isset($backendConfiguration[$view]['actions']) ? $backendConfiguration[$view]['actions'] : array();
- $backendActions = $this->normalizeActionsConfiguration($backendActions);
+ $backendActions = $this->normalizeActionsConfiguration($backendActions, $defaultActions);
 
  $defaultViewActions = array_replace($defaultActions, $backendActions);
  $defaultViewActions = $this->filterRemovedActions($defaultViewActions);
 
  $entityActions = isset($entityConfiguration[$view]['actions']) ? $entityConfiguration[$view]['actions'] : array();
- $entityActions = $this->normalizeActionsConfiguration($entityActions);
+ $entityActions = $this->normalizeActionsConfiguration($entityActions, $defaultViewActions);
 
  $viewActions = array_replace($defaultViewActions, $entityActions);
  $viewActions = $this->filterRemovedActions($viewActions);
@@ -189,15 +189,15 @@ private function getDefaultActions($view)
  {
  // basic configuration for default actions
  $actions = $this->normalizeActionsConfiguration(array(
- array('name' => 'delete', 'label' => 'action.delete', 'type' => 'method', 'icon' => 'trash'),
- array('name' => 'edit', 'label' => 'action.edit', 'type' => 'method', 'icon' => 'edit'),
- array('name' => 'new', 'label' => 'action.new', 'type' => 'method'),
- array('name' => 'search', 'label' => 'action.search', 'type' => 'method'),
- array('name' => 'show', 'label' => 'action.show', 'type' => 'method'),
- array('name' => 'list', 'label' => 'action.list', 'type' => 'method'),
+ array('name' => 'delete', 'label' => 'action.delete', 'icon' => 'trash'),
+ array('name' => 'edit', 'label' => 'action.edit', 'icon' => 'edit'),
+ array('name' => 'new', 'label' => 'action.new'),
+ array('name' => 'search', 'label' => 'action.search'),
+ array('name' => 'show', 'label' => 'action.show'),
+ array('name' => 'list', 'label' => 'action.list'),
  ));
 
- // configure which actions are enabled for each view
+ // define which actions are enabled for each view
  $actionsPerView = array(
  'edit' => array('delete' => $actions['delete'], 'list' => $actions['list']),
  'list' => array('show' => $actions['show'], 'edit' => $actions['edit'], 'search' => $actions['search'], 'new' => $actions['new']),
@@ -229,46 +229,46 @@ private function getDefaultActions($view)
  * list:
  * actions: ['search', { name: 'show', label: 'Show', 'icon': 'user' }, 'grantAccess']
  *
- * @param array $actionConfiguration
+ * @param array $actionsConfiguration
+ * @param array $defaultActionsConfiguration
  *
  * @return array
  */
- private function normalizeActionsConfiguration(array $actionConfiguration)
+ private function normalizeActionsConfiguration(array $actionsConfiguration, array $defaultActionsConfiguration = array())
  {
  $configuration = array();
 
- foreach ($actionConfiguration as $action) {
- if (!is_string($action) && !is_array($action)) {
- throw new \RuntimeException('The values of the "actions" option can only be strings or arrays.');
- }
-
- // config format #1
+ foreach ($actionsConfiguration as $action) {
  if (is_string($action)) {
- $action = array('name' => $action);
+ // config format #1
+ $actionConfiguration = array('name' => $action);
+ } elseif (is_array($action)) {
+ // config format #2
+ $actionConfiguration = $action;
+ } else {
+ throw new \RuntimeException('The values of the "actions" option can only be strings or arrays.');
  }
 
- $normalizedConfiguration = array_replace($this->defaultActionConfiguration, $action);
-
  // 'name' is the only mandatory option for actions
- if (!isset($action['name'])) {
+ if (!isset($actionConfiguration['name'])) {
  throw new \RuntimeException('When using the expanded configuration format for actions, you must define their "name" option.');
  }
 
+ $actionName = $actionConfiguration['name'];
+
  // 'name' value is used as the class method name or the Symfony route name
  // check that its value complies with the PHP method name regexp (the leading dash
  // is exceptionally allowed to support the configuration format of removed actions)
- if (!preg_match('/^-?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $action['name'], $matchActionName)) {
- throw new \InvalidArgumentException(sprintf('The name of the "%s" action contains invalid characters (allowed: letters, numbers, underscores).', $action['name']));
+ if (!preg_match('/^-?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $actionName, $matchActionName)) {
+ throw new \InvalidArgumentException(sprintf('The name of the "%s" action contains invalid characters (allowed: letters, numbers, underscores).', $actionName));
  }
 
- if (!isset($action['type'])) {
- $action['type'] = 'method';
- }
+ $normalizedConfiguration = array_replace($this->defaultActionConfiguration, $actionConfiguration);
 
  $actionName = $normalizedConfiguration['name'];
 
  // use the special 'action.<action name>' label for the default actions
- if (null === $normalizedConfiguration['label'] && in_array($actionName, array('delete', 'edit', 'new', 'search', 'show', 'list'))) {
+ if (!isset($normalizedConfiguration['label']) && in_array($actionName, array('delete', 'edit', 'new', 'search', 'show', 'list'))) {
  $normalizedConfiguration['label'] = 'action.'.$actionName;
  }
 
@@ -279,6 +279,20 @@ private function normalizeActionsConfiguration(array $actionConfiguration)
  $normalizedConfiguration['label'] = $label;
  }
 
+ if (count($defaultActionsConfiguration)) {
+ // if the user defines an action with the same name of a default action,
+ // he/she is in fact overriding the default configuration of that action.
+ // for example: actions: ['delete', 'list']
+ // this condition ensures that when the user doesn't define the value for
+ // some option of the action (for example the icon or the label) that
+ // option is actually added with the right default value. Otherwise,
+ // those options would be 'null' and the template would show some issues
+ if (array_key_exists($actionName, $defaultActionsConfiguration)) {
+ $normalizedConfiguration = array_filter($normalizedConfiguration); // remove empty/null config options
+ $normalizedConfiguration = array_replace($defaultActionsConfiguration[$actionName], $normalizedConfiguration);
+ }
+ }
+
  $configuration[$actionName] = $normalizedConfiguration;
  }
 

Tests/DependencyInjection/fixtures/configurations/input/admin_101.yml

@@ -0,0 +1,16 @@
+# TEST
+# when the user defines an action with the same name of a default action, he/she
+# is actually overriding the default options of that action. Therefore, if the
+# custom configuration doesn't provide a value for some option (e.g. the icon or
+# the label) the default value should be applied to avoid template issues
+# this test checks that the above is correct when using global actions
+
+# CONFIGURATION
+easy_admin:
+ edit:
+ actions: ['delete', 'list']
+ show:
+ actions: ['edit']
+ entities:
+ TestEntity:
+ class: AppBundle\Entity\TestEntity

Tests/DependencyInjection/fixtures/configurations/input/admin_102.yml

@@ -0,0 +1,16 @@
+# TEST
+# when the user defines an action with the same name of a default action, he/she
+# is actually overriding the default options of that action. Therefore, if the
+# custom configuration doesn't provide a value for some option (e.g. the icon or
+# the label) the default value should be applied to avoid template issues
+# this test checks that the above is correct when using entity actions
+
+# CONFIGURATION
+easy_admin:
+ entities:
+ TestEntity:
+ class: AppBundle\Entity\TestEntity
+ edit:
+ actions: ['delete', 'list']
+ show:
+ actions: ['edit']

Tests/DependencyInjection/fixtures/configurations/input/admin_103.yml

@@ -0,0 +1,20 @@
+# TEST
+# when the user defines an action with the same name of a default action, he/she
+# is actually overriding the default options of that action. Therefore, if the
+# custom configuration doesn't provide a value for some option (e.g. the icon or
+# the label) the default value should be applied to avoid template issues
+# this test checks that the above is correct when using both global and entity actions
+
+# CONFIGURATION
+easy_admin:
+ edit:
+ actions: ['delete', 'list']
+ show:
+ actions: ['edit']
+ entities:
+ TestEntity:
+ class: AppBundle\Entity\TestEntity
+ edit:
+ actions: ['delete', 'list']
+ show:
+ actions: ['edit']

Tests/DependencyInjection/fixtures/configurations/output/config_065.yml

@@ -109,7 +109,7 @@ easy_admin:
  type: method
  label: custom-edit-label
  class: ''
- icon: null
+ icon: edit
  custom_action_for_show:
  name: custom_action_for_show
  type: method

Tests/DependencyInjection/fixtures/configurations/output/config_076.yml

@@ -56,7 +56,7 @@ easy_admin:
  type: method
  label: action.delete
  class: ''
- icon: null
+ icon: trash
  list:
  name: list
  type: method
@@ -83,7 +83,7 @@ easy_admin:
  type: method
  label: action.edit
  class: ''
- icon: null
+ icon: edit
  fields: { }
  new:
  actions:

Tests/DependencyInjection/fixtures/configurations/output/config_078.yml

@@ -58,7 +58,7 @@ easy_admin:
  type: method
  label: action.delete
  class: ''
- icon: null
+ icon: trash
  list:
  name: list
  type: method
@@ -85,7 +85,7 @@ easy_admin:
  type: method
  label: action.edit
  class: ''
- icon: null
+ icon: edit
  fields: { }
  new:
  actions:

Tests/DependencyInjection/fixtures/configurations/output/config_101.yml

@@ -0,0 +1,110 @@
+easy_admin:
+ edit:
+ actions:
+ - delete
+ - list
+ show:
+ actions:
+ - edit
+ entities:
+ TestEntity:
+ class: AppBundle\Entity\TestEntity
+ label: TestEntity
+ name: TestEntity
+ edit:
+ fields: { }
+ actions:
+ delete:
+ name: delete
+ type: method
+ label: action.delete
+ class: ''
+ icon: trash
+ list:
+ name: list
+ type: method
+ label: action.list
+ class: ''
+ icon: null
+ list:
+ fields: { }
+ actions:
+ show:
+ name: show
+ type: method
+ label: action.show
+ class: ''
+ icon: null
+ edit:
+ name: edit
+ type: method
+ label: action.edit
+ class: ''
+ icon: null
+ search:
+ name: search
+ type: method
+ label: action.search
+ class: ''
+ icon: null
+ new:
+ name: new
+ type: method
+ label: action.new
+ class: ''
+ icon: null
+ list:
+ name: list
+ type: method
+ label: action.list
+ class: ''
+ icon: null
+ new:
+ fields: { }
+ actions:
+ list:
+ name: list
+ type: method
+ label: action.list
+ class: ''
+ icon: null
+ show:
+ fields: { }
+ actions:
+ delete:
+ name: delete
+ type: method
+ label: action.delete
+ class: ''
+ icon: trash
+ list:
+ name: list
+ type: method
+ label: action.list
+ class: ''
+ icon: null
+ edit:
+ name: edit
+ type: method
+ label: action.edit
+ class: ''
+ icon: edit
+ design:
+ assets:
+ css: { }
+ js: { }
+ theme: default
+ color_scheme: dark
+ brand_color: '#E67E22'
+ form_theme:
+ - '@EasyAdmin/form/bootstrap_3_horizontal_layout.html.twig'
+ site_name: 'Easy Admin'
+ formats:
+ date: Y-m-d
+ time: 'H:i:s'
+ datetime: 'F j, Y H:i'
+ list:
+ actions: { }
+ max_results: 15
+ new:
+ actions: { }