Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
alanmels opened this issue Dec 23, 2022 · 3 comments

Comments

@alanmels
Copy link
Member

alanmels commented Dec 23, 2022

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Member Author

alanmels commented Dec 24, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants