Menu Nodes should use label field for display in Sonata Admin

[benglass]

Currently menu nodes do not use their label field as the display field within Sonata Admin. This is because MenuNode does not implement __toString and the MenuNodeAdmin does not implement toString. The result is that the name field is being used which is generally all lowercase and does not match with other items when displayed in a tree.

I don't have a preference on which of these 2 approaches is used (__toString on MenuNode vs toString on MenuNodeAdmin) but if you guys do then let me know and I'll create a pull request.

[dbu]

i checked and it seems only blocks still implement __toString. i would prefer to have toString on the admin. if i read the PhpcrOdmTree correctly, this will also be used by all trees. in fact, we should have this for content and blockbundle (at least where applicable) as well.

@lsmith77 i guess it would be good to clean this up today, before the release. do you agree? and agree on that we overwrite toString in those admins? i think the node name is less helpful to an editor than the menu label / content title.

[lsmith77]

we already overwrite the toString method in the base admin class

[dbu]

yes, this is why we at least get the nodename. but we do not use getLabel() on the menu or getTitle on content

[lsmith77]

ok .. we should then maybe have an additional standard method to override so that less code needs to be duplicated

[dbu]

i don't think there is a standard. it depends on the document class what method it has and makes sense. using getLabel() for menu and getTitle for content sounds right.

[lsmith77]

you misunderstood me. i wasn't saying a standard method on the models but a method in the admin base class that can be overwritten without having to duplicate the entire toString method

[dbu]

well, in MenuNodeAdmin we can do

return $this->getSubject()->getLabel() ? : parent::toString();

i don't think we need to refactor the base admin class for this.

[lsmith77]

true

[benglass]

@dbu @lsmith77 ok so I will submit a pull request with the overridden MenuNodeAdmin using the code example from dbu. It was mentioned there were other admins where this is applicable so we may want to create separate issues for them.

[lsmith77]

yup!

[dbu]

thanks. instead of opening issues, i did pull requests: https://github.com/symfony-cmf/ContentBundle/pull/83 https://github.com/symfony-cmf/BlockBundle/pull/148 https://github.com/symfony-cmf/SimpleCmsBundle/pull/83