Display custom fields on order admin view page

This patch adds a section to the right of the order admin page and displays all fields configured on the "Manage display" page that aren't displayed elsewhere on the page. For this to work, I had to look up which fields in fact were already in the template and pass these to twig's "without" filter.

Retitling.

Second this, seems like a good idea to have a baseline support for custom fields. Updated the patch to support the new class names introduced in 2.10.

Fixing issue with the patch format

Patch #6 works for me.

#6 works for me as well

When will this patch be merged into the module?

#6 is working for me too.

If you've reviewed and tested the patch you need to set it to RTBC.

For me the patch looks clean as well. Although it would be good to have someone from the commerce team to review as well.

Hi, I've just realised this is an issue with the user template as well: commerce-order--user.html.twig. I just added the code in patch #6 to the user template, and it seems to work. Is it worth opening up a new issue for this?

Hi roblog, I concluded more or less the same in #2831952-71: Create an entity_print renderer for orders (to allow order PDF output). I think it makes sense to have consistent behavior across both views, so it makes sense to combine them in one patch.

Please see it attached.

I plan on using this patch myself, but I also wanted to quickly share how to display the value of the order fields in the order email receipt:

{% if order_entity.field_purchase_order_number.value %}
    PO #: {{ order_entity.field_purchase_order_number.value }}
{% endif %}

I know that's a bit off-topic... hopefully that will be helpful for anyone else who is using fields on their orders.

Tested patch #14, it works perfectly.

Moved to RTBC

I have also tested the patch in #14

It creates a section in the sidebar called "Other" where it places the all fields that were not expressly placed elsewhere in the template.

This should handle most user cases where a custom layout is not needed.

Thank you for the patch. Does this issue need anything else to be commited?

I can agree that this should cover a lot of people who expect fields they add to show up, but people could still get surprised by changes to view mode settings not changing the order page. How should that be explained? Documentation on https://docs.drupalcommerce.org? A blurb on the order settings page so people will actually see it while changing those settings?

@tbradbury, I think this can be eventually solved with #2952529: Support for Layout Builder module.

+++ b/modules/order/templates/commerce-order--admin.html.twig
@@ -79,6 +79,17 @@
+      {% if order|without('order_items', 'total_price', 'activity', 'completed', 'placed', 'changed', 'uid', 'mail', 'ip_address', 'billing_information', 'shipping_information', 'state') %}
...
+            {{ order|without('order_items', 'total_price', 'activity', 'completed', 'placed', 'changed', 'uid', 'mail', 'ip_address', 'billing_information', 'shipping_information', 'state') }}

+++ b/modules/order/templates/commerce-order--user.html.twig
@@ -39,5 +39,6 @@
+    {{ order|without('order_items', 'total_price', 'activity', 'completed', 'placed', 'changed', 'uid', 'mail', 'ip_address', 'billing_information', 'shipping_information', 'state') }}

😬I wish there was an easier way to do this.

I wonder if we can put these in a variable via preprocess and pass it to without. Or if we still had a way to not display items which have been printed.

  // Early return if this element was pre-rendered (no need to re-render).
  if (isset($arg['#printed']) && $arg['#printed'] == TRUE && isset($arg['#markup']) && strlen($arg['#markup']) > 0) {
    return (string) $arg['#markup'];
  }
  $arg['#printed'] = FALSE;

#printed is currently only used to shortcut duplicate renders.

Bummer. I tried making a Twig filter which checks #printed and prevent duplicate rendering. However, the Twig autoescape doesn't modify the render array by reference (like Drupal 7 did), so the source render array has no idea it was already rendered.

@mglaman I had a look, but the Twig without filter simply creates a copy of the entity and leaves the original: https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Template!TwigExte...

The alternative I could think of is to create a clone in template_preprocess_commerce_order() and suppress all these fields there. But even that is not really nice..

function template_preprocess_commerce_order(array &$variables) {

  ...

  // Create a new render variable with all default fields removed. 
  $variables['sanitized_order'] = array_diff_key($variables['order'], array_flip(['order_items', 'total_price', 'activity', 'completed', 'placed', 'changed', 'uid', 'mail', 'ip_address', 'billing_information', 'shipping_information', 'state']));
}

