Original issue report

When I try to style fields on a product display node that are inherited from the product_reference to the product entity, it doesn't work when using multiple build modes.

With the help of Display Suite I added a custom build mode to the product entity, as well as to the product display node.
However the fields on the product display node get rendered via the product entity "Default" view mode, which is not the intended behavior.

This seems to be caused by the fact that field_view_field() takes the view mode name for core view modes with a prefix (e.g. node_full, node_teaser, node_rss, ...) while custom view modes through Display Suite don't.

I created a patch that tries to address this problem.
This has already come up in #1084084: Managing display of Product fields and #733044: Add build modes for products, but appeared to have been in the wrong issue context.
Hence this dedicated issue, and an updated patch.
If this belongs within a "bigger picture" issue, let me know.

Sven

Note to future readers

The committed fix to this issue has the effect of exposing more display modes to the 'manage display' section of products. If you have an existing site where you tried to manipulate the display of product fields by setting "Use custom display settings" for YourCustomViewMode, this will not work. You need to:

  • clear all caches
  • at admin/commerce/products/types/product/display, turn on "Use custom display settings" for "Node: YourCustomViewMode"
  • migrate your field display settings from "YourCustomViewMode" to "Node: YourCustomViewMode"
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

svendecabooter’s picture

Status: Active » Needs review
FileSize
1.3 KB

Patch attached.

swentel’s picture

Subscribing (as the maintainer of DS) - I'm indeed wondering why the entity type is prefixed at all. Even core does not do this when rendering say nodes or other entity types. Even if you would implement hook_entity_info_alter() yourself and add custom view modes through that (which DS is just doing), it won't work either. Patch looks RTBC imo.

liupascal’s picture

Could it be related to this issue somehow ? http://drupal.org/node/1084084#comment-4577586
edit : stupid me, didn't realize it was the same issue as the one in the original post :/

svendecabooter’s picture

@swentel:
I was assuming the entity type prefix was something that was required by core (although I couldn't find much reference to support that statement), and that Drupal Commerce has to add the prefix to get it working with core. But you say that's not true? I'll try to dig into it some more to wrap my head around the bigger context in which this issue appears, because I'm no expert on the inner workings of view modes (contrary to you :))

Thanks for keeping an eye on this!

rszrama’s picture

Status: Needs review » Active

The prefix is coming from the fact that the Product Reference module defines new view modes for each view mode of the entities that reference products. So if you add a product reference field to a node, then products will have new view modes representing each view mode nodes using the "node_[view mode]" pattern. If you created a custom view mode for your display nodes, I'd expect the Product Reference module to create a similar view mode for products that uses the same pattern. Is it not doing this? If not, it could just be a module weighting issue...

Also, this patch assumes the default view modes as used by nodes, but the Product Reference module supports this behavior for any entity.

I think there should be a cleaner solution, so why don't we see if adjusting the module weight so Display Suite alters the entity info before Product Reference solves this.

BenK’s picture

Wanted to chime in with one thought pointed about by swentel (Display Suite maintainer). This issue may be related to the notices we're experiencing in Display Suite on this thread:

http://drupal.org/node/1251076

--Ben

swentel’s picture

I've looked at the code a bit, and it's probably a racing condition indeed. Can anyone test playing around with weights, sven ?

BenK’s picture

I've also noticed that when displaying a view mode supplied by Display Suite that also contains a Drupal Commerce price field, I'm getting the following notice:

Notice: unserialize() [function.unserialize]: Error at offset 0 of 5 bytes in commerce_price_field_load() (line 85 of /Users/benkaplan/git/commerce_kickstart/profiles/commerce_kickstart/modules/contrib/commerce/modules/price/commerce_price.module).

Is this related to this issue or is this a different issue?

--Ben

rszrama’s picture

Ahh, that might be key. Someone just posted that same error message in a separate issue.

swentel’s picture

@rszrama - are you in London - maybe we should sit together and see if can pinpoint this - I'm getting some other request on DS as well - but I think if can can 'fix' (I'm quoting this on purpose, since I'm not sure where the problem is - even if there is one) the view modes problem, than a lot of issues will easily go away. Sven, care to join as well on a little brainstorm ?

Damien Tournoud’s picture

Category: bug » support

Display suite should probably move its hook_entity_info() to the beginning of the list, and we should probably move commerce_product_reference_entity_info_alter() to the end of the list.

svendecabooter’s picture

@swentel sure let me know when you want to work on this. I'll see what I can do :)

jbova’s picture

As the issue title suggests, this occurs with any custom view modes, regardless of whether or not Display Suite is used.

