Skip to content

Commit

Permalink
feature #39 Improved the way entity fields are displayed in listings …
Browse files Browse the repository at this point in the history 
…and forms (javiereguiluz)

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

Discussion
----------

Improved the way entity fields are displayed in listings and forms

### The problem we want to solve

EasyAdmin must work right out of the box. Listings and forms should display any entity field correctly, regardless how wrong those properties are defined.

### The issues we're facing

In an ideal world, Doctrine entities would be well defined and would contain no errors. In that ideal world, we could use some simple Twig code to display those entity properties in forms and listings.

However, lots of applications contain errors in Doctrine entities, such as declaring a property `protected` and not providing any `getter` method to access its value. In those cases, Twig would display these awful error messages and EasyAdmin would crash:

![twig_error](https://cloud.githubusercontent.com/assets/73419/5792319/55e84c46-9f13-11e4-9961-930d1c0d9360.png)

### How I propose to solve them

Twig is great when things go right. But Twig is very weak for "defensive programming". That's why I propose to move all the logic used to display entity fields to a custom Twig extension. If you check the code of the extension included in this PR, you can see that performs a lot of "safeguard actions" to never throw an error: `try ... catch`, `reflection`, `null` and `instanceof` checks, etc.

In my local tests, the results are very promising, because the application now never crashes:

**Simple associations are linked to their "show" action and null values are not a problem**

![simple_associations_and_null](https://cloud.githubusercontent.com/assets/73419/5792324/f1b7449c-9f13-11e4-9f1b-65ba75d503ac.png)

**Null property values are no longer a problem**

![null_properties](https://cloud.githubusercontent.com/assets/73419/5792325/f81c7730-9f13-11e4-99be-0f591e2ad55d.png)

**When an association is a collection, we display the number of elements included in the collection**

![association_collections](https://cloud.githubusercontent.com/assets/73419/5792331/166dafec-9f14-11e4-9d47-5e654edf99f7.png)

**The worst case scenario: a property without a getter and non-public**

![innaccessible_properties](https://cloud.githubusercontent.com/assets/73419/5792333/24be3562-9f14-11e4-8c2e-4d4a10eb78db.png)

They even include a help message explaining why they are inaccessible:

![innaccessible_properties_help_message](https://cloud.githubusercontent.com/assets/73419/5792335/3129ad7c-9f14-11e4-83e3-ae868774ebaf.png)

By the way, this PR fixes #24, #23 and #5.

Commits
-------

a1e1880 Improved the way entity fields are displayed in listings and forms
  • Loading branch information
@javiereguiluz
javiereguiluz committed Jan 18, 2015
2 parents b9dffb9 + a1e1880 commit 47c3d64
Show file tree
Hide file tree
Showing 7 changed files with 218 additions and 117 deletions.
93 changes: 60 additions & 33 deletions Controller/AdminController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Doctrine\Bundle\DoctrineBundle\Mapping\DisconnectedMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Pagerfanta\Pagerfanta;
use Pagerfanta\Adapter\DoctrineORMAdapter;
Expand Down Expand Up @@ -77,14 +76,14 @@ protected function initialize(Request $request)
));
}

$this->em = $this->getDoctrine()->getManager();

if (null !== $entityName = $request->query->get('entity')) {
if (!array_key_exists($entityName, $this->config['entities'])) {
return $this->render404error('@EasyAdmin/error/undefined_entity.html.twig', array('entity_name' => $entityName));
}

$this->entity['name'] = $entityName;
$this->entity['class'] = $this->config['entities'][$entityName]['class'];
$this->entity['metadata'] = $this->getEntityMetadata($this->entity['class']);
$this->entity = $this->getEntityMetadata($entityName);
}

if (!$request->query->has('sortField')) {
Expand All @@ -95,12 +94,11 @@ protected function initialize(Request $request)
}

$this->request = $request;
$this->em = $this->getDoctrine()->getManager();
}

public function listAction()
{
$fields = $this->getFieldsForList($this->entity['metadata']->fieldMappings);
$fields = $this->getFieldsForList($this->entity['fieldMappings']);
$paginator = $this->findAll($this->entity['class'], $this->request->query->get('page', 1), $this->config['list_max_results'], $this->request->query->get('sortField'), $this->request->query->get('sortDirection'));

return $this->render('@EasyAdmin/list.html.twig', array(
Expand All @@ -117,7 +115,7 @@ public function editAction()
throw $this->createNotFoundException(sprintf('Unable to find entity (%s #%d).', $this->entity['name'], $this->request->query->get('id')));
}

$fields = $this->getFieldsForEdit($this->entity['metadata']->fieldMappings);
$fields = $this->getFieldsForEdit($this->entity['fieldMappings']);
$editForm = $this->createEditForm($item, $fields);
$deleteForm = $this->createDeleteForm($this->entity['name'], $this->request->query->get('id'));

Expand All @@ -144,7 +142,7 @@ public function showAction()
throw $this->createNotFoundException(sprintf('Unable to find entity (%s #%d).', $this->entity['name'], $this->request->query->get('id')));
}

$fields = $this->getFieldsForShow($this->entity['metadata']->fieldMappings);
$fields = $this->getFieldsForShow($this->entity['fieldMappings']);
$deleteForm = $this->createDeleteForm($this->entity['name'], $this->request->query->get('id'));

return $this->render('@EasyAdmin/show.html.twig', array(
Expand All @@ -161,7 +159,7 @@ public function newAction()
$entityFullyQualifiedClassName = $this->entity['class'];
$item = new $entityFullyQualifiedClassName();

$fields = $this->getFieldsForNew($this->entity['metadata']->fieldMappings);
$fields = $this->getFieldsForNew($this->entity['fieldMappings']);
$newForm = $this->createNewForm($item, $fields);

$newForm->handleRequest($this->request);
Expand Down Expand Up @@ -204,9 +202,9 @@ public function deleteAction()

public function searchAction()
{
$searchableFields = $this->getSearchableFields($this->entity['metadata']->fieldMappings);
$searchableFields = $this->getSearchableFields($this->entity['fieldMappings']);
$paginator = $this->findBy($this->entity['class'], $this->request->query->get('query'), $searchableFields, $this->request->query->get('page', 1), $this->config['list_max_results']);
$fields = $this->getFieldsForSearch($this->entity['metadata']->fieldMappings);
$fields = $this->getFieldsForSearch($this->entity['fieldMappings']);

return $this->render('@EasyAdmin/list.html.twig', array(
'config' => $this->config,
Expand All @@ -219,27 +217,44 @@ public function searchAction()
/**
* Takes the FQCN of the Doctrine entity and returns all its configured metadata.
*/
protected function getEntityMetadata($entityClass)
protected function getEntityMetadata($entityName)
{
$factory = new DisconnectedMetadataFactory($this->getDoctrine());
$metadata = $factory->getClassMetadata($entityClass)->getMetadata();
$metadata = $metadata[0];

// add fields for relationships
$associationFieldMappings = array();
foreach ($metadata->associationMappings as $fieldName => $relation) {
if (ClassMetadataInfo::ONE_TO_MANY !== $relation['type']) {
$associationFieldMappings[$fieldName] = array(
$entityMetadata = array();

$entityMetadata['name'] = $entityName;
$entityMetadata['class'] = $this->config['entities'][$entityName]['class'];

$doctrineMetadata = $this->em->getMetadataFactory()->getMetadataFor($entityMetadata['class']);

// TODO: Check if the entity performs any kind of inheritance: $doctrineMetadata->isInheritanceTypeNone()

if ('id' !== $doctrineMetadata->identifier[0]) {
throw new \RuntimeException(sprintf("The '%s' entity isn't valid because it doesn't define a primary key called 'id'.", $entityMetadata['class']));
}

// add regular entity fields
foreach ($doctrineMetadata->fieldMappings as $fieldName => $fieldMetadata) {
// field names must be processed to avoid problems in Twig templates
$fieldName = str_replace('_', '', $fieldName);

$entityMetadata['fieldMappings'][$fieldName] = $fieldMetadata;
}

// add fields for entity associations (except many-to-many)
foreach ($doctrineMetadata->associationMappings as $fieldName => $associationMetadata) {
if (ClassMetadataInfo::MANY_TO_MANY !== $associationMetadata['type']) {
$entityMetadata['fieldMappings'][$fieldName] = array(
'association' => $associationMetadata['type'],
'fieldName' => $fieldName,
'fetch' => $associationMetadata['fetch'],
'isOwningSide' => $associationMetadata['isOwningSide'],
'type' => 'association',
'targetEntity' => $relation['targetEntity'],
'targetEntity' => $associationMetadata['targetEntity'],
);
}
}

$metadata->fieldMappings = array_merge($metadata->fieldMappings, $associationFieldMappings);

return $metadata;
return $entityMetadata;
}

/**
Expand Down Expand Up @@ -408,10 +423,27 @@ protected function findBy($entityClass, $searchQuery, array $searchableFields, $

protected function createEditForm($entity, array $entityFieldsMapping)
{
$form = $this->createFormBuilder($entity);
$formTypeMap = array(
'boolean' => 'checkbox',
'datetime' => 'datetime',
'datetimetz' => 'datetime',
'text' => 'textarea',
);

$form = $this->createFormBuilder($entity, array(
'data_class' => $this->entity['class'],
));

foreach ($entityFieldsMapping as $name => $metadata) {
$form->add($name, null, array());
if ('association' === $metadata['type'] && !$metadata['isOwningSide']) {
continue;
}

$formFieldType = array_key_exists($metadata['type'], $formTypeMap)
? $formTypeMap[$metadata['type']]
: null;

$form->add($name, $formFieldType, array());
}

return $form->getForm();
Expand Down Expand Up @@ -450,12 +482,7 @@ protected function getFieldsForNew(array $entityFields)

protected function createNewForm($entity, array $entityFieldsMapping)
{
$form = $this->createFormBuilder($entity);
foreach ($entityFieldsMapping as $name => $metadata) {
$form->add($name, null, array());
}

return $form->getForm();
return $this->createEditForm($entity, $entityFieldsMapping);
}

protected function getNameOfTheFirstConfiguredEntity()
Expand Down
7 changes: 7 additions & 0 deletions DependencyInjection/EasyAdminExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\Config\FileLocator;

class EasyAdminExtension extends Extension
{
Expand All @@ -25,13 +27,18 @@ class EasyAdminExtension extends Extension

public function load(array $configs, ContainerBuilder $container)
{
// process configuration parameters
$configuration = new Configuration();
$config = $this->processConfiguration($configuration, $configs);

$options = array_replace($this->defaultConfigOptions, $config);
$options['entities'] = $this->processEntityConfiguration($options['entities']);

$container->setParameter('easy_admin.config', $options);

// load bundle's services
$loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('services.xml');
}

protected function processEntityConfiguration(array $entitiesConfiguration)
Expand Down
14 changes: 14 additions & 0 deletions Resources/config/services.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0" ?>
<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>

<service id="easyadmin.twig.extension" class="JavierEguiluz\Bundle\EasyAdminBundle\Twig\EasyAdminTwigExtension" public="false">
<argument type="service" id="router"></argument>
<tag name="twig.extension" />
</service>

</services>
</container>
6 changes: 6 additions & 0 deletions Resources/public/stylesheet/admin.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ span.label-danger {
background: #D44542;
}

/* Badges
------------------------------------------------------------------------- */
span.badge {
background-color: #D47843;
}

/* Buttons
------------------------------------------------------------------------- */
a.btn,
Expand Down
48 changes: 1 addition & 47 deletions Resources/views/list.html.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,43 +86,7 @@
{% set isSortingField = metadata.fieldName == app.request.get('sortField') %}

<td class="{{ isSortingField ? 'sorted' : '' }}">
{% if metadata.type in ['date', 'datetime', 'datetimetz', 'time'] %}
{{ attribute(item, field|replace({'_': ''}))|date }}
{% elseif metadata.type in ['boolean'] %}
<span class="label label-{{ attribute(item, field|replace({'_': ''})) ? 'success' : 'danger' }}">
{{ attribute(item, field|replace({'_': ''})) ? 'YES' : 'NO' }}
</span>
{% elseif metadata.type in ['array', 'simple_array'] %}
{{ attribute(item, field|replace({'_': ''}))|join(', ') }}
{% elseif metadata.type in ['string', 'text'] %}
{{ attribute(item, field|replace({'_': ''}))[:128] }}
{% elseif field != 'id' and metadata.type in ['bigint', 'integer', 'smallint', 'decimal', 'float'] %}
{{ attribute(item, field|replace({'_': ''}))|number_format }}
{% elseif metadata.type in ['association'] %}
{% set associatedEntityName = metadata.targetEntity|split('\\')|last %}
{% set associatedItem = attribute(item, field|replace({'_': ''})) %}

{% if not associatedItem.__isInitialized__|default(false) %}
{% if associatedItem is iterable %}
<span class="label">{{ associatedItem.count|default(0) }}</span>
<small>{{ associatedEntityName }}</small>
{% elseif associatedItem is not null and associatedItem['id'] is defined %}
<a href="{{ path('admin', { action: 'show', entity: associatedEntityName, id: associatedItem.id }) }}">{{ associatedItem }}</a>
{% elseif associatedItem is null %}
<span class="label">null</span>
{% else %}
{{ associatedItem }}
{% endif %}
{% elseif associatedItem is not null %}
<a href="{{ path('admin', { action: 'show', entity: associatedEntityName, id: associatedItem.id }) }}">{{ associatedItem }}</a>
{% else %}
<span class="label">null</span>
{% endif %}
{% elseif metadata.type in ['blob', 'binary'] %}
<span class="label">{{ metadata.type }}</span>
{% else %}
{{ attribute(item, field|replace({'_': ''}))|default(null) }}
{% endif %}
{{ entity_field(item, field, metadata) }}
</td>
{% endfor %}
<td class="actions">
Expand All @@ -131,16 +95,6 @@
{{ ('actions.'~action)|trans }}
</a>
{% endfor %}

{#
<a href="{{ path('admin', { action: 'show', entity: entity.name, id: attribute(item, 'id') }) }}">
Show
</a>
<a href="{{ path('admin', { action: 'edit', entity: entity.name, id: attribute(item, 'id') }) }}">
Edit
</a>
#}

</td>
</tr>
{% else %}
Expand Down
38 changes: 1 addition & 37 deletions Resources/views/show.html.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,7 @@
</label>
<div class="col-sm-10">
<div class="form-control {{ metadata.type|lower }}">
{% if metadata.type in ['date', 'datetime', 'datetimetz', 'time'] %}
{{ attribute(item, field|replace({'_': ''}))|date }}
{% elseif metadata.type in ['boolean'] %}
<span class="label label-{{ attribute(item, field|replace({'_': ''})) ? 'success' : 'danger' }}">
{{ attribute(item, field|replace({'_': ''})) ? 'YES' : 'NO' }}
</span>
{% elseif metadata.type in ['array', 'simple_array'] %}
{{ attribute(item, field|replace({'_': ''}))|join(', ') }}
{% elseif metadata.type in ['string', 'text'] %}
{{ attribute(item, field|replace({'_': ''}))[:128] }}
{% elseif field != 'id' and metadata.type in ['bigint', 'integer', 'smallint', 'decimal', 'float'] %}
{{ attribute(item, field|replace({'_': ''}))|number_format }}
{% elseif metadata.type in ['association'] %}
{% set associatedEntityName = metadata.targetEntity|split('\\')|last %}
{% set associatedItem = attribute(item, field|replace({'_': ''})) %}

{% if not associatedItem.__isInitialized__|default(false) %}
{% if associatedItem is iterable %}
<span class="label">{{ associatedItem.count|default(0) }}</span>
<small>{{ associatedEntityName }}</small>
{% elseif associatedItem is not null and associatedItem['id'] is defined %}
<a href="{{ path('admin', { action: 'show', entity: associatedEntityName, id: associatedItem.id }) }}">{{ associatedItem }}</a>
{% elseif associatedItem is null %}
<span class="label">null</span>
{% else %}
{{ associatedItem }}
{% endif %}
{% elseif associatedItem is not null %}
<a href="{{ path('admin', { action: 'show', entity: associatedEntityName, id: associatedItem.id }) }}">{{ associatedItem }}</a>
{% else %}
<span class="label">null</span>
{% endif %}
{% elseif metadata.type in ['blob', 'binary'] %}
<span class="label">{{ metadata.type }}</span>
{% else %}
{{ attribute(item, field|replace({'_': ''})) }}
{% endif %}
{{ entity_field(item, field, metadata) }}
</div>
</div>
</div>
Expand Down

0 comments on commit 47c3d64

Please sign in to comment.