Currently we have 3 actions on prepopulated fields :
- "Do nothing" which do pretty well what it says :)
- "Hide field" which removes the field from the UI
- "Disable field" which let the field as is but disabled, not really aestetic.

I propose to add a new action which would hide the real field and show an "item" field instead containing the entity title.

Patch coming soon.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DuaelFr’s picture

Assigned: DuaelFr » Unassigned
Status: Active » Needs review
FileSize
3.27 KB

Here is the patch !
I added a few tests but feel free to improve them as I am not really good for it.

This patch is part of the #1day1patch initiative.

Status: Needs review » Needs work
DuaelFr’s picture

I said I was not good at patches...

chipway’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
amitaibu’s picture

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,17 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            $form[$field_name][$lang][$delta]['label']['#markup'] = preg_replace('#^(.*)\s\([0-9]+\)$#', '$1', $form[$field_name][$lang][$delta]['label']['#default_value']);

Can you explain this part?

+++ b/plugins/behavior/EntityReferencePrepopulateInstanceBehavior.class.php
@@ -15,6 +15,7 @@ class EntityReferencePrepopulateInstanceBehavior extends EntityReference_Behavio
+        'value' => t('Only show entity title'),

value => item

DuaelFr’s picture

The first part removes the entity ID to only let its title (better for UX I think).
I don't understand your second comment, the key of my option is named 'value' to make reference to the form field type used to hide the field.

amitaibu’s picture

> The first part removes the entity ID to only let its title (better for UX I think).

Can't we just use entity_label() form the entity ID instead?

DuaelFr’s picture

The entity_id is extracted by entityreference with a regular expression so I think using regexp is ok.
If you think entity_label is lighter and more understandable than my method, I have no reason to refuse :)

RoySegall’s picture

Status: Needs work » Needs review
FileSize
4.11 KB

Basicly the test is ok expect for two things:
1. You try to verify that the field was replaced by the label but you weren't in the node edit/create form, so the text was not available.
2. You didn't verify in the test that the value option is applied on the edit of the node. I added this part in test.

amitaibu’s picture

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,17 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            $form[$field_name][$lang][$delta]['label']['#markup'] = preg_replace('#^(.*)\s\([0-9]+\)$#', '$1', $form[$field_name][$lang][$delta]['label']['#default_value']);

I suspect you test this only with autocomplete, and not with a select list (where we won't have the entity ID in brackets). So let's use entity_label().

DuaelFr’s picture

Thank you Roy !
If I understand what you say in your first point, the previous test "Hide the field." cannot be valid because it is missing the $this->drupalGet('node/add/' . $this->node1->type, $options); too.

Here is a new patch to use entity_label instead of a regexp. I let you fix the "Hide the field" test in another issue.

Status: Needs review » Needs work
DuaelFr’s picture

What happened ?
I don't understand why this patch didn't pass...

RoySegall’s picture

I'm trying to figure now.

RoySegall’s picture

After some investigation the correct way is:

$form[$field_name][$lang][$delta]['label']['#markup'] = $id ? entity_label($field['settings']['target_type'], entity_load_single($entity_type, $id)) : entity_label($field['settings']['target_type'], $entities[$ids[$delta]]);

Because when creating the entity the $id don't have a value. Any way, i think this is a bit messy.

Also, have you tried this feature with select list as @amitaibu mention before? With select list the the feature don't work because element_children($form[$field_name][$lang]) is returning nothing is the list will not be a markup but remains a select list.

amitaibu’s picture

Instead of working with the form try calling entityreference_prepopulate_get_values()

DuaelFr’s picture

Status: Needs work » Needs review
FileSize
2.46 KB
4.65 KB

@Roy : I think $id is the edited entity id not the referenced entity id so I think I should not use it here.

I fully rewrited my function to use more $ids which is populated by entityreference_prepopulate_get_values() and to find something that will work for all widgets. I think that tests will continue to work as they only test the concept (*crossing fingers*).

Status: Needs review » Needs work
amitaibu’s picture

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,35 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+          foreach ($entities as $tmp_entity) {

maybe $tmp_entity => $entity_item ?

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,35 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            $entities_labels[] = entity_label($field['settings']['target_type'], $tmp_entity);

XSS! :) use check_plain()

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,35 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            $form[$field_name][$lang . '_label']['#markup'] = theme('item_list', array('items' => $entities_labels));

In that case don't use #markup, use the #theme => 'item_list'

DuaelFr’s picture

Status: Needs work » Needs review
FileSize
1.35 KB
4.7 KB

Done !

Status: Needs review » Needs work
DuaelFr’s picture

Status: Needs work » Needs review
FileSize
1.2 KB
4.7 KB

Tests fixed (I hope)

RoySegall’s picture

I'm attaching a minor fix the patch: i try to verify that when the widget type is select list and the user editing the entity the label of the entities should appear - that didn't occurred. When editing, the $ids variable does not holds the populated field values. I fixed that and verified that in the simple test.

RoySegall’s picture

amitaibu’s picture

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,45 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            // No prepopulated values has been found. Load the value from the

has => have

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,45 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+          foreach (element_children($form[$field_name][$lang]) as $delta => $element) {
+            unset($form[$field_name][$lang][$delta]);

We can just create a new element as assign to to $form[$field_name][$lang] -- instead of iterating and unsetting the existing element.

RoySegall’s picture

FileSize
36.02 KB

removing the unset is working for me but for the simple test can't handle it:

RoySegall’s picture

DuaelFr’s picture

+++ b/entityreference_prepopulate.moduleundefined
@@ -109,6 +109,44 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+          $form[$field_name][$lang]['#type'] = 'value';
+          $form[$field_name][$lang]['#value'] = $ids;

I had styles issues without unsetting #theme on multiple fields.

+++ b/entityreference_prepopulate.moduleundefined
@@ -109,6 +109,44 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+          if (count($entities_labels) == 1) {
+            $form[$field_name][$lang] = array(
+              '#type' => 'item',
+              '#title' => $form[$field_name]['und']['#title'],
+              '#markup' => $entities_labels[0],
+            );
+          }
+          else {
+            $form[$field_name][$lang] = array(
+              '#theme' => 'item_list',
+              '#title' => $form[$field_name]['und']['#title'],
+              '#items' => $entities_labels,
+            );

You are overriding $form[$field_name][$lang] so the values will be lost on the form submission. This is the reason why I used $lang . '_label' as form field key.

RoySegall’s picture

@DuaelFr you'r right with you said. I don't know how the prepopulated save the value in the fields but i made a few changes.

amitaibu’s picture

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,49 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            // No prepopulated values has been found. Load the value from the

has => have

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,49 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+            foreach ($entity->{$field_name}[$langcode] as $value) {

Why not use $wrapper?

+++ b/entityreference_prepopulate.module
@@ -109,6 +109,49 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+          $title = $form[$field_name]['und']['#title'];

'und' should probably be $langcode?

XSS - make sure to use check_plain()

RoySegall’s picture

RoySegall’s picture

amitaibu’s picture

+++ b/entityreference_prepopulate.moduleundefined
@@ -109,6 +109,55 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+              for ($i = 0; $i < $wrapper->{$field_name}->count(); $i++) {

no need for this, just do $ids = $wrapper->{$field_name}->value(array('identifier' => TRUE));

amitaibu’s picture

+++ b/entityreference_prepopulate.moduleundefined
@@ -109,6 +109,55 @@ function entityreference_prepopulate_field_attach_form($entity_type, $entity, &$
+          if (!$ids) {

btw, When would we have an $ids with values?

RoySegall’s picture

We have $ids when we in a URL like: node/add/article?field_node_ref=1

chris_h’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

I've been using this in production for nearly a year now and works great

jgullstr’s picture

Added #2126203: new action type in prepopulate : Display rendered entity as related issue, as this behavior is possible to replicate using a display mode.

hbjosemaria’s picture

If this works, why isn't it commited yet? Now I have to patch ERP :<

delacosta456’s picture

hello ... hi please

for this issue , which patch can apply safely please ?

Anybody’s picture

Status: Reviewed & tested by the community » Fixed

#36 was committed to 7.x-1.x-dev! :) Thanks all!

  • Anybody committed a77e94e on 7.x-1.x
    Issue #2028735 by RoySegall, DuaelFr, amitaibu, chris_h: Add a new...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.