Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz’s picture

Status: Active » Needs review
FileSize
7.9 KB

Let's get the discussion started.

This patch works for me.

amitaibu’s picture

I'll go over it, first thing that "jumped" out was commerce-product related stuff -- I don't think ER should deal, it should be added by an alter hook via commerce-product.

amitaibu’s picture

Status: Needs review » Needs work
+++ b/entityreference.moduleundefined
@@ -815,6 +815,36 @@ function entityreference_autocomplete_callback($type, $field_name, $entity_type,
+function entityreference_field_type_settings($field) {

@@ -835,7 +865,7 @@ function entityreference_field_formatter_info() {
+      'field types' => array('entityreference', 'commerce_product_reference', 'taxonomy_term_reference'),

This probably needs to trigger a hook and a drupal_alter(), instead of hardcoding the list.

+++ b/entityreference.moduleundefined
@@ -815,6 +815,36 @@ function entityreference_autocomplete_callback($type, $field_name, $entity_type,
+    $settings['column'] = 'product_id';

We can probably get this info from entity_get_info()

+++ b/entityreference.moduleundefined
@@ -835,7 +865,7 @@ function entityreference_field_formatter_info() {
+      'field types' => array('entityreference', 'commerce_product_reference', 'taxonomy_term_reference'),

I think we can get the field types from the new entityreference_field_type_settings()

pamreed’s picture

Hi
Do you see this patch getting into the module? Or maybe better question is How can it work with commerce product reference fields? Thank you.

itamar’s picture

Status: Needs work » Needs review
FileSize
8.8 KB

Re-rolling bojan's patch according to Amitai's comments - Removing commerce related definitions from entityreference.

itamar’s picture

I added the commerce-product-reference specific definitions here: http://drupal.org/node/1788880

Damien Tournoud’s picture

     'settings' => array(
       // Default to the core target entity type node.
       'target_type' => 'node',
+      // The column that holds the referenced ID.
+      'target_column' => 'target_id',
       // The handler for this field.
       'handler' => 'base',
       // The handler settings.

The target column is *not* a field setting for entity reference. We should not stuff it in there...

Let's find another place for it.

bojanz’s picture

I don't like where #5 went either.

Commerce now has a "rendered product" formatter, so let's just remove any mention of that field type, support only core fields (term reference, basically), and remove the need to have any additional hooks.

amitaibu’s picture

Now that commerce doesn't need it, I think we should actually take another approach -- make taxonomy term obsolete (as we'd want to do for 8.x):

bojanz’s picture

It's too late for me to change the taxonomy_term_reference fields in Kickstart, so I will still need a patch such as this one. I'm probably not the only one.

vasike’s picture

#8 patch rerolled

nwy’s picture

hswong3i’s picture

#11 patch rerolled

hswong3i’s picture

ooo miss...

roborn’s picture

Status: Needs review » Needs work

The last submitted patch, 1580348-universal-formatters-14.patch, failed testing.

roborn’s picture

Status: Needs work » Needs review
FileSize
8.69 KB

#14 doesn't apply cleanly

guy_schneerson’s picture

Thanks guys #14 was applied cleanly for me and works great.

apanag’s picture

discipolo’s picture

Issue summary: View changes

me too

rmathew’s picture

#17 applies cleanly for me, using 7.x-1.1

asb’s picture

Status: Needs review » Needs work

Coming from https://www.drupal.org/project/commerce_fancy_attributes, where I've been instructed to apply "the" patch from #1580348: Make the formatters work with other reference fields.

For me, the patch from #17 doesn't work at all:

$ /var/www/drupal/profiles/commerce_kickstart/modules/contrib/entityreference# patch -p1 < 1580348-universal-formatters-17.patch 
patching file entityreference.module
Hunk #1 succeeded at 1096 (offset 26 lines).
Hunk #2 FAILED at 1116.
Hunk #3 FAILED at 1131.
Hunk #4 FAILED at 1141.
Hunk #5 FAILED at 1173.
Hunk #6 FAILED at 1181.
Hunk #7 FAILED at 1197.
Hunk #8 FAILED at 1221.
Hunk #9 FAILED at 1249.
Hunk #10 FAILED at 1265.
Hunk #11 FAILED at 1276.
Hunk #12 FAILED at 1286.
11 out of 12 hunks FAILED -- saving rejects to file entityreference.module.rej

In #10, bojanz mentioned about two years ago that he'd need this pach for Commerce Kickstart, so I don't know if something is included in Kickstart 2. However, I want this bloody "Rendered entity" formatter mentioned at https://www.drupal.org/project/commerce_fancy_attributes, and it does not exist in Kickstart 2.

I might also add that appying and reverting this patch kills my Kickstart 2 installation:

PHP Fatal error:  Cannot redeclare entityreference_field_type_settings() (previously declared in /var/www/drupal/profiles/commerce_kickstart/modules/contrib/entityreference/entityreference.module:1080) in /var/www/drupal/profiles/commerce_kickstart/modules/contrib/entityreference/entityreference.module on line 1122
Drush command terminated abnormally due to an unrecoverable error.

So be careful when following the instructions from https://www.drupal.org/project/commerce_fancy_attributes.

bojanz’s picture

This patch is already included in Kickstart v2, always has been.

You applied the patch twice, hence the error you saw.

asb’s picture

Good to know, but I still don't see the "Rendered entity" formatter mentioned at https://www.drupal.org/project/commerce_fancy_attributes :-(

edysmp’s picture

Status: Needs work » Reviewed & tested by the community

Applied on a commerce 2.x site with fancy attributes and the render entity formatter just works.

hswong3i’s picture

Status: Reviewed & tested by the community » Needs work

With latest commit this patch no longer working ;-(

florisg’s picture

Status: Needs work » Needs review
FileSize
8.74 KB

Re rolled #17 against latest git checkout of 7.x-1.x

hswong3i’s picture

hswong3i’s picture

  • spotzero committed c083739 on 7.x-1.x authored by bojanz
    Issue #1580348 by hswong3i, bojanz, roborn, itamar, vasike, idevit: Make...
spotzero’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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

xumepadismal’s picture

Re-roll in #29 breaks #1674792: Rendered entity is not language aware committed earlier.

$result[$delta] = entity_view($target_type, array($item[$column] => $target_entity), $settings['view_mode'], $langcode, FALSE);

It should be $target_langcode instead of $langcode:

Should we revert and commit proper re-roll or just create a separate issue?

UPD: Sorry, missed that one: #2850416: Rendered entity is not language aware (again)

minorOffense’s picture

Resolved in the other issue.