Empty block name in entity reference field [#1916938]

Hello.
I found a nasty bug. The bean block has two fields - label and title by default.
Field label is mandatory and title field is optional.

In Hook function bean_entity_info ()

indicated that as the label to use the field title

       'entity keys' => array (
         'id' => 'bid',
         'bundle' => 'type',
         'label' => 'title',
         'name' => 'delta',
         'revision' => 'vid',
         'default revision' => 'default_revision',
       )

I use modules and entityreference & references_dialog to attach the bean block to the node. Because the title is not a mandatory field, you may have the following image:
http://i.imgur.com/weXy407.png

And should be so:
http://i.imgur.com/vqjwYC6.png

This error is then that entity api tries to put as a title of bean block field "title", but user can leave this field blank. If this situation occured - user is confused.

I see two ways to resolve the problem:

1. Make mandatory title field.
2. Set the 'label' => 'label' in 'entity keys'

Which variant is better for you? I will write patch.

Comment	File	Size	Author
#12	Screen Shot 2013-03-14 at 2.41.49 PM.png	18.3 KB	saltednut
#10	1916938-admin-screen.png	30.82 KB	saltednut
#10	1916938-block-list.png	15.5 KB	saltednut
#10	1916938-pre-patch.png	72.08 KB	saltednut
#10	1916938-patch-a.png	30.78 KB	saltednut
#10	1916938-patch-b.png	25.29 KB	saltednut
#10	bean-defaultLabel-insanity-1916938-10-a.patch	583 bytes	saltednut
#10
#10	bean-defaultLabel-insanity-1916938-10-b.patch	615 bytes	saltednut
#10
#4	labe_callback-1916938-4.patch	1.02 KB	indytechcook
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

indytechcook CreditAttribution: indytechcook commented 14 February 2013 at 14:52

Since entity reference uses the entity_label() function and bean uses the entity_class_label callback, the label() then the defaultLabel() methods are used on the class. The "correct" way would be to override the defaultLabel() method and show the label field if the title field is empty.

Comment #2

indytechcook CreditAttribution: indytechcook commented 14 February 2013 at 15:00

More information

The purpose of the title field is the same as the "Block description" or "The title of the block as shown to the user."
THe purpose of the label field is the same as the "Block title" or "Block description"

One of the most difficult aspects of this module is keeping it's structure similar to the block module. I do find this confuses people at times.

The defaultLabel() method should follow this logic.

First check if there is a label. If there is then use it.
If there is no label then use the title.

Corresponding issue in entity reference module: #1911266: autocomplete field should use field selected for sorting

Comment #3

indytechcook CreditAttribution: indytechcook commented 14 February 2013 at 15:06

There have been several issues around the label/title usage.

#1836798: Block Label used rather than Title on block/delta and block/delta/view
#1845974: Block Admin label is correct

We need the following tests with this issue:

Title is correct on bean view page with an empty title.
Title is correct on bean view page with a title filled out.
Title is correct on block list page with an empty title.
Title is correct on block list page with a title filled out.

Comment #4

indytechcook CreditAttribution: indytechcook commented 14 February 2013 at 15:39

Status:	Active	» Needs review
Issue tags:		+Needs tests

File	Size
labe_callback-1916938-4.patch	1.02 KB

Here is a patch without the tests. Setting to needs review to run current tests but I'd still like to see other tests added before this gets committed.

Comment #5

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 14 February 2013 at 15:48

Just as a general comment, you shouldn't include changes to the CHANGELOG.txt file in the patch, that should be manually added when you're committing the changes.

The patch in #4 works for me.

Comment #6

eugene.ilyin CreditAttribution: eugene.ilyin commented 14 February 2013 at 16:55

>The patch in #4 works for me.
ok, this patch will be applied? my help is not need?

Comment #7

indytechcook CreditAttribution: indytechcook commented 27 February 2013 at 14:00

Status:

Needs review

» Fixed

Posted to wrong issue.

Comment #8

indytechcook CreditAttribution: indytechcook commented 27 February 2013 at 14:06

http://drupal.org/commitlog/commit/22232/e62e9bf85390c58acb6318c8ec7b8a3...

Comment #9

saltednut

she/her

English

Chicago

CreditAttribution: saltednut commented 27 February 2013 at 21:51

Status:

Fixed

» Active

Should be active?

Tagged as Needs tests
The getTranslation method will call entity_get_all_property_info($this->entityType); -- for Beans, this returns empty. I have seen errors as a result:

Notice: Undefined index: title in Entity->getTranslation() (line 256 of /entity/includes/entity.inc).

Comment #10

saltednut

she/her

English

Chicago

CreditAttribution: saltednut commented 27 February 2013 at 23:37

Status:

Active

» Needs review

File	Size
bean-defaultLabel-insanity-1916938-10-b.patch	615 bytes

bean-defaultLabel-insanity-1916938-10-a.patch	583 bytes

1916938-patch-b.png	25.29 KB
1916938-patch-a.png	30.78 KB
1916938-pre-patch.png	72.08 KB
1916938-block-list.png	15.5 KB
1916938-admin-screen.png	30.82 KB

I've tested this with Entity 1.0 and the latest bean dev.

I've also tested this using latest dev and entity 7.x-1.x dev.

Did anyone else test this patch before it was committed?

The defaultLabel() method should follow this logic.

First check if there is a label. If there is then use it.
If there is no label then use the title.

I don't think this will give the result we want.

When a Bean is rendered (ex: in a region), using this logic, you'll show the label - when you really want to see the title.

Notice: Undefined index: title in Entity->getTranslation() (line 256 of /entity/includes/entity.inc).

I'm attaching two patches, both without the translation. One uses the label as the defaultLabel, one uses title as defaultLabel.

Bean Admin UI Bean

Bean on the Listings Page

Bean Using Latest Dev

Removed getTranslation (patch a)

Removed getTranslation and swapped label with title (patch b)

Note: both patches also fix some erroneous quote styling.

Comment #11

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 28 February 2013 at 20:23

Patch b from #10 resolves the problem of the block's title being incorrect on display, but I've not yet tested it with EntityReference.

Comment #12

saltednut

she/her

English

Chicago

CreditAttribution: saltednut commented 14 March 2013 at 19:42

File	Size
Screen Shot 2013-03-14 at 2.41.49 PM.png	18.3 KB

Committed b per #1935770: Bean label is used as block subject

I believe this issue is resolved now. Tested latest dev with an entity reference and I am seeing Bean titles in the EntityReference field.
Screen Shot 2013-03-14 at 2.41.49 PM.png