Then in the templates we can use sanitized_order. This should be backwards compatible for all users who have overridden templates which use order.*?

It is still bothering me as users will have no control over what field is output where, as already mentioned in #18. This is so different compared to what people are used to with nodes.

I still think #2952529: Support for Layout Builder module should provide a nice solution for displaying an order with a sidebar and provide users with all the control they need.

Well, for over a year I had assumed that #2952529: Support for Layout Builder module would also include commerce orders, but after reading through everything, it appears that issue is very specific for products.

The reason that the layout builder does not work for orders is because of
#3137212: Implement a generateSampleValue() method for the StateItem field type
#3137225: Target bundles for entity reference fields should have the same key and value. (As Matt already discovered in #2952529-29: Support for Layout Builder module)
and #3137226: Target bundles for entity reference fields should have the same key and value. (For physical orders from commerce_shipping).

This would allow the layout builder to create a two column layout giving a user full control over what field goes where. Some additional styling could be applied to the sidebar. In order to build the same detail elements as we have now, it might be possible to use hook_entity_extra_field_info() to generate some pseudo fields?

commerce-order--admin.html.twig and commerce-order--user.html.twig could then be adjusted to look like this :

<div{{ attributes }}>
  {{ order }}
</div>

Or they can be removed to follow the default commerce-order.html.twig

So, yeah, this neat idea of providing a better default order appearance for folks in the alpha stages is having some repercussions.

One thing I was wondering is if we could fake/leverage concepts from the deprecated experimental module Field Layout.

There's a "region" setting and it's either Content or Hidden. I wonder if we could get a Sidebar region added to the form for just orders.

One thing I was wondering is if we could fake/leverage concepts from the deprecated experimental module Field Layout.

I have no idea how you've implemented the sidebar for the checkout system, but perhaps it is reusable? But even then, you would still need additional logic to achieve the same sidebar grouping you have now.

Before I test this patch, is it supposed to support entity_print?

Just tested #14 with current dev release and it works. Custom field appears in the side bar. It doesn't work with layout builder though.

I've created a new meta issue for layout builder on orders: #3175579: [meta] Layout builder for orders. It should work regardless of this patch.

+1 to RTBC. We're looking into the possibility of showing additional field in the admin order template as part of the custom/contrib module and #14 allows us to do so.

The patch no longer applies, it needs a reroll... I find it also annoying that the same list of fields in hardcoded 3 times (2 times in the same template), wondering if we could simply build that list in a preprocess at least.

Note that the "without" filter only supports passing an array from Drupal 8.9 (See #3093577: Let Twig without() filter take both arrays and strings as arguments), since we support 8.8, we cannot actually pass an array via a preprocess... So I guess the patch from #14 is fine for now... Not ideal, but better than not printing custom fields...

Once we drop Drupal 8.8 support, we could do this (in template_preprocess_commerce_order):

  if (in_array($view_mode, ['user', 'admin'])) {
    $variables['manually_printed_fields'] = ['order_items', 'total_price', 'activity', 'completed', 'placed', 'changed', 'uid', 'mail', 'ip_address', 'billing_information', 'shipping_information', 'state'];
  }

An alternative would be to fill an additional array in the following loop:

  foreach (Element::children($variables['elements']) as $key) {
    $variables['order'][$key] = $variables['elements'][$key];
  }

With only custom fields (i.e if the field is not in the list of manually rendered fields, fill another array).

Implemented a different approach that puts "additional" order fields that we're not manually printing in a separate variable, for easier rendering.

This prevents us from hardcoding the same field list in 2 different templates and 3 times.

I'm +1 for #33. The template was added years ago when we were bright-eyed and bushy-tailed on making a unified View and Edit screen when Twig ruled all. It's a quick fix and escape hatch. Solves most problems by exposing the fields, at least.

It is better to commit #33 than letting this continue to drag on, in my opinion.

@mglaman: Thanks for the review! Committed!