I created a new view mode for both the 'node' & 'commerce_product' entities using a custom module. After configuring each display to show the image field at different sizes, the settings are not honored.

A modified version of the patch in #1 would change the display for my custom view mode, but break the original teaser view. It does seem to be related to how $entity_type . '_' is prepended to the $view_mode parameter to field_view_field function call. In my case, $entity_type = 'node'.

The only dirty hack I could work in for a temporary fix is to check the $view_mode variable to see if it is my custom view mode, "second_teaser". This had to be done in both the commerce_cart.module and commerce_product_reference.module.

--- modules.b/cart/commerce_cart.module 2011-08-23 14:30:31.000000000 -0400
+++ modules/cart/commerce_cart.module   2011-09-08 09:53:58.000000000 -0400
@@ -2039,7 +2039,11 @@
       // reference field is attached to.
       list($entity_id, $vid, $bundle) = entity_extract_ids($context['entity_type'], $context['entity']);
       $cart_context['class_prefix'] = $context['entity_type'] . '-' . $entity_id;
-      $cart_context['view_mode'] = $context['entity_type'] . '_' . $element['#view_mode'];
+      if ( $element['#view_mode'] != 'second_teaser' ) {
+        $cart_context['view_mode'] = $context['entity_type'] . '_' . $element['#view_mode'];
+      } else {
+        $cart_context['view_mode'] = $element['#view_mode'];
+      }
 
       $entity_uri = entity_uri($context['entity_type'], $element['#object']);
 
modules/product_reference/commerce_product_reference.module
--- modules.b/product_reference/commerce_product_reference.module       2011-08-23 14:30:31.000000000 -0400
+++ modules/product_reference/commerce_product_reference.module 2011-09-08 09:50:16.000000000 -0400
@@ -223,7 +223,11 @@
         // Loop through the fields on the referenced product's type.
         foreach (field_info_instances('commerce_product', $product->type) as $product_field_name => $product_field) {
           // Add the product field to the entity's content array.
-          $entity->content['product:' . $product_field_name] = field_view_field('commerce_product', $product, $product_field_name, $entity_type . '_' . $view_mode, $langcode);
+          if ( $view_mode != 'second_teaser' ) {
+            $entity->content['product:' . $product_field_name] = field_view_field('commerce_product', $product, $product_field_name, $entity_type . '_' . $view_mode, $langcode);
+          } else {
+            $entity->content['product:' . $product_field_name] = field_view_field('commerce_product', $product, $product_field_name, $view_mode, $langcode);
+          }
 
           // For multiple value references, add context information so the cart
           // form can do dynamic replacement of fields.

zambrey’s picture

I would like to bump this issue. Do we have any updates on it?
What direction should we take to fix this problem?
DS is growing rapidly these days so I imagine more concerns from users when they discover possibilities with that module.

adam.t’s picture

3 months have passed since the last bump - so here's another one.

I'm building a complex website with intensive usage of view modes, so it is understandable that this bogus functionality is a real pita.

I would appreciate any info on if there's any work being put into resolving this issue, or should we revert to some dirty hacks if we'd like to customize product fields properly.

Thank you,
Adam

elmerbotha’s picture

I'm with Adam, except for the fact that I am not capable of hacking anything. This functionality in my opinion is crucial and after configuring and adding products, product types, views, etc. I am stranded because my custom view mode is effectively being ignored.

Could we get any final and definitive feedback on this?

Thanks

Elmer

timodwhit’s picture

Its seems there are multiple discussions all faced around the same thing: People looking to unify their "View Modes" on the "Nodes" and on the "Products".

Just to reiterate: When someone creates a custom view mode, they are unable to effectively call upon the view mode in the product.

Any resolve to this yet?

adam.t’s picture

I temporarily got around this issue, by dropping view modes completely, and use views with "panels fields" instead. This way I can still rearrange fields on a set of layouts, and in addition, I can use the same system that I use for laying out the whole site (panels).

It was trickier to replace some other useful features of display suite, like alternating view modes, but I was able to do it by utilizing panels contexts.

But all these things would be so much easier by using view modes.

jaxtheking’s picture

I badly needed a fix too (no, I don't mean drugs) for this issue. Display Suite is such an important addition to the ease of manageability in D7 and it's a shame it can't be used properly (or at all) in D7 commerce sites.
After studying all comments here and going through some of the code, I came up with my own interim fix/hack.
The issue seems to be around prepending $entity_type.'_' for core view modes but NOT for custom view modes added by DS.
I made an array containing the names of core view modes in D7. If the view mode is from core, then the entity type gets prepended (node_), otherwise it does not. Seems to work ok for nodes/product displays. This way I have been able to use Views to display suggested products incl an add to cart button and a resized thumbnail!

In modules/product_reference/commerce_product_reference.module, change line 286 from

  $reference_view_mode = $entity_type . '_' . $view_mode;

into

  $core_view_modes = array('full', 'teaser', 'rss', 'search_index', 'search_result', 'token', 'revision');
  if (in_array($view_mode, $core_view_modes)) {
	$reference_view_mode = $entity_type . '_' . $view_mode;
  } else {
    $reference_view_mode = $view_mode;
  }

I tried to populate the core_views_modes array dynamically but I could not find a function in the API which did not include the custom one(s). Anyway, I think they'll hardly be changed before D8...
I hope this helps some of the strugglers out there!

rszrama’s picture

Thanks for the update - gives me some good leads to follow to see about a core solution. One difficulty will be that we can't really go about changing the names of view modes in the 1.x branch, because it could be too disruptive for existing sites that have worked around this limitation. We'll need to find a solution that preserves backwards compatibility if we want 1.x to be compatible with Display Suite, otherwise we'll have to figure out how to develop a fix in contrib or push it off until 2.x.

swentel’s picture

@rszrame : Display Suite has 7.x-2.x branch now as well so you might just push it off to commerce 2.x. Haven't digged this completely as I have never had to use commerce in a project until now - crossing fingers though, there might be one coming up (but not as early as august), so I can debug along when that starts, but not before that.

vasike’s picture

Status: Active » Needs review
FileSize
968 bytes

a little different approach: added an extra check before setting to the "default" view mode.

extra: i noticed that the displays/view modes (in $product_field['display'] values) are still available if they are disabled, including the default view modes.
but i think this a core issue.

d.sibaud’s picture

the vasike latest patch it's working like charme! Thank you

Summit’s picture

Yep, #21 is working great! Would love to see it committed!
greetings, Martijn

vasike’s picture

Status: Needs review » Reviewed & tested by the community
Damien Tournoud’s picture

Status: Reviewed & tested by the community » Needs work

I already explained what needed to happen back in #11.

vasike’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

thanks Damien. then it's about changing the weight of the module as Ryan said (#5).
here is the patch for it.

rszrama’s picture

Status: Needs review » Needs work

What if the Display Suite weight has been increased already? ; )

You should probably use either a MAX function or look at the Display Suite weight directly and one up it.

Damien Tournoud’s picture

We only need to change the weight of commerce_product_reference_entity_info_alter() using hook_module_implements_alter() and place our implementation at the end of the list.

vasike’s picture

Status: Needs work » Needs review
FileSize
990 bytes

thanks again Damien.
here is the patch

timodwhit’s picture

Works for me. Is the initial load of the view mode in the product display more taxing the server? Anyone else having an issue.

Also, since the patch works will it be rolled into the next release?

rszrama’s picture

Status: Needs review » Fixed

Committed with a little more documentation. Thanks guys!

Commit: http://drupalcode.org/project/commerce.git/commitdiff/3f5ed07

brunorios1’s picture

the problem persists here.
i updated the drupal commerce to the last -dev and flushed all caches.

i'm trying to reproduce the faceted search of commerce kickstart 2 with search_api.
i created a custom view mode "Product List" with Display Suite and added to product display and product entity.
then i created a view with a field "rendered content" displayed with the view mode "Product List" mentioned above.

the products are listed in the view, but the fields are rendered by the "Default" view mode of the product entity.

am i missing something?

thanks!

rszrama’s picture

I guess you're missing something if it's working in CK 2.x but isn't in your reconstruction. Look again at how the distribution is configuring things and ensure you have placed the view mode in the wrong place or something. My hunch here is that it sounds like you're using a relationship from the product display node to the referenced product and rendering the product fields directly. In this case, those rendered product fields will know nothing of whatever view mode you're rendering nodes in the View in, so you may need to find either a setting to toggle what view mode to use for the product fields or actually alter the View directly in code.

In other words: the patch is fine / this issue is resolved, but it does not apply to your configuration.

brunorios1’s picture

You're right.

I tried to reproduce the issue in a clean installation, but i couldn't. The patch is ok!
I think my problem is being caused by some trash in the database or module conflicts.

Thanks!

Status: Fixed » Closed (fixed)

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

roderik’s picture

Issue summary: View changes

Just a little explanation (it happens that people don't see the extra display modes directly, the 7.x-1.4 announcement didn't make this very clear, and almost all issue comments assume that 'customviewmode' will be used after the fix, not 'node_customviewmode' .