sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

What does the "identifier" field option do? #4554

Closed martixy closed 5 years ago

martixy commented 7 years ago

Sonata packages

$ composer show sonata-project/*
sonata-project/admin-bundle              3.20.1 The missing Symfony Admin Generator
sonata-project/block-bundle              3.3.2  Symfony SonataBlockBundle
sonata-project/cache                     1.0.7  Cache library
sonata-project/core-bundle               3.4.0  Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.2.1  Symfony SonataDatagridBundle
sonata-project/doctrine-orm-admin-bundle 3.1.6  Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/exporter                  1.7.1  Lightweight Exporter library

PHP version

$ php -v
PHP 7.1.7-1+ubuntu16.04.1+deb.sury.org+1 (cli) (built: Jul  7 2017 09:41:45) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.1.7-1+ubuntu16.04.1+deb.sury.org+1, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.5, Copyright (c) 2002-2017, by Derick Rethans

Subject

I was reading here. Either I am misunderstanding what the identifier property does, it doesn't do what it says it does.

As I understood it, if set to true it will create a link to edit the associated object(if it has an admin). But it always creates a link and it doesn't respect a false value.

greg0ire commented 7 years ago

it doesn't respect a false value

What do you mean by that?

OskarStark commented 7 years ago

Entity Post with id and name property

If you use ->addIdentifier('name') it links to the Post-Edit, it has nothing todo with referenced entities. You could also add ->addIdentifier('id') AND ->addIdentifier('name'), in this case you could remove the list action for edit to save some space if you want

martixy commented 7 years ago

Right... so this is tied to the addIdentifier method. Which makes me question its existence at all.

What do you mean by that?

->addIdentifier('name') and ->add('name', null, ['identifier' => false]) (or ['identifier' => ''] for that matter) are identical - i.e. it doesn't respect the value of the property, it just looks for its presence or absence.

The wording in the documentation I linked is also incredibly poor.

What it implies however is useful and I'd like to put that as a suggestion. That is - remove the identifier option entirely, and introduce a better-named one that controls whether it creates a link to edit the referenced object.

greg0ire commented 7 years ago

I wasn't even aware this option existed. I guess it is does nothing, we should deprecate it, or make it treat falsy values by not adding a link.

The wording in the documentation I linked is also incredibly poor.

Can you contribute something about that?

That is - remove the identifier option entirely, and introduce a better-named one that controls whether it creates a link to edit the referenced object.

:+1: Great idea! Can you contribute it, but with a deprecation instead of a removal?

martixy commented 7 years ago

I'm new to the "contributing" scene. So don't know... maybe. I don't even know where the documentation lives.

greg0ire commented 7 years ago

It lives in Resource/doc, and you can read CONTRIBUTING.md in this repo, contributing to the docs or code is described at length in it.