backdrop-contrib / entity_plus

This module wraps in a variety of additional entity-related functionality from various sources. Partial port of D7 Entity API.
https://backdropcms.org/project/entity_plus
GNU General Public License v2.0
3 stars 11 forks source link

The entity_plus_view() function returns incorrect result in certain conditions #143

Open alanmels opened 1 year ago

alanmels commented 1 year ago

First, let me paste just the relevant part of a long custom function:

$new_product = uc_product_load_variant($result[$cycle]->nid);
$new_product->qty = $product->qty;
$new_items[] = $new_product;
dpm($new_items);
$display_items = uc_order_product_view_multiple($new_items);

At this stage, the $new_items array contains two Node type objects as dpm() shows:

Screenshot 2022-12-23 at 4 24 37 PM

Further processing through the uc_order_product_view_multiple() function turns the Node objects to UcOrderProduct objects.

function uc_order_product_view_multiple($order_products, $view_mode = 'full', $langcode = NULL, $page = NULL) {
  $order_product_entities = array();
  // Ensure we pass entities of the correct type to entity_plus_view.
  // @see https://github.com/backdrop-contrib/ubercart/issues/309
  foreach ($order_products as $id => $item) {
    if (!is_a($item, 'Entity') || $item->entityType() !== 'uc_order_product') {
      $order_product_entities[$id] = entity_create('uc_order_product', (array) $item);
    } else {
      $order_product_entities[$id] = $item;
    }
  }
  sdpm($order_product_entities);
  return entity_plus_view('uc_order_product', $order_product_entities, $view_mode, $langcode, $page);
}

Screenshot 2022-12-23 at 4 24 43 PM

The problem starts when the entity_plus_view() looses one of the array members and returns incomplete result. Unfortunately, the function was returning just one item:

function uc_order_product_view_multiple($order_products, $view_mode = 'full', $langcode = NULL, $page = NULL) {
  $order_product_entities = array();
  // Ensure we pass entities of the correct type to entity_plus_view.
  // @see https://github.com/backdrop-contrib/ubercart/issues/309
  foreach ($order_products as $id => $item) {
    if (!is_a($item, 'Entity') || $item->entityType() !== 'uc_order_product') {
      $order_product_entities[$id] = entity_create('uc_order_product', (array) $item);
    } else {
      $order_product_entities[$id] = $item;
    }
  }
  sdpm(entity_plus_view('uc_order_product', $order_product_entities, $view_mode, $langcode, $page));
  return entity_plus_view('uc_order_product', $order_product_entities, $view_mode, $langcode, $page);
}

until I changed the entity_plus_view() which looks like:

function entity_plus_view($entity_plus_type, $entities, $view_mode = 'full', $langcode = NULL, $page = NULL) {
  $info = entity_get_info($entity_plus_type);
  if (isset($info['view callback'])) {
    $entities = entity_plus_key_array_by_property($entities, $info['entity keys']['id']);
    return $info['view callback']($entities, $view_mode, $langcode, $entity_plus_type);
  }
  elseif (in_array('EntityPlusControllerInterface', class_implements($info['controller class']))) {
    return entity_get_controller($entity_plus_type)->view($entities, $view_mode, $langcode, $page);
  }
  return FALSE;
}

to:

function entity_plus_view($entity_plus_type, $entities, $view_mode = 'full', $langcode = NULL, $page = NULL) {
  $info = entity_get_info($entity_plus_type);
  if (isset($info['view callback'])) {
    $entities = entity_plus_key_array_by_property($entities, $info['entity keys']['id']);
    return $info['view callback']($entities, $view_mode, $langcode, $entity_plus_type);
  }
  elseif (in_array('EntityPlusControllerInterface', class_implements($info['controller class']))) {
    foreach ($entities as $id => $entity) {
      if (empty($entity->order_product_id)) {
        $entity->order_product_id = $id;
      }
    }
    dpm(entity_get_controller($entity_plus_type)->view($entities, $view_mode, $langcode, $page));
    return entity_get_controller($entity_plus_type)->view($entities, $view_mode, $langcode, $page);
  }
  return FALSE;
}

Turns out if $entity->order_product_id is not assigned yet, then the entity_plus_view() returns just one item (probably the last one overwrites the first one).

I'm now trying to figure out if there is a way for the newly formatted Node objects (order products) to already have order_product_id before passing to uc_order_product_view_multiple() function, but I thought to report this anyway, so you, guys, could consider changing the entity_plus_view() function slightly to make it foolproof against similar cases when UcOrderProduct entities with no order_product_id are served to it.

argiepiano commented 1 year ago

@alanmels thank you for posting this comment. As a maintainer for both Entity Plus and Ubercart I'm very familiar with the functions you are using here.

In a nutshell: you are trying to use entity_plus_view on the wrong type of entity, and on entities that have not been saved and don't have IDs yet. entity_plus_view is intended to be used with saved entities that possess IDs.

I see several issues with your code above. You are using a few Ubercart functions as if they were a full API, but unfortunately those functions are used as part of a whole process. You are bypassing several steps in the process.

  1. It's true that Ubercart products are nodes. But as soon as you put a product into your cart, that entity is saved as a uc_cart_item entity, which is saved and has its own cart_item_id. So, the first problem I see in your code is that you are passing nodes to a function that takes saved uc_cart_item entities.
  2. The second problem is that the function uc_order_product_view_multiple() is supposed to be used at a specific moment of the checkout process: when a customer moves from the Shopping cart form, into the Checkout form
  3. At that specific moment, the uc_order entity has been saved before invoking that function. This means that the order at this moment contains an array of either uc_cart_item entities WITH order_product_ids (which are added when you save the order), OR uc_order_product entities with order_product_ids. (This BTW is one weakness of Ubercart architecture - the fact that the function uc_order_product_view_multiple() can receive either uc_cart_item entities, or uc_order_product entities.)

So, my suggestion to you is that you take a close look at the whole process. It's possible to do all this programmatically, without doing all the UI forms. You would first need to add a node product to the shopping cart, then get the cart into the order and save it, then pass those items to uc_order_product_view_multiple() (more or less - other details may be missing in this description).

As a side note: I don't think it's a good idea to change an Entity Plus API function (entity_plus_view) to accommodate a problem that can be preventing by correctly using the Ubercart processes.

I can help further with this if you need feedback or a second opinion 😄

argiepiano commented 1 year ago

I want to clarify/expand my comment - the nodes you are passing to uc_order_product_view_multiple() ARE indeed saved and contain a nid. The issue is that uc_order_product_view_multiple() expects entities with order_product_ids set, which are obtained as Ubercart loads the nodes into the cart (creating uc_cart_item entities) and then loads those into an order and saves the order.

alanmels commented 1 year ago

@argiepiano I do appreciate much your insights. I've got the whole process working in regular conditions, however I'm trying to change the product items on the cart/checkout page depending on the user-chosen cycle recurring cycle, and because I need to reflect the change in individual item prices and the subtotal, I have to use an ajax_callback where all the regular workflow gets awry. Let me try few more things and come back to the issue.